Re: usb/gadget: GPF in usb_gadget_unregister_driver

[Krzysztof Opasiak]

On 12/05/2016 01:50 PM, Andrey Konovalov wrote:
> On Sat, Dec 3, 2016 at 6:13 PM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>> On Sat, Dec 03, 2016 at 05:36:35PM +0100, Andrey Konovalov wrote:
>>> Hi!
>>>
>>> I've got the following error report while running the syzkaller fuzzer.
>>>
>>> On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2).
>>>
>>> general protection fault: 0000 [#1] SMP KASAN
>>> Dumping ftrace buffer:
>>>    (ftrace buffer empty)
>>> Modules linked in:
>>> CPU: 0 PID: 10564 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #4
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>> task: ffff88006cd0db40 task.stack: ffff88006a050000
>>> RIP: 0010:[<ffffffff8201aa9e>]  [<ffffffff8201aa9e>]
>>> __list_del_entry+0xce/0x280 lib/list_debug.c:57
>>> RSP: 0018:ffff88006a056ea8  EFLAGS: 00010246
>>> RAX: dffffc0000000000 RBX: 1ffff1000d40add6 RCX: ffffffff860ceac8
>>> RDX: 0000000000000000 RSI: ffff88006cd0e340 RDI: ffffffff860cead0
>>> RBP: ffff88006a056f38 R08: 0000000000000000 R09: 0000000000000000
>>> R10: 0000000000000006 R11: 0000000000000000 R12: 0000000000000000
>>> R13: 0000000000000000 R14: ffffffff860cea00 R15: ffff88006a056f10
>>> FS:  0000000000000000(0000) GS:ffff88003ec00000(0000) knlGS:0000000000000000
>>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> CR2: 000000002000d000 CR3: 0000000005c1d000 CR4: 00000000000006f0
>>> Stack:
>>>  ffffffff859785e0 0000000041b58ab3 ffffffff8593717a ffffffff8201a9d0
>>>  1ffff1000d40ade0 ffff88006cd0db40 ffffffff817e5a3c ffff88006cd0e338
>>>  0000000000000a06 1ffff1000d40ade4 ffff88006cd0e340 0000000000000000
>>> Call Trace:
>>>  [<ffffffff8201ac5d>] list_del+0xd/0x70 lib/list_debug.c:77
>>>  [<ffffffff830ebf80>] usb_gadget_unregister_driver+0x120/0x240
>>> drivers/usb/gadget/udc/core.c:1365
>>>  [<ffffffff832245d0>] dev_release+0x80/0x160
>>> drivers/usb/gadget/legacy/inode.c:1187
>>>  [<ffffffff81805852>] __fput+0x332/0x7f0 fs/file_table.c:208
>>>  [<ffffffff81805d95>] ____fput+0x15/0x20 fs/file_table.c:244
>>>  [<ffffffff81338b9b>] task_work_run+0x19b/0x270 kernel/task_work.c:116
>>>  [<     inline     >] exit_task_work include/linux/task_work.h:21
>>>  [<ffffffff812c7eca>] do_exit+0x16aa/0x2530 kernel/exit.c:828
>>>  [<ffffffff812cd749>] do_group_exit+0x149/0x420 kernel/exit.c:932
>>>  [<ffffffff812faa9d>] get_signal+0x76d/0x17b0 kernel/signal.c:2307
>>>  [<ffffffff811cfee2>] do_signal+0xd2/0x2120 arch/x86/kernel/signal.c:807
>>>  [<ffffffff81003d00>] exit_to_usermode_loop+0x170/0x200
>>> arch/x86/entry/common.c:156
>>>  [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:190
>>>  [<ffffffff81007293>] syscall_return_slowpath+0x3d3/0x420
>>> arch/x86/entry/common.c:259
>>>  [<ffffffff84f47f62>] entry_SYSCALL_64_fastpath+0xc0/0xc2
>>> Code: c5 0f 84 e2 00 00 00 48 b8 00 02 00 00 00 00 ad de 49 39 c4 0f
>>> 84 ec 00 00 00 4c 89 e2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80>
>>> 3c 02 00 0f 85 35 01 00 00 4d 8b 04 24 49 39 c8 0f 85 e1 00
>>> RIP  [<ffffffff8201aa9e>] __list_del_entry+0xce/0x280 lib/list_debug.c:57
>>>  RSP <ffff88006a056ea8>
>>> ---[ end trace 883f708e6720200f ]---
>>> Kernel panic - not syncing: Fatal exception
>>> Dumping ftrace buffer:
>>>    (ftrace buffer empty)
>>> Kernel Offset: disabled
>>
>> Any hints as to what you were doing when this happend?
> 
> I've looked at the code.
> 
> As far as I can see, it's possible for usb_gadget_probe_driver() to
> return 0 without adding driver to gadget_driver_pending_list (in case
> strcmp() returns 0 and driver->match_existing_only == true).
> As a result dev->gadget_registered will be set to true in dev_config()
> and dev_release() will call usb_gadget_unregister_driver(), which in
> turn will try to remove driver from gadget_driver_pending_list.
> 
> Does it make any sense?
> 

Yes. We should add gadget to pending list only if suitable UDC has not
been found and gadget driver is willing to wait for new UDC to appear.
Otherwise we call udc_bind_to_gadget() and if this success then we
should just return 0 as we took some free udc. usually you should have
empty pending list;)

Your description sounds really quite similar to what Felix fixed in his
patch[1]. This patch fix this function in case when we found matching
UDC but it is busy.

Footnotes:
1 - http://marc.info/?l=linux-usb&m=148054830631070&w=2

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics