On Thu, Apr 11, 2024 at 02:43:03PM +0200, Oliver Neukum wrote:
> When an interface is parsed the number of endpoints claimed to exist
> is compared to the number of endpoint descriptors actually found.
> Duplicated endpoints are not parsed in usb_parse_endpoint but
> usb_parse_interface counts them. That makes no sense.
It _does_ make sense. If there are 4 endpoint descriptors in the buffer
then "the number of endpoint descriptors actually found" is 4, even if
some of them are duplicates.
> To correct this usb_parse_endpoint needs to return feedback
> about the validity of parsed endpoints.
If you make this change then the following error message:
dev_notice(ddev, "config %d interface %d altsetting %d has %d "
"endpoint descriptor%s, different from the interface "
"descriptor's value: %d\n",
cfgno, inum, asnum, n, plural(n), num_ep_orig);
would be wrong, since n would not be the number of endpoint descriptors
but rather the number of descriptors after duplicates were removed.
This does not need to be changed.
Alan Stern
>
> Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
> ---
> drivers/usb/core/config.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 055910fc6b19..50acc9021247 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -254,7 +254,7 @@ static bool config_endpoint_is_duplicate(struct usb_host_config *config,
> static int usb_parse_endpoint(struct device *ddev, int cfgno,
> struct usb_host_config *config, int inum, int asnum,
> struct usb_host_interface *ifp, int num_ep,
> - unsigned char *buffer, int size)
> + unsigned char *buffer, int size, bool *valid)
> {
> struct usb_device *udev = to_usb_device(ddev);
> unsigned char *buffer0 = buffer;
> @@ -270,6 +270,7 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno,
>
> buffer += d->bLength;
> size -= d->bLength;
> + *valid = false;
>
> if (d->bDescriptorType != USB_DT_ENDPOINT)
> goto skip_to_next_endpoint_or_interface_descriptor;
> @@ -313,6 +314,7 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno,
> }
> }
>
> + *valid = true;
> endpoint = &ifp->endpoint[ifp->desc.bNumEndpoints];
> ++ifp->desc.bNumEndpoints;
>
> @@ -581,14 +583,17 @@ static int usb_parse_interface(struct device *ddev, int cfgno,
> /* Parse all the endpoint descriptors */
> n = 0;
> while (size >= sizeof(struct usb_descriptor_header)) { /* minimum length to get bDescriptorType */
> + bool valid;
> +
> if (((struct usb_descriptor_header *) buffer)->bDescriptorType
> == USB_DT_INTERFACE)
> break;
> retval = usb_parse_endpoint(ddev, cfgno, config, inum, asnum,
> - alt, num_ep, buffer, size);
> + alt, num_ep, buffer, size, &valid);
> if (retval < 0)
> return retval;
> - ++n;
> + if (valid)
> + ++n;
>
> buffer += retval;
> size -= retval;
> --
> 2.44.0
>
>
