Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: atm: ueagle-atm: reject descriptors that confuse probe and disconnect
@ 2026-07-17  8:07 Diego Fernando Mancera Gomez
  2026-07-17  9:10 ` Stanislaw Gruszka
  0 siblings, 1 reply; 2+ messages in thread
From: Diego Fernando Mancera Gomez @ 2026-07-17  8:07 UTC (permalink / raw)
  To: Matthieu CASTET, Stanislaw Gruszka
  Cc: Chas Williams, Greg Kroah-Hartman, Mauricio Faria de Oliveira,
	linux-usb, netdev, linux-atm-general, linux-kernel,
	Diego Fernando Mancera Gomez, syzbot+e62a973f8322b3bbe3ac

uea_probe() distinguishes a pre-firmware device from a post-firmware one
using the USB id (UEA_IS_PREFIRM()), and stores a different object as the
interface data in each case: a 'struct completion' for a pre-firmware
device (to be waited on in .disconnect()), or a 'struct usbatm_data' for a
post-firmware one.

uea_disconnect() instead tells the two apart by the number of interfaces
of the active configuration (a pre-firmware device exposes a single
interface, ADI930 has 2 and eagle has 3), and casts the interface data
accordingly.

Because the two handlers use different criteria, a crafted device that
advertises a pre-firmware id together with a multi-interface descriptor
(or a post-firmware id with a single interface) makes them disagree: the
small 'struct completion' stored by uea_probe() is then passed to
usbatm_usb_disconnect(), which casts it to 'struct usbatm_data' and takes
instance->serialize, reading past the end of the allocation:

  BUG: KASAN: slab-out-of-bounds in __mutex_lock+0x152a/0x1b80
  Read of size 8 at addr ffff8880470e2c60 by task kworker/1:2/982
  ...
   __mutex_lock+0x152a/0x1b80
   usbatm_usb_disconnect+0x70/0x820
   uea_disconnect+0x133/0x2c0
   usb_unbind_interface+0x1dd/0x9e0
  ...
  which belongs to the cache kmalloc-96 of size 96
  The buggy address is located 0 bytes to the right of
   allocated 96-byte region [ffff8880470e2c00, ffff8880470e2c60)

Reject such inconsistent descriptors in uea_probe() so that both handlers
always make the same pre/post-firmware decision.

Reported-by: syzbot+e62a973f8322b3bbe3ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e62a973f8322b3bbe3ac
Fixes: e2674dfbed8a ("usb: atm: ueagle-atm: wait for pre-firmware load in .disconnect()")
Signed-off-by: Diego Fernando Mancera Gomez <diegomancera.dev@gmail.com>
---
 drivers/usb/atm/ueagle-atm.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 4e71ed679a76..4266a0cb7e3b 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -2549,6 +2549,7 @@ static struct usbatm_driver uea_usbatm_driver = {
 static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_device *usb = interface_to_usbdev(intf);
+	bool single_iface = usb->config->desc.bNumInterfaces == 1;
 	int ret;
 
 	uea_dbg(usb, "ADSL device found with vid (%#X) pid (%#X) Rev (%#X): %s\n",
@@ -2557,6 +2558,22 @@ static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		le16_to_cpu(usb->descriptor.bcdDevice),
 		chip_name[UEA_CHIP_VERSION(id)]);
 
+	/*
+	 * uea_probe() decides between the pre-firmware and post-firmware case
+	 * from the USB id and stores a different object as interface data in
+	 * each case: a struct completion for a pre-firmware device, a struct
+	 * usbatm_data for a post-firmware one. uea_disconnect() instead tells
+	 * the two apart by the number of interfaces (a pre-firmware device
+	 * exposes a single interface, ADI930 has 2 and eagle has 3). A crafted
+	 * device advertising a pre-firmware id together with a multi-interface
+	 * descriptor (or the other way around) makes the two disagree, so that
+	 * usbatm_usb_disconnect() treats the small completion object as a
+	 * struct usbatm_data and reads out of bounds. Reject such inconsistent
+	 * descriptors so both paths make the same decision.
+	 */
+	if (UEA_IS_PREFIRM(id) != single_iface)
+		return -ENODEV;
+
 	usb_reset_device(usb);
 
 	if (UEA_IS_PREFIRM(id)) {
-- 
2.43.0


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

* Re: [PATCH] usb: atm: ueagle-atm: reject descriptors that confuse probe and disconnect
  2026-07-17  8:07 [PATCH] usb: atm: ueagle-atm: reject descriptors that confuse probe and disconnect Diego Fernando Mancera Gomez
@ 2026-07-17  9:10 ` Stanislaw Gruszka
  0 siblings, 0 replies; 2+ messages in thread
From: Stanislaw Gruszka @ 2026-07-17  9:10 UTC (permalink / raw)
  To: Diego Fernando Mancera Gomez
  Cc: Matthieu CASTET, Chas Williams, Greg Kroah-Hartman,
	Mauricio Faria de Oliveira, linux-usb, netdev, linux-atm-general,
	linux-kernel, syzbot+e62a973f8322b3bbe3ac

On Fri, Jul 17, 2026 at 02:07:04AM -0600, Diego Fernando Mancera Gomez wrote:
> uea_probe() distinguishes a pre-firmware device from a post-firmware one
> using the USB id (UEA_IS_PREFIRM()), and stores a different object as the
> interface data in each case: a 'struct completion' for a pre-firmware
> device (to be waited on in .disconnect()), or a 'struct usbatm_data' for a
> post-firmware one.
> 
> uea_disconnect() instead tells the two apart by the number of interfaces
> of the active configuration (a pre-firmware device exposes a single
> interface, ADI930 has 2 and eagle has 3), and casts the interface data
> accordingly.
> 
> Because the two handlers use different criteria, a crafted device that
> advertises a pre-firmware id together with a multi-interface descriptor
> (or a post-firmware id with a single interface) makes them disagree: the
> small 'struct completion' stored by uea_probe() is then passed to
> usbatm_usb_disconnect(), which casts it to 'struct usbatm_data' and takes
> instance->serialize, reading past the end of the allocation:
> 
>   BUG: KASAN: slab-out-of-bounds in __mutex_lock+0x152a/0x1b80
>   Read of size 8 at addr ffff8880470e2c60 by task kworker/1:2/982
>   ...
>    __mutex_lock+0x152a/0x1b80
>    usbatm_usb_disconnect+0x70/0x820
>    uea_disconnect+0x133/0x2c0
>    usb_unbind_interface+0x1dd/0x9e0
>   ...
>   which belongs to the cache kmalloc-96 of size 96
>   The buggy address is located 0 bytes to the right of
>    allocated 96-byte region [ffff8880470e2c00, ffff8880470e2c60)
> 
> Reject such inconsistent descriptors in uea_probe() so that both handlers
> always make the same pre/post-firmware decision.
> 
> Reported-by: syzbot+e62a973f8322b3bbe3ac@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e62a973f8322b3bbe3ac
> Fixes: e2674dfbed8a ("usb: atm: ueagle-atm: wait for pre-firmware load in .disconnect()")
> Signed-off-by: Diego Fernando Mancera Gomez <diegomancera.dev@gmail.com>
Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>

Thanks
Stanislaw

> ---
>  drivers/usb/atm/ueagle-atm.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
> index 4e71ed679a76..4266a0cb7e3b 100644
> --- a/drivers/usb/atm/ueagle-atm.c
> +++ b/drivers/usb/atm/ueagle-atm.c
> @@ -2549,6 +2549,7 @@ static struct usbatm_driver uea_usbatm_driver = {
>  static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  {
>  	struct usb_device *usb = interface_to_usbdev(intf);
> +	bool single_iface = usb->config->desc.bNumInterfaces == 1;
>  	int ret;
>  
>  	uea_dbg(usb, "ADSL device found with vid (%#X) pid (%#X) Rev (%#X): %s\n",
> @@ -2557,6 +2558,22 @@ static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  		le16_to_cpu(usb->descriptor.bcdDevice),
>  		chip_name[UEA_CHIP_VERSION(id)]);
>  
> +	/*
> +	 * uea_probe() decides between the pre-firmware and post-firmware case
> +	 * from the USB id and stores a different object as interface data in
> +	 * each case: a struct completion for a pre-firmware device, a struct
> +	 * usbatm_data for a post-firmware one. uea_disconnect() instead tells
> +	 * the two apart by the number of interfaces (a pre-firmware device
> +	 * exposes a single interface, ADI930 has 2 and eagle has 3). A crafted
> +	 * device advertising a pre-firmware id together with a multi-interface
> +	 * descriptor (or the other way around) makes the two disagree, so that
> +	 * usbatm_usb_disconnect() treats the small completion object as a
> +	 * struct usbatm_data and reads out of bounds. Reject such inconsistent
> +	 * descriptors so both paths make the same decision.
> +	 */
> +	if (UEA_IS_PREFIRM(id) != single_iface)
> +		return -ENODEV;
> +
>  	usb_reset_device(usb);
>  
>  	if (UEA_IS_PREFIRM(id)) {
> -- 
> 2.43.0
> 

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

end of thread, other threads:[~2026-07-17  9:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17  8:07 [PATCH] usb: atm: ueagle-atm: reject descriptors that confuse probe and disconnect Diego Fernando Mancera Gomez
2026-07-17  9:10 ` Stanislaw Gruszka

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