Linux USB
 help / color / mirror / Atom feed
* usb: ljca: possible stack overflow in ljca_enumerate_gpio()
@ 2026-06-17 10:42 Maoyi Xie
  2026-06-17 13:11 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 2+ messages in thread
From: Maoyi Xie @ 2026-06-17 10:42 UTC (permalink / raw)
  To: Lixu Zhang, Sakari Ailus; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

Hi all,

I think ljca_enumerate_gpio() in drivers/usb/misc/usb-ljca.c can write past
the valid_pin array on the stack. I would appreciate it if you could take a
look.

The GPIO descriptor comes from the device. It carries pins_per_bank and
bank_num. The function has a small fixed array on the stack.

	u32 valid_pin[LJCA_MAX_GPIO_NUM / BITS_PER_TYPE(u32)];

That is two u32 entries. Two checks run before the loop. One matches the
reply length against the descriptor.

	if (ret != struct_size(desc, bank_desc, desc->bank_num))
		return -EINVAL;

The other bounds the pin product.

	gpio_num = desc->pins_per_bank * desc->bank_num;
	if (gpio_num > LJCA_MAX_GPIO_NUM)
		return -EINVAL;

Then the loop walks bank_num.

	for (i = 0; i < desc->bank_num; i++)
		valid_pin[i] = get_unaligned_le32(&desc->bank_desc[i].valid_pins);

Nothing checks bank_num against the size of valid_pin. The reply is capped at
60 bytes, so the struct_size check limits bank_num to 9. A device that reports
bank_num 9 with pins_per_bank 7 still passes both checks. gpio_num is 63 and
the reply is 56 bytes. The loop then writes nine u32 into the two entry array,
seven past the end.

I reproduced the write on 7.1-rc7 by running the same loop over the same stack
array with a bank_num of 9.

  BUG: KASAN: stack-out-of-bounds in ljca_enumerate_gpio
  Write of size 4 ... [32, 40) 'valid_pin'

The fix I tried rejects a bank_num too large for valid_pin before the loop.

	 	if (gpio_num > LJCA_MAX_GPIO_NUM)
	 		return -EINVAL;
	+
	+	if (desc->bank_num > ARRAY_SIZE(valid_pin))
	+		return -EINVAL;

This needs a malicious or broken LJCA device, not a remote attacker. It still
looks like a stack overflow to me. Does this look right, and is that the place
to bound it? Happy to send a proper patch once you confirm.

Thanks,
Maoyi
https://maoyixie.com/

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: usb: ljca: possible stack overflow in ljca_enumerate_gpio()
  2026-06-17 10:42 usb: ljca: possible stack overflow in ljca_enumerate_gpio() Maoyi Xie
@ 2026-06-17 13:11 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2026-06-17 13:11 UTC (permalink / raw)
  To: Maoyi Xie; +Cc: Lixu Zhang, Sakari Ailus, linux-usb, linux-kernel

On Wed, Jun 17, 2026 at 06:42:01PM +0800, Maoyi Xie wrote:
> Hi all,
> 
> I think ljca_enumerate_gpio() in drivers/usb/misc/usb-ljca.c can write past
> the valid_pin array on the stack. I would appreciate it if you could take a
> look.
> 
> The GPIO descriptor comes from the device. It carries pins_per_bank and
> bank_num. The function has a small fixed array on the stack.
> 
> 	u32 valid_pin[LJCA_MAX_GPIO_NUM / BITS_PER_TYPE(u32)];
> 
> That is two u32 entries. Two checks run before the loop. One matches the
> reply length against the descriptor.
> 
> 	if (ret != struct_size(desc, bank_desc, desc->bank_num))
> 		return -EINVAL;
> 
> The other bounds the pin product.
> 
> 	gpio_num = desc->pins_per_bank * desc->bank_num;
> 	if (gpio_num > LJCA_MAX_GPIO_NUM)
> 		return -EINVAL;
> 
> Then the loop walks bank_num.
> 
> 	for (i = 0; i < desc->bank_num; i++)
> 		valid_pin[i] = get_unaligned_le32(&desc->bank_desc[i].valid_pins);
> 
> Nothing checks bank_num against the size of valid_pin. The reply is capped at
> 60 bytes, so the struct_size check limits bank_num to 9. A device that reports
> bank_num 9 with pins_per_bank 7 still passes both checks. gpio_num is 63 and
> the reply is 56 bytes. The loop then writes nine u32 into the two entry array,
> seven past the end.
> 
> I reproduced the write on 7.1-rc7 by running the same loop over the same stack
> array with a bank_num of 9.
> 
>   BUG: KASAN: stack-out-of-bounds in ljca_enumerate_gpio
>   Write of size 4 ... [32, 40) 'valid_pin'
> 
> The fix I tried rejects a bank_num too large for valid_pin before the loop.
> 
> 	 	if (gpio_num > LJCA_MAX_GPIO_NUM)
> 	 		return -EINVAL;
> 	+
> 	+	if (desc->bank_num > ARRAY_SIZE(valid_pin))
> 	+		return -EINVAL;
> 
> This needs a malicious or broken LJCA device, not a remote attacker. It still
> looks like a stack overflow to me. Does this look right, and is that the place
> to bound it? Happy to send a proper patch once you confirm.

We trust the hardware to do the right thing once a driver is bound to a
device.  That being said, making checks like this to prevent obviously
malicious or broken devices from crashing the kernel are almost always
accepted, so just make up a patch and submit it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-17 13:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17 10:42 usb: ljca: possible stack overflow in ljca_enumerate_gpio() Maoyi Xie
2026-06-17 13:11 ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox