* [PATCH] USB: serial: sierra: validate URB length in instat callback
@ 2026-10-18 22:33 Tristan Madani
2026-10-18 22:40 ` sashiko-bot
2026-10-19 6:41 ` Johan Hovold
0 siblings, 2 replies; 3+ messages in thread
From: Tristan Madani @ 2026-10-18 22:33 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Kevin Lloyd, linux-usb, stable,
Tristan Madani
From: Tristan Madani <tristan@talencesecurity.com>
sierra_instat_callback() reads one byte past the usb_ctrlrequest
header at offset sizeof(struct usb_ctrlrequest) without first
checking that urb->actual_length is large enough to contain it.
A short USB interrupt transfer could cause an out-of-bounds read.
Add a length check matching the pattern already present in the
option driver (option_instat_callback), which validates the same
field access.
Fixes: 033a3fb980b041 ("USB: Sierra Wireless driver update")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/usb/serial/sierra.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 6e443aacae07d..8f800cde25229 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -575,6 +575,11 @@ static void sierra_instat_callback(struct urb *urb)
__func__);
return;
}
+ if (urb->actual_length < sizeof(*req_pkt) + 1) {
+ dev_dbg(&port->dev, "%s: short interrupt transfer\n",
+ __func__);
+ return;
+ }
if ((req_pkt->bRequestType == 0xA1) &&
(req_pkt->bRequest == 0x20)) {
int old_dcd_state;
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] USB: serial: sierra: validate URB length in instat callback
2026-10-18 22:33 [PATCH] USB: serial: sierra: validate URB length in instat callback Tristan Madani
@ 2026-10-18 22:40 ` sashiko-bot
2026-10-19 6:41 ` Johan Hovold
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-18 22:40 UTC (permalink / raw)
To: Tristan Madani; +Cc: linux-usb
> From: Tristan Madani <tristan@talencesecurity.com>
>
> sierra_instat_callback() reads one byte past the usb_ctrlrequest
> header at offset sizeof(struct usb_ctrlrequest) without first
> checking that urb->actual_length is large enough to contain it.
> A short USB interrupt transfer could cause an out-of-bounds read.
>
> Add a length check matching the pattern already present in the
> option driver (option_instat_callback), which validates the same
> field access.
>
> Fixes: 033a3fb980b041 ("USB: Sierra Wireless driver update")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261018223352.2021650-1-tristmd@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] USB: serial: sierra: validate URB length in instat callback
2026-10-18 22:33 [PATCH] USB: serial: sierra: validate URB length in instat callback Tristan Madani
2026-10-18 22:40 ` sashiko-bot
@ 2026-10-19 6:41 ` Johan Hovold
1 sibling, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2026-10-19 6:41 UTC (permalink / raw)
To: Tristan Madani
Cc: Greg Kroah-Hartman, Kevin Lloyd, linux-usb, stable,
Tristan Madani
On Sun, Oct 18, 2026 at 10:33:52PM +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
>
> sierra_instat_callback() reads one byte past the usb_ctrlrequest
> header at offset sizeof(struct usb_ctrlrequest) without first
> checking that urb->actual_length is large enough to contain it.
> A short USB interrupt transfer could cause an out-of-bounds read.
>
> Add a length check matching the pattern already present in the
> option driver (option_instat_callback), which validates the same
> field access.
>
> Fixes: 033a3fb980b041 ("USB: Sierra Wireless driver update")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> drivers/usb/serial/sierra.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> index 6e443aacae07d..8f800cde25229 100644
> --- a/drivers/usb/serial/sierra.c
> +++ b/drivers/usb/serial/sierra.c
> @@ -575,6 +575,11 @@ static void sierra_instat_callback(struct urb *urb)
> __func__);
> return;
> }
> + if (urb->actual_length < sizeof(*req_pkt) + 1) {
> + dev_dbg(&port->dev, "%s: short interrupt transfer\n",
> + __func__);
> + return;
> + }
This silently breaks a theoretical device that submits requests without
payload not yet handled by the driver (the else branch below).
Please split this up in two checks; one for the header and one for the
line state data as in the option driver.
And since you prevent any further interrupts if you ever hit this, you
should use dev_err().
> if ((req_pkt->bRequestType == 0xA1) &&
> (req_pkt->bRequest == 0x20)) {
> int old_dcd_state;
Johan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-19 6:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18 22:33 [PATCH] USB: serial: sierra: validate URB length in instat callback Tristan Madani
2026-10-18 22:40 ` sashiko-bot
2026-10-19 6:41 ` Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox