Re: [PATCH v2] USB: serial: cypress_m8: validate interrupt packet headers

[Johan Hovold <johan@kernel.org>]

On Fri, May 22, 2026 at 10:54:42PM +0800, Zhang Cen wrote:
> cypress_read_int_callback() parses the interrupt-in buffer according to
> the selected Cypress packet format. Format 1 has a two-byte status/count
> header and format 2 has a one-byte combined status/count header. The
> usb-serial core sizes the interrupt-in buffer from the endpoint
> descriptor's wMaxPacketSize, and successful interrupt transfers can
> complete short when URB_SHORT_NOT_OK is not set.
> 
> Check that the completed packet contains the selected header before
> reading it. Malformed short reports are ignored and the interrupt URB is
> resubmitted through the existing retry path, preventing out-of-bounds
> header-byte reads.
> 
> KASAN report as below:
> KASAN slab-out-of-bounds in cypress_read_int_callback+0x240/0x7f0
> Read of size 1
> Call trace:
>   cypress_read_int_callback() (drivers/usb/serial/cypress_m8.c:1009)
>   __usb_hcd_giveback_urb()
>   dummy_timer()
> 
> Fixes: 3416eaa1f8f8 ("USB: cypress_m8: Packet format is separate from characteristic size")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
> ---
> v2:
> Check only urb->actual_length before reading the packet-format header.
> Reuse the existing i header-length variable instead of adding a new one.
> Shorten the KASAN trace in the commit message.

Thanks for the v2.

>  drivers/usb/serial/cypress_m8.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
> index afff1a0f4298b..49c0f3e379bd0 100644
> --- a/drivers/usb/serial/cypress_m8.c
> +++ b/drivers/usb/serial/cypress_m8.c
> @@ -1060,18 +1060,27 @@ static void cypress_read_int_callback(struct urb *urb)
>  	default:
>  	case packet_format_1:
>  		/* This is for the CY7C64013... */
> -		priv->current_status = data[0] & 0xF8;
> -		bytes = data[1] + 2;
>  		i = 2;
> +		if (result < i)
> +			break;
> +		priv->current_status = data[0] & 0xF8;
> +		bytes = data[1] + i;
>  		break;

I know I asked you to move the initialisation of i (the header length),
but when applying I ended up changing this so that the sanity checks use
constants instead, which I found more readable.

The end result is here:

	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-linus&id=9f9bfc80c67f35a275820da7e83a35dface08281

Johan