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

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

[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

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

[Alan Stern]

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

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

[Michal Pecio]

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

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

[Alan Stern]

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

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

[Michal Pecio]

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

[PATCH v2] usb: core: hub: Re-guess ep0 maxpacket size on reset

[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

Re: [PATCH v2] usb: core: hub: Re-guess ep0 maxpacket size on reset

[Michal Pecio]

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
>