[PATCH] usb: gadget: Do not handle OS Descriptors config separately

[PATCH] usb: gadget: Do not handle OS Descriptors config separately

[Elson Roy Serrao]

Consider a multi-configuration composition in which the last
configuration uses OS descriptors. Since this configuration will
be sent first, the host may choose this config if it matches the
choosing criteria and ignore the user configured order of the
multi-config composition. Moreover linux based hosts do not re-order
the received configurations based on their indices but process them
in FIFO order. This may result in the host never choosing the user
desired configuration because of the re-order caused by sending the
OS descriptor config first. To avoid this, follow the user configured
order and do not handle OS descriptor config separately.

Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/gadget/composite.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index fa7dd6c..685003a 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -589,18 +589,9 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
 	w_value &= 0xff;
 
 	pos = &cdev->configs;
-	c = cdev->os_desc_config;
-	if (c)
-		goto check_config;
-
 	while ((pos = pos->next) !=  &cdev->configs) {
 		c = list_entry(pos, typeof(*c), list);
 
-		/* skip OS Descriptors config which is handled separately */
-		if (c == cdev->os_desc_config)
-			continue;
-
-check_config:
 		/* ignore configs that won't work at this speed */
 		switch (speed) {
 		case USB_SPEED_SUPER_PLUS:
-- 
2.7.4

Re: [PATCH] usb: gadget: Do not handle OS Descriptors config separately

[Greg KH]

On Mon, Feb 13, 2023 at 03:45:25PM -0800, Elson Roy Serrao wrote:
> Consider a multi-configuration composition in which the last
> configuration uses OS descriptors. Since this configuration will
> be sent first, the host may choose this config if it matches the
> choosing criteria and ignore the user configured order of the
> multi-config composition.

That is up to the host, so why is this an issue?

> Moreover linux based hosts do not re-order
> the received configurations based on their indices but process them
> in FIFO order.

s/linux/Linux/

> This may result in the host never choosing the user
> desired configuration because of the re-order caused by sending the
> OS descriptor config first. To avoid this, follow the user configured
> order and do not handle OS descriptor config separately.

I do not understand, what reordering is happening, and by whom?

> 
> Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>

What commit does this fix?

> ---
>  drivers/usb/gadget/composite.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index fa7dd6c..685003a 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -589,18 +589,9 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
>  	w_value &= 0xff;
>  
>  	pos = &cdev->configs;
> -	c = cdev->os_desc_config;
> -	if (c)
> -		goto check_config;
> -
>  	while ((pos = pos->next) !=  &cdev->configs) {
>  		c = list_entry(pos, typeof(*c), list);
>  
> -		/* skip OS Descriptors config which is handled separately */
> -		if (c == cdev->os_desc_config)
> -			continue;
> -
> -check_config:

This feels wrong, are you sure that this code isn't here for a reason?
How did you test this?

thanks,

greg k-h