* Re: [PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback
2026-07-14 18:11 ` [PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback Jay Vadayath
@ 2026-07-14 18:25 ` Oliver Neukum
2026-07-15 7:21 ` Johan Hovold
1 sibling, 0 replies; 3+ messages in thread
From: Oliver Neukum @ 2026-07-14 18:25 UTC (permalink / raw)
To: Jay Vadayath, gregkh, johan; +Cc: linux-usb, linux-kernel, stable, Lukas Dresel
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback
2026-07-14 18:11 ` [PATCH] USB: serial: sierra: fix slab out-of-bounds read in sierra_instat_callback Jay Vadayath
2026-07-14 18:25 ` Oliver Neukum
@ 2026-07-15 7:21 ` Johan Hovold
1 sibling, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2026-07-15 7:21 UTC (permalink / raw)
To: Jay Vadayath; +Cc: gregkh, linux-usb, linux-kernel, stable, Lukas Dresel
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
^ permalink raw reply [flat|nested] 3+ messages in thread