Re: [PATCH] media: gspca: fix stack overrun in build_isoc_ep_tb()

[Hans Verkuil <hverkuil+cisco@kernel.org>]

On 28/09/2026 13:07, pwngets wrote:
> build_isoc_ep_tb() fills the ep_tb[] table that gspca_init_transfer()
> declares as "struct ep_tb_s ep_tb[MAX_ALT]".  The bandwidth sentinel is
> stored at the top of the loop, i.e. before the "found" test that decides
> whether the pass produced a usable entry:
> 
> 	for (;;) {
> 		ep_tb->bandwidth = 2000 * 2000 * 120;
> 		found = 0;
> 		for (j = 0; j < nbalt; j++) {
> 			...
> 		}
> 		if (!found)
> 			break;
> 		last_bw = ep_tb->bandwidth;
> 		i++;
> 		ep_tb++;
> 	}
> 
> Every pass that does not break raises last_bw, and the candidates are
> the bandwidths of the nbalt altsettings, so at most MAX_ALT passes fill
> an entry.  The pass that ends the loop has already written
> ep_tb[MAX_ALT].bandwidth by then -- one element past the end of the
> array.
> 
> A device that presents MAX_ALT (16) alternate settings carrying
> isochronous IN endpoints with pairwise distinct, increasing bandwidths
> reaches that pass with nbalt == MAX_ALT.  On x86-64 the store is 4
> bytes at ep_tb + 132, which is the upper half of the stack canary, so
> kernels built with CONFIG_STACKPROTECTOR_STRONG (the default on the
> major distributions) panic in __stack_chk_fail() when
> gspca_init_transfer() returns.
> 
> Only the device descriptors are needed: the store happens while
> VIDIOC_STREAMON is being handled, before any isochronous transfer takes
> place.
> 
> Give the table room for the terminating entry as well.
> 
> Assisted-by: Claude Code

No Signed-off-by and incomprehensible AI prose.

Rejected.

Regards,

	Hans

> ---
>  drivers/media/usb/gspca/gspca.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
> index 594d73e..64bb818 100644
> --- a/drivers/media/usb/gspca/gspca.c
> +++ b/drivers/media/usb/gspca/gspca.c
> @@ -753,7 +753,7 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
>  	struct usb_interface *intf;
>  	struct usb_host_endpoint *ep;
>  	struct urb *urb;
> -	struct ep_tb_s ep_tb[MAX_ALT];
> +	struct ep_tb_s ep_tb[MAX_ALT + 1];
>  	int n, ret, xfer, alt, alt_idx;
>  
>  	/* reset the streaming variables */