Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
> On Sun, May 08, 2022 at 12:02:43AM +0800, Schspa Shi wrote:
>> 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.
>>
>> We can fix it by adding a new STATE_DEV_REGISTERING device state to
>> avoid double register.
>
> Are you sure that this patch will fix the problem found by syzbot? That
> is, are you sure that the problem really was caused by two threads
> registering the same driver concurrently?
>
Yes, from the console log from syzbot.
T8324 alloced driver_private was released by T8326.
> The fact that the error was "use after free" suggests that the problem
> might be something else. It looks like one of the threads was trying to
> access the driver structure after the other thread had done something
> that caused it to be deallocated, which suggests an error in reference
> counting.
>
The direct cause of this place is because of the refcount error, but the
root cause is still caused by multiple registrations
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.
And all the logs reported by syzbot can be matched to this scenario.
> Yes, this could be caused by two threads both registering the same
> driver. But the evidence doesn't prove that this is what happened, as
> far as I can see.
>
> Alan Stern
>
BRs
Schspa Shi
