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

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

[pwngets]

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
---
 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 */
-- 
2.34.1

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

[Hans Verkuil]

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 */