linux-kernel - Re: [PATCH] usb: gadget: f_mass_storage: Disable eps during disconnect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211026144140.GA1288435@rowland.harvard.edu>
Date:   Tue, 26 Oct 2021 10:41:40 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     Wesley Cheng <quic_wcheng@...cinc.com>
Cc:     balbi@...nel.org, gregkh@...uxfoundation.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Wesley Cheng <wcheng@...eaurora.org>
Subject: Re: [PATCH] usb: gadget: f_mass_storage: Disable eps during
 disconnect

On Mon, Oct 25, 2021 at 05:44:56PM -0700, Wesley Cheng wrote:
> From: Wesley Cheng <wcheng@...eaurora.org>
> 
> When receiving a disconnect event from the UDC, the mass storage
> function driver currently runs the handle_exception() routine
> asynchronously.  For UDCs that support runtime PM, there is a
> possibility the UDC is already suspended by the time the
> do_set_interface() is executed.  This can lead to HW register access
> while the UDC is already suspended.
> 
> Signed-off-by: Wesley Cheng <wcheng@...eaurora.org>
> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 3cabf7692ee1..752439690fda 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -2342,6 +2342,16 @@ static void fsg_disable(struct usb_function *f)
>  {
>  	struct fsg_dev *fsg = fsg_from_func(f);
>  
> +	/* Disable the endpoints */
> +	if (fsg->bulk_in_enabled) {
> +		usb_ep_disable(fsg->bulk_in);

According to the kerneldoc, this routine must be called in process 
context.

> +		fsg->bulk_in_enabled = 0;
> +	}
> +	if (fsg->bulk_out_enabled) {
> +		usb_ep_disable(fsg->bulk_out);
> +		fsg->bulk_out_enabled = 0;
> +	}
> +
>  	__raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE, NULL);
>  }

Looks like you'll have to find a different way to avoid the problem.  
For example, if an exception is pending then you might prevent the 
gadget from going into runtime suspend until the exception has been 
handled.

Alan Stern