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

[Greg KH <gregkh@linuxfoundation.org>]

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