Linux USB
 help / color / mirror / Atom feed
* [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31
@ 2026-10-19 15:15 Lucas Pascal
  2026-10-19 16:11 ` sashiko-bot
  2026-10-19 19:06 ` Shuah Khan
  0 siblings, 2 replies; 4+ messages in thread
From: Lucas Pascal @ 2026-10-19 15:15 UTC (permalink / raw)
  To: Valentina Manea, Shuah Khan
  Cc: Hongren Zheng, Greg Kroah-Hartman, linux-usb, Lucas Pascal,
	stable

vhci_hcd numbers every CMD_SUBMIT and CMD_UNLINK from a per-HCD atomic_t
counter, and keeps the value in unsigned long fields of struct vhci_priv
and struct vhci_unlink. On the wire, the seqnum is a __u32, and the peer
echoes it back as such.

Once the counter passes INT_MAX, atomic_inc_return() returns a negative
int, which is sign-extended when stored in an unsigned long on 64-bit.
Commit 15b93f340fa6 ("usbip: Fix seqnum sign extension issue in
vhci_tx_urb") fixed this for submitted URBs with a (u32) cast, but
vhci_urb_dequeue() still stores the unlink seqnum without one. The
CMD_UNLINK goes out with the correct, truncated 32-bit value, but when
the RET_UNLINK comes back, dequeue_pending_unlink() compares the
sign-extended value with the 32-bit one and never finds it:

  vhci_hcd: unlink->seqnum 18446744071562096407
  vhci_hcd: cannot find the pending unlink 2147512087

The unlinked URB is then never given back. A task waiting for it in
usb_kill_urb() blocks forever, typically with the device lock held by
usbdev_do_ioctl(), and everything else touching that device (open,
ioctl, driver binding, rmmod, reboot) piles up behind it in D state.
Only a forced reboot recovers. On a machine importing a few FTDI-based
JTAG adapters through a local usbip loopback, the counter crossed 2^31
after about 30 days of uptime:

  INFO: task fpServer:2209312 blocked for more than 122 seconds.
  Call Trace:
   usb_kill_urb
   usbdev_do_ioctl
   usbdev_ioctl

The wire protocol uses 32-bit seqnums, so store them as u32 as well.
The counter then wraps around modulo 2^32 consistently on both the
submit and the unlink paths, instead of relying on a cast at each
assignment. Adjust the format specifiers that printed these fields
with %lu.

Fixes: 04679b3489e0 ("Staging: USB/IP: add client driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Lucas Pascal <lucas.pascal@pqshield.com>
---
Hi,

A bit of context on this fix:

- We heavily use USB/IP on CI/CD runners, in order to have full
  control/isolation for multiple USB development boards plugged into
  a powered hub. So USB/IP host and client are on the same machine.
  Once configured, a runner host stays up ad vitam.
- I found the bug while debugging a runner host lockup on Ubuntu
  7.0.0-31-generic, with an uptime > 1month. The log lines above are from
  that machine.
- The diagnosis, this patch and its changelog were prepared with the
  help of an AI coding assistant (Claude Opus 5.5), then reviewed by me.
- Build-tested on x86_64 and i386 with CONFIG_USBIP_DEBUG=y: no new
  warnings (W=1 reports a pre-existing -Wformat-truncation in
  vhci_sysfs.c, unrelated).
- checkpatch.pl --strict: clean.
- The patch was tested in QEMU (virtme-ng), not on the physical runner,
  using dummy_hcd + g_zero in loopback mode (bulk IN read only complete
  when unlinked), exported through a USB/IP loopback (bind + attach to
  localhost), running usbtest test 11 (unlink reads, 3000 iterations), with
  a test-only change starting vhci_hcd->seqnum at INT_MAX - 2000.
  Without this patch: the first unlink past 2^31 is lost
  ("unlink->seqnum 18446744071562067969", "cannot find the pending
  unlink 2147483649"), and testusb hangs in D state.
  With this patch: 5007 unlinks past 2^31, none lost, testusb passes.
- The explicit (u32) cast added by 15b93f340fa6 in vhci_tx_urb() is now
  redundant. I left it alone to keep this fix minimal.

Thanks,
Lucas


 drivers/usb/usbip/vhci.h    | 6 +++---
 drivers/usb/usbip/vhci_rx.c | 4 ++--
 drivers/usb/usbip/vhci_tx.c | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 5659dce1526e..64c824c7d165 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -50,7 +50,7 @@ struct vhci_device {

 /* urb->hcpriv, use container_of() */
 struct vhci_priv {
-	unsigned long seqnum;
+	u32 seqnum;
 	struct list_head list;

 	struct vhci_device *vdev;
@@ -59,12 +59,12 @@ struct vhci_priv {

 struct vhci_unlink {
 	/* seqnum of this request */
-	unsigned long seqnum;
+	u32 seqnum;

 	struct list_head list;

 	/* seqnum of the unlink target */
-	unsigned long unlink_seqnum;
+	u32 unlink_seqnum;
 };

 enum hub_speed {
diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index a678e7c89837..c8a27778478d 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -118,9 +118,9 @@ static struct vhci_unlink *dequeue_pending_unlink(struct vhci_device *vdev,
 	spin_lock_irqsave(&vdev->priv_lock, flags);

 	list_for_each_entry_safe(unlink, tmp, &vdev->unlink_rx, list) {
-		pr_info("unlink->seqnum %lu\n", unlink->seqnum);
+		pr_info("unlink->seqnum %u\n", unlink->seqnum);
 		if (unlink->seqnum == pdu->base.seqnum) {
-			usbip_dbg_vhci_rx("found pending unlink, %lu\n",
+			usbip_dbg_vhci_rx("found pending unlink, %u\n",
 					  unlink->seqnum);
 			list_del(&unlink->list);

diff --git a/drivers/usb/usbip/vhci_tx.c b/drivers/usb/usbip/vhci_tx.c
index 32e6fabccf72..4b602849c14b 100644
--- a/drivers/usb/usbip/vhci_tx.c
+++ b/drivers/usb/usbip/vhci_tx.c
@@ -74,7 +74,7 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev)
 		memset(&msg, 0, sizeof(msg));
 		memset(&iov, 0, sizeof(iov));

-		usbip_dbg_vhci_tx("setup txdata urb seqnum %lu\n",
+		usbip_dbg_vhci_tx("setup txdata urb seqnum %u\n",
 				  priv->seqnum);

 		if (urb->num_sgs && usb_pipeout(urb->pipe))
@@ -201,7 +201,7 @@ static int vhci_send_cmd_unlink(struct vhci_device *vdev)
 		memset(&msg, 0, sizeof(msg));
 		memset(&iov, 0, sizeof(iov));

-		usbip_dbg_vhci_tx("setup cmd unlink, %lu\n", unlink->seqnum);
+		usbip_dbg_vhci_tx("setup cmd unlink, %u\n", unlink->seqnum);

 		/* 1. setup usbip_header */
 		pdu_header.base.command = USBIP_CMD_UNLINK;
--
2.43.0

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

* Re: [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31
  2026-10-19 15:15 [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31 Lucas Pascal
@ 2026-10-19 16:11 ` sashiko-bot
  2026-10-19 19:06 ` Shuah Khan
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-19 16:11 UTC (permalink / raw)
  To: Lucas Pascal; +Cc: linux-usb

> vhci_hcd numbers every CMD_SUBMIT and CMD_UNLINK from a per-HCD atomic_t
> counter, and keeps the value in unsigned long fields of struct vhci_priv
> and struct vhci_unlink. On the wire, the seqnum is a __u32, and the peer
> echoes it back as such.
> 
> Once the counter passes INT_MAX, atomic_inc_return() returns a negative
> int, which is sign-extended when stored in an unsigned long on 64-bit.
> Commit 15b93f340fa6 ("usbip: Fix seqnum sign extension issue in
> vhci_tx_urb") fixed this for submitted URBs with a (u32) cast, but
> vhci_urb_dequeue() still stores the unlink seqnum without one. The
> CMD_UNLINK goes out with the correct, truncated 32-bit value, but when
> the RET_UNLINK comes back, dequeue_pending_unlink() compares the
> sign-extended value with the 32-bit one and never finds it:
> 
>   vhci_hcd: unlink->seqnum 18446744071562096407
> [ ... ]
> 
> Fixes: 04679b3489e0 ("Staging: USB/IP: add client driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Lucas Pascal <lucas.pascal@pqshield.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019151537.112304-1-lucas.pascal@pqshield.com?part=1


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

* Re: [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31
  2026-10-19 15:15 [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31 Lucas Pascal
  2026-10-19 16:11 ` sashiko-bot
@ 2026-10-19 19:06 ` Shuah Khan
  2026-10-20 15:45   ` Lucas Pascal
  1 sibling, 1 reply; 4+ messages in thread
From: Shuah Khan @ 2026-10-19 19:06 UTC (permalink / raw)
  To: Lucas Pascal, Valentina Manea, Shuah Khan
  Cc: Hongren Zheng, Greg Kroah-Hartman, linux-usb, stable, Shuah Khan

On 10/19/26 09:15, Lucas Pascal wrote:
> vhci_hcd numbers every CMD_SUBMIT and CMD_UNLINK from a per-HCD atomic_t
> counter, and keeps the value in unsigned long fields of struct vhci_priv
> and struct vhci_unlink. On the wire, the seqnum is a __u32, and the peer
> echoes it back as such.
> 
> Once the counter passes INT_MAX, atomic_inc_return() returns a negative
> int, which is sign-extended when stored in an unsigned long on 64-bit.
> Commit 15b93f340fa6 ("usbip: Fix seqnum sign extension issue in
> vhci_tx_urb") fixed this for submitted URBs with a (u32) cast, but
> vhci_urb_dequeue() still stores the unlink seqnum without one. The
> CMD_UNLINK goes out with the correct, truncated 32-bit value, but when
> the RET_UNLINK comes back, dequeue_pending_unlink() compares the
> sign-extended value with the 32-bit one and never finds it:
> 
>    vhci_hcd: unlink->seqnum 18446744071562096407
>    vhci_hcd: cannot find the pending unlink 2147512087
> 
> The unlinked URB is then never given back. A task waiting for it in
> usb_kill_urb() blocks forever, typically with the device lock held by
> usbdev_do_ioctl(), and everything else touching that device (open,
> ioctl, driver binding, rmmod, reboot) piles up behind it in D state.
> Only a forced reboot recovers. On a machine importing a few FTDI-based
> JTAG adapters through a local usbip loopback, the counter crossed 2^31
> after about 30 days of uptime:
> 
>    INFO: task fpServer:2209312 blocked for more than 122 seconds.
>    Call Trace:
>     usb_kill_urb
>     usbdev_do_ioctl
>     usbdev_ioctl
> 
> The wire protocol uses 32-bit seqnums, so store them as u32 as well.
> The counter then wraps around modulo 2^32 consistently on both the
> submit and the unlink paths, instead of relying on a cast at each
> assignment. Adjust the format specifiers that printed these fields
> with %lu.
> 
> Fixes: 04679b3489e0 ("Staging: USB/IP: add client driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Lucas Pascal <lucas.pascal@pqshield.com>
> ---
> Hi,
> 
> A bit of context on this fix:
> 
> - We heavily use USB/IP on CI/CD runners, in order to have full
>    control/isolation for multiple USB development boards plugged into
>    a powered hub. So USB/IP host and client are on the same machine.
>    Once configured, a runner host stays up ad vitam.
> - I found the bug while debugging a runner host lockup on Ubuntu
>    7.0.0-31-generic, with an uptime > 1month. The log lines above are from
>    that machine.
> - The diagnosis, this patch and its changelog were prepared with the
>    help of an AI coding assistant (Claude Opus 5.5), then reviewed by me.
> - Build-tested on x86_64 and i386 with CONFIG_USBIP_DEBUG=y: no new
>    warnings (W=1 reports a pre-existing -Wformat-truncation in
>    vhci_sysfs.c, unrelated).
> - checkpatch.pl --strict: clean.
> - The patch was tested in QEMU (virtme-ng), not on the physical runner,
>    using dummy_hcd + g_zero in loopback mode (bulk IN read only complete
>    when unlinked), exported through a USB/IP loopback (bind + attach to
>    localhost), running usbtest test 11 (unlink reads, 3000 iterations), with
>    a test-only change starting vhci_hcd->seqnum at INT_MAX - 2000.
>    Without this patch: the first unlink past 2^31 is lost
>    ("unlink->seqnum 18446744071562067969", "cannot find the pending
>    unlink 2147483649"), and testusb hangs in D state.
>    With this patch: 5007 unlinks past 2^31, none lost, testusb passes.

Can you reproduce this with usbip_host and client exchanging messages?
have you noticed "seqnum max" messages in dmesg while you were running
tests?

thanks,
-- Shuah

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

* Re: [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31
  2026-10-19 19:06 ` Shuah Khan
@ 2026-10-20 15:45   ` Lucas Pascal
  0 siblings, 0 replies; 4+ messages in thread
From: Lucas Pascal @ 2026-10-20 15:45 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Valentina Manea, Hongren Zheng, Greg Kroah-Hartman, linux-usb,
	stable

Hi Shuah,

Sorry for the duplicates: my earlier replies went to you only, or were
sent with HTML bits by Gmail and rejected by the list. This is a
(hopefully right) plain-text resend to everyone.

On Mon, 19 Oct 2026 at 21:06, Shuah Khan <skhan@linuxfoundation.org> wrote:
> Can you reproduce this with usbip_host and client exchanging messages?

The QEMU test I mentioned may cover that already. It goes as follows:
- dummy_hcd is activated and g_zero is "plugged into" it.
- Host side: usbipd + "usbip bind" hand the device to usbip_host.
- Client side: "usbip attach -r 127.0.0.1" imports it through
  vhci_hcd.
- Transport: the real usbip_host <-> vhci_hcd protocol over TCP
  (loopback), no simulated peer. Same setup as the production machine
  where I hit the bug.
- Traffic test itself: I reused the usbtest test case 11 (unlink reads)
  with 3000 iterations:
  /opt/testusb -D /dev/bus/usb/<M>/<N> -t 11 -s 512 -c 3000
- As I modified vhci_hcd->seqnum at INT_MAX - 2000, the test crosses
  2^31 within seconds.

From there I can think of other tests:
- using another test case which does not focus on unlinks (case 1, 2?),
- host and client on different machines.

Would either be helpful?

> have you noticed "seqnum max" messages in dmesg while you were running
> tests?

Not in the QEMU before/after test itself. seqnum starts at
INT_MAX - 2000, above 0xffff, so given the driver code it won't trigger
the message:

  if (priv->seqnum == 0xffff)
      dev_info(&urb->dev->dev, "seqnum max\n");

However I did see it on the production runner server, ~20m after it
booted. The counter then kept going until it crossed 2^31 a month later:

2026-09-15T16:01:36+01:00 fwtest35 kernel: Linux version 7.0.0-31-generic (buildd@lcy02-amd64-060) (x86_64-linux-gnu-gcc-13 (Ubuntu 13.3.0-6ubuntu2~24.04.1) 13.3.0, GNU ld (GNU Binutils for Ubuntu) 2.42) #31~24.04.1-Ubuntu SMP PREEMPT_DYNAMIC Mon Aug 10 09:38:02 UTC 2 (Ubuntu 7.0.0-31.31~24.04.1-generic 7.0.14)
[...]
2026-09-15T16:24:45+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 37862
2026-09-15T16:24:45+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-09-15T16:24:45+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 37865
2026-09-15T16:24:45+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-09-15T16:24:45+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 37867
2026-09-15T16:24:45+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-09-15T16:24:48+01:00 fwtest35 kernel: usb 5-5: seqnum max
2026-09-15T16:24:49+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 75555
2026-09-15T16:24:49+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-09-15T16:24:49+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 75556
2026-09-15T16:24:49+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-09-15T16:24:49+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 75557
[...]
2026-10-15T17:31:35+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 2147467545
2026-10-15T17:31:35+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-10-15T17:31:35+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 2147467547
2026-10-15T17:31:35+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-10-15T17:31:35+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 2147467551
2026-10-15T17:31:35+01:00 fwtest35 kernel: vhci_hcd: urb->status -104
2026-10-15T17:31:39+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 18446744071562096407
2026-10-15T17:31:39+01:00 fwtest35 kernel: vhci_hcd: cannot find the pending unlink 2147512087
2026-10-15T17:31:39+01:00 fwtest35 kernel: vhci_hcd: unlink->seqnum 18446744071562096629
2026-10-15T17:31:39+01:00 fwtest35 kernel: vhci_hcd: cannot find the pending unlink 2147512309

These last numbers show the bug:
- 2147467551 = 0x7fffc11f, < 0x80000000 - the bug doesn't trigger before
  2^31,
- 18446744071562096629 = 0xffffffff80006ff5 - the bug triggers,
  seqnum > 2^31 are sign-extended with 0xffffffff,
- 2147512309 = 0x80006ff5 - the expected seqnum.

Thank you,
Lucas

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

end of thread, other threads:[~2026-10-20 15:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 15:15 [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31 Lucas Pascal
2026-10-19 16:11 ` sashiko-bot
2026-10-19 19:06 ` Shuah Khan
2026-10-20 15:45   ` Lucas Pascal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox