Linux USB
 help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: Lucas Pascal <lucas.pascal@pqshield.com>,
	Valentina Manea <valentina.manea.m@gmail.com>,
	Shuah Khan <shuah@kernel.org>
Cc: Hongren Zheng <i@zenithal.me>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, stable@vger.kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] usbip: vhci_hcd: store seqnums as u32 to fix lost unlinks after 2^31
Date: Mon, 19 Oct 2026 13:06:06 -0600	[thread overview]
Message-ID: <3237a1ae-c38c-4544-97c9-0dfaf0a0bc94@linuxfoundation.org> (raw)
In-Reply-To: <20261019151537.112304-1-lucas.pascal@pqshield.com>

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

  parent reply	other threads:[~2026-10-19 19:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-10-20 15:45   ` Lucas Pascal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3237a1ae-c38c-4544-97c9-0dfaf0a0bc94@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=i@zenithal.me \
    --cc=linux-usb@vger.kernel.org \
    --cc=lucas.pascal@pqshield.com \
    --cc=shuah@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=valentina.manea.m@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox