[PATCHv2] usb: serial: sierra: deal with insufficiently long notifications

[PATCHv2] usb: serial: sierra: deal with insufficiently long notifications

[Oliver Neukum]

A malicious device can send too short messages which
would cause the driver to read beyond the part of the buffer
holding valid data.
Ignore too short packets.

Fixes: 033a3fb980b04 ("USB: Sierra Wireless driver update")
Reported-by: Jay Vadayath <jkrshnmenon@gmail.com>
Reported-by: Lukas Dresel <lukas@artiphishell.com>
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---

v2:
- fixed description and added reporters and fixes tag

 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:
 		usb_mark_last_busy(serial->dev);
 		err = usb_submit_urb(urb, GFP_ATOMIC);
 		if (err && err != -EPERM)
-- 
2.55.0

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

[Johan Hovold]

On Wed, Jul 15, 2026 at 11:04:32AM +0200, Oliver Neukum wrote:
> A malicious device can send too short messages which
> would cause the driver to read beyond the part of the buffer
> holding valid data.
> Ignore too short packets.
> 
> Fixes: 033a3fb980b04 ("USB: Sierra Wireless driver update")
> Reported-by: Jay Vadayath <jkrshnmenon@gmail.com>
> Reported-by: Lukas Dresel <lukas@artiphishell.com>

You should add a Link to the report(s) as well. I only saw the one from
Jay.

> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
> 
> v2:
> - fixed description and added reporters and fixes tag
> 
>  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:

Please move and rename this one as I suggested in my reply to v1.

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

Johan