linux-kernel - Re: [PATCH] usb: gadget: f_midi: Fixing wMaxPacketSize exceeded issue during MIDI bind retries

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: <2024121845-cactus-geology-8df3@gregkh>
Date: Wed, 18 Dec 2024 06:31:08 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Selvarasu Ganesan <selvarasu.g@...sung.com>
Cc: quic_jjohnson@...cinc.com, kees@...nel.org, abdul.rahim@...ahoo.com,
	m.grzeschik@...gutronix.de, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, jh0801.jung@...sung.com,
	dh10.jung@...sung.com, naushad@...sung.com, akash.m5@...sung.com,
	rc93.raju@...sung.com, taehyun.cho@...sung.com,
	hongpooh.kim@...sung.com, eomji.oh@...sung.com,
	shijie.cai@...sung.com, alim.akhtar@...sung.com,
	stable@...r.kernel.org
Subject: Re: [PATCH] usb: gadget: f_midi: Fixing wMaxPacketSize exceeded
 issue during MIDI bind retries

On Sun, Dec 08, 2024 at 08:53:20PM +0530, Selvarasu Ganesan wrote:
> The current implementation sets the wMaxPacketSize of bulk in/out
> endpoints to 1024 bytes at the end of the f_midi_bind function. However,
> in cases where there is a failure in the first midi bind attempt,
> consider rebinding.

What considers rebinding?  Your change does not modify that.

> This scenario may encounter an f_midi_bind issue due
> to the previous bind setting the bulk endpoint's wMaxPacketSize to 1024
> bytes, which exceeds the ep->maxpacket_limit where configured TX/RX
> FIFO's maxpacket size of 512 bytes for IN/OUT endpoints in support HS
> speed only.
> This commit addresses this issue by resetting the wMaxPacketSize before
> endpoint claim.

resets it to what?  Where did the magic numbers come from?  How do we
know this is now full speed and not high speed?

> 
> Fixes: 46decc82ffd5 ("usb: gadget: unconditionally allocate hs/ss descriptor in bind operation")
> Cc: stable@...r.kernel.org
> Signed-off-by: Selvarasu Ganesan <selvarasu.g@...sung.com>
> ---
>  drivers/usb/gadget/function/f_midi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 837fcdfa3840..5caa0e4eb07e 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -907,6 +907,15 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
>  
>  	status = -ENODEV;
>  
> +	/*
> +	 * Reset wMaxPacketSize with maximum packet size of FS bulk transfer before
> +	 * endpoint claim. This ensures that the wMaxPacketSize does not exceed the
> +	 * limit during bind retries where configured TX/RX FIFO's maxpacket size
> +	 * of 512 bytes for IN/OUT endpoints in support HS speed only.
> +	 */
> +	bulk_in_desc.wMaxPacketSize = cpu_to_le16(64);
> +	bulk_out_desc.wMaxPacketSize = cpu_to_le16(64);

Where did "64" come from?  How do we know this is full speed?  Later
on in this function the endpoint sizes are set, why set them here to
these small values when you do not know the speed?

Or, if it had failed before, reset the values on the failure, not here
before you start anything up, right?

thanks,

greg k-h