Re: [PATCH] usb: serial: sierra: deal with insufficiently long notifications

[Johan Hovold <johan@kernel.org>]

On Tue, Jul 14, 2026 at 08:33:05PM +0200, Oliver Neukum wrote:
> A malicious device can send too short messages which
> would cause the driver to overflow the buffer and hit

"overflow" makes it sounds like memory corruption but this is just a
potential 1-byte read beyond the buffer (or of stale data inside the
buffer).

> stale data in their interpretation.
> Ignore too short packets.

Even if this was found by an LLM it was still reported to use by a
person so please add a Reported-by and Link tag along with a Fixes tag.

> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/usb/serial/sierra.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> index 6e443aacae07..f50833055360 100644
> --- a/drivers/usb/serial/sierra.c
> +++ b/drivers/usb/serial/sierra.c
> @@ -575,10 +575,19 @@ static void sierra_instat_callback(struct urb *urb)
>  				__func__);
>  			return;
>  		}
> +
> +		if (urb->actual_length < sizeof(struct usb_ctrlrequest))
> +			goto skip_too_short;
> +
>  		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(struct usb_ctrlrequest) + 1)
> +				goto skip_too_short;
> +
> +			signals = *((unsigned char *)
>  					urb->transfer_buffer +
>  					sizeof(struct usb_ctrlrequest));
>  
> @@ -603,6 +612,7 @@ static void sierra_instat_callback(struct urb *urb)
>  
>  	/* Resubmit urb so we continue receiving IRQ data */
>  	if (status != -ESHUTDOWN && status != -ENOENT) {
> +skip_too_short:

And please move the label before the conditional and rename it after
what it does (e.g. "out_resubmit" or just "resubmit").

>  		usb_mark_last_busy(serial->dev);
>  		err = usb_submit_urb(urb, GFP_ATOMIC);
>  		if (err && err != -EPERM)

Johan