[PATCH v2 1/3] usb: core: Fix root hub descriptor wBytesPerInterval

[PATCH v2 1/3] usb: core: Fix root hub descriptor wBytesPerInterval

[Michal Pecio]

Per USB3 9.6.7, it's "the total number of bytes this endpoint will
transfer every service interval". There seems to be no good reason
to have wBytesPerInterval < wMaxPacketSize - either one is too low
or the other too high. Here, wBytesPerInterval is too low for hubs
with more than 15 ports and xHCI spec allows such root hubs.

This is inconsequential for emulated root hubs, but we may want to
override and log suspiciously low wBytesPerInterval in descriptors,
so fix this to prevent nuisance warnings.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---

v2: same as v1

 drivers/usb/core/hcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 89221f1ce769..fc8130f94ca5 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -339,7 +339,8 @@ static const u8 ss_rh_config_descriptor[] = {
 		     /* Companion */
 	0x00,        /* __u8 ss_bMaxBurst; allows 1 TX between ACKs */
 	0x00,        /* __u8 ss_bmAttributes; 1 packet per service interval */
-	0x02, 0x00   /* __le16 ss_wBytesPerInterval; 15 bits for max 15 ports */
+		     /* __le16 ss_wBytesPerInterval; same as wMaxPacketSize */
+	(USB_MAXCHILDREN + 1 + 7) / 8, 0x00,
 };
 
 /* authorized_default behaviour:
-- 
2.48.1

[PATCH v2 2/3] usb: core: Fix up Interrupt IN endpoints with bogus wBytesPerInterval

[Michal Pecio]

Tao Xue found that some common devices violate USB3 section 9.6.7
by reporting wBytesPerInterval lower than the size of packets they
actually send. I confirmed that AX88179 may set it to 0 and RTL8153
CDC configuration sets it to 8 but sends both 8 and 16 byte packets:

S Ii:11:007:3 -115:128 16 <
C Ii:11:007:3 0:128 8 = a1000000 01000000
S Ii:11:007:3 -115:128 16 <
C Ii:11:007:3 0:128 16 = a12a0000 01000800 00000000 00000000

Most xHCI host controllers neglect interrupt bandwidth reservations
and let such devices exceed theirs, some fail the URB with EOVERFLOW.

Assume that wBytesPerInterval lower than wMaxPacketSize is bogus and
increase it to the worst case maximum on interrupt IN endpoints. This
solves xHCI problems and appears to have no other effect. Interrupt
transfers are not limited to one interval and drivers submit URBs of
class defined size without looking at wBytesPerInterval. Any multi-
interval transfer is considered terminated by a packet shorter than
wMaxPacketSize regardless of wBytesPerInterval - see USB3 8.10.3.

Stay in spec on OUT endpoints and isochronous. No buggy devices are
known and we don't want to risk sending more data than the device
is prepared to handle or confusing isoc drivers regarding altsetting
capacities guaranteed by the device itself. And don't complain when
wMaxPacketSize <= wBytesPerInterval < wMaxPacketSize * (bMaxBurst+1)
because enabling this seems to be the exact goal of the spec.

Reported-and-tested-by: Tao Xue <xuetao09@huawei.com>
Closes: https://lore.kernel.org/linux-usb/20260402021400.28853-1-xuetao09@huawei.com/
Cc: stable@vger.kernel.org
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---

v2:
- added reporter's tested-by
- use usb_endpoint_is_int_in() as suggested by Alan

 drivers/usb/core/config.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 6a1fd967e0a6..74945cd30cd2 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -191,7 +191,14 @@ static void usb_parse_ss_endpoint_companion(struct device *ddev, int cfgno,
 			(desc->bMaxBurst + 1);
 	else
 		max_tx = 999999;
-	if (le16_to_cpu(desc->wBytesPerInterval) > max_tx) {
+	/*
+	 * wBytesPerInterval > max_tx is bogus, but USB3 spec doesn't forbid the opposite.
+	 * Experience shows that wBytesPerInterval < wMaxPacketSize on common interrupt IN
+	 * endpoints is usually bogus too, and recent HCs enforce interrupt BW limits.
+	 */
+	if (le16_to_cpu(desc->wBytesPerInterval) > max_tx ||
+	    (le16_to_cpu(desc->wBytesPerInterval) < usb_endpoint_maxp(&ep->desc) &&
+	     usb_endpoint_is_int_in(&ep->desc))) {
 		dev_notice(ddev, "%s endpoint with wBytesPerInterval of %d in "
 				"config %d interface %d altsetting %d ep %d: "
 				"setting to %d\n",
-- 
2.48.1

[PATCH v2 3/3] usb: core: Clean up SuperSpeed/eUSB2 descriptor validation logging

[Michal Pecio]

Core usually prints endpoint addresses with 0x%X format.
Change this code to use it too, instead of just %d.
Particularly for IN, 0x83 seems more readable than 131.

While at that, fix checkpatch warnings about multi-line
quoted strings, as well as missing or doubled whitespace
in those strings.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---

v2: new patch in series

 drivers/usb/core/config.c | 37 +++++++++++--------------------------
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 74945cd30cd2..6b718f8a6697 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -56,8 +56,7 @@ static void usb_parse_ssp_isoc_endpoint_companion(struct device *ddev,
 	desc = (struct usb_ssp_isoc_ep_comp_descriptor *) buffer;
 	if (desc->bDescriptorType != USB_DT_SSP_ISOC_ENDPOINT_COMP ||
 	    size < USB_DT_SSP_ISOC_EP_COMP_SIZE) {
-		dev_notice(ddev, "Invalid SuperSpeedPlus isoc endpoint companion"
-			 "for config %d interface %d altsetting %d ep %d.\n",
+		dev_notice(ddev, "Invalid SuperSpeedPlus isoc endpoint companion for config %d interface %d altsetting %d ep 0x%X.\n",
 			 cfgno, inum, asnum, ep->desc.bEndpointAddress);
 		return;
 	}
@@ -91,7 +90,7 @@ static void usb_parse_eusb2_isoc_endpoint_companion(struct device *ddev,
 		size -= h->bLength;
 	}
 
-	dev_notice(ddev, "No eUSB2 isoc ep %d companion for config %d interface %d altsetting %d\n",
+	dev_notice(ddev, "No eUSB2 isoc ep 0x%X companion for config %d interface %d altsetting %d\n",
 		   ep->desc.bEndpointAddress, cfgno, inum, asnum);
 }
 
@@ -115,9 +114,7 @@ static void usb_parse_ss_endpoint_companion(struct device *ddev, int cfgno,
 	}
 
 	if (desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP) {
-		dev_notice(ddev, "No SuperSpeed endpoint companion for config %d "
-				" interface %d altsetting %d ep %d: "
-				"using minimum values\n",
+		dev_notice(ddev, "No SuperSpeed endpoint companion for config %d interface %d altsetting %d ep 0x%X: using minimum values\n",
 				cfgno, inum, asnum, ep->desc.bEndpointAddress);
 
 		/* Fill in some default values.
@@ -141,42 +138,32 @@ static void usb_parse_ss_endpoint_companion(struct device *ddev, int cfgno,
 
 	/* Check the various values */
 	if (usb_endpoint_xfer_control(&ep->desc) && desc->bMaxBurst != 0) {
-		dev_notice(ddev, "Control endpoint with bMaxBurst = %d in "
-				"config %d interface %d altsetting %d ep %d: "
-				"setting to zero\n", desc->bMaxBurst,
-				cfgno, inum, asnum, ep->desc.bEndpointAddress);
+		dev_notice(ddev, "Control endpoint with bMaxBurst = %d in config %d interface %d altsetting %d ep 0x%X: setting to zero\n",
+				desc->bMaxBurst, cfgno, inum, asnum, ep->desc.bEndpointAddress);
 		ep->ss_ep_comp.bMaxBurst = 0;
 	} else if (desc->bMaxBurst > 15) {
-		dev_notice(ddev, "Endpoint with bMaxBurst = %d in "
-				"config %d interface %d altsetting %d ep %d: "
-				"setting to 15\n", desc->bMaxBurst,
-				cfgno, inum, asnum, ep->desc.bEndpointAddress);
+		dev_notice(ddev, "Endpoint with bMaxBurst = %d in config %d interface %d altsetting %d ep 0x%X: setting to 15\n",
+				desc->bMaxBurst, cfgno, inum, asnum, ep->desc.bEndpointAddress);
 		ep->ss_ep_comp.bMaxBurst = 15;
 	}
 
 	if ((usb_endpoint_xfer_control(&ep->desc) ||
 			usb_endpoint_xfer_int(&ep->desc)) &&
 				desc->bmAttributes != 0) {
-		dev_notice(ddev, "%s endpoint with bmAttributes = %d in "
-				"config %d interface %d altsetting %d ep %d: "
-				"setting to zero\n",
+		dev_notice(ddev, "%s endpoint with bmAttributes = %d in config %d interface %d altsetting %d ep 0x%X: setting to zero\n",
 				usb_endpoint_xfer_control(&ep->desc) ? "Control" : "Bulk",
 				desc->bmAttributes,
 				cfgno, inum, asnum, ep->desc.bEndpointAddress);
 		ep->ss_ep_comp.bmAttributes = 0;
 	} else if (usb_endpoint_xfer_bulk(&ep->desc) &&
 			desc->bmAttributes > 16) {
-		dev_notice(ddev, "Bulk endpoint with more than 65536 streams in "
-				"config %d interface %d altsetting %d ep %d: "
-				"setting to max\n",
+		dev_notice(ddev, "Bulk endpoint with more than 65536 streams in config %d interface %d altsetting %d ep 0x%X: setting to max\n",
 				cfgno, inum, asnum, ep->desc.bEndpointAddress);
 		ep->ss_ep_comp.bmAttributes = 16;
 	} else if (usb_endpoint_xfer_isoc(&ep->desc) &&
 		   !USB_SS_SSP_ISOC_COMP(desc->bmAttributes) &&
 		   USB_SS_MULT(desc->bmAttributes) > 3) {
-		dev_notice(ddev, "Isoc endpoint has Mult of %d in "
-				"config %d interface %d altsetting %d ep %d: "
-				"setting to 3\n",
+		dev_notice(ddev, "Isoc endpoint has Mult of %d in config %d interface %d altsetting %d ep 0x%X: setting to 3\n",
 				USB_SS_MULT(desc->bmAttributes),
 				cfgno, inum, asnum, ep->desc.bEndpointAddress);
 		ep->ss_ep_comp.bmAttributes = 2;
@@ -199,9 +186,7 @@ static void usb_parse_ss_endpoint_companion(struct device *ddev, int cfgno,
 	if (le16_to_cpu(desc->wBytesPerInterval) > max_tx ||
 	    (le16_to_cpu(desc->wBytesPerInterval) < usb_endpoint_maxp(&ep->desc) &&
 	     usb_endpoint_is_int_in(&ep->desc))) {
-		dev_notice(ddev, "%s endpoint with wBytesPerInterval of %d in "
-				"config %d interface %d altsetting %d ep %d: "
-				"setting to %d\n",
+		dev_notice(ddev, "%s endpoint with wBytesPerInterval of %d in config %d interface %d altsetting %d ep 0x%X: setting to %d\n",
 				usb_endpoint_xfer_isoc(&ep->desc) ? "Isoc" : "Int",
 				le16_to_cpu(desc->wBytesPerInterval),
 				cfgno, inum, asnum, ep->desc.bEndpointAddress,
-- 
2.48.1

Re: [PATCH v2 1/3] usb: core: Fix root hub descriptor wBytesPerInterval

[Mathias Nyman]

On 5/4/26 12:13, Michal Pecio wrote:
> Per USB3 9.6.7, it's "the total number of bytes this endpoint will
> transfer every service interval". There seems to be no good reason
> to have wBytesPerInterval < wMaxPacketSize - either one is too low
> or the other too high. Here, wBytesPerInterval is too low for hubs
> with more than 15 ports and xHCI spec allows such root hubs.
> 

There shouldn't be a USB3 hub with more than 15 ports as
USB 3.x specification limits USB3 ports to 15.

See USB 3.2 section 10.15.2.1 Hub Descriptor
"bNbrPorts:	Number of downstream facing ports that this hub supports. The
		maximum number of ports of ports a hub can support is 15."


Hub driver also fails if hub has more than 15 ports.
hub.c hub_configure():

    maxchild = USB_MAXCHILDREN;
         if (hub_is_superspeed(hdev))
                 maxchild = min_t(unsigned, maxchild, USB_SS_MAXPORTS);

         if (hub->descriptor->bNbrPorts > maxchild) {
                 message = "hub has too many ports!";
                 ret = -ENODEV;
                 goto fail;

ch11.h:#define USB_SS_MAXPORTS           15

The USB 3.2 specification 10.15.1 'Standard Descriptors for Hub Class' also
shows hardcoded values of '2' for both the wBytesPerInterval and wMaxPacketSize
for the hub interrupt endpoint.

Thanks
Mathias

Re: [PATCH v2 1/3] usb: core: Fix root hub descriptor wBytesPerInterval

[Michal Pecio]

On Wed, 6 May 2026 13:17:46 +0300, Mathias Nyman wrote:
> On 5/4/26 12:13, Michal Pecio wrote:
> > Per USB3 9.6.7, it's "the total number of bytes this endpoint will
> > transfer every service interval". There seems to be no good reason
> > to have wBytesPerInterval < wMaxPacketSize - either one is too low
> > or the other too high. Here, wBytesPerInterval is too low for hubs
> > with more than 15 ports and xHCI spec allows such root hubs.
> >   
> 
> There shouldn't be a USB3 hub with more than 15 ports as
> USB 3.x specification limits USB3 ports to 15.
> 
> See USB 3.2 section 10.15.2.1 Hub Descriptor
> "bNbrPorts:	Number of downstream facing ports that this hub supports. The
> 		maximum number of ports of ports a hub can support is 15."
> 
> 
> Hub driver also fails if hub has more than 15 ports.
> hub.c hub_configure():
> 
>     maxchild = USB_MAXCHILDREN;
>          if (hub_is_superspeed(hdev))
>                  maxchild = min_t(unsigned, maxchild, USB_SS_MAXPORTS);
> 
>          if (hub->descriptor->bNbrPorts > maxchild) {
>                  message = "hub has too many ports!";
>                  ret = -ENODEV;
>                  goto fail;
> 
> ch11.h:#define USB_SS_MAXPORTS           15
> 
> The USB 3.2 specification 10.15.1 'Standard Descriptors for Hub Class' also
> shows hardcoded values of '2' for both the wBytesPerInterval and wMaxPacketSize
> for the hub interrupt endpoint.

Thanks, looks like we should be able to get away with reducing
wMaxPacketSize then. Currently it's calculated to fit USB_MAXCHILDREN
i.e. 31 ports, supposedly dictated by the needs of (obsolete) WUSB.

Any preferences whether to replace USB_MAXCHILDREN with USB_SS_MAXPORTS
in the existing formula for USB3 hubs or just hardcode 0x02?

Regards,
Michal

Re: [PATCH v2 1/3] usb: core: Fix root hub descriptor wBytesPerInterval

[Mathias Nyman]

On 5/6/26 13:31, Michal Pecio wrote:
> On Wed, 6 May 2026 13:17:46 +0300, Mathias Nyman wrote:
>> On 5/4/26 12:13, Michal Pecio wrote:
>>> Per USB3 9.6.7, it's "the total number of bytes this endpoint will
>>> transfer every service interval". There seems to be no good reason
>>> to have wBytesPerInterval < wMaxPacketSize - either one is too low
>>> or the other too high. Here, wBytesPerInterval is too low for hubs
>>> with more than 15 ports and xHCI spec allows such root hubs.
>>>    
>>
>> There shouldn't be a USB3 hub with more than 15 ports as
>> USB 3.x specification limits USB3 ports to 15.
>>
>> See USB 3.2 section 10.15.2.1 Hub Descriptor
>> "bNbrPorts:	Number of downstream facing ports that this hub supports. The
>> 		maximum number of ports of ports a hub can support is 15."
>>
>>
>> Hub driver also fails if hub has more than 15 ports.
>> hub.c hub_configure():
>>
>>      maxchild = USB_MAXCHILDREN;
>>           if (hub_is_superspeed(hdev))
>>                   maxchild = min_t(unsigned, maxchild, USB_SS_MAXPORTS);
>>
>>           if (hub->descriptor->bNbrPorts > maxchild) {
>>                   message = "hub has too many ports!";
>>                   ret = -ENODEV;
>>                   goto fail;
>>
>> ch11.h:#define USB_SS_MAXPORTS           15
>>
>> The USB 3.2 specification 10.15.1 'Standard Descriptors for Hub Class' also
>> shows hardcoded values of '2' for both the wBytesPerInterval and wMaxPacketSize
>> for the hub interrupt endpoint.
> 
> Thanks, looks like we should be able to get away with reducing
> wMaxPacketSize then. Currently it's calculated to fit USB_MAXCHILDREN
> i.e. 31 ports, supposedly dictated by the needs of (obsolete) WUSB.
> 
> Any preferences whether to replace USB_MAXCHILDREN with USB_SS_MAXPORTS
> in the existing formula for USB3 hubs or just hardcode 0x02?

Maybe prefer the hardcoded 0x02 value as spec does it as well

Thanks
Mathias