Re: [PATCH v2] USB: serial: sierra: validate URB length in instat callback

[Johan Hovold <johan@kernel.org>]

On Mon, Oct 19, 2026 at 10:38:49AM +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> sierra_instat_callback() reads request fields and one byte past the
> usb_ctrlrequest header without first checking that urb->actual_length
> is large enough.  A short USB interrupt transfer could cause an
> out-of-bounds read.
> 
> Add two length checks matching the pattern already present in the
> option driver (option_instat_callback): one for the request header
> before accessing bRequestType, and one for the line-state data byte
> inside the serial-state notification handler.  Use dev_err() since
> failing these checks causes the interrupt URB not to be resubmitted.
> 
> Fixes: 033a3fb980b041 ("USB: Sierra Wireless driver update")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> Changes in v2 (Johan Hovold):
>  - Split into two checks: one for the header before accessing
>    bRequestType, and one for the line-state data byte inside
>    the serial-state handler, matching the option driver pattern
>  - Use dev_err() instead of dev_dbg() since early return prevents
>    further interrupts

Thanks for the update.

Now applied, with one change below.

>  drivers/usb/serial/sierra.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> index 6e443aacae07d..03ed76dfbb2e3 100644
> --- a/drivers/usb/serial/sierra.c
> +++ b/drivers/usb/serial/sierra.c
> @@ -575,10 +575,23 @@ static void sierra_instat_callback(struct urb *urb)
>  				__func__);
>  			return;
>  		}
> +		if (urb->actual_length < sizeof(*req_pkt)) {
> +			dev_err(&port->dev, "%s: short transfer\n",
> +				__func__);
> +			return;
> +		}
>  		if ((req_pkt->bRequestType == 0xA1) &&
>  				(req_pkt->bRequest == 0x20)) {
>  			int old_dcd_state;
> -			unsigned char signals = *((unsigned char *)
> +			unsigned char signals;
> +
> +			if (urb->actual_length < sizeof(*req_pkt) + 1) {
> +				dev_err(&port->dev,
> +					"%s: short interrupt transfer\n",
> +					__func__);
> +				return;
> +			}
> +			signals = *((unsigned char *)
>  					urb->transfer_buffer +
>  					sizeof(struct usb_ctrlrequest));

I used sizeof(*req_pkt) here too for consistency.

Johan