[PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31

[Lucas Pascal <lucas.pascal@pqshield.com>]

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