linux-kernel - Re: [PATCH 1/2] usbip: Fix seqnum sign extension issue in vhci_tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJ=gGT3Z85Hshfw5eb5JgM0CA-V_O1MW1hbfbdyp8Be0bG6SvA@mail.gmail.com>
Date: Mon, 13 Jan 2025 21:29:23 +0800
From: xndcn <xndchn@...il.com>
To: Shuah Khan <skhan@...uxfoundation.org>
Cc: Valentina Manea <valentina.manea.m@...il.com>, Shuah Khan <shuah@...nel.org>, 
	Hongren Zheng <i@...ithal.me>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	"open list:USB OVER IP DRIVER" <linux-usb@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] usbip: Fix seqnum sign extension issue in vhci_tx_urb

without the patch:
> [ 384.276605] vhci_hcd vhci_hcd.0: USB/IP Virtual Host Controller
> [ 384.278487] vhci_hcd vhci_hcd.0: new USB bus registered, assigned bus number 1
> [ 384.278509] vhci_hcd: created sysfs vhci_hcd.0
> [ 384.278532] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 6.12
> [ 384.278535] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 384.278536] usb usb1: Product: USB/IP Virtual Host Controller
> [ 384.278538] usb usb1: Manufacturer: Linux 6.12.0-00971-g158f238aa69d-dirty vhci_hcd
> [ 384.278539] usb usb1: SerialNumber: vhci_hcd.0
> [ 384.278630] hub 1-0:1.0: USB hub found
> [ 384.278637] hub 1-0:1.0: 8 ports detected
> [ 384.278740] vhci_hcd vhci_hcd.0: USB/IP Virtual Host Controller
> [ 384.278781] vhci_hcd vhci_hcd.0: new USB bus registered, assigned bus number 2
> [ 384.278788] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
> [ 384.278801] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 6.12
> [ 384.278802] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 384.278803] usb usb2: Product: USB/IP Virtual Host Controller
> [ 384.278804] usb usb2: Manufacturer: Linux 6.12.0-00971-g158f238aa69d-dirty vhci_hcd
> [ 384.278805] usb usb2: SerialNumber: vhci_hcd.0
> [ 384.278866] hub 2-0:1.0: USB hub found
> [ 384.278869] hub 2-0:1.0: 8 ports detected
> [ 384.279071] insmod (400) used greatest stack depth: 11960 bytes left
> [ 550.127351] vhci_hcd vhci_hcd.0: pdev(0) rhport(0) sockfd(3)
> [ 550.127356] vhci_hcd vhci_hcd.0: devid(0) speed(3) speed_str(high-speed)
> [ 550.127359] vhci_hcd vhci_hcd.0: Device attached
> [ 550.341007] usb 1-1: new high-speed USB device number 2 using vhci_hcd
> [ 550.452995] usb 1-1: SetAddress Request (2) to port 0
> [ 550.464332] vhci_hcd: cannot find a urb of seqnum 2147483648 max seqnum -2147483648
> [ 550.464842] vhci_hcd: stop threads
> [ 550.464843] vhci_hcd: release socket
> [ 550.464845] vhci_hcd: disconnect device

and with this patch:
> [ 746.253823] vhci_hcd vhci_hcd.0: USB/IP Virtual Host Controller
> [ 746.254660] vhci_hcd vhci_hcd.0: new USB bus registered, assigned bus number 1
> [ 746.254669] vhci_hcd: created sysfs vhci_hcd.0
> [ 746.254895] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 6.12
> [ 746.254897] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 746.254898] usb usb1: Product: USB/IP Virtual Host Controller
> [ 746.254899] usb usb1: Manufacturer: Linux 6.12.0-00971-g158f238aa69d-dirty vhci_hcd
> [ 746.254899] usb usb1: SerialNumber: vhci_hcd.0
> [ 746.254964] hub 1-0:1.0: USB hub found
> [ 746.254985] hub 1-0:1.0: 8 ports detected
> [ 746.255042] vhci_hcd vhci_hcd.0: USB/IP Virtual Host Controller
> [ 746.255066] vhci_hcd vhci_hcd.0: new USB bus registered, assigned bus number 2
> [ 746.255072] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
> [ 746.255081] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 6.12
> [ 746.255082] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 746.255083] usb usb2: Product: USB/IP Virtual Host Controller
> [ 746.255089] usb usb2: Manufacturer: Linux 6.12.0-00971-g158f238aa69d-dirty vhci_hcd
> [ 746.255089] usb usb2: SerialNumber: vhci_hcd.0
> [ 746.255118] hub 2-0:1.0: USB hub found
> [ 746.255120] hub 2-0:1.0: 8 ports detected
> [ 756.967922] vhci_hcd vhci_hcd.0: pdev(0) rhport(0) sockfd(3)
> [ 756.967928] vhci_hcd vhci_hcd.0: devid(0) speed(3) speed_str(high-speed)
> [ 756.967933] vhci_hcd vhci_hcd.0: Device attached
> [ 757.184367] usb 1-1: new high-speed USB device number 2 using vhci_hcd
> [ 757.296479] usb 1-1: SetAddress Request (2) to port 0
> [ 757.309845] usb 1-1: New USB device found, idVendor=1234, idProduct=1234, bcdDevice= 2.80
> [ 757.309848] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 757.309849] usb 1-1: Product: foo
> [ 757.309850] usb 1-1: Manufacturer: bar
> [ 757.309851] usb 1-1: SerialNumber: 0

to make the bug easier to reproduce ,I have changed the initial value of segnum:

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 8dac1edc74d4..92a60e89b459 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1192,7 +1192,7 @@ static int vhci_start(struct usb_hcd *hcd)
                vdev->rhport = rhport;
        }

- atomic_set(&vhci_hcd->seqnum, 0);
+ atomic_set(&vhci_hcd->seqnum, 2147483646);

        hcd->power_budget = 0; /* no limit */
        hcd->uses_new_polling = 1;

On Fri, Jan 10, 2025 at 12:02 AM Shuah Khan <skhan@...uxfoundation.org> wrote:
>
> On 1/3/25 08:18, xndcn wrote:
> > Thanks.
> >
> >> How did you find the problem?
> >> Why does it make sense to cast it to u32?
> >
> > After running with usbip enough time, I happened to see logs like this:
> >> [ 293.863125] vhci_hcd vhci_hcd.0: Device attached
> >> [ 294.081110] usb 1-1: new high-speed USB device number 2 using vhci_hcd
> >> [ 294.193163] usb 1-1: SetAddress Request (2) to port 0
> >> [ 294.204334] vhci_hcd: cannot find a urb of seqnum 2147483648 max seqnum -2147483648
> >> [ 294.204850] vhci_hcd: stop threads
> >> [ 294.204851] vhci_hcd: release socket
> >> [ 294.204853] vhci_hcd: disconnect device
> >
> > Then I notice that on 64bit platform, when
> > atomic_inc_return(&vhci_hcd->seqnum) returns (2147483647 + 1, or
> > 0x80000000),
> > priv->seqnum (which is unsigned long, i.e. u64 on 64bit) will be
> > extends to 0xffffffff80000000
> > So we can fix the issue by cast it to u32.
> >
>
> Can you send me the dmesg without and with your patch?
>
> thanks,
> -- Shuah
>