Forwarded: [PATCH] usb: atm: ueagle: fix use-after-free in uea_upload_pre_firmware()

[syzbot <syzbot+3d45d763d18796f97412@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] usb: atm: ueagle: fix use-after-free in uea_upload_pre_firmware()
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master


uea_load_firmware() calls request_firmware_nowait() passing a raw
struct usb_device pointer as the callback context, without holding
a reference to it.

If the USB device is disconnected before the firmware workqueue fires,
the usb_device is freed while uea_upload_pre_firmware() is still
executing, causing a slab-use-after-free:

  BUG: KASAN: slab-use-after-free in __intf_to_usbdev
  include/linux/usb.h:752 [inline]
  BUG: KASAN: slab-use-after-free in uea_upload_pre_firmware+0x8d/0x640
  drivers/usb/atm/ueagle-atm.c:598
  Read of size 8 at addr ffff88802b0710b8 by task kworker/0:2/1664

The root cause: Eagle ADSL modems enumerate twice. On first plug-in
they present a pre-firmware PID; uea_probe() calls uea_load_firmware()
which queues the firmware upload asynchronously and returns immediately.
The USB core considers probing complete. If the cable is pulled at this
point, the usb_device is torn down while the firmware callback is still
pending or running on the workqueue.

Fix by calling usb_get_dev() before queuing the request to pin the
usb_device in memory for the lifetime of the async operation, and
usb_put_dev() in the callback once it is finished with the pointer.
On the error path where request_firmware_nowait() itself fails, drop
the reference immediately since the callback will never fire.

Reported-by: syzbot+3d45d763d18796f97412@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3d45d763d18796f97412
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/usb/atm/ueagle-atm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index d610cdcef7d0..686cc58fb89f 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -663,6 +663,7 @@ static void uea_upload_pre_firmware(const struct firmware *fw_entry,
 	uea_err(usb, "firmware is corrupted\n");
 err:
 	release_firmware(fw_entry);
+	usb_put_dev(usb);
 }
 
 /*
@@ -693,12 +694,14 @@ static int uea_load_firmware(struct usb_device *usb, unsigned int ver)
 		break;
 	}
 
+	usb_get_dev(usb);
 	ret = request_firmware_nowait(THIS_MODULE, 1, fw_name, &usb->dev,
 					GFP_KERNEL, usb,
 					uea_upload_pre_firmware);
-	if (ret)
+	if (ret) {
 		uea_err(usb, "firmware %s is not available\n", fw_name);
-	else
+		usb_put_dev(usb);
+	} else
 		uea_info(usb, "loading firmware %s\n", fw_name);
 
 	return ret;
-- 
2.43.0