All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
@ 2026-11-04 12:44 Pengfei Lv
  2026-11-04 12:57 ` sashiko-bot
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Pengfei Lv @ 2026-11-04 12:44 UTC (permalink / raw)
  To: gregkh
  Cc: stern, linux-usb, linux-kernel, stable, xuhaiyang5, xuetao09,
	caiyadong, suzhuangluan, Pengfei Lv

Per USB 2.0 spec section 5.5.3, the ep0 maxpacket size is fixed for
all device speeds except full-speed devices, where it can be 8, 16,
32, or 64 bytes and can only be determined by reading the device
descriptor.  During the initial enumeration of a full-speed device,
the USB core guesses 64 bytes (hub_port_init() with initial=true);
after reading the descriptor, the real value (e.g. 8) is stored in
udev->ep0.desc.wMaxPacketSize and propagated to the xHCI ep0 context
via an Evaluate Context command (xhci_check_maxpacket()).

After a port reset (usb_reset_and_verify_device() -> hub_port_init()
with initial=false), the "if (initial)" guard added by commit
ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()") skips the ep0 maxpacket
guess block, so udev->ep0.desc.wMaxPacketSize retains the real value
(e.g. 8).  The xHCI Reset Device command preserves the ep0 context
as well, so the xHC ep0 max_packet also stays at 8.

When the subsequent re-enumeration then uses the "new scheme" and
issues a 64-byte GET_DESCRIPTOR (get_bMaxPacketSize0 with size=64),
xhci_check_maxpacket() sees hw==sw==8 and does not issue an Evaluate
Context command.  The xHC queues a Data TRB of length 64 but applies
ep0 max_packet=8 for short-packet detection.  If the device's first
data packet is exactly 8 bytes and it sends no ZLP, the xHC does not
recognise a short packet, keeps issuing IN tokens for the remaining
56 bytes, and the control transfer hangs in the Data stage until URB
timeout.  (A standard 18-byte descriptor returned as 8+8+2 is not
affected because the final 2-byte packet is a short packet.)

Fix this by removing the "if (initial)" guard so that the ep0
maxpacket size is re-guessed from udev->speed on every reset.  For
full-speed devices this re-establishes the 64-byte guess, which makes
xhci_check_maxpacket() issue an Evaluate Context command that brings
the xHC ep0 context back to max_packet=64 before the GET_DESCRIPTOR
data stage; the data stage then terminates correctly via short-packet
detection, and the ep0 maxpacket is subsequently corrected to the
real value as usual.

For high-speed, low-speed, and SuperSpeed-or-faster devices the
re-guess is a no-op (guessed value equals the fixed real value).
The descriptor-immutability checks (!initial && maxp0 !=
udev->descriptor.bMaxPacketSize0) remain unchanged.

Fixes: ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()")
Cc: stable@vger.kernel.org
Signed-off-by: Pengfei Lv <lvpengfei20@huawei.com>
---
 drivers/usb/core/hub.c | 46 ++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 199d3f535a89..51f66b24b353 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4969,31 +4969,29 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 	}
 	oldspeed = udev->speed;
 
-	if (initial) {
-		/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
-		 * it's fixed size except for full speed devices.
+	/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
+	 * it's fixed size except for full speed devices.
+	 */
+	switch (udev->speed) {
+	case USB_SPEED_SUPER_PLUS:
+	case USB_SPEED_SUPER:
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
+		break;
+	case USB_SPEED_HIGH:		/* fixed at 64 */
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
+		break;
+	case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
+		/* to determine the ep0 maxpacket size, try to read
+		 * the device descriptor to get bMaxPacketSize0 and
+		 * then correct our initial guess.
 		 */
-		switch (udev->speed) {
-		case USB_SPEED_SUPER_PLUS:
-		case USB_SPEED_SUPER:
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
-			break;
-		case USB_SPEED_HIGH:		/* fixed at 64 */
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
-			break;
-		case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
-			/* to determine the ep0 maxpacket size, try to read
-			 * the device descriptor to get bMaxPacketSize0 and
-			 * then correct our initial guess.
-			 */
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
-			break;
-		case USB_SPEED_LOW:		/* fixed at 8 */
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
-			break;
-		default:
-			goto fail;
-		}
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
+		break;
+	case USB_SPEED_LOW:		/* fixed at 8 */
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
+		break;
+	default:
+		goto fail;
 	}
 
 	speed = usb_speed_string(udev->speed);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-04 12:44 [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset Pengfei Lv
@ 2026-11-04 12:57 ` sashiko-bot
  2026-11-04 14:59 ` Alan Stern
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-11-04 12:57 UTC (permalink / raw)
  To: Pengfei Lv; +Cc: linux-usb

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: core: hub: missing usb_ep0_reinit() in hub_port_init() after ep0 guess
--

commit 8488ba17cb81d9272c99646f158ad82af5c82843
Author: Pengfei Lv <lvpengfei20@huawei.com>

usb: core: hub: Re-guess ep0 maxpacket size on reset

This commit removes the if (initial) guard around the ep0 maxpacket size
initialization in hub_port_init() so that udev->ep0.desc.wMaxPacketSize is
re-guessed from udev->speed during port resets as well as initial
enumeration.

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 199d3f535a891..51f66b24b3538 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4969,31 +4969,29 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>  	}
>  	oldspeed = udev->speed;
>  
> -	if (initial) {
> -		/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
> -		 * it's fixed size except for full speed devices.
> +	/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
> +	 * it's fixed size except for full speed devices.
> +	 */
> +	switch (udev->speed) {
> +	case USB_SPEED_SUPER_PLUS:
> +	case USB_SPEED_SUPER:
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
> +		break;
> +	case USB_SPEED_HIGH:		/* fixed at 64 */
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> +		break;
> +	case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
> +		/* to determine the ep0 maxpacket size, try to read
> +		 * the device descriptor to get bMaxPacketSize0 and
> +		 * then correct our initial guess.
>  		 */
> -		switch (udev->speed) {
> -		case USB_SPEED_SUPER_PLUS:
> -		case USB_SPEED_SUPER:
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
> -			break;
> -		case USB_SPEED_HIGH:		/* fixed at 64 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> -			break;
> -		case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
> -			/* to determine the ep0 maxpacket size, try to read
> -			 * the device descriptor to get bMaxPacketSize0 and
> -			 * then correct our initial guess.
> -			 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> -			break;
> -		case USB_SPEED_LOW:		/* fixed at 8 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
> -			break;
> -		default:
> -			goto fail;
> -		}
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> +		break;

[Severity: High]
Does hub_port_init() also need to call usb_ep0_reinit(udev) after resetting
udev->ep0.desc.wMaxPacketSize to 64 when !initial?

The commit message notes that re-establishing the 64-byte guess makes
xhci_check_maxpacket() issue an Evaluate Context command before the
GET_DESCRIPTOR data stage. However, commit e34900f46cd6 ("xhci: Reconfigure
endpoint 0 max packet size only during endpoint reset") removed that check
from xhci_urb_enqueue() and replaced it with xhci_check_ep0_maxpacket(),
which is only invoked from xhci_endpoint_reset() via usb_ep0_reinit():

drivers/usb/host/xhci.c:xhci_endpoint_reset() {
	if (usb_endpoint_xfer_control(&host_ep->desc) && ep_index == 0) {
		...
		xhci_check_ep0_maxpacket(xhci, vdev);

		/* Nothing else should be done here for ep0 during ep reset */
		return;
	}
}

During a port reset in usb_reset_and_verify_device(), usb_ep0_reinit(udev)
is called before hub_port_init():

drivers/usb/core/hub.c:usb_reset_and_verify_device() {
	for (i = 0; i < SET_CONFIG_TRIES; ++i) {

		/* ep0 maxpacket size may change; let the HCD know about it.
		 * Other endpoints will be handled by re-enumeration. */
		usb_ep0_reinit(udev);
		ret = hub_port_init(parent_hub, udev, port1, i, &descriptor);
}

On the first attempt (i == 0), udev->ep0.desc.wMaxPacketSize still holds the
pre-reset value (e.g. 8) when usb_ep0_reinit(udev) runs, matching the xHC
output EP0 context, so xhci_check_ep0_maxpacket() does nothing.

Then hub_port_init() resets the port (leaving the slot in SLOT_STATE_DEFAULT
with max_packet = 8) and sets udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64)
in software without calling usb_ep0_reinit(udev). When do_new_scheme is true,
hub_enable_device() -> xhci_enable_device() -> xhci_setup_device() sees the
slot is already in SLOT_STATE_DEFAULT and returns early without updating the
EP0 context:

drivers/usb/host/xhci.c:xhci_setup_device() {
	if (setup == SETUP_CONTEXT_ONLY) {
		if (GET_SLOT_STATE(le32_to_cpu(slot_ctx->dev_state)) ==
		    SLOT_STATE_DEFAULT) {
			xhci_dbg(xhci, "Slot already in default state\n");
			goto out;
		}
	}
}

As a result, when get_bMaxPacketSize0() submits the 64-byte GET_DESCRIPTOR
URB on i == 0, the xHC hardware EP0 context still has max_packet = 8, so the
first hub_port_init() call still times out across all GET_DESCRIPTOR_TRIES *
GET_MAXPACKET0_TRIES attempts before usb_reset_and_verify_device() retries
with i == 1.

In addition, for standard full-speed devices with bMaxPacketSize0 = 8 whose
18-byte descriptor read succeeds on i == 0 in do_new_scheme,
hub_set_address() calls usb_ep0_reinit(udev) while
udev->ep0.desc.wMaxPacketSize is still 64, issuing an Evaluate Context
command to change max_packet from 8 to 64 right before hub_port_init() sets
wMaxPacketSize back to 8 and calls usb_ep0_reinit(udev) a second time.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104124402.1269650-1-lvpengfei20@huawei.com?part=1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-04 12:44 [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset Pengfei Lv
  2026-11-04 12:57 ` sashiko-bot
@ 2026-11-04 14:59 ` Alan Stern
  2026-11-04 17:36   ` Michal Pecio
  2026-11-05  8:41 ` Pengfei Lv
  2026-11-06  3:16 ` [PATCH v2] " Pengfei Lv
  3 siblings, 1 reply; 11+ messages in thread
From: Alan Stern @ 2026-11-04 14:59 UTC (permalink / raw)
  To: Pengfei Lv
  Cc: gregkh, linux-usb, linux-kernel, stable, xuhaiyang5, xuetao09,
	caiyadong, suzhuangluan

On Wed, Nov 04, 2026 at 08:44:02PM +0800, Pengfei Lv wrote:
> Per USB 2.0 spec section 5.5.3, the ep0 maxpacket size is fixed for
> all device speeds except full-speed devices, where it can be 8, 16,
> 32, or 64 bytes and can only be determined by reading the device
> descriptor.  During the initial enumeration of a full-speed device,
> the USB core guesses 64 bytes (hub_port_init() with initial=true);
> after reading the descriptor, the real value (e.g. 8) is stored in
> udev->ep0.desc.wMaxPacketSize and propagated to the xHCI ep0 context
> via an Evaluate Context command (xhci_check_maxpacket()).
> 
> After a port reset (usb_reset_and_verify_device() -> hub_port_init()
> with initial=false), the "if (initial)" guard added by commit
> ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()") skips the ep0 maxpacket
> guess block, so udev->ep0.desc.wMaxPacketSize retains the real value
> (e.g. 8).  The xHCI Reset Device command preserves the ep0 context
> as well, so the xHC ep0 max_packet also stays at 8.
> 
> When the subsequent re-enumeration then uses the "new scheme" and
> issues a 64-byte GET_DESCRIPTOR (get_bMaxPacketSize0 with size=64),
> xhci_check_maxpacket() sees hw==sw==8 and does not issue an Evaluate
> Context command.  The xHC queues a Data TRB of length 64 but applies
> ep0 max_packet=8 for short-packet detection.

That's exactly what we want it to do.

>  If the device's first
> data packet is exactly 8 bytes and it sends no ZLP, the xHC does not
> recognise a short packet, keeps issuing IN tokens for the remaining
> 56 bytes, and the control transfer hangs in the Data stage until URB
> timeout.

That is the correct behavior.  Indeed, the same issue applies to _any_ 
64-byte transfer (not just control and not just Get-Descriptor) when the 
maxpacket value is 8.  If the device sends an 8-byte reply with no ZLP 
following, the transfer will hang in the data stage.  That's _supposed_ 
to happen.

>  (A standard 18-byte descriptor returned as 8+8+2 is not
> affected because the final 2-byte packet is a short packet.)

Quite right.  Why wouldn't the device reply with a standard 18-byte 
descriptor in this situation?

Have you observed this problem in actual testing or is it purely 
theoretical?

> Fix this by removing the "if (initial)" guard so that the ep0
> maxpacket size is re-guessed from udev->speed on every reset.  For
> full-speed devices this re-establishes the 64-byte guess, which makes
> xhci_check_maxpacket() issue an Evaluate Context command that brings
> the xHC ep0 context back to max_packet=64 before the GET_DESCRIPTOR
> data stage; the data stage then terminates correctly via short-packet
> detection, and the ep0 maxpacket is subsequently corrected to the
> real value as usual.
> 
> For high-speed, low-speed, and SuperSpeed-or-faster devices the
> re-guess is a no-op (guessed value equals the fixed real value).
> The descriptor-immutability checks (!initial && maxp0 !=
> udev->descriptor.bMaxPacketSize0) remain unchanged.

As far as I can see, there is no need to do this.

Alan Stern

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-04 14:59 ` Alan Stern
@ 2026-11-04 17:36   ` Michal Pecio
  2026-11-05  5:06     ` Alan Stern
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Pecio @ 2026-11-04 17:36 UTC (permalink / raw)
  To: Alan Stern
  Cc: Pengfei Lv, gregkh, linux-usb, linux-kernel, stable, xuhaiyang5,
	xuetao09, caiyadong, suzhuangluan

On Wed, 4 Nov 2026 09:59:24 -0500, Alan Stern wrote:
> On Wed, Nov 04, 2026 at 08:44:02PM +0800, Pengfei Lv wrote:
> > Per USB 2.0 spec section 5.5.3, the ep0 maxpacket size is fixed for
> > all device speeds except full-speed devices, where it can be 8, 16,
> > 32, or 64 bytes and can only be determined by reading the device
> > descriptor.  During the initial enumeration of a full-speed device,
> > the USB core guesses 64 bytes (hub_port_init() with initial=true);
> > after reading the descriptor, the real value (e.g. 8) is stored in
> > udev->ep0.desc.wMaxPacketSize and propagated to the xHCI ep0 context
> > via an Evaluate Context command (xhci_check_maxpacket()).
> > 
> > After a port reset (usb_reset_and_verify_device() -> hub_port_init()
> > with initial=false), the "if (initial)" guard added by commit
> > ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()") skips the ep0 maxpacket
> > guess block, so udev->ep0.desc.wMaxPacketSize retains the real value
> > (e.g. 8).  The xHCI Reset Device command preserves the ep0 context
> > as well, so the xHC ep0 max_packet also stays at 8.
> > 
> > When the subsequent re-enumeration then uses the "new scheme" and
> > issues a 64-byte GET_DESCRIPTOR (get_bMaxPacketSize0 with size=64),
> > xhci_check_maxpacket() sees hw==sw==8 and does not issue an Evaluate
> > Context command.  The xHC queues a Data TRB of length 64 but applies
> > ep0 max_packet=8 for short-packet detection.  
> 
> That's exactly what we want it to do.

Should we want it, though?

Were there known devices with bMaxPacketSize0 <= 16 which required the
whole transfer to complete before they are reset?

That's not what happens in the initial iteration, so it seems any such
devices would always take two iterations to begin working.

Is this what the only correct Proprietary Operating System does?

> >  If the device's first
> > data packet is exactly 8 bytes and it sends no ZLP, the xHC does not
> > recognise a short packet, keeps issuing IN tokens for the remaining
> > 56 bytes, and the control transfer hangs in the Data stage until URB
> > timeout.  
> 
> That is the correct behavior.  Indeed, the same issue applies to _any_ 
> 64-byte transfer (not just control and not just Get-Descriptor) when the 
> maxpacket value is 8.  If the device sends an 8-byte reply with no ZLP 
> following, the transfer will hang in the data stage.  That's _supposed_ 
> to happen.

Right, all those details about HC behavior and xHCI internals aren't
important here. The device is out of spec, apparently expecting the
host to always use MPS=64 for this particular request. It should send
the full descriptor in response to further polling.

More useful would be information about impact: what this abomination
is, does it always fail to enumerate on the first try and trigger this
unsuccessful retry, or only in exceptional cases? And of course,

> Have you observed this problem in actual testing or is it purely 
> theoretical?

Regards,
Michal

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-04 17:36   ` Michal Pecio
@ 2026-11-05  5:06     ` Alan Stern
  2026-11-05 12:30       ` Michal Pecio
  0 siblings, 1 reply; 11+ messages in thread
From: Alan Stern @ 2026-11-05  5:06 UTC (permalink / raw)
  To: Michal Pecio
  Cc: Pengfei Lv, gregkh, linux-usb, linux-kernel, stable, xuhaiyang5,
	xuetao09, caiyadong, suzhuangluan

On Wed, Nov 04, 2026 at 06:36:03PM +0100, Michal Pecio wrote:
> On Wed, 4 Nov 2026 09:59:24 -0500, Alan Stern wrote:
> > On Wed, Nov 04, 2026 at 08:44:02PM +0800, Pengfei Lv wrote:
> > > When the subsequent re-enumeration then uses the "new scheme" and
> > > issues a 64-byte GET_DESCRIPTOR (get_bMaxPacketSize0 with size=64),
> > > xhci_check_maxpacket() sees hw==sw==8 and does not issue an Evaluate
> > > Context command.  The xHC queues a Data TRB of length 64 but applies
> > > ep0 max_packet=8 for short-packet detection.  
> > 
> > That's exactly what we want it to do.
> 
> Should we want it, though?
> 
> Were there known devices with bMaxPacketSize0 <= 16 which required the
> whole transfer to complete before they are reset?
> 
> That's not what happens in the initial iteration, so it seems any such
> devices would always take two iterations to begin working.
> 
> Is this what the only correct Proprietary Operating System does?

I don't know offhand of any positive or negative examples.  But if a 
device was going to hang if it was asked for the remaining part of 
the device descriptor after a non-initial reset, how would it ever get 
through initialization?  The procedure does two resets, and after the 
second reset the device has to send the full descriptor.

I don't know what other OSes do about this.

Alan Stern

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-04 12:44 [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset Pengfei Lv
  2026-11-04 12:57 ` sashiko-bot
  2026-11-04 14:59 ` Alan Stern
@ 2026-11-05  8:41 ` Pengfei Lv
  2026-11-05 15:46   ` Alan Stern
  2026-11-06  3:16 ` [PATCH v2] " Pengfei Lv
  3 siblings, 1 reply; 11+ messages in thread
From: Pengfei Lv @ 2026-11-05  8:41 UTC (permalink / raw)
  To: linux-usb
  Cc: stern, michal.pecio, gregkh, sashiko, xuhaiyang5, xuetao09,
	caiyadong, suzhuangluan, Pengfei Lv

Alan Stern wrote:
> > xhci_check_maxpacket() sees hw==sw==8 and does not issue an Evaluate
> > Context command.
>
> That's exactly what we want it to do.

Thank you both for the detailed review.  Let me answer the questions
and share what we observed in actual testing, and then address the
concern about initialization raised later in the thread.

This is a real problem observed in production on the 6.6 LTS kernel,
not theoretical.  The device is a Full-Speed USB banking token (U-key)
with bMaxPacketSize0 = 8.  The failure is 100% reproducible in a
virtual-machine USB passthrough scenario: after the VM sends
USBDEVFS_RESET to re-claim the device, the reset re-enumeration hangs
every time on kernels containing ff33299ec8bb.

> Have you observed this problem in actual testing or is it purely
> theoretical?

We captured the bus traffic with an Ellisys analyzer on both a 5.10
kernel (before ff33299ec8bb, works) and a 6.6 kernel (after
ff33299ec8bb, hangs).  The device behavior is identical in both
captures; only the host's short-packet detection differs.

The failing transfer is the 64-byte GET_DESCRIPTOR(Device) that
get_bMaxPacketSize0() issues under the "new scheme".  Here is the
side-by-side comparison of that single control transfer:

  5.10 (works, ep0 max_packet = 64)        6.6 (hangs, ep0 max_packet = 8)
  ---------------------------------------  ---------------------------------------
  SETUP  80 06 00 01 00 00 40 00          SETUP  80 06 00 01 00 00 40 00
  IN (x2, device NAK)                     IN (x2, device NAK)
  IN     12 01 10 01 00 00 00 08          IN     12 01 10 01 00 00 00 08
  OUT    (status stage, transfer done)    IN (x 482748, device never replies)
                                          ... 5 s timeout ...

Both captures show the device returning the same 8-byte packet
(the first 8 bytes of the 18-byte device descriptor; the last byte
08 is bMaxPacketSize0) and then going silent — no more data, no
ZLP.  On 5.10, 8 < 64 is a short packet, so the transfer completes
in < 0.1 ms and enumeration proceeds.  On 6.6, 8 == 8 is not a
short packet, so the host keeps issuing IN tokens (482748 of them)
until the URB times out ~5 s later.

After the timeout the reset-retry path eventually falls back to the
"old scheme", which issues an 8-byte GET_DESCRIPTOR, and the device
becomes usable for that iteration.  But the root cause is still
present, so the next reset — whether triggered by the kernel retry
or by the VM application re-issuing USBDEVFS_RESET — takes the
"new scheme" path again and hangs the same way.  The device is
never reliably usable in the VM.

> > If the device's first data packet is exactly 8 bytes and it sends
> > no ZLP, the xHC does not recognise a short packet, keeps issuing
> > IN tokens for the remaining 56 bytes, and the control transfer
> > hangs in the Data stage until URB timeout.
>
> That is the correct behavior.

> > (A standard 18-byte descriptor returned as 8+8+2 is not affected
> > because the final 2-byte packet is a short packet.)
>
> Quite right.  Why wouldn't the device reply with a standard 18-byte
> descriptor in this situation?

This is the key point, and it also answers the later question about
why the device ever gets through initialization:

  - For a 64-byte GET_DESCRIPTOR (wLength=64, the get_bMaxPacketSize0
    probe), the device sends exactly one 8-byte packet and then stops
    responding to IN tokens.  It relies on the host's short-packet
    detection with max_packet=64 to terminate the transfer after that
    first packet.  Both the 5.10 and 6.6 captures confirm this; the
    device's bus behavior is identical.

  - For an 18-byte GET_DESCRIPTOR (wLength=18, the
    usb_get_device_descriptor() read that runs after SetAddress), the
    same device sends the full 18 bytes as 8 + 8 + 2.  The final
    2-byte packet is a short packet regardless of whether max_packet
    is 8 or 64, so this transfer succeeds on both kernels.

So the two GET_DESCRIPTOR transfers are not the same request and do
not get the same response.  The 64-byte probe is the one that hangs
when max_packet=8; the 18-byte read never hangs because its last
packet is always short.  That is why the device can complete
initialization (the 18-byte read works) yet still hang on reset
re-enumeration (the 64-byte probe does not).

> If the device would hang when asked to send the remaining descriptor
> bytes, how would it ever get through initialization?  The procedure
> does two resets, and after the second reset the device has to send
> the full descriptor.

It sends the full descriptor for the 18-byte read, and that is the
read that runs after the second reset (SetAddress is followed by
usb_get_device_descriptor(), wLength=18).  The transfer that hangs is
the earlier 64-byte probe, which only runs on the "new scheme" path
and only before SetAddress.

To understand why the first enumeration succeeds but a reset hangs,
it helps to track the xHC ep0 max_packet across the lifecycle:

  - First enumeration (power-on): the switch block in hub_port_init()
    guesses wMaxPacketSize=64 for a Full-Speed device, and the
    Address Device command writes that 64 into the xHC ep0 context,
    so the 64-byte probe sees 8 < 64, completes as a short packet,
    and reads the real bMaxPacketSize0=8.  Later in hub_port_init()
    the sw maxpacket is corrected to 8 and usb_ep0_reinit() is
    called; the xHCI driver then sees hw(64) != sw(8) and issues an
    Evaluate Context command that lowers the xHC ep0 context to 8.
    By the end of the first enumeration the xHC ep0 context holds 8.

  - Reset before ff33299ec8bb: the switch block ran *unconditionally*,
    so it re-guessed wMaxPacketSize=64 on every reset.  The ep0
    re-sync then issued an Evaluate Context command to raise the xHC
    ep0 context from 8 back to 64 before the 64-byte probe, so
    8 < 64 was a short packet and the probe succeeded.

  - Reset after ff33299ec8bb: the if(initial) guard skips the
    re-guess, so sw maxpacket stays at 8.  The xHC ep0 context is
    already 8 (left over from the first enumeration), the maxpacket
    check sees hw == sw == 8, issues no Evaluate Context, and the
    64-byte probe sees 8 == 8 — not a short packet — so the Data
    stage hangs.

In other words, before ff33299ec8bb the reset path re-raised the xHC
ep0 context from 8 to 64 (via the re-guess + Evaluate Context) and
the device worked.  After ff33299ec8bb, the re-guess is skipped, the
xHC ep0 context stays at 8, and the 64-byte probe hangs.  This is a
regression introduced by ff33299ec8bb, not a pre-existing device
incompatibility — the very same device on the very same bus works on
5.10 and on 6.6 with old_scheme_first=Y (which skips the 64-byte
probe entirely).

I agree the device is not strictly compliant (it should continue
answering IN tokens for the 64-byte request).  However, this class of
USB banking token is widely deployed in enterprise environments, and
the regression breaks a configuration that previously worked.

Michal Pecio wrote:
> Should we want it, though?  What is the impact?

The impact is that a Full-Speed device with bMaxPacketSize0=8 that
relies on host-side short-packet detection for the 64-byte descriptor
probe becomes unusable across VM USB passthrough resets on 6.6+,
while it works on 5.10 and on 6.6 with old_scheme_first=Y.  The
device is not rare in our deployment; it is the standard enterprise
banking token form factor.

The purpose of ff33299ec8bb was to stop overwriting udev->descriptor
after registration, to fix a syzbot-reported KASAN race.  The ep0
wMaxPacketSize re-guess is a separate concern: udev->ep0.desc is not
part of udev->descriptor and is not the field involved in the
read_descriptors() race.  Would it be acceptable to keep the
descriptor-immutability protection while restoring the wMaxPacketSize
re-guess on the reset path, so the xHC ep0 context stays at 64 until
the real bMaxPacketSize0 is re-read from the descriptor?

Best regards,
Pengfei Lv

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-05  5:06     ` Alan Stern
@ 2026-11-05 12:30       ` Michal Pecio
  0 siblings, 0 replies; 11+ messages in thread
From: Michal Pecio @ 2026-11-05 12:30 UTC (permalink / raw)
  To: Alan Stern
  Cc: Pengfei Lv, gregkh, linux-usb, linux-kernel, stable, xuhaiyang5,
	xuetao09, caiyadong, suzhuangluan

On Thu, 5 Nov 2026 00:06:24 -0500, Alan Stern wrote:
> On Wed, Nov 04, 2026 at 06:36:03PM +0100, Michal Pecio wrote:
> > Were there known devices with bMaxPacketSize0 <= 16 which required
> > the whole transfer to complete before they are reset?
> > 
> > That's not what happens in the initial iteration, so it seems any
> > such devices would always take two iterations to begin working.
> > 
> > Is this what the only correct Proprietary Operating System does?  
> 
> I don't know offhand of any positive or negative examples.

Pengfei believes that the current behavior has only existed since 2023,
so perhaps no device depends on it for enumeration yet. And proposed
reset matches initial enumeration, so it seems low risk.

> But if a device was going to hang if it was asked for the remaining
> part of the device descriptor after a non-initial reset, how would it
> ever get through initialization?  The procedure does two resets, and
> after the second reset the device has to send the full descriptor.

I guess a device can harcode enumeration, including the 2nd reset, as
fixed sequence. Or it may send 8-byte descriptors while in the default
state or when wLength=64. Not sure why it would do that, though.

> I don't know what other OSes do about this.

I tried W10 in QEMU with qemu-xhci host controller, the device was an
audio dongle with a HID function and bMaxPacketSize0=8 passed through.
I disconnected D- to get errors without port disconnection detected.

Windows doesn't seem to switch packet size back to 64, it only disables
non-default endpoints, checks port status, resets it, switches xHCI
device state to "default" and then reads 64 bytes of device descriptor.

This succeeds because host kernel has already configured proper packet
size by this time. Not sure what Windows would od if this hanged.

Later of course second reset, address device, normal enumeration.

Regards,
Michal

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-05  8:41 ` Pengfei Lv
@ 2026-11-05 15:46   ` Alan Stern
  0 siblings, 0 replies; 11+ messages in thread
From: Alan Stern @ 2026-11-05 15:46 UTC (permalink / raw)
  To: Pengfei Lv
  Cc: linux-usb, michal.pecio, gregkh, sashiko, xuhaiyang5, xuetao09,
	caiyadong, suzhuangluan

On Thu, Nov 05, 2026 at 04:41:59PM +0800, Pengfei Lv wrote:
> Alan Stern wrote:
> > Quite right.  Why wouldn't the device reply with a standard 18-byte
> > descriptor in this situation?
> 
> This is the key point, and it also answers the later question about
> why the device ever gets through initialization:
> 
>   - For a 64-byte GET_DESCRIPTOR (wLength=64, the get_bMaxPacketSize0
>     probe), the device sends exactly one 8-byte packet and then stops
>     responding to IN tokens.  It relies on the host's short-packet
>     detection with max_packet=64 to terminate the transfer after that
>     first packet.  Both the 5.10 and 6.6 captures confirm this; the
>     device's bus behavior is identical.
> 
>   - For an 18-byte GET_DESCRIPTOR (wLength=18, the
>     usb_get_device_descriptor() read that runs after SetAddress), the
>     same device sends the full 18 bytes as 8 + 8 + 2.  The final
>     2-byte packet is a short packet regardless of whether max_packet
>     is 8 or 64, so this transfer succeeds on both kernels.
> 
> So the two GET_DESCRIPTOR transfers are not the same request and do
> not get the same response.  The 64-byte probe is the one that hangs
> when max_packet=8; the 18-byte read never hangs because its last
> packet is always short.  That is why the device can complete
> initialization (the 18-byte read works) yet still hang on reset
> re-enumeration (the 64-byte probe does not).

Okay, this is a good point.  It should be mentioned in the patch 
description, demonstrating that the problem is a real one.

> I agree the device is not strictly compliant (it should continue
> answering IN tokens for the 64-byte request).  However, this class of
> USB banking token is widely deployed in enterprise environments, and
> the regression breaks a configuration that previously worked.

Mention that too.

> The purpose of ff33299ec8bb was to stop overwriting udev->descriptor
> after registration, to fix a syzbot-reported KASAN race.  The ep0
> wMaxPacketSize re-guess is a separate concern: udev->ep0.desc is not
> part of udev->descriptor and is not the field involved in the
> read_descriptors() race.  Would it be acceptable to keep the
> descriptor-immutability protection while restoring the wMaxPacketSize
> re-guess on the reset path, so the xHC ep0 context stays at 64 until
> the real bMaxPacketSize0 is re-read from the descriptor?

Yes, that would be acceptable.

Alan Stern

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-04 12:44 [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset Pengfei Lv
                   ` (2 preceding siblings ...)
  2026-11-05  8:41 ` Pengfei Lv
@ 2026-11-06  3:16 ` Pengfei Lv
  2026-11-06  3:34   ` sashiko-bot
  2026-11-06  7:17   ` Michal Pecio
  3 siblings, 2 replies; 11+ messages in thread
From: Pengfei Lv @ 2026-11-06  3:16 UTC (permalink / raw)
  To: gregkh
  Cc: stern, michal.pecio, linux-usb, linux-kernel, stable, xuhaiyang5,
	xuetao09, caiyadong, suzhuangluan, Pengfei Lv

Per USB 2.0 spec section 5.5.3, the ep0 maxpacket size is fixed for
all device speeds except full-speed devices, where it can be 8, 16,
32, or 64 bytes and can only be determined by reading the device
descriptor.  During the initial enumeration of a full-speed device,
the USB core guesses 64 bytes (hub_port_init() with initial=true);
after reading the descriptor, the real value (e.g. 8) is stored in
udev->ep0.desc.wMaxPacketSize and propagated to the xHCI ep0 context
via an Evaluate Context command.

After a port reset (usb_reset_and_verify_device() -> hub_port_init()
with initial=false), the "if (initial)" guard added by commit
ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()")
skips the ep0 maxpacket guess block, so
udev->ep0.desc.wMaxPacketSize retains the real value (e.g. 8).  The
xHCI Reset Device command preserves the ep0 context as well, so the
xHC ep0 max_packet also stays at 8.

When the subsequent re-enumeration then uses the "new scheme" and
issues a 64-byte GET_DESCRIPTOR (get_bMaxPacketSize0 with size=64),
the xHC queues a Data TRB of length 64 but applies ep0 max_packet=8
for short-packet detection.  If the device's first data packet is
exactly 8 bytes and it sends no ZLP, the xHC does not recognise a
short packet, keeps issuing IN tokens for the remaining 56 bytes, and
the control transfer hangs in the Data stage until URB timeout.

This is not merely theoretical.  It was observed in production on a
6.6 LTS kernel with a Full-Speed USB banking token (bMaxPacketSize0
= 8) in a virtual-machine USB passthrough scenario; the reset
re-enumeration hangs 100% of the time after the VM issues
USBDEVFS_RESET.  Bus captures with an Ellisys analyzer confirm that
the device sends exactly one 8-byte packet for the 64-byte request
and then stops responding to IN tokens, relying on the host's
short-packet detection with max_packet=64 to terminate the transfer.
For the subsequent 18-byte GET_DESCRIPTOR (usb_get_device_descriptor,
wLength=18) the same device sends the full 18 bytes as 8 + 8 + 2;
the final 2-byte packet is always a short packet regardless of
max_packet, so that transfer succeeds on both kernels.  This is why
the device completes initial enumeration yet hangs on reset
re-enumeration.

The device is not strictly compliant (it should continue answering
IN tokens for the 64-byte request), but this class of USB banking
token is widely deployed in enterprise environments, and the
regression introduced by commit ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()") breaks a configuration that
previously worked.

Fix this by removing the "if (initial)" guard so that the ep0 maxpacket
size is re-guessed from udev->speed on every reset, and by calling
usb_ep0_reinit() afterwards when !initial so that the new software
value is propagated to the xHCI ep0 context via an Evaluate Context
command before the GET_DESCRIPTOR data stage.

The usb_ep0_reinit() call is necessary because commit e34900f46cd6
("xhci: Reconfigure endpoint 0 max packet size only during endpoint reset")
moved the ep0 maxpacket sync point from xhci_urb_enqueue()
(xhci_check_maxpacket, lazy per-URB) to xhci_endpoint_reset()
(xhci_check_ep0_maxpacket, eager), reachable only via
usb_ep0_reinit() -> usb_enable_endpoint(reset_ep=true) ->
usb_hcd_reset_endpoint() -> xhci_endpoint_reset().  Without the
explicit usb_ep0_reinit() after the re-guess, no Evaluate Context
command is issued on the reset path: the pre-existing
usb_ep0_reinit() in usb_reset_and_verify_device() runs before
hub_port_init() and thus syncs the stale value, and the subsequent
hub_enable_device() -> xhci_setup_device(SETUP_CONTEXT_ONLY) returns
early when the slot is already in SLOT_STATE_DEFAULT.

The !initial guard on the new usb_ep0_reinit() call ensures that
initial enumeration is unaffected: on first enumeration the ep0
context is initialised by the Address Device command, so no extra
Evaluate Context is needed.  For high-speed, low-speed, and
SuperSpeed-or-faster devices, xhci_endpoint_reset() returns early
(speed != USB_SPEED_FULL), so the call is a no-op on xHCI, and on
non-xHCI host controllers endpoint_reset for control endpoints is
likewise a no-op (e.g. ehci_endpoint_reset only handles bulk and
interrupt endpoints).  The descriptor-immutability checks (!initial
&& maxp0 != udev->descriptor.bMaxPacketSize0) remain unchanged;
udev->ep0.desc is not part of udev->descriptor and is not the field
involved in the read_descriptors() race that commit ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()") fixed, so
restoring the wMaxPacketSize re-guess does not reopen that race.

Fixes: ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()")
Cc: stable@vger.kernel.org
Signed-off-by: Pengfei Lv <lvpengfei20@huawei.com>
---

Changes in v2:
- Add usb_ep0_reinit(udev) after the ep0 maxpacket re-guess block,
  guarded by !initial, to propagate the new wMaxPacketSize to the
  xHCI ep0 context.  v1 only removed the if(initial) guard, which
  is insufficient after commit e34900f46cd6 ("xhci: Reconfigure endpoint 0 max packet size only during endpoint reset")
  moved the xHCI ep0
  maxpacket sync point from xhci_urb_enqueue() (xhci_check_maxpacket,
  lazy per-URB) to xhci_endpoint_reset() (xhci_check_ep0_maxpacket,
  eager), reachable only via usb_ep0_reinit().  The !initial guard
  avoids a redundant Evaluate Context on first enumeration, where the
  Address Device command already initialises the ep0 context to 64.
  (Sashiko AI review)
- Expand commit message with observed-in-production evidence and the
  wLength=64 vs wLength=18 device behavior distinction, per Alan
  Stern's review.
- Note: on kernels before e34900f46cd6 (e.g. 6.6 LTS), the xHCI ep0
  maxpacket sync happens lazily in xhci_urb_enqueue() via
  xhci_check_maxpacket(), so removing the if(initial) guard alone is
  sufficient and the extra usb_ep0_reinit() call is a no-op for ep0
  maxpacket sync (xhci_endpoint_reset returns early for control
  endpoints on those kernels).  This patch targets current mainline
  where e34900f46cd6 is already present.  Would the maintainers
  prefer a separate, simpler patch (without the usb_ep0_reinit()
  addition) for stable branches predating e34900f46cd6, or is this
  single patch acceptable for backport given that the
  usb_ep0_reinit() call is harmless on those kernels?
---
 drivers/usb/core/hub.c | 49 +++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 199d3f535a89..a6c260d39094 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4969,33 +4969,34 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 	}
 	oldspeed = udev->speed;
 
-	if (initial) {
-		/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
-		 * it's fixed size except for full speed devices.
+	/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
+	 * it's fixed size except for full speed devices.
+	 */
+	switch (udev->speed) {
+	case USB_SPEED_SUPER_PLUS:
+	case USB_SPEED_SUPER:
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
+		break;
+	case USB_SPEED_HIGH:		/* fixed at 64 */
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
+		break;
+	case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
+		/* to determine the ep0 maxpacket size, try to read
+		 * the device descriptor to get bMaxPacketSize0 and
+		 * then correct our initial guess.
 		 */
-		switch (udev->speed) {
-		case USB_SPEED_SUPER_PLUS:
-		case USB_SPEED_SUPER:
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
-			break;
-		case USB_SPEED_HIGH:		/* fixed at 64 */
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
-			break;
-		case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
-			/* to determine the ep0 maxpacket size, try to read
-			 * the device descriptor to get bMaxPacketSize0 and
-			 * then correct our initial guess.
-			 */
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
-			break;
-		case USB_SPEED_LOW:		/* fixed at 8 */
-			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
-			break;
-		default:
-			goto fail;
-		}
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
+		break;
+	case USB_SPEED_LOW:		/* fixed at 8 */
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
+		break;
+	default:
+		goto fail;
 	}
 
+	if (!initial)
+		usb_ep0_reinit(udev);
+
 	speed = usb_speed_string(udev->speed);
 
 	/*
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-06  3:16 ` [PATCH v2] " Pengfei Lv
@ 2026-11-06  3:34   ` sashiko-bot
  2026-11-06  7:17   ` Michal Pecio
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-11-06  3:34 UTC (permalink / raw)
  To: Pengfei Lv; +Cc: linux-usb

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: core: hub: usb_ep0_reinit() on unaddressed xHCI slot in hub_port_init()
--

commit d0ddaa8105de736e42e89c86a81968c860e72871
Author: Pengfei Lv <lvpengfei20@huawei.com>

usb: core: hub: Re-guess ep0 maxpacket size on reset

This commit removes the initial guard around the ep0 maxpacket size guess in
hub_port_init() and calls usb_ep0_reinit() on reset so that full-speed devices
with bMaxPacketSize0 < 64 have their ep0 maxpacket reset to 64 in both USB
core and the xHCI ep0 context before re-enumeration issues a 64-byte
GET_DESCRIPTOR request.

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 199d3f535a891..a6c260d390945 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4969,33 +4969,34 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>  	}
>  	oldspeed = udev->speed;
>  
> -	if (initial) {
> -		/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
> -		 * it's fixed size except for full speed devices.
> +	/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
> +	 * it's fixed size except for full speed devices.
> +	 */
> +	switch (udev->speed) {
> +	case USB_SPEED_SUPER_PLUS:
> +	case USB_SPEED_SUPER:
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
> +		break;
> +	case USB_SPEED_HIGH:		/* fixed at 64 */
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> +		break;
> +	case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
> +		/* to determine the ep0 maxpacket size, try to read
> +		 * the device descriptor to get bMaxPacketSize0 and
> +		 * then correct our initial guess.
>  		 */
> -		switch (udev->speed) {
> -		case USB_SPEED_SUPER_PLUS:
> -		case USB_SPEED_SUPER:
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
> -			break;
> -		case USB_SPEED_HIGH:		/* fixed at 64 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> -			break;
> -		case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
> -			/* to determine the ep0 maxpacket size, try to read
> -			 * the device descriptor to get bMaxPacketSize0 and
> -			 * then correct our initial guess.
> -			 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> -			break;
> -		case USB_SPEED_LOW:		/* fixed at 8 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
> -			break;
> -		default:
> -			goto fail;
> -		}
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> +		break;
> +	case USB_SPEED_LOW:		/* fixed at 8 */
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
> +		break;
> +	default:
> +		goto fail;
>  	}
>  
> +	if (!initial)
> +		usb_ep0_reinit(udev);

[Severity: Medium]
Can calling usb_ep0_reinit(udev) here before hub_enable_device() or
hub_set_address() trigger an invalid Evaluate Context command when the xHCI
slot has been newly allocated during reset?

Earlier in drivers/usb/core/hub.c:hub_port_init(), hub_port_reset() invokes
hcd->driver->reset_device(), which resolves to
drivers/usb/host/xhci.c:xhci_discover_or_reset_device(). When the xHCI slot
was freed and reallocated via xhci_alloc_dev() (for example on Etron hosts
with XHCI_ETRON_HOST, or after host controller power loss / reset on resume),
the newly allocated virt_dev has a zeroed out_ctx in SLOT_STATE_DISABLED
because Address Device (xhci_setup_device()) has not run yet:

drivers/usb/host/xhci.c:xhci_discover_or_reset_device() {
	...
	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx);
	if (GET_SLOT_STATE(le32_to_cpu(slot_ctx->dev_state)) ==
						SLOT_STATE_DISABLED)
		return 0;

	if (xhci->quirks & XHCI_ETRON_HOST) {
		ret = xhci_disable_and_free_slot(xhci, udev->slot_id);
		if (!ret) {
			ret = xhci_alloc_dev(hcd, udev);
	...
}

When usb_ep0_reinit(udev) is then called for a full-speed device via
usb_enable_endpoint() -> usb_hcd_reset_endpoint() -> xhci_endpoint_reset() ->
xhci_check_ep0_maxpacket(), neither xhci_endpoint_reset() nor
xhci_check_ep0_maxpacket() checks whether vdev->out_ctx is still in
SLOT_STATE_DISABLED:

drivers/usb/host/xhci.c:xhci_check_ep0_maxpacket() {
	...
	ep_ctx = xhci_get_ep_ctx(xhci, vdev->out_ctx, 0);
	hw_max_packet_size = MAX_PACKET_DECODED(le32_to_cpu(ep_ctx->ep_info2));
	max_packet_size = usb_endpoint_maxp(&vdev->udev->ep0.desc);

	if (hw_max_packet_size == max_packet_size)
		return 0;
	...
}

Because vdev->out_ctx is still zeroed, hw_max_packet_size is 0 != 64, so
xhci_check_ep0_maxpacket() copies the zeroed out_ctx ep0 into in_ctx ep0 and
issues an Evaluate Context command on an unaddressed slot in the Enabled
state, which the xHC rejects with COMP_CONTEXT_STATE_ERROR (logging a warning
in xhci_evaluate_context_result()).

Furthermore, if a previous attempt in xhci_setup_device() failed with
COMP_USB_TRANSACTION_ERROR, it reallocates the slot via xhci_alloc_dev() and
calls xhci_setup_addressable_virt_dev(xhci, udev), which populates
vdev->in_ctx (including setting slot_ctx->dev_info and initializing in_ctx
ep0) while vdev->out_ctx remains zeroed. If hub_port_init() retries and calls
usb_ep0_reinit(udev) before the next xhci_setup_device(),
xhci_check_ep0_maxpacket() overwrites vdev->in_ctx ep0 from the zeroed
vdev->out_ctx while leaving slot_ctx->dev_info non-zero:

drivers/usb/host/xhci.c:xhci_setup_device() {
	...
	if (!slot_ctx->dev_info)
		xhci_setup_addressable_virt_dev(xhci, udev);
	/* Otherwise, update the control endpoint ring enqueue pointer. */
	else
		xhci_copy_ep0_dequeue_into_input_ctx(xhci, udev);
	...
}

Wouldn't xhci_setup_device() then skip xhci_setup_addressable_virt_dev() and
submit Address Device with a zeroed ep0 context (EP_TYPE == 0)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106031625.1824732-1-lvpengfei20@huawei.com?part=1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] usb: core: hub: Re-guess ep0 maxpacket size on reset
  2026-11-06  3:16 ` [PATCH v2] " Pengfei Lv
  2026-11-06  3:34   ` sashiko-bot
@ 2026-11-06  7:17   ` Michal Pecio
  1 sibling, 0 replies; 11+ messages in thread
From: Michal Pecio @ 2026-11-06  7:17 UTC (permalink / raw)
  To: Pengfei Lv
  Cc: gregkh, stern, linux-usb, linux-kernel, stable, xuhaiyang5,
	xuetao09, caiyadong, suzhuangluan

On Fri, 6 Nov 2026 11:16:25 +0800, Pengfei Lv wrote:
> Per USB 2.0 spec section 5.5.3, the ep0 maxpacket size is fixed for
> all device speeds except full-speed devices, where it can be 8, 16,
> 32, or 64 bytes and can only be determined by reading the device
> descriptor.  During the initial enumeration of a full-speed device,
> the USB core guesses 64 bytes (hub_port_init() with initial=true);
> after reading the descriptor, the real value (e.g. 8) is stored in
> udev->ep0.desc.wMaxPacketSize and propagated to the xHCI ep0 context
> via an Evaluate Context command.

  ... and applied to hardware by the usb_ep0_reinit() call.

Is there anything xHCI-specific about this issue?
Any properly working HCD should behave equivalently.

> 
> After a port reset (usb_reset_and_verify_device() -> hub_port_init()
> with initial=false), the "if (initial)" guard added by commit
> ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()")
>
> skips the ep0 maxpacket guess block, so
> udev->ep0.desc.wMaxPacketSize retains the real value (e.g. 8).  The
> xHCI Reset Device command preserves the ep0 context as well, so the
> xHC ep0 max_packet also stays at 8.

So it's another unimportant xHCI implementation detail.

Doesn't need to be mentioned at all; it's obvious that if we don't
change ep0.desc then there is no reason to update the HW either.
 
> When the subsequent re-enumeration then uses the "new scheme" and
> issues a 64-byte GET_DESCRIPTOR (get_bMaxPacketSize0 with size=64),
> the xHC queues a Data TRB

xHC is the HW, it doesn't queue any TRBs.

Why not just omit all those irrelevant details of how xHCI implements
policies decided by this code?

> of length 64 but applies ep0 max_packet=8
> for short-packet detection.  If the device's first data packet is
> exactly 8 bytes and it sends no ZLP,

It certainly shouldn't be sending any ZLP if descriptor size is not
a multiple of packet size. This whole paragraph boils down to:

  If a buggy device responds with one 8 byte packet and NAKs all
  further polling, the HC doesn't recognize a short transfer but
  keeps polling for the rest of the descriptor till unlink.

> the xHC does not recognise a
> short packet, keeps issuing IN tokens for the remaining 56 bytes, and
> the control transfer hangs in the Data stage until URB timeout.
> 
> This is not merely theoretical.  It was observed in production on a

s/is not merely theoretical. It// would still be fine.

> 6.6 LTS kernel with a Full-Speed USB banking token (bMaxPacketSize0
> = 8) in a virtual-machine USB passthrough scenario; the reset
> re-enumeration hangs 100% of the time after the VM issues
> USBDEVFS_RESET.  Bus captures with an Ellisys analyzer confirm that
> the device sends exactly one 8-byte packet for the 64-byte request
> and then stops responding to IN tokens, relying on the host's
> short-packet detection with max_packet=64 to terminate the transfer.
> For the subsequent 18-byte GET_DESCRIPTOR (usb_get_device_descriptor,
> wLength=18) the same device sends the full 18 bytes as 8 + 8 + 2;
> the final 2-byte packet is always a short packet regardless of
> max_packet, so that transfer succeeds on both kernels.  This is why
> the device completes initial enumeration yet hangs on reset
> re-enumeration.
> 
> The device is not strictly compliant (it should continue answering
> IN tokens for the 64-byte request), but this class of USB banking
> token is widely deployed in enterprise environments, and the
> regression introduced by commit ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()") breaks a configuration that
> previously worked.
> 
> Fix this by removing the "if (initial)" guard so that the ep0 maxpacket
> size is re-guessed from udev->speed on every reset, and by calling
> usb_ep0_reinit() afterwards when !initial so that the new software
> value is propagated to the xHCI ep0 context via an Evaluate Context
> command before the GET_DESCRIPTOR data stage.

More unnecessary xHCI details.

> The usb_ep0_reinit() call is necessary because commit e34900f46cd6
> ("xhci: Reconfigure endpoint 0 max packet size only during endpoint reset")
> moved the ep0 maxpacket sync point from xhci_urb_enqueue()
> (xhci_check_maxpacket, lazy per-URB) to xhci_endpoint_reset()
> (xhci_check_ep0_maxpacket, eager), reachable only via
> usb_ep0_reinit() -> usb_enable_endpoint(reset_ep=true) ->
> usb_hcd_reset_endpoint() -> xhci_endpoint_reset().  Without the
> explicit usb_ep0_reinit() after the re-guess, no Evaluate Context
> command is issued on the reset path: the pre-existing
> usb_ep0_reinit() in usb_reset_and_verify_device() runs before
> hub_port_init() and thus syncs the stale value, and the subsequent
> hub_enable_device() -> xhci_setup_device(SETUP_CONTEXT_ONLY) returns
> early when the slot is already in SLOT_STATE_DEFAULT.

Does this need to be explained?

usb_ep0_reinit() is quite self-explanatory, or its effects could be
explained with one HCD-agnostic sentence if you must.

> The !initial guard on the new usb_ep0_reinit() call ensures that
> initial enumeration is unaffected: on first enumeration the ep0
> context is initialised by the Address Device command, so no extra
> Evaluate Context is needed.

This, of course, makes assumption that xhci-hcd has guessed the same
initial values as USB core. (Which it currently does).

Not sure if this optimization is worth bothering with. For xhci-hcd
at least, usb_ep0_reniti() exits quickly if no change occurred. And
you say it's a no-op for others? Great, who cares if we call it.

>  For high-speed, low-speed, and
> SuperSpeed-or-faster devices, xhci_endpoint_reset() returns early
> (speed != USB_SPEED_FULL), so the call is a no-op on xHCI, and on
> non-xHCI host controllers endpoint_reset for control endpoints is
> likewise a no-op (e.g. ehci_endpoint_reset only handles bulk and
> interrupt endpoints).  The descriptor-immutability checks (!initial
> && maxp0 != udev->descriptor.bMaxPacketSize0) remain unchanged;
> udev->ep0.desc is not part of udev->descriptor and is not the field
> involved in the read_descriptors() race that commit ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()") fixed, so
> restoring the wMaxPacketSize re-guess does not reopen that race.
> 
> Fixes: ff33299ec8bb ("USB: core: Fix race by not overwriting udev->descriptor in hub_port_init()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pengfei Lv <lvpengfei20@huawei.com>
> ---
> 
> Changes in v2:
> - Add usb_ep0_reinit(udev) after the ep0 maxpacket re-guess block,
>   guarded by !initial, to propagate the new wMaxPacketSize to the
>   xHCI ep0 context.  v1 only removed the if(initial) guard, which
>   is insufficient after commit e34900f46cd6 ("xhci: Reconfigure endpoint 0 max packet size only during endpoint reset")
>   moved the xHCI ep0
>   maxpacket sync point from xhci_urb_enqueue() (xhci_check_maxpacket,
>   lazy per-URB) to xhci_endpoint_reset() (xhci_check_ep0_maxpacket,
>   eager), reachable only via usb_ep0_reinit().  The !initial guard
>   avoids a redundant Evaluate Context on first enumeration, where the
>   Address Device command already initialises the ep0 context to 64.
>   (Sashiko AI review)
> - Expand commit message with observed-in-production evidence and the
>   wLength=64 vs wLength=18 device behavior distinction, per Alan
>   Stern's review.
> - Note: on kernels before e34900f46cd6 (e.g. 6.6 LTS), the xHCI ep0
>   maxpacket sync happens lazily in xhci_urb_enqueue() via
>   xhci_check_maxpacket(), so removing the if(initial) guard alone is
>   sufficient and the extra usb_ep0_reinit() call is a no-op for ep0
>   maxpacket sync (xhci_endpoint_reset returns early for control
>   endpoints on those kernels).  This patch targets current mainline
>   where e34900f46cd6 is already present.  Would the maintainers
>   prefer a separate, simpler patch (without the usb_ep0_reinit()
>   addition) for stable branches predating e34900f46cd6, or is this
>   single patch acceptable for backport given that the
>   usb_ep0_reinit() call is harmless on those kernels?
> ---
>  drivers/usb/core/hub.c | 49 +++++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 199d3f535a89..a6c260d39094 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4969,33 +4969,34 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>  	}
>  	oldspeed = udev->speed;
>  
> -	if (initial) {
> -		/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
> -		 * it's fixed size except for full speed devices.
> +	/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
> +	 * it's fixed size except for full speed devices.
> +	 */
> +	switch (udev->speed) {
> +	case USB_SPEED_SUPER_PLUS:
> +	case USB_SPEED_SUPER:
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
> +		break;
> +	case USB_SPEED_HIGH:		/* fixed at 64 */
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> +		break;
> +	case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
> +		/* to determine the ep0 maxpacket size, try to read
> +		 * the device descriptor to get bMaxPacketSize0 and
> +		 * then correct our initial guess.
>  		 */
> -		switch (udev->speed) {
> -		case USB_SPEED_SUPER_PLUS:
> -		case USB_SPEED_SUPER:
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(512);
> -			break;
> -		case USB_SPEED_HIGH:		/* fixed at 64 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> -			break;
> -		case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
> -			/* to determine the ep0 maxpacket size, try to read
> -			 * the device descriptor to get bMaxPacketSize0 and
> -			 * then correct our initial guess.
> -			 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> -			break;
> -		case USB_SPEED_LOW:		/* fixed at 8 */
> -			udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
> -			break;
> -		default:
> -			goto fail;
> -		}
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> +		break;
> +	case USB_SPEED_LOW:		/* fixed at 8 */
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(8);
> +		break;
> +	default:
> +		goto fail;
>  	}
>  
> +	if (!initial)
> +		usb_ep0_reinit(udev);
> +
>  	speed = usb_speed_string(udev->speed);
>  
>  	/*
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-11-06  7:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04 12:44 [PATCH] usb: core: hub: Re-guess ep0 maxpacket size on reset Pengfei Lv
2026-11-04 12:57 ` sashiko-bot
2026-11-04 14:59 ` Alan Stern
2026-11-04 17:36   ` Michal Pecio
2026-11-05  5:06     ` Alan Stern
2026-11-05 12:30       ` Michal Pecio
2026-11-05  8:41 ` Pengfei Lv
2026-11-05 15:46   ` Alan Stern
2026-11-06  3:16 ` [PATCH v2] " Pengfei Lv
2026-11-06  3:34   ` sashiko-bot
2026-11-06  7:17   ` Michal Pecio

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.