Re: [PATCH] usb: atm: ueagle-atm: reject descriptors that confuse probe and disconnect

[Stanislaw Gruszka <stf_xl@wp.pl>]

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
>