| Message ID | 20240411124722.17343-6-oneukum@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,1/6] usb: usb_parse_endpoint ignore reserved bits | expand |
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@suse.com> > --- > 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 > >
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;
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. To correct this usb_parse_endpoint needs to return feedback about the validity of parsed endpoints. Signed-off-by: Oliver Neukum <oneukum@suse.com> --- drivers/usb/core/config.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)