Re: [PATCH] usb: serial: fix slab out-of-bounds read in interrupt URB callback

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jul 13, 2026 at 01:00:12AM +0800, Jiale Yao wrote:
> The interrupt URB buffer is allocated in setup_port_interrupt_in() based
> on the endpoint's wMaxPacketSize:
> 
>     buffer_size = usb_endpoint_maxp(epd);
>     port->interrupt_in_buffer = kmalloc(buffer_size, GFP_KERNEL);
> 
> When a USB device declares wMaxPacketSize = 8 on its interrupt IN
> endpoint, the buffer is allocated from kmalloc-8 cache (exactly 8 bytes).
> 
> If the device sends a short packet (actual_length < wMaxPacketSize),
> the URB completes with status == 0 and the callback proceeds to read:
> 
>     data[sizeof(struct usb_ctrlrequest)]
> 
> which evaluates to data[8], accessing 1 byte beyond the allocated 8-byte
> buffer. This results in a slab out-of-bounds read.
> 
> Fix this by adding a bounds check before accessing data[8], ensuring that
> the actual length is sufficient to contain a full USB control request
> header.
> 
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/usb/serial/option.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index 4c4009b8a46d..4aae54e1eacf 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -2672,6 +2672,13 @@ static void option_instat_callback(struct urb *urb)
>  			dev_dbg(dev, "%s: NULL req_pkt\n", __func__);
>  			return;
>  		}
> +
> +		if (urb->actual_length < sizeof(struct usb_ctrlrequest) + 1) {
> +			dev_dbg(dev, "%s: short interrupt transfer: %d bytes\n",
> +				__func__, urb->actual_length);
> +			return;
> +		}

Close, shouldn't this be rewritten to be:

               if (urb->actual_length < sizeof(*req_pkt) + 1) {
                       dev_dbg(dev, "%s: short packet %u\n", __func__,
                               urb->actual_length);

As that's the structure you need to care about instead?

Also, nit, dev_dbg() already has the __func__ name in it, no need to
duplicate it again (yes, the other ones in this file do that, so it is
the style here...)

thanks,

greg k-h