linux-kernel - Re: [PATCH] usb: gadget: m66592-udc: Ignore feature requests for EP0

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: <2025062956-finalize-canal-9145@gregkh>
Date: Sun, 29 Jun 2025 08:10:05 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Seungjin Bae <eeodqql09@...il.com>
Cc: Kyungtae Kim <Kyungtae.Kim@...tmouth.edu>,
	Uwe Kleine-König <u.kleine-koenig@...libre.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: gadget: m66592-udc: Ignore feature requests for EP0

On Sat, Jun 28, 2025 at 10:59:53PM -0400, Seungjin Bae wrote:
> The `set_feature() and clear_feature() functions handle requests to set or clear the ENDPOINT_HALT feature.
> Currently, these requests are processed for any endpoint, including the control endpoint (EP0).
> 
> The ENDPOINT_HALT feature is not defined for control endpoints according to the USB specification 9.4.5.

Nit, shouldn't you wrap these lines at 72 columns?  Didn't checkpatch
complain about this?

> 
> Fixes: 4cf2503c6801a ("USB: m66592-udc: peripheral controller driver for M66592")
> Co-developed-by: Kyungtae Kim <Kyungtae.Kim@...tmouth.edu>
> Signed-off-by: Kyungtae Kim <Kyungtae.Kim@...tmouth.edu>
> Signed-off-by: Seungjin Bae <eeodqql09@...il.com>
> ---

Any reason why you didn't also cc: stable for this?

>  drivers/usb/gadget/udc/m66592-udc.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
> index 715791737499..38cc11ae80b6 100644
> --- a/drivers/usb/gadget/udc/m66592-udc.c
> +++ b/drivers/usb/gadget/udc/m66592-udc.c
> @@ -1010,8 +1010,13 @@ static void clear_feature(struct m66592 *m66592, struct usb_ctrlrequest *ctrl)
>  		struct m66592_ep *ep;
>  		struct m66592_request *req;
>  		u16 w_index = le16_to_cpu(ctrl->wIndex);
> +		u16 ep_num = w_index & USB_ENDPOINT_NUMBER_MASK;
>  
> -		ep = m66592->epaddr2ep[w_index & USB_ENDPOINT_NUMBER_MASK];
> +		if (ep_num == 0) {
> +			control_end(m66592, 1);
> +			break;
> +		}
> +		ep = m66592->epaddr2ep[ep_num];

What in-kernel user calls this function for endpoint 0?


>  		pipe_stop(m66592, ep->pipenum);
>  		control_reg_sqclr(m66592, ep->pipenum);
>  
> @@ -1067,8 +1072,13 @@ static void set_feature(struct m66592 *m66592, struct usb_ctrlrequest *ctrl)
>  	case USB_RECIP_ENDPOINT: {
>  		struct m66592_ep *ep;
>  		u16 w_index = le16_to_cpu(ctrl->wIndex);
> +		u16 ep_num = w_index & USB_ENDPOINT_NUMBER_MASK;
>  
> -		ep = m66592->epaddr2ep[w_index & USB_ENDPOINT_NUMBER_MASK];
> +		if (ep_num == 0) {
> +			control_end(m66592, 1);
> +			break;
> +		}
> +		ep = m66592->epaddr2ep[ep_num];

Same here, what in-kernel user calls this for endpoint 0?

And was this tested on real hardware?

thanks,

greg k-h