Re: [PATCH] usb: core: Fix bandwidth for devices with invalid wBytesPerInterval

[Xuetao (kirin)]

在 2026/4/2 11:51, Greg KH 写道:
> On Thu, Apr 02, 2026 at 10:14:00AM +0800, Tao Xue wrote:
> > As specified in Section 4.14.2 of the xHCI Specification, the xHC
> > reserves bandwidth for periodic endpoints according to bInterval and
> > wBytesPerInterval (Max ESIT Payload).
> >
> > Some peripherals report an invalid wBytesPerInterval in their device
> > descriptor, which is either 0 or smaller than the actual data length
> > transmitted. This issue is observed on ASIX AX88179 series USB 3.0
> > Ethernet adapters.
> >
> > These errors may lead to unexpected behavior on certain USB host
> > controllers, causing USB peripherals to malfunction.
> >
> > To address the issue, return max(wBytesPerInterval, max_payload) when
> > calculating bandwidth reservation.
> >
> > Fixes: 9238f25d5d32 ("USB: xhci: properly set endpoint context fields for periodic eps.")
> > Cc: <stable@xxxxxxxxxx>
> > Signed-off-by: Tao Xue <xuetao09@xxxxxxxxxx>
> > ---
> >   drivers/usb/core/usb.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index e9a10a33534c..8f2e05a5a015 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -1125,6 +1125,8 @@ EXPORT_SYMBOL_GPL(usb_free_noncoherent);
> >   u32 usb_endpoint_max_periodic_payload(struct usb_device *udev,
> >   				      const struct usb_host_endpoint *ep)
> >   {
> > +	u32 max_payload;
> > +
> >   	if (!usb_endpoint_xfer_isoc(&ep->desc) &&
> >   	    !usb_endpoint_xfer_int(&ep->desc))
> >   		return 0;
> > @@ -1135,7 +1137,12 @@ u32 usb_endpoint_max_periodic_payload(struct usb_device *udev,
> >   			return le32_to_cpu(ep->ssp_isoc_ep_comp.dwBytesPerInterval);
> >   		fallthrough;
> >   	case USB_SPEED_SUPER:
> > -		return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
> > +		max_payload = usb_endpoint_maxp(&ep->desc) * (ep->ss_ep_comp.bMaxBurst + 1);
> > +		if (usb_endpoint_xfer_isoc(&ep->desc))
> > +			return max_t(u32, max_payload * USB_SS_MULT(ep->ss_ep_comp.bmAttributes),
> > +					ep->ss_ep_comp.wBytesPerInterval);
> > +		else
> > +			return max_t(u32, max_payload, ep->ss_ep_comp.wBytesPerInterval);
>
> You dropped the conversion from le16 to cpu?  Why?
>
> thanks,
>
> greg k-h

Hi Greg,

Thank you for the review.

1、You're right, that was an oversight. I should keep the le16_to_cpu().
Here's the corrected version:

    max_payload = usb_endpoint_maxp(&ep->desc) * 
(ep->ss_ep_comp.bMaxBurst + 1);
    if (usb_endpoint_xfer_isoc(&ep->desc))
        return max_t(u32, max_payload * 
USB_SS_MULT(ep->ss_ep_comp.bmAttributes),
                        le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval));
    else
        return max_t(u32, max_payload, 
le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval));

2、Following Alan's suggestion in another email, should I check whether 
wBytesPerInterval is a valid value and handle it in the 
usb_parse_ss_endpoint_companion() ?

However, when parsing the device descriptor, we do not know whether the 
actual data length transmitted by the peripheral is greater than 
wBytesPerInterval.

Therefore, would it be sufficient to only add a check for whether 
wBytesPerInterval is 0 in the existing flow, and if it is 0, set 
wBytesPerInterval to cpu_to_le16(max_tx) by default?

For example, modify it in the following way：

     if (le16_to_cpu(desc->wBytesPerInterval) > max_tx || 
le16_to_cpu(desc->wBytesPerInterval) == 0) {
        dev_notice(ddev, "%s endpoint with wBytesPerInterval of %d in "
                "config %d interface %d altsetting %d ep %d: "
                "setting to %d\n",
                usb_endpoint_xfer_isoc(&ep->desc) ? "Isoc" : "Int",
                le16_to_cpu(desc->wBytesPerInterval),
                cfgno, inum, asnum, ep->desc.bEndpointAddress,
                max_tx);
        ep->ss_ep_comp.wBytesPerInterval = cpu_to_le16(max_tx);
    }

 Could you please give me some advice? Thanks.