The usb_gadget_register_driver can be called multi time by to
threads via USB_RAW_IOCTL_RUN ioctl syscall, which will lead
to multiple registrations.
Call trace:
driver_register+0x220/0x3a0 drivers/base/driver.c:171
usb_gadget_register_driver_owner+0xfb/0x1e0
drivers/usb/gadget/udc/core.c:1546
raw_ioctl_run drivers/usb/gadget/legacy/raw_gadget.c:513 [inline]
raw_ioctl+0x1883/0x2730 drivers/usb/gadget/legacy/raw_gadget.c:1220
ioctl USB_RAW_IOCTL_RUN
This routine allows two processes to register the same driver instance
via ioctl syscall. which lead to a race condition.
Please refer to the following scenarios.
T1 T2
------------------------------------------------------------------
usb_gadget_register_driver_owner
driver_register driver_register
driver_find driver_find
bus_add_driver bus_add_driver
priv alloced <context switch>
drv->p = priv;
<schedule out>
kobject_init_and_add // refcount = 1;
//couldn't find an available UDC or it's busy
<context switch>
priv alloced
drv->priv = priv;
kobject_init_and_add
---> refcount = 1 <------
// register success
<context switch>
===================== another ioctl/process ======================
driver_register
driver_find
k = kset_find_obj()
---> refcount = 2 <------
<context out>
driver_unregister
// drv->p become T2's priv
---> refcount = 1 <------
<context switch>
kobject_put(k)
---> refcount = 0 <------
return priv->driver;
--------UAF here----------
There will be UAF in this scenario.
We can fix it by adding a new STATE_DEV_REGISTERING device state to
avoid double register.
Reported-by: syzbot+dc7c3ca638e773db07f6@xxxxxxxxxxxxxxxxxxxxxxxxx
Link: https://lore.kernel.org/all/000000000000e66c2805de55b15a@xxxxxxxxxx/
Signed-off-by: Schspa Shi <schspa@xxxxxxxxx>
---
Changelog:
v1 -> v2:
- Add a STATE_DEV_REGISTERING as Alan Stern suggested.
v2 -> v3:
- Adjust STATE_DEV_REGISTERING position to reflect the actual
order in which the states occur.
- Add the fault scenarios to comments.
---
drivers/usb/gadget/legacy/raw_gadget.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index b3be8db1ff63..241740024c50 100644
--- a/drivers/usb/gadget/legacy/raw_gadget.c
+++ b/drivers/usb/gadget/legacy/raw_gadget.c
@@ -145,6 +145,7 @@ enum dev_state {
STATE_DEV_INVALID = 0,
STATE_DEV_OPENED,
STATE_DEV_INITIALIZED,
+ STATE_DEV_REGISTERING,
STATE_DEV_RUNNING,
STATE_DEV_CLOSED,
STATE_DEV_FAILED
@@ -508,6 +509,7 @@ static int raw_ioctl_run(struct raw_dev *dev, unsigned long value)
ret = -EINVAL;
goto out_unlock;
}
+ dev->state = STATE_DEV_REGISTERING;
spin_unlock_irqrestore(&dev->lock, flags);
ret = usb_gadget_register_driver(&dev->driver);
--
2.24.3 (Apple Git-128)
