[PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback

[PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback

[Jay Vadayath]

The interrupt-in URB buffer is allocated based on the endpoint's
wMaxPacketSize. A device declaring wMaxPacketSize == 8 gets an 8-byte
buffer from kmalloc-8. When such a device delivers a short packet,
sierra_instat_callback() still dereferences transfer_buffer as struct
usb_ctrlrequest and reads a further byte at data[sizeof(*req_pkt)], one
byte past the end of the allocation.

Reject the URB when fewer than sizeof(struct usb_ctrlrequest) + 1 bytes
were received.

Cc: stable@vger.kernel.org
Reported-by: Jay Vadayath <jkrshnmenon@gmail.com>
Reported-by: Lukas Dresel <lukas@artiphishell.com>
Signed-off-by: Jay Vadayath <jkrshnmenon@gmail.com>
---
Apologies for the wall of text in the original mail. I wanted to include
the artifacts inline so the bug could be independently verified. Just the
patch this time.

Same shape of fix as Jiale Yao's option.c patch:
https://lore.kernel.org/all/20260712170012.3503601-1-yaojiale02@163.com/

Tested with the reproducer against v7.2-rc3. The KASAN splat does not
appear with the patch applied.

 drivers/usb/serial/sierra.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 6e443aacae07..4c6e7120695e 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -575,6 +575,13 @@ static void sierra_instat_callback(struct urb *urb)
 				__func__);
 			return;
 		}
+
+		if (urb->actual_length < sizeof(struct usb_ctrlrequest) + 1) {
+			dev_dbg(&port->dev, "%s: short interrupt transfer: %d bytes\n",
+				__func__, urb->actual_length);
+			return;
+		}
+
 		if ((req_pkt->bRequestType == 0xA1) &&
 				(req_pkt->bRequest == 0x20)) {
 			int old_dcd_state;
--
2.51.2

Re: [PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback

[Oliver Neukum]

On 14.07.26 20:11, Jay Vadayath wrote:
> The interrupt-in URB buffer is allocated based on the endpoint's
> wMaxPacketSize. A device declaring wMaxPacketSize == 8 gets an 8-byte
> buffer from kmalloc-8. When such a device delivers a short packet,
> sierra_instat_callback() still dereferences transfer_buffer as struct
> usb_ctrlrequest and reads a further byte at data[sizeof(*req_pkt)], one
> byte past the end of the allocation.
> 
> Reject the URB when fewer than sizeof(struct usb_ctrlrequest) + 1 bytes
> were received.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Jay Vadayath <jkrshnmenon@gmail.com>
> Reported-by: Lukas Dresel <lukas@artiphishell.com>
> Signed-off-by: Jay Vadayath <jkrshnmenon@gmail.com>

Nacked-by: Oliver Neukum <oneukum@suse.com>

> +
> +		if (urb->actual_length < sizeof(struct usb_ctrlrequest) + 1) {
> +			dev_dbg(&port->dev, "%s: short interrupt transfer: %d bytes\n",
> +				__func__, urb->actual_length);
> +			return;
> +		}

I am sorry, but you cannot do this. Not here.

> +
>   		if ((req_pkt->bRequestType == 0xA1) &&
>   				(req_pkt->bRequest == 0x20)) {

This is the test you need to check how long the reply needs to be.

If we look at the full evaluation of the package from the driver we have:

                 }
                 if ((req_pkt->bRequestType == 0xA1) &&
                                 (req_pkt->bRequest == 0x20)) {
                         int old_dcd_state;
                         unsigned char signals = *((unsigned char *)
                                         urb->transfer_buffer +
                                         sizeof(struct usb_ctrlrequest));

                         dev_dbg(&port->dev, "%s: signal x%x\n", __func__,
                                 signals);

                         old_dcd_state = portdata->dcd_state;
                         portdata->cts_state = 1;
                         portdata->dcd_state = ((signals & 0x01) ? 1 : 0);
                         portdata->dsr_state = ((signals & 0x02) ? 1 : 0);
                         portdata->ri_state = ((signals & 0x08) ? 1 : 0);

                         if (old_dcd_state && !portdata->dcd_state)
                                 tty_port_tty_hangup(&port->port, true);
                 } else {
                         dev_dbg(&port->dev, "%s: type %x req %x\n",
                                 __func__, req_pkt->bRequestType,
                                 req_pkt->bRequest);
                 }

In this branch:

                 if ((req_pkt->bRequestType == 0xA1) &&
                                 (req_pkt->bRequest == 0x20)) {

replies must be at least sizeof(struct usb_ctrlrequest) + 1 long.
But in the else branch a length of sizeof(struct usb_ctrlrequest) will do,
because they do not evaluate the signal.

In other words, you stop processing messages if a message the driver does not
care about, but is valid under the specification arrives. That breaks the driver.

	Regards
		Oliver

Re: [PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback

[Johan Hovold]

On Tue, Jul 14, 2026 at 11:11:42AM -0700, Jay Vadayath wrote:
> The interrupt-in URB buffer is allocated based on the endpoint's
> wMaxPacketSize. A device declaring wMaxPacketSize == 8 gets an 8-byte
> buffer from kmalloc-8. When such a device delivers a short packet,
> sierra_instat_callback() still dereferences transfer_buffer as struct
> usb_ctrlrequest and reads a further byte at data[sizeof(*req_pkt)], one
> byte past the end of the allocation.
> 
> Reject the URB when fewer than sizeof(struct usb_ctrlrequest) + 1 bytes
> were received.
> 

Fixes tag missing.

> Cc: stable@vger.kernel.org
> Reported-by: Jay Vadayath <jkrshnmenon@gmail.com>

You shouldn't add a Reported-by for your own patches.

> Reported-by: Lukas Dresel <lukas@artiphishell.com>

Where was this reported?

> Signed-off-by: Jay Vadayath <jkrshnmenon@gmail.com>

You also need to mention that you used an LLM to find, write the commit
message, test and/or fix this issue as documented here:

	Documentation/process/generated-content.rst
	Documentation/process/coding-assistants.rst

> ---
> Apologies for the wall of text in the original mail. I wanted to include
> the artifacts inline so the bug could be independently verified. Just the
> patch this time.
> 
> Same shape of fix as Jiale Yao's option.c patch:
> https://lore.kernel.org/all/20260712170012.3503601-1-yaojiale02@163.com/

And as I and Oliver pointed out in that thread, that fix is not correct.

Johan