Re: [PATCH] usb: chipidea: udc: fix NULL ptr dereference in isr_setup_status_phase

[Peter Chen]

On Mon, Sep 05, 2016 at 07:29:58PM +0200, Clemens Gruber wrote:
> Problems with the signal integrity of the high speed USB data lines or
> noise on reference ground lines can cause the i.MX6 USB controller to
> violate USB specs and exhibit unexpected behavior.
> 
> It was observed that USBi_UI interrupts were triggered first and when
> isr_setup_status_phase was called, ci->status was NULL, which lead to a
> NULL pointer dereference kernel panic.
> 
> This patch fixes the kernel panic, emits a warning once and returns
> -EPIPE to halt the device and let the host get stalled.
> It also adds a comment to point people, who are experiencing this issue,
> to their USB hardware design.
> 
> Signed-off-by: Clemens Gruber <clemens.gruber@xxxxxxxxxxxx>
> ---
>  drivers/usb/chipidea/udc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index dfec5a1..b933568 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -949,6 +949,15 @@ static int isr_setup_status_phase(struct ci_hdrc *ci)
>  	int retval;
>  	struct ci_hw_ep *hwep;
>  
> +	/*
> +	 * Unexpected USB controller behavior, caused by bad signal integrity
> +	 * or ground reference problems, can lead to isr_setup_status_phase
> +	 * being called with ci->status equal to NULL.
> +	 * If this situation occurs, you should review your USB hardware design.
> +	 */
> +	if (WARN_ON_ONCE(!ci->status))
> +		return -EPIPE;
> +
>  	hwep = (ci->ep0_dir == TX) ? ci->ep0out : ci->ep0in;
>  	ci->status->context = ci;
>  	ci->status->complete = isr_setup_status_complete;

Thanks, I will apply it.

-- 

Best Regards,
Peter Chen