[PATCH] usb: storage: sierra_ms: reject short SWoC info transfers

[PATCH] usb: storage: sierra_ms: reject short SWoC info transfers

[Syed Labeeq Sajid Bukhari]

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

sierra_get_swoc_info() requests sizeof(struct swoc_info) (60) bytes
from the device via usb_control_msg(), but its callers only treat a
negative return value as failure. A device that answers the
vendor-specific GetSwocInfo request with a short IN transfer is
therefore accepted, leaving the tail of the freshly allocated
(kmalloc(), non-zeroing) swoc_info buffer uninitialized.

truinst_show() subsequently prints swocInfo->rev, swocInfo->LinuxSKU
and swocInfo->LinuxVer from that buffer into the world-readable
(0444) "truinst" sysfs attribute. An emulated/malicious USB device
(VID 0x1199, PID 0x0fff) can exploit this to disclose up to 5 bytes
of stale kernel heap memory (kmalloc-64) to unprivileged userspace,
once per sysfs read, indefinitely. On kernels built without
init_on_alloc this leaks recently freed heap contents.

Only accept the transfer when the full structure was received.
sierra_ms_init() already retries failed queries, so well-behaved
devices are unaffected.

Fixes: 32fe5e393455 ("USB Storage Sierra: TRU-Install feature update")
Cc: stable@vger.kernel.org
Signed-off-by: Syed Labeeq <syedlabeeq@gmail.com>
---
 drivers/usb/storage/sierra_ms.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/storage/sierra_ms.c b/drivers/usb/storage/sierra_ms.c
index 177fa6cd143ab2837640c26f8336781ddd3cf9cb..8755fda42eed2afd3235283e990a35aac14cb829
100644
--- a/drivers/usb/storage/sierra_ms.c
+++ b/drivers/usb/storage/sierra_ms.c
@@ -76,6 +76,12 @@
  (void *) swocInfo, /* void *data        */
  sizeof(struct swoc_info), /* __u16 size     */
  USB_CTRL_SET_TIMEOUT); /* int timeout     */
+ /*
+ * A short IN transfer leaves the tail of swocInfo uninitialized;
+ * only a full transfer is valid.
+ */
+ if (result != sizeof(struct swoc_info))
+ return -EIO;

  swocInfo->LinuxSKU = le16_to_cpu(swocInfo->LinuxSKU);
  swocInfo->LinuxVer = le16_to_cpu(swocInfo->LinuxVer);
-- 
2.43.0

[-- Attachment #2: 0001-usb-storage-sierra_ms-reject-short-SWoC-info-transfers.patch --]
[-- Type: text/x-patch, Size: 2162 bytes --]

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Syed Labeeq <syedlabeeq@gmail.com>
Date: Fri, 04 Sep 2026 17:30:59 +0500
Subject: [PATCH] usb: storage: sierra_ms: reject short SWoC info transfers

sierra_get_swoc_info() requests sizeof(struct swoc_info) (60) bytes
from the device via usb_control_msg(), but its callers only treat a
negative return value as failure. A device that answers the
vendor-specific GetSwocInfo request with a short IN transfer is
therefore accepted, leaving the tail of the freshly allocated
(kmalloc(), non-zeroing) swoc_info buffer uninitialized.

truinst_show() subsequently prints swocInfo->rev, swocInfo->LinuxSKU
and swocInfo->LinuxVer from that buffer into the world-readable
(0444) "truinst" sysfs attribute. An emulated/malicious USB device
(VID 0x1199, PID 0x0fff) can exploit this to disclose up to 5 bytes
of stale kernel heap memory (kmalloc-64) to unprivileged userspace,
once per sysfs read, indefinitely. On kernels built without
init_on_alloc this leaks recently freed heap contents.

Only accept the transfer when the full structure was received.
sierra_ms_init() already retries failed queries, so well-behaved
devices are unaffected.

Fixes: 32fe5e393455 ("USB Storage Sierra: TRU-Install feature update")
Cc: stable@vger.kernel.org
Signed-off-by: Syed Labeeq <syedlabeeq@gmail.com>
---
 drivers/usb/storage/sierra_ms.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/storage/sierra_ms.c b/drivers/usb/storage/sierra_ms.c
index 177fa6cd143ab2837640c26f8336781ddd3cf9cb..8755fda42eed2afd3235283e990a35aac14cb829 100644
--- a/drivers/usb/storage/sierra_ms.c
+++ b/drivers/usb/storage/sierra_ms.c
@@ -76,6 +76,12 @@
 			(void *) swocInfo,		/* void *data        */
 			sizeof(struct swoc_info),	/* __u16 size 	     */
 			USB_CTRL_SET_TIMEOUT);		/* int timeout 	     */
+	/*
+	 * A short IN transfer leaves the tail of swocInfo uninitialized;
+	 * only a full transfer is valid.
+	 */
+	if (result != sizeof(struct swoc_info))
+		return -EIO;
 
 	swocInfo->LinuxSKU = le16_to_cpu(swocInfo->LinuxSKU);
 	swocInfo->LinuxVer = le16_to_cpu(swocInfo->LinuxVer);
-- 
2.43.0

Re: [PATCH] usb: storage: sierra_ms: reject short SWoC info transfers

[Greg KH]

On Fri, Sep 04, 2026 at 07:52:59PM +0500, Syed Labeeq Sajid Bukhari wrote:
> sierra_get_swoc_info() requests sizeof(struct swoc_info) (60) bytes
> from the device via usb_control_msg(), but its callers only treat a
> negative return value as failure. A device that answers the
> vendor-specific GetSwocInfo request with a short IN transfer is
> therefore accepted, leaving the tail of the freshly allocated
> (kmalloc(), non-zeroing) swoc_info buffer uninitialized.
> 
> truinst_show() subsequently prints swocInfo->rev, swocInfo->LinuxSKU
> and swocInfo->LinuxVer from that buffer into the world-readable
> (0444) "truinst" sysfs attribute. An emulated/malicious USB device
> (VID 0x1199, PID 0x0fff) can exploit this to disclose up to 5 bytes
> of stale kernel heap memory (kmalloc-64) to unprivileged userspace,
> once per sysfs read, indefinitely. On kernels built without
> init_on_alloc this leaks recently freed heap contents.
> 
> Only accept the transfer when the full structure was received.
> sierra_ms_init() already retries failed queries, so well-behaved
> devices are unaffected.
> 
> Fixes: 32fe5e393455 ("USB Storage Sierra: TRU-Install feature update")
> Cc: stable@vger.kernel.org
> Signed-off-by: Syed Labeeq <syedlabeeq@gmail.com>

Did you forget an Assisted-by: tag?


> ---
>  drivers/usb/storage/sierra_ms.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/storage/sierra_ms.c b/drivers/usb/storage/sierra_ms.c
> index 177fa6cd143ab2837640c26f8336781ddd3cf9cb..8755fda42eed2afd3235283e990a35aac14cb829
> 100644
> --- a/drivers/usb/storage/sierra_ms.c
> +++ b/drivers/usb/storage/sierra_ms.c
> @@ -76,6 +76,12 @@
>   (void *) swocInfo, /* void *data        */
>   sizeof(struct swoc_info), /* __u16 size     */
>   USB_CTRL_SET_TIMEOUT); /* int timeout     */
> + /*
> + * A short IN transfer leaves the tail of swocInfo uninitialized;
> + * only a full transfer is valid.
> + */
> + if (result != sizeof(struct swoc_info))
> + return -EIO;

This is corrupted and can not be applied :(

Also, are you sure the device will not send "short" data?  We've had
bugs in the past where we have added this type of check and it turned
out that it broke valid devices, so be careful.

Was this tested with a real device?

thanks,

greg k-h

Re: [PATCH] usb: storage: sierra_ms: reject short SWoC info transfers

[Syed Labeeq Sajid Bukhari]

Hi Greg,

Thanks for the quick look, and apologies for the corrupted patch -- I sent
sent it via a web mail client, which messed up with the whitespaces. I will
resend it properly with git send-email, with the patch also attached.

On the short-transfer concern:

The SWoC query explicitly asks the device for exactly
sizeof(struct swoc_info) (wLength = 60). This vendor command has a
fixed-size response, and the driver already implicitly assumes a full
answer today: containsFullLinuxPackage() and truinst_show()
unconditionally read fields at fixed offsets (bytes 9-12) that a short
transfer would never have written. So with a short answer the current code
already makes its mode-switch decision on uninitialized garbage. The
patch only makes that failure explicit instead of silent.

Two further points:

* sierra_ms_init() retries the query up to 3 times on failure, so a
  transient short transfer during probe is tolerated, not fatal.

* There is core precedent: usb_control_msg_recv() in usb/core/message.c
  rejects short transfers with -EREMOTEIO, i.e. the USB core itself
  treats a short control-IN as an error.

To answer the testing question directly and honestly: no, I do not have
the physical hardware, so this was not tested against a real Sierra
TRU-Install device. It was tested with a fully controllable emulated
device (raw_gadget on dummy_hcd, VID 0x1199 PID 0x0fff) against a
self-built 6.12.108 kernel:

* full 60-byte response: enumeration and truinst reads behave exactly as
  before (attribute created, values printed);
* short response: the query now fails with -EIO instead of exposing
  uninitialized heap through the world-readable truinst attribute.

If you would rather avoid any risk on some devices you mentioned, I
can respin this the other way: allocate the swoc_info buffer with
kzalloc() in truinst_show()/sierra_ms_init() instead of rejecting short
transfers. That closes the uninitialized-data exposure without changing
behavior for short answers (the unwritten tail becomes deterministic
zeros). Happy to send whichever variant you prefer.

thanks,
syed labeeq

On Fri, Sep 4, 2026 at 8:09 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Sep 04, 2026 at 07:52:59PM +0500, Syed Labeeq Sajid Bukhari wrote:
> > sierra_get_swoc_info() requests sizeof(struct swoc_info) (60) bytes
> > from the device via usb_control_msg(), but its callers only treat a
> > negative return value as failure. A device that answers the
> > vendor-specific GetSwocInfo request with a short IN transfer is
> > therefore accepted, leaving the tail of the freshly allocated
> > (kmalloc(), non-zeroing) swoc_info buffer uninitialized.
> >
> > truinst_show() subsequently prints swocInfo->rev, swocInfo->LinuxSKU
> > and swocInfo->LinuxVer from that buffer into the world-readable
> > (0444) "truinst" sysfs attribute. An emulated/malicious USB device
> > (VID 0x1199, PID 0x0fff) can exploit this to disclose up to 5 bytes
> > of stale kernel heap memory (kmalloc-64) to unprivileged userspace,
> > once per sysfs read, indefinitely. On kernels built without
> > init_on_alloc this leaks recently freed heap contents.
> >
> > Only accept the transfer when the full structure was received.
> > sierra_ms_init() already retries failed queries, so well-behaved
> > devices are unaffected.
> >
> > Fixes: 32fe5e393455 ("USB Storage Sierra: TRU-Install feature update")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Syed Labeeq <syedlabeeq@gmail.com>
>
> Did you forget an Assisted-by: tag?
>
>
> > ---
> >  drivers/usb/storage/sierra_ms.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/storage/sierra_ms.c b/drivers/usb/storage/sierra_ms.c
> > index 177fa6cd143ab2837640c26f8336781ddd3cf9cb..8755fda42eed2afd3235283e990a35aac14cb829
> > 100644
> > --- a/drivers/usb/storage/sierra_ms.c
> > +++ b/drivers/usb/storage/sierra_ms.c
> > @@ -76,6 +76,12 @@
> >   (void *) swocInfo, /* void *data        */
> >   sizeof(struct swoc_info), /* __u16 size     */
> >   USB_CTRL_SET_TIMEOUT); /* int timeout     */
> > + /*
> > + * A short IN transfer leaves the tail of swocInfo uninitialized;
> > + * only a full transfer is valid.
> > + */
> > + if (result != sizeof(struct swoc_info))
> > + return -EIO;
>
> This is corrupted and can not be applied :(
>
> Also, are you sure the device will not send "short" data?  We've had
> bugs in the past where we have added this type of check and it turned
> out that it broke valid devices, so be careful.
>
> Was this tested with a real device?
>
> thanks,
>
> greg k-h

Re: [PATCH] usb: storage: sierra_ms: reject short SWoC info transfers

[Greg KH]

On Fri, Sep 04, 2026 at 08:27:46PM +0500, Syed Labeeq Sajid Bukhari wrote:
> Hi Greg,
> 
> Thanks for the quick look, and apologies for the corrupted patch -- I sent
> sent it via a web mail client, which messed up with the whitespaces. I will
> resend it properly with git send-email, with the patch also attached.
> 
> On the short-transfer concern:
> 
> The SWoC query explicitly asks the device for exactly
> sizeof(struct swoc_info) (wLength = 60). This vendor command has a
> fixed-size response, and the driver already implicitly assumes a full
> answer today: containsFullLinuxPackage() and truinst_show()
> unconditionally read fields at fixed offsets (bytes 9-12) that a short
> transfer would never have written. So with a short answer the current code
> already makes its mode-switch decision on uninitialized garbage. The
> patch only makes that failure explicit instead of silent.

Ok, that's a good "proof" that this should be ok.

> Two further points:
> 
> * sierra_ms_init() retries the query up to 3 times on failure, so a
>   transient short transfer during probe is tolerated, not fatal.
> 
> * There is core precedent: usb_control_msg_recv() in usb/core/message.c
>   rejects short transfers with -EREMOTEIO, i.e. the USB core itself
>   treats a short control-IN as an error.

usb_control_msg_recv() is designed to work that way, it's not any sort
of "proof" that this is ok for this device :)

> To answer the testing question directly and honestly: no, I do not have
> the physical hardware, so this was not tested against a real Sierra
> TRU-Install device. It was tested with a fully controllable emulated
> device (raw_gadget on dummy_hcd, VID 0x1199 PID 0x0fff) against a
> self-built 6.12.108 kernel:
> 
> * full 60-byte response: enumeration and truinst reads behave exactly as
>   before (attribute created, values printed);
> * short response: the query now fails with -EIO instead of exposing
>   uninitialized heap through the world-readable truinst attribute.


That's a fake device, real devices are what really matters for testing,
you know this :)

> If you would rather avoid any risk on some devices you mentioned, I
> can respin this the other way: allocate the swoc_info buffer with
> kzalloc() in truinst_show()/sierra_ms_init() instead of rejecting short
> transfers. That closes the uninitialized-data exposure without changing
> behavior for short answers (the unwritten tail becomes deterministic
> zeros). Happy to send whichever variant you prefer.

kzalloc() is good, and is what we have done for other types of issues
like this:
	https://lore.kernel.org/r/20260819-usb_misc_random-v1-1-43a0dcee3a32@kroah.com
but as the data is being accessed, this should be ok.  Worst case, we
get a bug report and fix it afterward like we have for other times we
have messed up.

thanks,

greg k-h