linux-kernel - Re: [PATCH v1 2/3] usb: gadget: Use get_status callback to set remote wakeup capability

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: <20250408011758.qfdflgrrmahwmfqi@synopsys.com>
Date: Tue, 8 Apr 2025 01:18:26 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Prashanth K <prashanth.k@....qualcomm.com>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Kees Bakker <kees@...erbout.nl>,
        William McVicker <willmcvicker@...gle.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...nel.org" <stable@...nel.org>
Subject: Re: [PATCH v1 2/3] usb: gadget: Use get_status callback to set remote
 wakeup capability

On Thu, Apr 03, 2025, Prashanth K wrote:
> Currently when the host sends GET_STATUS request for an interface,
> we use get_status callbacks to set/clear remote wakeup capability
> of that interface. And if get_status callback isn't present for
> that interface, then we assume its remote wakeup capability based
> on bmAttributes.
> 
> Now consider a scenario, where we have a USB configuration with
> multiple interfaces (say ECM + ADB), here ECM is remote wakeup
> capable and as of now ADB isn't. And bmAttributes will indicate
> the device as wakeup capable. With the current implementation,
> when host sends GET_STATUS request for both interfaces, we will
> set FUNC_RW_CAP for both. This results in USB3 CV Chapter 9.15
> (Function Remote Wakeup Test) failures as host expects remote
> wakeup from both interfaces.
> 
> The above scenario is just an example, and the failure can be
> observed if we use configuration with any interface except ECM.
> Hence avoid configuring remote wakeup capability from composite
> driver based on bmAttributes, instead use get_status callbacks
> and let the function drivers decide this.
> 
> Cc: stable@...nel.org
> Fixes: 481c225c4802 ("usb: gadget: Handle function suspend feature selector")
> Signed-off-by: Prashanth K <prashanth.k@....qualcomm.com>
> ---
>  drivers/usb/gadget/composite.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 869ad99afb48..5c6da360e95b 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -2010,16 +2010,12 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>  			break;
>  
>  		if (f->get_status) {
> -			status = f->get_status(f);
> +			/* if D5 is not set, then device is not wakeup capable */
> +			if (f->config->bmAttributes & USB_CONFIG_ATT_WAKEUP)

We should allow function to execute get_status regardless of
USB_CONFIG_ATT_WAKEUP. There are other status beside wakeup.

> +				status = f->get_status(f);
> +
>  			if (status < 0)
>  				break;
> -		} else {
> -			/* Set D0 and D1 bits based on func wakeup capability */
> -			if (f->config->bmAttributes & USB_CONFIG_ATT_WAKEUP) {
> -				status |= USB_INTRF_STAT_FUNC_RW_CAP;


So right now we're not able to configure the function to enable RW
capable? Perhaps we need to update the composite configfs for this?

BR,
Thinh

> -				if (f->func_wakeup_armed)
> -					status |= USB_INTRF_STAT_FUNC_RW;
> -			}
>  		}
>  
>  		put_unaligned_le16(status & 0x0000ffff, req->buf);
> -- 
> 2.25.1
>