Re: [PATCH] USB/Gadget: Fix race between gether_disconnect and rx_submit

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Jun 19, 2019 at 06:41:10AM +0000, kvaradarajan wrote:
>   On spin lock release in rx_submit, gether_disconnect get
>   a chance to run, it makes port_usb NULL, rx_submit access
>   NULL port USB, hence null pointer crash.
> 
>   Fixed by releasing the lock in rx_submit after port_usb
>   is used.

Meta-comments about the patch information...

Why is this indented?  Please keep comments all the way to the left and
wrap the columns at 72.

> Signed-off-by: KVaradarajan <Kiruthika.Varadarajan@harman.com>

I need a "legal name" here, I don't think you sign documents that way.
It also needs to match the From: line of your email.

> ---
>  drivers/usb/gadget/function/u_ether.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index 737bd77..76cf1e4 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -186,11 +186,11 @@ static void defer_kevent(struct eth_dev *dev, int flag)
>  		out = dev->port_usb->out_ep;
>  	else
>  		out = NULL;
> -	spin_unlock_irqrestore(&dev->lock, flags);
>  
> -	if (!out)
> +	if (!out) {
> +		spin_unlock_irqrestore(&dev->lock, flags);
>  		return -ENOTCONN;
> -
> +	}
>  
>  	/* Padding up to RX_EXTRA handles minor disagreements with host.
>  	 * Normally we use the USB "terminate on short read" convention;
> @@ -215,6 +215,7 @@ static void defer_kevent(struct eth_dev *dev, int flag)
>  	if (dev->port_usb->is_fixed)
>  		size = max_t(size_t, size, dev->port_usb->fixed_out_len);
>  
> +	spin_unlock_irqrestore(&dev->lock, flags);

Patch looks sane to me.  I'll let Felipe do the real review after you
resend based on the information above.

thanks,

greg k-h