linux-kernel - Re: [PATCH] usb: dwc3: gadget: lower informal user notifaction dequeue operation

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]

Date:   Thu, 23 Mar 2023 23:41:06 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Marco Felsch <m.felsch@...gutronix.de>
CC:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "balbi@...nel.org" <balbi@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>
Subject: Re: [PATCH] usb: dwc3: gadget: lower informal user notifaction
 dequeue operation

Hi,

On Thu, Mar 23, 2023, Marco Felsch wrote:
> Printing an error message during usb_ep_dequeue() is more confusing than
> helpful since the usb_ep_dequeue() could be call during unbind() just
> in case that everything is canceld before unbinding the driver. Lower
> the dev_err() message to dev_dbg() to keep the message for developers.
> 
> Fixes: fcd2def66392 ("usb: dwc3: gadget: Refactor dwc3_gadget_ep_dequeue")
> Signed-off-by: Marco Felsch <m.felsch@...gutronix.de>
> ---
>  drivers/usb/dwc3/gadget.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 89dcfac01235f..6699db26cc7b5 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2106,7 +2106,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>  		}
>  	}
>  
> -	dev_err(dwc->dev, "request %pK was not queued to %s\n",
> +	dev_dbg(dwc->dev, "request %pK was not queued to %s\n",
>  		request, ep->name);
>  	ret = -EINVAL;
>  out:
> -- 
> 2.30.2
> 

How were you able to reproduce this error message?

During unbind(), the function driver would typically call to
usb_ep_disable(). Before the call usb_ep_disable() completes, all queued
and incompleted requests are expected to be returned with -ESHUTDOWN.
For you to see this error, this means that the function driver issued
usb_ep_dequeue() to an already disabled endpoint, and the request was
probably already given back.

Even though this error message is not critical and shouldn't affect the
driver's behavior, it's better to fix the function driver to handle this
race.

Thanks,
Thinh