* KASAN: use-after-free Read in usbhid_power
@ 2019-07-23 12:48 syzbot
2019-07-24 14:17 ` Oliver Neukum
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: syzbot @ 2019-07-23 12:48 UTC (permalink / raw)
To: andreyknvl, linux-kernel, linux-usb, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=11b13e78600000
kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
dashboard link: https://syzkaller.appspot.com/bug?extid=ef5de9c4f99c4edb4e49
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15482210600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1139b07c600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in usbhid_power+0xca/0xe0
/drivers/hid/usbhid/hid-core.c:1234
Read of size 8 at addr ffff8881ce8a4008 by task syz-executor373/1763
CPU: 0 PID: 1763 Comm: syz-executor373 Not tainted 5.2.0-rc6+ #15
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack /lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e /lib/dump_stack.c:113
print_address_description+0x67/0x231 /mm/kasan/report.c:188
__kasan_report.cold+0x1a/0x32 /mm/kasan/report.c:317
kasan_report+0xe/0x20 /mm/kasan/common.c:614
usbhid_power+0xca/0xe0 /drivers/hid/usbhid/hid-core.c:1234
hid_hw_power /./include/linux/hid.h:1038 [inline]
hidraw_open+0x20d/0x740 /drivers/hid/hidraw.c:282
chrdev_open+0x219/0x5c0 /fs/char_dev.c:413
do_dentry_open+0x497/0x1040 /fs/open.c:778
do_last /fs/namei.c:3416 [inline]
path_openat+0x1430/0x3ff0 /fs/namei.c:3533
do_filp_open+0x1a1/0x280 /fs/namei.c:3563
do_sys_open+0x3c0/0x580 /fs/open.c:1070
do_syscall_64+0xb7/0x560 /arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x401ad0
Code: 01 f0 ff ff 0f 83 c0 0b 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f
44 00 00 83 3d fd 5b 2d 00 00 75 14 b8 02 00 00 00 0f 05 <48> 3d 01 f0 ff
ff 0f 83 94 0b 00 00 c3 48 83 ec 08 e8 fa 00 00 00
RSP: 002b:00007ffff378ac48 EFLAGS: 00000246 ORIG_RAX: 0000000000000002
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000401ad0
RDX: 0000000000000000 RSI: 0000000000000800 RDI: 00007ffff378ac50
RBP: 6666666666666667 R08: 000000000000000f R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000402af0
R13: 0000000000402b80 R14: 0000000000000000 R15: 0000000000000000
Allocated by task 21:
save_stack+0x1b/0x80 /mm/kasan/common.c:71
set_track /mm/kasan/common.c:79 [inline]
__kasan_kmalloc /mm/kasan/common.c:489 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 /mm/kasan/common.c:462
slab_post_alloc_hook /mm/slab.h:437 [inline]
slab_alloc_node /mm/slub.c:2748 [inline]
__kmalloc_node_track_caller+0xee/0x370 /mm/slub.c:4363
__kmalloc_reserve.isra.0+0x39/0xe0 /net/core/skbuff.c:138
__alloc_skb+0xef/0x5a0 /net/core/skbuff.c:206
alloc_skb /./include/linux/skbuff.h:1054 [inline]
alloc_uevent_skb+0x7b/0x210 /lib/kobject_uevent.c:289
uevent_net_broadcast_untagged /lib/kobject_uevent.c:325 [inline]
kobject_uevent_net_broadcast /lib/kobject_uevent.c:408 [inline]
kobject_uevent_env+0x8ee/0x1150 /lib/kobject_uevent.c:592
device_add+0xade/0x16f0 /drivers/base/core.c:2110
usb_set_configuration+0xdf6/0x1670 /drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 /drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 /drivers/usb/core/driver.c:266
really_probe+0x281/0x660 /drivers/base/dd.c:509
driver_probe_device+0x104/0x210 /drivers/base/dd.c:670
__device_attach_driver+0x1c2/0x220 /drivers/base/dd.c:777
bus_for_each_drv+0x15c/0x1e0 /drivers/base/bus.c:454
__device_attach+0x217/0x360 /drivers/base/dd.c:843
bus_probe_device+0x1e4/0x290 /drivers/base/bus.c:514
device_add+0xae6/0x16f0 /drivers/base/core.c:2111
usb_new_device.cold+0x6a4/0xe61 /drivers/usb/core/hub.c:2536
hub_port_connect /drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change /drivers/usb/core/hub.c:5213 [inline]
port_event /drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1abd/0x3550 /drivers/usb/core/hub.c:5441
process_one_work+0x905/0x1570 /kernel/workqueue.c:2269
process_scheduled_works /kernel/workqueue.c:2331 [inline]
worker_thread+0x7ab/0xe20 /kernel/workqueue.c:2417
kthread+0x30b/0x410 /kernel/kthread.c:255
ret_from_fork+0x24/0x30 /arch/x86/entry/entry_64.S:352
Freed by task 243:
save_stack+0x1b/0x80 /mm/kasan/common.c:71
set_track /mm/kasan/common.c:79 [inline]
__kasan_slab_free+0x130/0x180 /mm/kasan/common.c:451
slab_free_hook /mm/slub.c:1421 [inline]
slab_free_freelist_hook /mm/slub.c:1448 [inline]
slab_free /mm/slub.c:2994 [inline]
kfree+0xd7/0x280 /mm/slub.c:3949
skb_free_head+0x8b/0xa0 /net/core/skbuff.c:588
skb_release_data+0x41f/0x7c0 /net/core/skbuff.c:608
skb_release_all+0x46/0x60 /net/core/skbuff.c:662
__kfree_skb /net/core/skbuff.c:676 [inline]
consume_skb /net/core/skbuff.c:736 [inline]
consume_skb+0xc0/0x2f0 /net/core/skbuff.c:730
skb_free_datagram+0x16/0xf0 /net/core/datagram.c:328
netlink_recvmsg+0x65e/0xea0 /net/netlink/af_netlink.c:2001
sock_recvmsg_nosec /net/socket.c:877 [inline]
sock_recvmsg /net/socket.c:894 [inline]
sock_recvmsg+0xca/0x110 /net/socket.c:890
___sys_recvmsg+0x271/0x5a0 /net/socket.c:2448
__sys_recvmsg+0xe9/0x1b0 /net/socket.c:2497
do_syscall_64+0xb7/0x560 /arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8881ce8a4000
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 8 bytes inside of
1024-byte region [ffff8881ce8a4000, ffff8881ce8a4400)
The buggy address belongs to the page:
page:ffffea00073a2900 refcount:1 mapcount:0 mapping:ffff8881dac02a00
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 dead000000000100 dead000000000200 ffff8881dac02a00
raw: 0000000000000000 00000000800e000e 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881ce8a3f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8881ce8a3f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8881ce8a4000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881ce8a4080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881ce8a4100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-23 12:48 KASAN: use-after-free Read in usbhid_power syzbot
@ 2019-07-24 14:17 ` Oliver Neukum
2019-07-24 14:24 ` Andrey Konovalov
2019-07-24 20:55 ` Oliver Neukum
2019-07-25 12:15 ` Oliver Neukum
2 siblings, 1 reply; 28+ messages in thread
From: Oliver Neukum @ 2019-07-24 14:17 UTC (permalink / raw)
To: syzbot, andreyknvl, syzkaller-bugs, linux-kernel, linux-usb
Am Dienstag, den 23.07.2019, 05:48 -0700 schrieb syzbot:
>
> Freed by task 243:
> save_stack+0x1b/0x80 /mm/kasan/common.c:71
> set_track /mm/kasan/common.c:79 [inline]
> __kasan_slab_free+0x130/0x180 /mm/kasan/common.c:451
> slab_free_hook /mm/slub.c:1421 [inline]
> slab_free_freelist_hook /mm/slub.c:1448 [inline]
> slab_free /mm/slub.c:2994 [inline]
> kfree+0xd7/0x280 /mm/slub.c:3949
> skb_free_head+0x8b/0xa0 /net/core/skbuff.c:588
> skb_release_data+0x41f/0x7c0 /net/core/skbuff.c:608
> skb_release_all+0x46/0x60 /net/core/skbuff.c:662
> __kfree_skb /net/core/skbuff.c:676 [inline]
> consume_skb /net/core/skbuff.c:736 [inline]
> consume_skb+0xc0/0x2f0 /net/core/skbuff.c:730
> skb_free_datagram+0x16/0xf0 /net/core/datagram.c:328
> netlink_recvmsg+0x65e/0xea0 /net/netlink/af_netlink.c:2001
> sock_recvmsg_nosec /net/socket.c:877 [inline]
> sock_recvmsg /net/socket.c:894 [inline]
> sock_recvmsg+0xca/0x110 /net/socket.c:890
> ___sys_recvmsg+0x271/0x5a0 /net/socket.c:2448
> __sys_recvmsg+0xe9/0x1b0 /net/socket.c:2497
> do_syscall_64+0xb7/0x560 /arch/x86/entry/common.c:301
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
How reliable is this trace? It seems very likely to me that this bug
is the same bug as
syzbot <syzbot+ded1794a717e3b235226@syzkaller.appspotmail.com>
KASAN: use-after-free Read in hidraw_ioctl
which shows a race with disconnect() instead of some networking code,
which I really cannot fathom.
Regards
Oliver
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-24 14:17 ` Oliver Neukum
@ 2019-07-24 14:24 ` Andrey Konovalov
0 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2019-07-24 14:24 UTC (permalink / raw)
To: Oliver Neukum; +Cc: syzbot, syzkaller-bugs, LKML, USB list
On Wed, Jul 24, 2019 at 4:17 PM Oliver Neukum <oneukum@suse.com> wrote:
>
> Am Dienstag, den 23.07.2019, 05:48 -0700 schrieb syzbot:
> >
> > Freed by task 243:
> > save_stack+0x1b/0x80 /mm/kasan/common.c:71
> > set_track /mm/kasan/common.c:79 [inline]
> > __kasan_slab_free+0x130/0x180 /mm/kasan/common.c:451
> > slab_free_hook /mm/slub.c:1421 [inline]
> > slab_free_freelist_hook /mm/slub.c:1448 [inline]
> > slab_free /mm/slub.c:2994 [inline]
> > kfree+0xd7/0x280 /mm/slub.c:3949
> > skb_free_head+0x8b/0xa0 /net/core/skbuff.c:588
> > skb_release_data+0x41f/0x7c0 /net/core/skbuff.c:608
> > skb_release_all+0x46/0x60 /net/core/skbuff.c:662
> > __kfree_skb /net/core/skbuff.c:676 [inline]
> > consume_skb /net/core/skbuff.c:736 [inline]
> > consume_skb+0xc0/0x2f0 /net/core/skbuff.c:730
> > skb_free_datagram+0x16/0xf0 /net/core/datagram.c:328
> > netlink_recvmsg+0x65e/0xea0 /net/netlink/af_netlink.c:2001
> > sock_recvmsg_nosec /net/socket.c:877 [inline]
> > sock_recvmsg /net/socket.c:894 [inline]
> > sock_recvmsg+0xca/0x110 /net/socket.c:890
> > ___sys_recvmsg+0x271/0x5a0 /net/socket.c:2448
> > __sys_recvmsg+0xe9/0x1b0 /net/socket.c:2497
> > do_syscall_64+0xb7/0x560 /arch/x86/entry/common.c:301
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> How reliable is this trace? It seems very likely to me that this bug
> is the same bug as
>
> syzbot <syzbot+ded1794a717e3b235226@syzkaller.appspotmail.com>
> KASAN: use-after-free Read in hidraw_ioctl
>
> which shows a race with disconnect() instead of some networking code,
> which I really cannot fathom.
To me the alloc/free stack trace doesn't seem to have anything to do
with the access stack trace here, so I wouldn't rely on them. This is
a limitation of KASAN and can happen when there's a wild memory access
or a huge out-of-bounds or some kind of a racy-use-after-free.
>
> Regards
> Oliver
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-23 12:48 KASAN: use-after-free Read in usbhid_power syzbot
2019-07-24 14:17 ` Oliver Neukum
@ 2019-07-24 20:55 ` Oliver Neukum
2019-07-24 21:02 ` Alan Stern
2019-07-24 21:16 ` syzbot
2019-07-25 12:15 ` Oliver Neukum
2 siblings, 2 replies; 28+ messages in thread
From: Oliver Neukum @ 2019-07-24 20:55 UTC (permalink / raw)
To: syzbot, andreyknvl, syzkaller-bugs, linux-usb
Am Dienstag, den 23.07.2019, 05:48 -0700 schrieb syzbot:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=11b13e78600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
> dashboard link: https://syzkaller.appspot.com/bug?extid=ef5de9c4f99c4edb4e49
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15482210600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1139b07c600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in usbhid_power+0xca/0xe0
> /drivers/hid/usbhid/hid-core.c:1234
> Read of size 8 at addr ffff8881ce8a4008 by task syz-executor373/1763
#syz test: https://github.com/google/kasan.git usb-fuzzer
From a3455c4527bdfe86536856ea96288b7dcc36590b Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 24 Jul 2019 22:49:57 +0200
Subject: [PATCH] usbhid: test for disconnected state in raw opening
If the device has been disconnected, we should not proceed to use
runtime PM on it.
Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index c7bc9db5b192..98b996ecf4d3 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
struct usbhid_device *usbhid = hid->driver_data;
int r = 0;
+ spin_lock_irq(&usbhid->lock);
+ if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
+ r = -ENODEV;
+ spin_unlock_irq(&usbhid->lock);
+ goto bail_out;
+ } else {
+ /* protect against disconnect */
+ usb_get_dev(interface_to_usbdev(usbhid->intf));
+ spin_unlock_irq(&usbhid->lock);
+ }
+
switch (lvl) {
case PM_HINT_FULLON:
r = usb_autopm_get_interface(usbhid->intf);
@@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
usb_autopm_put_interface(usbhid->intf);
break;
}
+ usb_put_dev(interface_to_usbdev(usbhid->intf));
+bail_out:
return r;
}
--
2.16.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-24 20:55 ` Oliver Neukum
@ 2019-07-24 21:02 ` Alan Stern
2019-07-25 9:33 ` Oliver Neukum
2019-07-24 21:16 ` syzbot
1 sibling, 1 reply; 28+ messages in thread
From: Alan Stern @ 2019-07-24 21:02 UTC (permalink / raw)
To: Oliver Neukum; +Cc: syzbot, andreyknvl, syzkaller-bugs, linux-usb
On Wed, 24 Jul 2019, Oliver Neukum wrote:
> Am Dienstag, den 23.07.2019, 05:48 -0700 schrieb syzbot:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
> > git tree: https://github.com/google/kasan.git usb-fuzzer
> > console output: https://syzkaller.appspot.com/x/log.txt?x=11b13e78600000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
> > dashboard link: https://syzkaller.appspot.com/bug?extid=ef5de9c4f99c4edb4e49
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15482210600000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1139b07c600000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in usbhid_power+0xca/0xe0
> > /drivers/hid/usbhid/hid-core.c:1234
> > Read of size 8 at addr ffff8881ce8a4008 by task syz-executor373/1763
>
> #syz test: https://github.com/google/kasan.git usb-fuzzer
>
> From a3455c4527bdfe86536856ea96288b7dcc36590b Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oneukum@suse.com>
> Date: Wed, 24 Jul 2019 22:49:57 +0200
> Subject: [PATCH] usbhid: test for disconnected state in raw opening
>
> If the device has been disconnected, we should not proceed to use
> runtime PM on it.
>
> Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
> drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index c7bc9db5b192..98b996ecf4d3 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> struct usbhid_device *usbhid = hid->driver_data;
> int r = 0;
>
> + spin_lock_irq(&usbhid->lock);
> + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> + r = -ENODEV;
> + spin_unlock_irq(&usbhid->lock);
> + goto bail_out;
> + } else {
> + /* protect against disconnect */
> + usb_get_dev(interface_to_usbdev(usbhid->intf));
> + spin_unlock_irq(&usbhid->lock);
> + }
> +
> switch (lvl) {
> case PM_HINT_FULLON:
> r = usb_autopm_get_interface(usbhid->intf);
> @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> usb_autopm_put_interface(usbhid->intf);
> break;
> }
> + usb_put_dev(interface_to_usbdev(usbhid->intf));
>
> +bail_out:
> return r;
> }
Isn't this treating the symptom instead of the cause?
Shouldn't the hid_device hold a reference to usbhid->intf throughout
its lifetime? That way this sort of problem wouldn't arise in any
routine, not just usbhid_power().
Alan Stern
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-24 20:55 ` Oliver Neukum
2019-07-24 21:02 ` Alan Stern
@ 2019-07-24 21:16 ` syzbot
2019-07-25 11:22 ` Andrey Konovalov
1 sibling, 1 reply; 28+ messages in thread
From: syzbot @ 2019-07-24 21:16 UTC (permalink / raw)
To: andreyknvl, linux-usb, oneukum, syzkaller-bugs
Hello,
syzbot tried to test the proposed patch but build/boot failed:
T1] devtmpfs: initialized
[ 2.873454][ T1] clocksource: jiffies: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 19112604462750000 ns
[ 2.873454][ T1] futex hash table entries: 512 (order: 4, 65536
bytes, linear)
[ 2.875783][ T1] PM: RTC time: 21:05:06, date: 2019-07-24
[ 2.882105][ T1] NET: Registered protocol family 16
[ 2.886588][ T1] audit: initializing netlink subsys (disabled)
[ 2.888579][ T22] audit: type=2000 audit(1564002306.507:1):
state=initialized audit_enabled=0 res=1
[ 2.888579][ T1] cpuidle: using governor menu
[ 2.893505][ T1] ACPI: bus type PCI registered
[ 2.894935][ T1] PCI: Using configuration type 1 for base access
[ 3.002926][ T1] HugeTLB registered 2.00 MiB page size, pre-allocated
0 pages
[ 3.003285][ T27] cryptomgr_test (27) used greatest stack depth: 30208
bytes left
[ 3.003285][ T29] kworker/u4:0 (29) used greatest stack depth: 27608
bytes left
[ 3.011409][ T43] kworker/u4:0 (43) used greatest stack depth: 27120
bytes left
[ 3.022044][ T1] ACPI: Added _OSI(Module Device)
[ 3.022884][ T1] ACPI: Added _OSI(Processor Device)
[ 3.023651][ T1] ACPI: Added _OSI(3.0 _SCP Extensions)
[ 3.031207][ T1] ACPI: Added _OSI(Processor Aggregator Device)
[ 3.031207][ T1] ACPI: Added _OSI(Linux-Dell-Video)
[ 3.031249][ T1] ACPI: Added _OSI(Linux-Lenovo-NV-HDMI-Audio)
[ 3.032156][ T1] ACPI: Added _OSI(Linux-HPI-Hybrid-Graphics)
[ 3.082907][ T1] ACPI: 2 ACPI AML tables successfully acquired and
loaded
[ 3.097843][ T1] ACPI: Interpreter enabled
[ 3.098853][ T1] ACPI: (supports S0 S3 S4 S5)
[ 3.099590][ T1] ACPI: Using IOAPIC for interrupt routing
[ 3.100591][ T1] PCI: Using host bridge windows from ACPI; if
necessary, use "pci=nocrs" and report a bug
[ 3.104257][ T1] ACPI: Enabled 16 GPEs in block 00 to 0F
[ 3.182493][ T1] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus
00-ff])
[ 3.183851][ T1] acpi PNP0A03:00: _OSC: OS supports [ASPM ClockPM
Segments MSI HPX-Type3]
[ 3.185519][ T1] acpi PNP0A03:00: fail to add MMCONFIG information,
can't access extended PCI configuration space under this bridge.
[ 3.192575][ T1] PCI host bridge to bus 0000:00
[ 3.193286][ T1] pci_bus 0000:00: root bus resource [io
0x0000-0x0cf7 window]
[ 3.194360][ T1] pci_bus 0000:00: root bus resource [io
0x0d00-0xffff window]
[ 3.195393][ T1] pci_bus 0000:00: root bus resource [mem
0x000a0000-0x000bffff window]
[ 3.196522][ T1] pci_bus 0000:00: root bus resource [mem
0xc0000000-0xfebfffff window]
[ 3.197715][ T1] pci_bus 0000:00: root bus resource [bus 00-ff]
[ 3.198886][ T1] pci 0000:00:00.0: [8086:1237] type 00 class 0x060000
[ 3.204649][ T1] pci 0000:00:01.0: [8086:7110] type 00 class 0x060100
[ 3.222865][ T1] pci 0000:00:01.3: [8086:7113] type 00 class 0x068000
[ 3.241306][ T1] pci 0000:00:01.3: quirk: [io 0xb000-0xb03f] claimed
by PIIX4 ACPI
[ 3.245348][ T1] pci 0000:00:03.0: [1af4:1004] type 00 class 0x000000
[ 3.251166][ T1] pci 0000:00:03.0: reg 0x10: [io 0xc000-0xc03f]
[ 3.255600][ T1] pci 0000:00:03.0: reg 0x14: [mem
0xfebfe000-0xfebfe07f]
[ 3.271694][ T1] pci 0000:00:04.0: [1af4:1000] type 00 class 0x020000
[ 3.277964][ T1] pci 0000:00:04.0: reg 0x10: [io 0xc040-0xc07f]
[ 3.283798][ T1] pci 0000:00:04.0: reg 0x14: [mem
0xfebff000-0xfebff07f]
[ 3.310020][ T1] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
[ 3.314509][ T1] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
[ 3.318340][ T1] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
[ 3.322364][ T1] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
[ 3.324796][ T1] ACPI: PCI Interrupt Link [LNKS] (IRQs *9)
[ 3.332253][ T1] vgaarb: loaded
[ 3.333359][ T1] SCSI subsystem initialized
[ 3.334153][ T1] ACPI: bus type USB registered
[ 3.334153][ T1] usbcore: registered new interface driver usbfs
[ 3.334153][ T1] usbcore: registered new interface driver hub
[ 3.341195][ T1] usbcore: registered new device driver usb
[ 3.341471][ T1] mc: Linux media interface: v0.10
[ 3.342295][ T1] videodev: Linux video capture interface: v2.00
[ 3.343599][ T1] pps_core: LinuxPPS API ver. 1 registered
[ 3.344464][ T1] pps_core: Software ver. 5.3.6 - Copyright 2005-2007
Rodolfo Giometti <giometti@linux.it>
[ 3.345876][ T1] PTP clock support registered
[ 3.346781][ T1] EDAC MC: Ver: 3.0.0
[ 3.346781][ T1] Advanced Linux Sound Architecture Driver Initialized.
[ 3.346781][ T1] PCI: Using ACPI for IRQ routing
[ 3.353286][ T1] Bluetooth: Core ver 2.22
[ 3.354202][ T1] NET: Registered protocol family 31
[ 3.354955][ T1] Bluetooth: HCI device and connection manager
initialized
[ 3.355989][ T1] Bluetooth: HCI socket layer initialized
[ 3.356767][ T1] Bluetooth: L2CAP socket layer initialized
[ 3.357644][ T1] Bluetooth: SCO socket layer initialized
[ 3.358472][ T1] NET: Registered protocol family 8
[ 3.359220][ T1] NET: Registered protocol family 20
[ 3.360093][ T1] NetLabel: Initializing
[ 3.360715][ T1] NetLabel: domain hash size = 128
[ 3.361148][ T1] NetLabel: protocols = UNLABELED CIPSOv4 CALIPSO
[ 3.362492][ T1] NetLabel: unlabeled traffic allowed by default
[ 3.363724][ T1] nfc: nfc_init: NFC Core ver 0.1
[ 3.363724][ T1] NET: Registered protocol family 39
[ 3.365420][ T1] clocksource: Switched to clocksource kvm-clock
[ 4.371843][ T1] VFS: Disk quotas dquot_6.6.0
[ 4.372992][ T1] VFS: Dquot-cache hash table entries: 512 (order 0,
4096 bytes)
[ 4.374598][ T1] *** VALIDATE hugetlbfs ***
[ 4.376107][ T1] pnp: PnP ACPI init
[ 4.387313][ T1] pnp: PnP ACPI: found 7 devices
[ 4.413711][ T1] thermal_sys: Registered thermal governor 'step_wise'
[ 4.413717][ T1] thermal_sys: Registered thermal governor 'user_space'
[ 4.419892][ T1] clocksource: acpi_pm: mask: 0xffffff max_cycles:
0xffffff, max_idle_ns: 2085701024 ns
[ 4.422972][ T1] pci_bus 0000:00: resource 4 [io 0x0000-0x0cf7
window]
[ 4.424189][ T1] pci_bus 0000:00: resource 5 [io 0x0d00-0xffff
window]
[ 4.425394][ T1] pci_bus 0000:00: resource 6 [mem
0x000a0000-0x000bffff window]
[ 4.426791][ T1] pci_bus 0000:00: resource 7 [mem
0xc0000000-0xfebfffff window]
[ 4.428947][ T1] NET: Registered protocol family 2
[ 4.431371][ T1] tcp_listen_portaddr_hash hash table entries: 4096
(order: 6, 294912 bytes, linear)
[ 4.433417][ T1] TCP established hash table entries: 65536 (order: 7,
524288 bytes, linear)
[ 4.435847][ T1] TCP bind hash table entries: 65536 (order: 10,
4194304 bytes, linear)
[ 4.443410][ T1] TCP: Hash tables configured (established 65536 bind
65536)
[ 4.445306][ T1] UDP hash table entries: 4096 (order: 7, 655360
bytes, linear)
[ 4.447824][ T1] UDP-Lite hash table entries: 4096 (order: 7, 655360
bytes, linear)
[ 4.450432][ T1] NET: Registered protocol family 1
[ 4.453345][ T1] RPC: Registered named UNIX socket transport module.
[ 4.454510][ T1] RPC: Registered udp transport module.
[ 4.455454][ T1] RPC: Registered tcp transport module.
[ 4.456446][ T1] RPC: Registered tcp NFSv4.1 backchannel transport
module.
[ 4.458621][ T1] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
[ 4.459864][ T1] PCI: CLS 0 bytes, default 64
[ 4.461378][ T1] PCI-DMA: Using software bounce buffering for IO
(SWIOTLB)
[ 4.462761][ T1] software IO TLB: mapped [mem 0xbbffd000-0xbfffd000]
(64MB)
[ 4.466413][ T1] RAPL PMU: API unit is 2^-32 Joules, 0 fixed
counters, 10737418240 ms ovfl timer
[ 4.467994][ T1] clocksource: tsc: mask: 0xffffffffffffffff
max_cycles: 0x212735223b2, max_idle_ns: 440795277976 ns
[ 4.469791][ T1] clocksource: Switched to clocksource tsc
[ 4.473423][ T1] check: Scanning for low memory corruption every 60
seconds
[ 4.478343][ T1] Initialise system trusted keyrings
[ 4.479810][ T1] workingset: timestamp_bits=40 max_order=21
bucket_order=0
[ 4.521420][ T1] NFS: Registering the id_resolver key type
[ 4.522729][ T1] Key type id_resolver registered
[ 4.523526][ T1] Key type id_legacy registered
[ 4.524942][ T1] 9p: Installing v9fs 9p2000 file system support
[ 4.531784][ T1] Key type asymmetric registered
[ 4.532779][ T1] Asymmetric key parser 'x509' registered
[ 4.533772][ T1] Block layer SCSI generic (bsg) driver version 0.4
loaded (major 247)
[ 4.535226][ T1] io scheduler mq-deadline registered
[ 4.536032][ T1] io scheduler kyber registered
[ 4.539777][ T1] usbcore: registered new interface driver udlfb
[ 4.541021][ T1] usbcore: registered new interface driver smscufx
[ 4.543014][ T1] input: Power Button as
/devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
[ 4.544520][ T1] ACPI: Power Button [PWRF]
[ 4.545743][ T1] input: Sleep Button as
/devices/LNXSYSTM:00/LNXSLPBN:00/input/input1
[ 4.547129][ T1] ACPI: Sleep Button [SLPF]
[ 4.560609][ T1] PCI Interrupt Link [LNKC] enabled at IRQ 11
[ 4.562043][ T1] virtio-pci 0000:00:03.0: virtio_pci: leaving for
legacy driver
[ 4.575190][ T1] PCI Interrupt Link [LNKD] enabled at IRQ 10
[ 4.576547][ T1] virtio-pci 0000:00:04.0: virtio_pci: leaving for
legacy driver
[ 4.582237][ T1] Serial: 8250/16550 driver, 4 ports, IRQ sharing
enabled
[ 4.606272][ T1] 00:03: ttyS0 at I/O 0x3f8 (irq = 4, base_baud =
115200) is a 16550A
[ 4.631884][ T1] 00:04: ttyS1 at I/O 0x2f8 (irq = 3, base_baud =
115200) is a 16550A
[ 4.656881][ T1] 00:05: ttyS2 at I/O 0x3e8 (irq = 6, base_baud =
115200) is a 16550A
[ 4.682193][ T1] 00:06: ttyS3 at I/O 0x2e8 (irq = 7, base_baud =
115200) is a 16550A
[ 4.687302][ T1] Non-volatile memory driver v1.3
[ 4.688815][ T1] Linux agpgart interface v0.103
[ 4.695497][ T1] usbcore: registered new interface driver udl
[ 4.719445][ T1] loop: module loaded
[ 4.721039][ T1] usbcore: registered new interface driver rtsx_usb
[ 4.723136][ T1] usbcore: registered new interface driver viperboard
[ 4.725065][ T1] usbcore: registered new interface driver dln2
[ 4.726853][ T1] usbcore: registered new interface driver pn533_usb
[ 4.729089][ T1] usbcore: registered new interface driver port100
[ 4.731604][ T1] usbcore: registered new interface driver nfcmrvl
[ 4.746210][ T1] scsi host0: Virtio SCSI HBA
[ 4.792288][ T1] kasan: CONFIG_KASAN_INLINE enabled
[ 4.793729][ T1] kasan: GPF could be caused by NULL-ptr deref or user
memory access
[ 4.795771][ T1] general protection fault: 0000 [#1] SMP KASAN
[ 4.797547][ T1] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.3.0-rc1+
#1
[ 4.799786][ T1] Hardware name: Google Google Compute Engine/Google
Compute Engine, BIOS Google 01/01/2011
[ 4.801153][ T1] RIP: 0010:dma_direct_max_mapping_size+0x73/0x19a
[ 4.801153][ T1] Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 1e
01 00 00 48 8b 9d 38 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 da 48 c1
ea 03 <80> 3c 02 00 0f 85 06 01 00 00 48 8d bd 48 03 00 00 48 8b 1b 48 b8
[ 4.801153][ T1] RSP: 0000:ffff8881da18f628 EFLAGS: 00010246
[ 4.801153][ T1] RAX: dffffc0000000000 RBX: 0000000000000000 RCX:
ffffffff812d716c
[ 4.801153][ T1] RDX: 0000000000000000 RSI: ffffffff812d7189 RDI:
ffff8881d829fa78
[ 4.801153][ T1] RBP: ffff8881d829f740 R08: ffff8881da180000 R09:
ffffed103ad210cb
[ 4.801153][ T1] R10: ffffed103ad210ca R11: ffff8881d6908657 R12:
ffff8881d829f740
[ 4.801153][ T1] R13: ffff8881d76a57b0 R14: 0000000000000200 R15:
0000000000000000
[ 4.801153][ T1] FS: 0000000000000000(0000)
GS:ffff8881db300000(0000) knlGS:0000000000000000
[ 4.801153][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4.801153][ T1] CR2: 0000000000000000 CR3: 0000000006a21000 CR4:
00000000001406e0
[ 4.801153][ T1] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 4.801153][ T1] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 4.801153][ T1] Call Trace:
[ 4.801153][ T1] dma_max_mapping_size+0xb5/0xf0
[ 4.801153][ T1] __scsi_init_queue+0x17e/0x510
[ 4.801153][ T1] scsi_mq_alloc_queue+0xcb/0x170
[ 4.801153][ T1] scsi_alloc_sdev+0x82e/0xc50
[ 4.801153][ T1] scsi_probe_and_add_lun+0x1ee5/0x2cd0
[ 4.801153][ T1] ? find_held_lock+0x2d/0x110
[ 4.801153][ T1] ? __pm_runtime_resume+0x111/0x180
[ 4.801153][ T1] ? scsi_alloc_sdev+0xc50/0xc50
[ 4.801153][ T1] ? mark_held_locks+0x9f/0xe0
[ 4.801153][ T1] ? _raw_spin_unlock_irqrestore+0x3e/0x50
[ 4.801153][ T1] ? lockdep_hardirqs_on+0x379/0x580
[ 4.801153][ T1] __scsi_scan_target+0x273/0xc30
[ 4.801153][ T1] ? find_held_lock+0x2d/0x110
[ 4.801153][ T1] ? __pm_runtime_resume+0x111/0x180
[ 4.801153][ T1] ? scsi_probe_and_add_lun+0x2cd0/0x2cd0
[ 4.801153][ T1] ? mark_lock+0xbc/0x1130
[ 4.801153][ T1] scsi_scan_channel.part.0+0x126/0x1a0
[ 4.801153][ T1] scsi_scan_host_selected+0x2bb/0x3f0
[ 4.801153][ T1] do_scsi_scan_host+0x1e8/0x260
[ 4.801153][ T1] scsi_scan_host+0x37c/0x440
[ 4.801153][ T1] virtscsi_probe+0x9b7/0xbb5
[ 4.801153][ T1] ? virtscsi_restore+0x240/0x240
[ 4.801153][ T1] virtio_dev_probe+0x463/0x710
[ 4.801153][ T1] ? virtio_device_restore+0x1f0/0x1f0
[ 4.801153][ T1] really_probe+0x281/0x650
[ 4.801153][ T1] driver_probe_device+0x101/0x1b0
[ 4.801153][ T1] device_driver_attach+0x108/0x140
[ 4.801153][ T1] __driver_attach+0xda/0x240
[ 4.801153][ T1] ? device_driver_attach+0x140/0x140
[ 4.801153][ T1] bus_for_each_dev+0x14b/0x1d0
[ 4.801153][ T1] ? subsys_dev_iter_exit+0x20/0x20
[ 4.801153][ T1] bus_add_driver+0x44e/0x5a0
[ 4.801153][ T1] driver_register+0x1c4/0x320
[ 4.801153][ T1] ? spi_transport_init+0x132/0x132
[ 4.801153][ T1] init+0xa1/0x115
[ 4.801153][ T1] do_one_initcall+0xf0/0x614
[ 4.801153][ T1] ? perf_trace_initcall_level+0x3e0/0x3e0
[ 4.801153][ T1] ? parameq+0x110/0x160
[ 4.801153][ T1] kernel_init_freeable+0x4a9/0x596
[ 4.801153][ T1] ? rest_init+0x371/0x371
[ 4.801153][ T1] kernel_init+0xd/0x1bf
[ 4.801153][ T1] ret_from_fork+0x24/0x30
[ 4.801153][ T1] Modules linked in:
[ 4.899360][ T1] ---[ end trace 6822302347232fe4 ]---
[ 4.900898][ T1] RIP: 0010:dma_direct_max_mapping_size+0x73/0x19a
[ 4.902841][ T1] Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 1e
01 00 00 48 8b 9d 38 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 da 48 c1
ea 03 <80> 3c 02 00 0f 85 06 01 00 00 48 8d bd 48 03 00 00 48 8b 1b 48 b8
[ 4.908805][ T1] RSP: 0000:ffff8881da18f628 EFLAGS: 00010246
[ 4.910502][ T1] RAX: dffffc0000000000 RBX: 0000000000000000 RCX:
ffffffff812d716c
[ 4.913042][ T1] RDX: 0000000000000000 RSI: ffffffff812d7189 RDI:
ffff8881d829fa78
[ 4.915172][ T1] RBP: ffff8881d829f740 R08: ffff8881da180000 R09:
ffffed103ad210cb
[ 4.917564][ T1] R10: ffffed103ad210ca R11: ffff8881d6908657 R12:
ffff8881d829f740
[ 4.919814][ T1] R13: ffff8881d76a57b0 R14: 0000000000000200 R15:
0000000000000000
[ 4.922373][ T1] FS: 0000000000000000(0000)
GS:ffff8881db300000(0000) knlGS:0000000000000000
[ 4.925294][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4.927267][ T1] CR2: 0000000000000000 CR3: 0000000006a21000 CR4:
00000000001406e0
[ 4.929685][ T1] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 4.931826][ T1] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 4.934300][ T1] Kernel panic - not syncing: Fatal exception
[ 4.936582][ T1] Kernel Offset: disabled
[ 4.937961][ T1] Rebooting in 86400 seconds..
Error text is too large and was truncated, full error text is at:
https://syzkaller.appspot.com/x/error.txt?x=138cfa5c600000
Tested on:
commit: 1154c0b0 wip
git tree: https://github.com/google/kasan.git usb-fuzzer
kernel config: https://syzkaller.appspot.com/x/.config?x=b228fb19779df17d
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=17678cc8600000
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-24 21:02 ` Alan Stern
@ 2019-07-25 9:33 ` Oliver Neukum
2019-07-25 15:09 ` Alan Stern
0 siblings, 1 reply; 28+ messages in thread
From: Oliver Neukum @ 2019-07-25 9:33 UTC (permalink / raw)
To: Alan Stern; +Cc: andreyknvl, syzkaller-bugs, syzbot, linux-usb
Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> On Wed, 24 Jul 2019, Oliver Neukum wrote:
>
> > drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > index c7bc9db5b192..98b996ecf4d3 100644
> > --- a/drivers/hid/usbhid/hid-core.c
> > +++ b/drivers/hid/usbhid/hid-core.c
> > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > struct usbhid_device *usbhid = hid->driver_data;
> > int r = 0;
> >
> > + spin_lock_irq(&usbhid->lock);
> > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > + r = -ENODEV;
> > + spin_unlock_irq(&usbhid->lock);
> > + goto bail_out;
> > + } else {
> > + /* protect against disconnect */
> > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > + spin_unlock_irq(&usbhid->lock);
> > + }
> > +
> > switch (lvl) {
> > case PM_HINT_FULLON:
> > r = usb_autopm_get_interface(usbhid->intf);
> > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > usb_autopm_put_interface(usbhid->intf);
> > break;
> > }
> > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> >
> > +bail_out:
> > return r;
> > }
>
> Isn't this treating the symptom instead of the cause?
Sort of. Holding a reference for the whole time would have merit,
but I doubt it is strictly necessary.
> Shouldn't the hid_device hold a reference to usbhid->intf throughout
> its lifetime? That way this sort of problem wouldn't arise in any
> routine, not just usbhid_power().
Unfortunately the semantics would still be wrong without the check
in corner cases. In case disconnect() is called without a physical
unplug, we must not touch the power state.
I am indeed afraid that in that case my putative fix is still racy.
But I don't to just introduce a mutex just for this. Any ideas?
Regards
Oliver
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-24 21:16 ` syzbot
@ 2019-07-25 11:22 ` Andrey Konovalov
0 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2019-07-25 11:22 UTC (permalink / raw)
To: syzbot; +Cc: USB list, Oliver Neukum, syzkaller-bugs
On Wed, Jul 24, 2019 at 11:16 PM syzbot
<syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot tried to test the proposed patch but build/boot failed:
5.3-rc1 has a boot time bug, so the usb-fuzzer branch is broken right
now. Could you try using the usb-fuzzer-usb-testing-2019.07.11 branch
instead for testing your patches?
>
> T1] devtmpfs: initialized
> [ 2.873454][ T1] clocksource: jiffies: mask: 0xffffffff max_cycles:
> 0xffffffff, max_idle_ns: 19112604462750000 ns
> [ 2.873454][ T1] futex hash table entries: 512 (order: 4, 65536
> bytes, linear)
> [ 2.875783][ T1] PM: RTC time: 21:05:06, date: 2019-07-24
> [ 2.882105][ T1] NET: Registered protocol family 16
> [ 2.886588][ T1] audit: initializing netlink subsys (disabled)
> [ 2.888579][ T22] audit: type=2000 audit(1564002306.507:1):
> state=initialized audit_enabled=0 res=1
> [ 2.888579][ T1] cpuidle: using governor menu
> [ 2.893505][ T1] ACPI: bus type PCI registered
> [ 2.894935][ T1] PCI: Using configuration type 1 for base access
> [ 3.002926][ T1] HugeTLB registered 2.00 MiB page size, pre-allocated
> 0 pages
> [ 3.003285][ T27] cryptomgr_test (27) used greatest stack depth: 30208
> bytes left
> [ 3.003285][ T29] kworker/u4:0 (29) used greatest stack depth: 27608
> bytes left
> [ 3.011409][ T43] kworker/u4:0 (43) used greatest stack depth: 27120
> bytes left
> [ 3.022044][ T1] ACPI: Added _OSI(Module Device)
> [ 3.022884][ T1] ACPI: Added _OSI(Processor Device)
> [ 3.023651][ T1] ACPI: Added _OSI(3.0 _SCP Extensions)
> [ 3.031207][ T1] ACPI: Added _OSI(Processor Aggregator Device)
> [ 3.031207][ T1] ACPI: Added _OSI(Linux-Dell-Video)
> [ 3.031249][ T1] ACPI: Added _OSI(Linux-Lenovo-NV-HDMI-Audio)
> [ 3.032156][ T1] ACPI: Added _OSI(Linux-HPI-Hybrid-Graphics)
> [ 3.082907][ T1] ACPI: 2 ACPI AML tables successfully acquired and
> loaded
> [ 3.097843][ T1] ACPI: Interpreter enabled
> [ 3.098853][ T1] ACPI: (supports S0 S3 S4 S5)
> [ 3.099590][ T1] ACPI: Using IOAPIC for interrupt routing
> [ 3.100591][ T1] PCI: Using host bridge windows from ACPI; if
> necessary, use "pci=nocrs" and report a bug
> [ 3.104257][ T1] ACPI: Enabled 16 GPEs in block 00 to 0F
> [ 3.182493][ T1] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus
> 00-ff])
> [ 3.183851][ T1] acpi PNP0A03:00: _OSC: OS supports [ASPM ClockPM
> Segments MSI HPX-Type3]
> [ 3.185519][ T1] acpi PNP0A03:00: fail to add MMCONFIG information,
> can't access extended PCI configuration space under this bridge.
> [ 3.192575][ T1] PCI host bridge to bus 0000:00
> [ 3.193286][ T1] pci_bus 0000:00: root bus resource [io
> 0x0000-0x0cf7 window]
> [ 3.194360][ T1] pci_bus 0000:00: root bus resource [io
> 0x0d00-0xffff window]
> [ 3.195393][ T1] pci_bus 0000:00: root bus resource [mem
> 0x000a0000-0x000bffff window]
> [ 3.196522][ T1] pci_bus 0000:00: root bus resource [mem
> 0xc0000000-0xfebfffff window]
> [ 3.197715][ T1] pci_bus 0000:00: root bus resource [bus 00-ff]
> [ 3.198886][ T1] pci 0000:00:00.0: [8086:1237] type 00 class 0x060000
> [ 3.204649][ T1] pci 0000:00:01.0: [8086:7110] type 00 class 0x060100
> [ 3.222865][ T1] pci 0000:00:01.3: [8086:7113] type 00 class 0x068000
> [ 3.241306][ T1] pci 0000:00:01.3: quirk: [io 0xb000-0xb03f] claimed
> by PIIX4 ACPI
> [ 3.245348][ T1] pci 0000:00:03.0: [1af4:1004] type 00 class 0x000000
> [ 3.251166][ T1] pci 0000:00:03.0: reg 0x10: [io 0xc000-0xc03f]
> [ 3.255600][ T1] pci 0000:00:03.0: reg 0x14: [mem
> 0xfebfe000-0xfebfe07f]
> [ 3.271694][ T1] pci 0000:00:04.0: [1af4:1000] type 00 class 0x020000
> [ 3.277964][ T1] pci 0000:00:04.0: reg 0x10: [io 0xc040-0xc07f]
> [ 3.283798][ T1] pci 0000:00:04.0: reg 0x14: [mem
> 0xfebff000-0xfebff07f]
> [ 3.310020][ T1] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
> [ 3.314509][ T1] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
> [ 3.318340][ T1] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
> [ 3.322364][ T1] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
> [ 3.324796][ T1] ACPI: PCI Interrupt Link [LNKS] (IRQs *9)
> [ 3.332253][ T1] vgaarb: loaded
> [ 3.333359][ T1] SCSI subsystem initialized
> [ 3.334153][ T1] ACPI: bus type USB registered
> [ 3.334153][ T1] usbcore: registered new interface driver usbfs
> [ 3.334153][ T1] usbcore: registered new interface driver hub
> [ 3.341195][ T1] usbcore: registered new device driver usb
> [ 3.341471][ T1] mc: Linux media interface: v0.10
> [ 3.342295][ T1] videodev: Linux video capture interface: v2.00
> [ 3.343599][ T1] pps_core: LinuxPPS API ver. 1 registered
> [ 3.344464][ T1] pps_core: Software ver. 5.3.6 - Copyright 2005-2007
> Rodolfo Giometti <giometti@linux.it>
> [ 3.345876][ T1] PTP clock support registered
> [ 3.346781][ T1] EDAC MC: Ver: 3.0.0
> [ 3.346781][ T1] Advanced Linux Sound Architecture Driver Initialized.
> [ 3.346781][ T1] PCI: Using ACPI for IRQ routing
> [ 3.353286][ T1] Bluetooth: Core ver 2.22
> [ 3.354202][ T1] NET: Registered protocol family 31
> [ 3.354955][ T1] Bluetooth: HCI device and connection manager
> initialized
> [ 3.355989][ T1] Bluetooth: HCI socket layer initialized
> [ 3.356767][ T1] Bluetooth: L2CAP socket layer initialized
> [ 3.357644][ T1] Bluetooth: SCO socket layer initialized
> [ 3.358472][ T1] NET: Registered protocol family 8
> [ 3.359220][ T1] NET: Registered protocol family 20
> [ 3.360093][ T1] NetLabel: Initializing
> [ 3.360715][ T1] NetLabel: domain hash size = 128
> [ 3.361148][ T1] NetLabel: protocols = UNLABELED CIPSOv4 CALIPSO
> [ 3.362492][ T1] NetLabel: unlabeled traffic allowed by default
> [ 3.363724][ T1] nfc: nfc_init: NFC Core ver 0.1
> [ 3.363724][ T1] NET: Registered protocol family 39
> [ 3.365420][ T1] clocksource: Switched to clocksource kvm-clock
> [ 4.371843][ T1] VFS: Disk quotas dquot_6.6.0
> [ 4.372992][ T1] VFS: Dquot-cache hash table entries: 512 (order 0,
> 4096 bytes)
> [ 4.374598][ T1] *** VALIDATE hugetlbfs ***
> [ 4.376107][ T1] pnp: PnP ACPI init
> [ 4.387313][ T1] pnp: PnP ACPI: found 7 devices
> [ 4.413711][ T1] thermal_sys: Registered thermal governor 'step_wise'
> [ 4.413717][ T1] thermal_sys: Registered thermal governor 'user_space'
> [ 4.419892][ T1] clocksource: acpi_pm: mask: 0xffffff max_cycles:
> 0xffffff, max_idle_ns: 2085701024 ns
> [ 4.422972][ T1] pci_bus 0000:00: resource 4 [io 0x0000-0x0cf7
> window]
> [ 4.424189][ T1] pci_bus 0000:00: resource 5 [io 0x0d00-0xffff
> window]
> [ 4.425394][ T1] pci_bus 0000:00: resource 6 [mem
> 0x000a0000-0x000bffff window]
> [ 4.426791][ T1] pci_bus 0000:00: resource 7 [mem
> 0xc0000000-0xfebfffff window]
> [ 4.428947][ T1] NET: Registered protocol family 2
> [ 4.431371][ T1] tcp_listen_portaddr_hash hash table entries: 4096
> (order: 6, 294912 bytes, linear)
> [ 4.433417][ T1] TCP established hash table entries: 65536 (order: 7,
> 524288 bytes, linear)
> [ 4.435847][ T1] TCP bind hash table entries: 65536 (order: 10,
> 4194304 bytes, linear)
> [ 4.443410][ T1] TCP: Hash tables configured (established 65536 bind
> 65536)
> [ 4.445306][ T1] UDP hash table entries: 4096 (order: 7, 655360
> bytes, linear)
> [ 4.447824][ T1] UDP-Lite hash table entries: 4096 (order: 7, 655360
> bytes, linear)
> [ 4.450432][ T1] NET: Registered protocol family 1
> [ 4.453345][ T1] RPC: Registered named UNIX socket transport module.
> [ 4.454510][ T1] RPC: Registered udp transport module.
> [ 4.455454][ T1] RPC: Registered tcp transport module.
> [ 4.456446][ T1] RPC: Registered tcp NFSv4.1 backchannel transport
> module.
> [ 4.458621][ T1] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
> [ 4.459864][ T1] PCI: CLS 0 bytes, default 64
> [ 4.461378][ T1] PCI-DMA: Using software bounce buffering for IO
> (SWIOTLB)
> [ 4.462761][ T1] software IO TLB: mapped [mem 0xbbffd000-0xbfffd000]
> (64MB)
> [ 4.466413][ T1] RAPL PMU: API unit is 2^-32 Joules, 0 fixed
> counters, 10737418240 ms ovfl timer
> [ 4.467994][ T1] clocksource: tsc: mask: 0xffffffffffffffff
> max_cycles: 0x212735223b2, max_idle_ns: 440795277976 ns
> [ 4.469791][ T1] clocksource: Switched to clocksource tsc
> [ 4.473423][ T1] check: Scanning for low memory corruption every 60
> seconds
> [ 4.478343][ T1] Initialise system trusted keyrings
> [ 4.479810][ T1] workingset: timestamp_bits=40 max_order=21
> bucket_order=0
> [ 4.521420][ T1] NFS: Registering the id_resolver key type
> [ 4.522729][ T1] Key type id_resolver registered
> [ 4.523526][ T1] Key type id_legacy registered
> [ 4.524942][ T1] 9p: Installing v9fs 9p2000 file system support
> [ 4.531784][ T1] Key type asymmetric registered
> [ 4.532779][ T1] Asymmetric key parser 'x509' registered
> [ 4.533772][ T1] Block layer SCSI generic (bsg) driver version 0.4
> loaded (major 247)
> [ 4.535226][ T1] io scheduler mq-deadline registered
> [ 4.536032][ T1] io scheduler kyber registered
> [ 4.539777][ T1] usbcore: registered new interface driver udlfb
> [ 4.541021][ T1] usbcore: registered new interface driver smscufx
> [ 4.543014][ T1] input: Power Button as
> /devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
> [ 4.544520][ T1] ACPI: Power Button [PWRF]
> [ 4.545743][ T1] input: Sleep Button as
> /devices/LNXSYSTM:00/LNXSLPBN:00/input/input1
> [ 4.547129][ T1] ACPI: Sleep Button [SLPF]
> [ 4.560609][ T1] PCI Interrupt Link [LNKC] enabled at IRQ 11
> [ 4.562043][ T1] virtio-pci 0000:00:03.0: virtio_pci: leaving for
> legacy driver
> [ 4.575190][ T1] PCI Interrupt Link [LNKD] enabled at IRQ 10
> [ 4.576547][ T1] virtio-pci 0000:00:04.0: virtio_pci: leaving for
> legacy driver
> [ 4.582237][ T1] Serial: 8250/16550 driver, 4 ports, IRQ sharing
> enabled
> [ 4.606272][ T1] 00:03: ttyS0 at I/O 0x3f8 (irq = 4, base_baud =
> 115200) is a 16550A
> [ 4.631884][ T1] 00:04: ttyS1 at I/O 0x2f8 (irq = 3, base_baud =
> 115200) is a 16550A
> [ 4.656881][ T1] 00:05: ttyS2 at I/O 0x3e8 (irq = 6, base_baud =
> 115200) is a 16550A
> [ 4.682193][ T1] 00:06: ttyS3 at I/O 0x2e8 (irq = 7, base_baud =
> 115200) is a 16550A
> [ 4.687302][ T1] Non-volatile memory driver v1.3
> [ 4.688815][ T1] Linux agpgart interface v0.103
> [ 4.695497][ T1] usbcore: registered new interface driver udl
> [ 4.719445][ T1] loop: module loaded
> [ 4.721039][ T1] usbcore: registered new interface driver rtsx_usb
> [ 4.723136][ T1] usbcore: registered new interface driver viperboard
> [ 4.725065][ T1] usbcore: registered new interface driver dln2
> [ 4.726853][ T1] usbcore: registered new interface driver pn533_usb
> [ 4.729089][ T1] usbcore: registered new interface driver port100
> [ 4.731604][ T1] usbcore: registered new interface driver nfcmrvl
> [ 4.746210][ T1] scsi host0: Virtio SCSI HBA
> [ 4.792288][ T1] kasan: CONFIG_KASAN_INLINE enabled
> [ 4.793729][ T1] kasan: GPF could be caused by NULL-ptr deref or user
> memory access
> [ 4.795771][ T1] general protection fault: 0000 [#1] SMP KASAN
> [ 4.797547][ T1] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.3.0-rc1+
> #1
> [ 4.799786][ T1] Hardware name: Google Google Compute Engine/Google
> Compute Engine, BIOS Google 01/01/2011
> [ 4.801153][ T1] RIP: 0010:dma_direct_max_mapping_size+0x73/0x19a
> [ 4.801153][ T1] Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 1e
> 01 00 00 48 8b 9d 38 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 da 48 c1
> ea 03 <80> 3c 02 00 0f 85 06 01 00 00 48 8d bd 48 03 00 00 48 8b 1b 48 b8
> [ 4.801153][ T1] RSP: 0000:ffff8881da18f628 EFLAGS: 00010246
> [ 4.801153][ T1] RAX: dffffc0000000000 RBX: 0000000000000000 RCX:
> ffffffff812d716c
> [ 4.801153][ T1] RDX: 0000000000000000 RSI: ffffffff812d7189 RDI:
> ffff8881d829fa78
> [ 4.801153][ T1] RBP: ffff8881d829f740 R08: ffff8881da180000 R09:
> ffffed103ad210cb
> [ 4.801153][ T1] R10: ffffed103ad210ca R11: ffff8881d6908657 R12:
> ffff8881d829f740
> [ 4.801153][ T1] R13: ffff8881d76a57b0 R14: 0000000000000200 R15:
> 0000000000000000
> [ 4.801153][ T1] FS: 0000000000000000(0000)
> GS:ffff8881db300000(0000) knlGS:0000000000000000
> [ 4.801153][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 4.801153][ T1] CR2: 0000000000000000 CR3: 0000000006a21000 CR4:
> 00000000001406e0
> [ 4.801153][ T1] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 4.801153][ T1] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [ 4.801153][ T1] Call Trace:
> [ 4.801153][ T1] dma_max_mapping_size+0xb5/0xf0
> [ 4.801153][ T1] __scsi_init_queue+0x17e/0x510
> [ 4.801153][ T1] scsi_mq_alloc_queue+0xcb/0x170
> [ 4.801153][ T1] scsi_alloc_sdev+0x82e/0xc50
> [ 4.801153][ T1] scsi_probe_and_add_lun+0x1ee5/0x2cd0
> [ 4.801153][ T1] ? find_held_lock+0x2d/0x110
> [ 4.801153][ T1] ? __pm_runtime_resume+0x111/0x180
> [ 4.801153][ T1] ? scsi_alloc_sdev+0xc50/0xc50
> [ 4.801153][ T1] ? mark_held_locks+0x9f/0xe0
> [ 4.801153][ T1] ? _raw_spin_unlock_irqrestore+0x3e/0x50
> [ 4.801153][ T1] ? lockdep_hardirqs_on+0x379/0x580
> [ 4.801153][ T1] __scsi_scan_target+0x273/0xc30
> [ 4.801153][ T1] ? find_held_lock+0x2d/0x110
> [ 4.801153][ T1] ? __pm_runtime_resume+0x111/0x180
> [ 4.801153][ T1] ? scsi_probe_and_add_lun+0x2cd0/0x2cd0
> [ 4.801153][ T1] ? mark_lock+0xbc/0x1130
> [ 4.801153][ T1] scsi_scan_channel.part.0+0x126/0x1a0
> [ 4.801153][ T1] scsi_scan_host_selected+0x2bb/0x3f0
> [ 4.801153][ T1] do_scsi_scan_host+0x1e8/0x260
> [ 4.801153][ T1] scsi_scan_host+0x37c/0x440
> [ 4.801153][ T1] virtscsi_probe+0x9b7/0xbb5
> [ 4.801153][ T1] ? virtscsi_restore+0x240/0x240
> [ 4.801153][ T1] virtio_dev_probe+0x463/0x710
> [ 4.801153][ T1] ? virtio_device_restore+0x1f0/0x1f0
> [ 4.801153][ T1] really_probe+0x281/0x650
> [ 4.801153][ T1] driver_probe_device+0x101/0x1b0
> [ 4.801153][ T1] device_driver_attach+0x108/0x140
> [ 4.801153][ T1] __driver_attach+0xda/0x240
> [ 4.801153][ T1] ? device_driver_attach+0x140/0x140
> [ 4.801153][ T1] bus_for_each_dev+0x14b/0x1d0
> [ 4.801153][ T1] ? subsys_dev_iter_exit+0x20/0x20
> [ 4.801153][ T1] bus_add_driver+0x44e/0x5a0
> [ 4.801153][ T1] driver_register+0x1c4/0x320
> [ 4.801153][ T1] ? spi_transport_init+0x132/0x132
> [ 4.801153][ T1] init+0xa1/0x115
> [ 4.801153][ T1] do_one_initcall+0xf0/0x614
> [ 4.801153][ T1] ? perf_trace_initcall_level+0x3e0/0x3e0
> [ 4.801153][ T1] ? parameq+0x110/0x160
> [ 4.801153][ T1] kernel_init_freeable+0x4a9/0x596
> [ 4.801153][ T1] ? rest_init+0x371/0x371
> [ 4.801153][ T1] kernel_init+0xd/0x1bf
> [ 4.801153][ T1] ret_from_fork+0x24/0x30
> [ 4.801153][ T1] Modules linked in:
> [ 4.899360][ T1] ---[ end trace 6822302347232fe4 ]---
> [ 4.900898][ T1] RIP: 0010:dma_direct_max_mapping_size+0x73/0x19a
> [ 4.902841][ T1] Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 1e
> 01 00 00 48 8b 9d 38 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 da 48 c1
> ea 03 <80> 3c 02 00 0f 85 06 01 00 00 48 8d bd 48 03 00 00 48 8b 1b 48 b8
> [ 4.908805][ T1] RSP: 0000:ffff8881da18f628 EFLAGS: 00010246
> [ 4.910502][ T1] RAX: dffffc0000000000 RBX: 0000000000000000 RCX:
> ffffffff812d716c
> [ 4.913042][ T1] RDX: 0000000000000000 RSI: ffffffff812d7189 RDI:
> ffff8881d829fa78
> [ 4.915172][ T1] RBP: ffff8881d829f740 R08: ffff8881da180000 R09:
> ffffed103ad210cb
> [ 4.917564][ T1] R10: ffffed103ad210ca R11: ffff8881d6908657 R12:
> ffff8881d829f740
> [ 4.919814][ T1] R13: ffff8881d76a57b0 R14: 0000000000000200 R15:
> 0000000000000000
> [ 4.922373][ T1] FS: 0000000000000000(0000)
> GS:ffff8881db300000(0000) knlGS:0000000000000000
> [ 4.925294][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 4.927267][ T1] CR2: 0000000000000000 CR3: 0000000006a21000 CR4:
> 00000000001406e0
> [ 4.929685][ T1] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 4.931826][ T1] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [ 4.934300][ T1] Kernel panic - not syncing: Fatal exception
> [ 4.936582][ T1] Kernel Offset: disabled
> [ 4.937961][ T1] Rebooting in 86400 seconds..
>
>
> Error text is too large and was truncated, full error text is at:
> https://syzkaller.appspot.com/x/error.txt?x=138cfa5c600000
>
>
> Tested on:
>
> commit: 1154c0b0 wip
> git tree: https://github.com/google/kasan.git usb-fuzzer
> kernel config: https://syzkaller.appspot.com/x/.config?x=b228fb19779df17d
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> patch: https://syzkaller.appspot.com/x/patch.diff?x=17678cc8600000
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-23 12:48 KASAN: use-after-free Read in usbhid_power syzbot
2019-07-24 14:17 ` Oliver Neukum
2019-07-24 20:55 ` Oliver Neukum
@ 2019-07-25 12:15 ` Oliver Neukum
2019-07-25 12:27 ` syzbot
2 siblings, 1 reply; 28+ messages in thread
From: Oliver Neukum @ 2019-07-25 12:15 UTC (permalink / raw)
To: syzbot, andreyknvl, syzkaller-bugs, linux-usb
Am Dienstag, den 23.07.2019, 05:48 -0700 schrieb syzbot:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=11b13e78600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
> dashboard link: https://syzkaller.appspot.com/bug?extid=ef5de9c4f99c4edb4e49
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15482210600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1139b07c600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in usbhid_power+0xca/0xe0
> /drivers/hid/usbhid/hid-core.c:1234
> Read of size 8 at addr ffff8881ce8a4008 by task syz-executor373/1763
#syz test: https://github.com/google/kasan.git usb-fuzzer-usb-testing-2019.07.11
From a3455c4527bdfe86536856ea96288b7dcc36590b Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 24 Jul 2019 22:49:57 +0200
Subject: [PATCH] usbhid: test for disconnected state in raw opening
If the device has been disconnected, we should not proceed to use
runtime PM on it.
Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index c7bc9db5b192..98b996ecf4d3 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
struct usbhid_device *usbhid = hid->driver_data;
int r = 0;
+ spin_lock_irq(&usbhid->lock);
+ if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
+ r = -ENODEV;
+ spin_unlock_irq(&usbhid->lock);
+ goto bail_out;
+ } else {
+ /* protect against disconnect */
+ usb_get_dev(interface_to_usbdev(usbhid->intf));
+ spin_unlock_irq(&usbhid->lock);
+ }
+
switch (lvl) {
case PM_HINT_FULLON:
r = usb_autopm_get_interface(usbhid->intf);
@@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
usb_autopm_put_interface(usbhid->intf);
break;
}
+ usb_put_dev(interface_to_usbdev(usbhid->intf));
+bail_out:
return r;
}
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-25 12:15 ` Oliver Neukum
@ 2019-07-25 12:27 ` syzbot
0 siblings, 0 replies; 28+ messages in thread
From: syzbot @ 2019-07-25 12:27 UTC (permalink / raw)
To: andreyknvl, linux-usb, oneukum, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer still triggered
crash:
KASAN: use-after-free Read in usbhid_power
==================================================================
BUG: KASAN: use-after-free in __lock_acquire+0x3a5d/0x5340
kernel/locking/lockdep.c:3665
Read of size 8 at addr ffff8881c82c68a0 by task syz-executor.2/2950
CPU: 1 PID: 2950 Comm: syz-executor.2 Not tainted 5.2.0-rc6+ #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
print_address_description+0x67/0x231 mm/kasan/report.c:188
__kasan_report.cold+0x1a/0x32 mm/kasan/report.c:317
kasan_report+0xe/0x20 mm/kasan/common.c:614
__lock_acquire+0x3a5d/0x5340 kernel/locking/lockdep.c:3665
lock_acquire+0x100/0x2b0 kernel/locking/lockdep.c:4303
__raw_spin_lock_irq include/linux/spinlock_api_smp.h:128 [inline]
_raw_spin_lock_irq+0x2d/0x40 kernel/locking/spinlock.c:167
spin_lock_irq include/linux/spinlock.h:363 [inline]
usbhid_power+0x4a/0x240 drivers/hid/usbhid/hid-core.c:1232
hid_hw_power include/linux/hid.h:1038 [inline]
hidraw_open+0x20d/0x740 drivers/hid/hidraw.c:282
chrdev_open+0x219/0x5c0 fs/char_dev.c:413
do_dentry_open+0x497/0x1040 fs/open.c:778
do_last fs/namei.c:3416 [inline]
path_openat+0x1430/0x3ff0 fs/namei.c:3533
do_filp_open+0x1a1/0x280 fs/namei.c:3563
do_sys_open+0x3c0/0x580 fs/open.c:1070
do_syscall_64+0xb7/0x560 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x413711
Code: 75 14 b8 02 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48
83 ec 08 e8 0a fa ff ff 48 89 04 24 b8 02 00 00 00 0f 05 <48> 8b 3c 24 48
89 c2 e8 53 fa ff ff 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:00007fd4b6cf27a0 EFLAGS: 00000293 ORIG_RAX: 0000000000000002
RAX: ffffffffffffffda RBX: 6666666666666667 RCX: 0000000000413711
RDX: 0000000000000000 RSI: 0000000000000800 RDI: 00007fd4b6cf2850
RBP: 000000000075bfc8 R08: 000000000000000f R09: 0000000000000000
R10: 00007fd4b6cf39d0 R11: 0000000000000293 R12: 00007fd4b6cf36d4
R13: 00000000004c8a7a R14: 00000000004df748 R15: 00000000ffffffff
Allocated by task 2939:
save_stack+0x1b/0x80 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_kmalloc mm/kasan/common.c:489 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:462
slab_post_alloc_hook mm/slab.h:437 [inline]
slab_alloc_node mm/slub.c:2748 [inline]
__kmalloc_node_track_caller+0xee/0x370 mm/slub.c:4363
__kmalloc_reserve.isra.0+0x39/0xe0 net/core/skbuff.c:138
__alloc_skb+0xef/0x5a0 net/core/skbuff.c:206
alloc_skb include/linux/skbuff.h:1054 [inline]
netlink_alloc_large_skb net/netlink/af_netlink.c:1179 [inline]
netlink_sendmsg+0x8d6/0xcc0 net/netlink/af_netlink.c:1897
sock_sendmsg_nosec net/socket.c:646 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:665
___sys_sendmsg+0x803/0x920 net/socket.c:2286
__sys_sendmsg+0xec/0x1b0 net/socket.c:2324
do_syscall_64+0xb7/0x560 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 2939:
save_stack+0x1b/0x80 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:451
slab_free_hook mm/slub.c:1421 [inline]
slab_free_freelist_hook mm/slub.c:1448 [inline]
slab_free mm/slub.c:2994 [inline]
kfree+0xd7/0x280 mm/slub.c:3949
skb_free_head+0x8b/0xa0 net/core/skbuff.c:588
skb_release_data+0x41f/0x7c0 net/core/skbuff.c:608
skb_release_all+0x46/0x60 net/core/skbuff.c:662
__kfree_skb net/core/skbuff.c:676 [inline]
consume_skb net/core/skbuff.c:736 [inline]
consume_skb+0xc0/0x2f0 net/core/skbuff.c:730
netlink_unicast_kernel net/netlink/af_netlink.c:1308 [inline]
netlink_unicast+0x4d7/0x690 net/netlink/af_netlink.c:1333
netlink_sendmsg+0x80b/0xcc0 net/netlink/af_netlink.c:1922
sock_sendmsg_nosec net/socket.c:646 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:665
___sys_sendmsg+0x803/0x920 net/socket.c:2286
__sys_sendmsg+0xec/0x1b0 net/socket.c:2324
do_syscall_64+0xb7/0x560 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8881c82c6880
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 32 bytes inside of
1024-byte region [ffff8881c82c6880, ffff8881c82c6c80)
The buggy address belongs to the page:
page:ffffea000720b100 refcount:1 mapcount:0 mapping:ffff8881dac02a00
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 dead000000000100 dead000000000200 ffff8881dac02a00
raw: 0000000000000000 00000000000e000e 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881c82c6780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881c82c6800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8881c82c6880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881c82c6900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881c82c6980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Tested on:
commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git
usb-fuzzer-usb-testing-2019.07.11
console output: https://syzkaller.appspot.com/x/log.txt?x=125edb68600000
kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=16e76a7c600000
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-25 9:33 ` Oliver Neukum
@ 2019-07-25 15:09 ` Alan Stern
2019-08-08 18:54 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Alan Stern @ 2019-07-25 15:09 UTC (permalink / raw)
To: Oliver Neukum; +Cc: andreyknvl, syzkaller-bugs, syzbot, linux-usb
On Thu, 25 Jul 2019, Oliver Neukum wrote:
> Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> >
> > > drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > > index c7bc9db5b192..98b996ecf4d3 100644
> > > --- a/drivers/hid/usbhid/hid-core.c
> > > +++ b/drivers/hid/usbhid/hid-core.c
> > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > struct usbhid_device *usbhid = hid->driver_data;
> > > int r = 0;
> > >
> > > + spin_lock_irq(&usbhid->lock);
> > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > + r = -ENODEV;
> > > + spin_unlock_irq(&usbhid->lock);
> > > + goto bail_out;
> > > + } else {
> > > + /* protect against disconnect */
> > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > + spin_unlock_irq(&usbhid->lock);
> > > + }
> > > +
> > > switch (lvl) {
> > > case PM_HINT_FULLON:
> > > r = usb_autopm_get_interface(usbhid->intf);
> > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > usb_autopm_put_interface(usbhid->intf);
> > > break;
> > > }
> > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > >
> > > +bail_out:
> > > return r;
> > > }
> >
> > Isn't this treating the symptom instead of the cause?
>
> Sort of. Holding a reference for the whole time would have merit,
> but I doubt it is strictly necessary.
Just to be crystal clear, I was talking about a device reference --
usb_{get,put}_dev or usb_{get,put}_intf -- not a runtime PM reference.
(Incidentally, your patch could be simplified by using usb_get_intf
instead of usb_get_dev.)
> > Shouldn't the hid_device hold a reference to usbhid->intf throughout
> > its lifetime? That way this sort of problem wouldn't arise in any
> > routine, not just usbhid_power().
>
> Unfortunately the semantics would still be wrong without the check
> in corner cases. In case disconnect() is called without a physical
> unplug, we must not touch the power state.
> I am indeed afraid that in that case my putative fix is still racy.
> But I don't to just introduce a mutex just for this. Any ideas?
That's a separate issue. USB drivers -- indeed, all drivers -- are
required to balance their runtime PM gets and puts (although in the
case of a physical disconnection it doesn't matter). Are you asking
about the best way to do this?
Normally a driver's release or disconnect routine will stop all
asynchronous accesses to the device (interrupt handlers, work queues,
URBs, and so on). At that point the only remaining runtime PM activity
will be whatever the routine itself does. So it can see if any extra
runtime PM gets or puts are needed, and do whatever is necessary.
Does that answer your question? I can't tell for sure...
Note: I did not try to track down the reason for the invalid access
reported by syzbot. It looked like a simple use-after-free, which
would normally be fixed by taking the appropriate reference. Which is
what your patch does, except that it holds the reference only for a
short time instead of over the entire lifetime of the private data
structure (the usbhid structure), which is what normally happens.
Alan Stern
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-07-25 15:09 ` Alan Stern
@ 2019-08-08 18:54 ` Andrey Konovalov
2019-08-08 19:37 ` Alan Stern
2019-08-09 20:26 ` Oliver Neukum
0 siblings, 2 replies; 28+ messages in thread
From: Andrey Konovalov @ 2019-08-08 18:54 UTC (permalink / raw)
To: Alan Stern; +Cc: Oliver Neukum, syzkaller-bugs, syzbot, USB list, Hillf Danton
On Thu, Jul 25, 2019 at 5:09 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Thu, 25 Jul 2019, Oliver Neukum wrote:
>
> > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > >
> > > > drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> > > > 1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > int r = 0;
> > > >
> > > > + spin_lock_irq(&usbhid->lock);
> > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > + r = -ENODEV;
> > > > + spin_unlock_irq(&usbhid->lock);
> > > > + goto bail_out;
> > > > + } else {
> > > > + /* protect against disconnect */
> > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > + spin_unlock_irq(&usbhid->lock);
> > > > + }
> > > > +
> > > > switch (lvl) {
> > > > case PM_HINT_FULLON:
> > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > usb_autopm_put_interface(usbhid->intf);
> > > > break;
> > > > }
> > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > >
> > > > +bail_out:
> > > > return r;
> > > > }
> > >
> > > Isn't this treating the symptom instead of the cause?
> >
> > Sort of. Holding a reference for the whole time would have merit,
> > but I doubt it is strictly necessary.
>
> Just to be crystal clear, I was talking about a device reference --
> usb_{get,put}_dev or usb_{get,put}_intf -- not a runtime PM reference.
>
> (Incidentally, your patch could be simplified by using usb_get_intf
> instead of usb_get_dev.)
>
> > > Shouldn't the hid_device hold a reference to usbhid->intf throughout
> > > its lifetime? That way this sort of problem wouldn't arise in any
> > > routine, not just usbhid_power().
> >
> > Unfortunately the semantics would still be wrong without the check
> > in corner cases. In case disconnect() is called without a physical
> > unplug, we must not touch the power state.
> > I am indeed afraid that in that case my putative fix is still racy.
> > But I don't to just introduce a mutex just for this. Any ideas?
>
> That's a separate issue. USB drivers -- indeed, all drivers -- are
> required to balance their runtime PM gets and puts (although in the
> case of a physical disconnection it doesn't matter). Are you asking
> about the best way to do this?
>
> Normally a driver's release or disconnect routine will stop all
> asynchronous accesses to the device (interrupt handlers, work queues,
> URBs, and so on). At that point the only remaining runtime PM activity
> will be whatever the routine itself does. So it can see if any extra
> runtime PM gets or puts are needed, and do whatever is necessary.
>
> Does that answer your question? I can't tell for sure...
>
> Note: I did not try to track down the reason for the invalid access
> reported by syzbot. It looked like a simple use-after-free, which
> would normally be fixed by taking the appropriate reference. Which is
> what your patch does, except that it holds the reference only for a
> short time instead of over the entire lifetime of the private data
> structure (the usbhid structure), which is what normally happens.
This report looks like very similar to these two:
https://syzkaller.appspot.com/bug?extid=b156665cf4d1b5e00c76
https://syzkaller.appspot.com/bug?extid=3cbe5cd105d2ad56a1df
Maybe we should mark those two as duplicates.
Hillf suggested a fix on one of them, but it looks different from what
you propose:
https://groups.google.com/d/msg/syzkaller-bugs/xW7LvKfpyn0/SpKbs5ZLEAAJ
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-08-08 18:54 ` Andrey Konovalov
@ 2019-08-08 19:37 ` Alan Stern
2019-08-09 7:35 ` AW: " Schmid, Carsten
2019-08-09 17:36 ` Andrey Konovalov
2019-08-09 20:26 ` Oliver Neukum
1 sibling, 2 replies; 28+ messages in thread
From: Alan Stern @ 2019-08-08 19:37 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Oliver Neukum, syzkaller-bugs, syzbot, USB list, Hillf Danton
On Thu, 8 Aug 2019, Andrey Konovalov wrote:
> On Thu, Jul 25, 2019 at 5:09 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Thu, 25 Jul 2019, Oliver Neukum wrote:
> >
> > > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > > >
> > > > > drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> > > > > 1 file changed, 13 insertions(+)
> > > > >
> > > > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > > int r = 0;
> > > > >
> > > > > + spin_lock_irq(&usbhid->lock);
> > > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > > + r = -ENODEV;
> > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > + goto bail_out;
> > > > > + } else {
> > > > > + /* protect against disconnect */
> > > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > + }
> > > > > +
> > > > > switch (lvl) {
> > > > > case PM_HINT_FULLON:
> > > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > > usb_autopm_put_interface(usbhid->intf);
> > > > > break;
> > > > > }
> > > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > > >
> > > > > +bail_out:
> > > > > return r;
> > > > > }
> This report looks like very similar to these two:
>
> https://syzkaller.appspot.com/bug?extid=b156665cf4d1b5e00c76
> https://syzkaller.appspot.com/bug?extid=3cbe5cd105d2ad56a1df
It also seems to resemble extids a7a6b9c609b9457c62c6,
62a1e04fd3ec2abf099e, and 75e6910bf03e266a277f, although this may be an
illusion.
> Maybe we should mark those two as duplicates.
>
> Hillf suggested a fix on one of them, but it looks different from what
> you propose:
>
> https://groups.google.com/d/msg/syzkaller-bugs/xW7LvKfpyn0/SpKbs5ZLEAAJ
Go ahead and try it out on all of them. I don't have a clear feeling
about it, not having worked on usbhid in quite a while.
Alan Stern
^ permalink raw reply [flat|nested] 28+ messages in thread
* AW: KASAN: use-after-free Read in usbhid_power
2019-08-08 19:37 ` Alan Stern
@ 2019-08-09 7:35 ` Schmid, Carsten
2019-08-09 7:55 ` Greg KH
2019-08-09 17:36 ` Andrey Konovalov
1 sibling, 1 reply; 28+ messages in thread
From: Schmid, Carsten @ 2019-08-09 7:35 UTC (permalink / raw)
To: Alan Stern, Andrey Konovalov
Cc: Oliver Neukum, syzkaller-bugs, syzbot, USB list, Hillf Danton
Hi all having use-after-free issues in USB shutdowns:
I hunted for a similar case in the intel_xhci_usb_sw driver.
What i have found and proposed is (from yesterday):
---
[PATCH] kernel/resource.c: invalidate parent when freed resource has childs
When a resource is freed and has children, the childrens are
left without any hint that their parent is no more valid.
This caused at least one use-after-free in the xhci-hcd using
ext-caps driver when platform code released platform devices.
Fix this by setting child's parent to zero and warn.
Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
---
Rationale:
When hunting for the root cause of a crash on a 4.14.86 kernel, i
have found the root cause and checked it being still present
upstream. Our case:
Having xhci-hcd and intel_xhci_usb_sw active we can see in
/proc/meminfo: (exceirpt)
b3c00000-b3c0ffff : 0000:00:15.0
b3c00000-b3c0ffff : xhci-hcd
b3c08070-b3c0846f : intel_xhci_usb_sw
intel_xhci_usb_sw being a child of xhci-hcd.
Doing an unbind command
echo 0000:00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
leads to xhci-hcd being freed in __release_region.
The intel_xhci_usb_sw resource is accessed in platform code
in platform_device_del with
for (i = 0; i < pdev->num_resources; i++) {
struct resource *r = &pdev->resource[i];
if (r->parent)
release_resource(r);
}
as the resource's parent has not been updated, the release_resource
uses the parent:
p = &old->parent->child;
which is now invalid.
Fix this by marking the parent invalid in the child and give a warning
in dmesg.
---
kernel/resource.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/kernel/resource.c b/kernel/resource.c
index 158f04ec1d4f..95340cb0b1c2 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent, resource_size_t start,
write_unlock(&resource_lock);
if (res->flags & IORESOURCE_MUXED)
wake_up(&muxed_resource_wait);
+
+ write_lock(&resource_lock);
+ if (res->child) {
+ printk(KERN_WARNING "__release_region: %s has child %s,"
+ "invalidating childs parent\n",
+ res->name, res->child->name);
+ res->child->parent = NULL;
+ }
+ write_unlock(&resource_lock);
free_resource(res);
return;
}
--
2.17.1
> -----Ursprüngliche Nachricht-----
> Von: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] Im Auftrag von Alan Stern
> Gesendet: Donnerstag, 8. August 2019 21:37
> An: Andrey Konovalov <andreyknvl@google.com>
> Cc: Oliver Neukum <oneukum@suse.com>; syzkaller-bugs <syzkaller-
> bugs@googlegroups.com>; syzbot
> <syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com>; USB list
> <linux-usb@vger.kernel.org>; Hillf Danton <hdanton@sina.com>
> Betreff: Re: KASAN: use-after-free Read in usbhid_power
>
> On Thu, 8 Aug 2019, Andrey Konovalov wrote:
>
> > On Thu, Jul 25, 2019 at 5:09 PM Alan Stern <stern@rowland.harvard.edu>
> wrote:
> > >
> > > On Thu, 25 Jul 2019, Oliver Neukum wrote:
> > >
> > > > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > > > >
> > > > > > drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> > > > > > 1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-
> core.c
> > > > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device
> *hid, int lvl)
> > > > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > > > int r = 0;
> > > > > >
> > > > > > + spin_lock_irq(&usbhid->lock);
> > > > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > > > + r = -ENODEV;
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + goto bail_out;
> > > > > > + } else {
> > > > > > + /* protect against disconnect */
> > > > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + }
> > > > > > +
> > > > > > switch (lvl) {
> > > > > > case PM_HINT_FULLON:
> > > > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device
> *hid, int lvl)
> > > > > > usb_autopm_put_interface(usbhid->intf);
> > > > > > break;
> > > > > > }
> > > > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > > > >
> > > > > > +bail_out:
> > > > > > return r;
> > > > > > }
>
> > This report looks like very similar to these two:
> >
> > https://syzkaller.appspot.com/bug?extid=b156665cf4d1b5e00c76
> > https://syzkaller.appspot.com/bug?extid=3cbe5cd105d2ad56a1df
>
> It also seems to resemble extids a7a6b9c609b9457c62c6,
> 62a1e04fd3ec2abf099e, and 75e6910bf03e266a277f, although this may be an
> illusion.
>
> > Maybe we should mark those two as duplicates.
> >
> > Hillf suggested a fix on one of them, but it looks different from what
> > you propose:
> >
> > https://groups.google.com/d/msg/syzkaller-
> bugs/xW7LvKfpyn0/SpKbs5ZLEAAJ
>
> Go ahead and try it out on all of them. I don't have a clear feeling
> about it, not having worked on usbhid in quite a while.
>
> Alan Stern
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-08-09 7:35 ` AW: " Schmid, Carsten
@ 2019-08-09 7:55 ` Greg KH
2019-08-09 9:34 ` AW: " Schmid, Carsten
0 siblings, 1 reply; 28+ messages in thread
From: Greg KH @ 2019-08-09 7:55 UTC (permalink / raw)
To: Schmid, Carsten
Cc: Alan Stern, Andrey Konovalov, Oliver Neukum, syzkaller-bugs,
syzbot, USB list, Hillf Danton
On Fri, Aug 09, 2019 at 07:35:32AM +0000, Schmid, Carsten wrote:
> Hi all having use-after-free issues in USB shutdowns:
> I hunted for a similar case in the intel_xhci_usb_sw driver.
> What i have found and proposed is (from yesterday):
> ---
> [PATCH] kernel/resource.c: invalidate parent when freed resource has childs
>
> When a resource is freed and has children, the childrens are
> left without any hint that their parent is no more valid.
> This caused at least one use-after-free in the xhci-hcd using
> ext-caps driver when platform code released platform devices.
>
> Fix this by setting child's parent to zero and warn.
>
> Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
> ---
> Rationale:
> When hunting for the root cause of a crash on a 4.14.86 kernel, i
> have found the root cause and checked it being still present
> upstream. Our case:
> Having xhci-hcd and intel_xhci_usb_sw active we can see in
> /proc/meminfo: (exceirpt)
> b3c00000-b3c0ffff : 0000:00:15.0
> b3c00000-b3c0ffff : xhci-hcd
> b3c08070-b3c0846f : intel_xhci_usb_sw
> intel_xhci_usb_sw being a child of xhci-hcd.
>
> Doing an unbind command
> echo 0000:00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
> leads to xhci-hcd being freed in __release_region.
> The intel_xhci_usb_sw resource is accessed in platform code
> in platform_device_del with
> for (i = 0; i < pdev->num_resources; i++) {
> struct resource *r = &pdev->resource[i];
> if (r->parent)
> release_resource(r);
> }
> as the resource's parent has not been updated, the release_resource
> uses the parent:
> p = &old->parent->child;
> which is now invalid.
> Fix this by marking the parent invalid in the child and give a warning
> in dmesg.
> ---
> kernel/resource.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 158f04ec1d4f..95340cb0b1c2 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent, resource_size_t start,
> write_unlock(&resource_lock);
> if (res->flags & IORESOURCE_MUXED)
> wake_up(&muxed_resource_wait);
> +
> + write_lock(&resource_lock);
Nit, should't this be written so that you only drop/get the lock if the
above if statement is true?
> + if (res->child) {
> + printk(KERN_WARNING "__release_region: %s has child %s,"
> + "invalidating childs parent\n",
> + res->name, res->child->name);
What can userspace/anyone do about this if it triggers?
Can't we fix the root problem in the xhci driver to properly order how
it tears things down? If it has intel_xhci_usb_driver as a "child"
shouldn't that be unbound/freed before the parent is?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 7:55 ` Greg KH
@ 2019-08-09 9:34 ` Schmid, Carsten
2019-08-09 10:20 ` Hans de Goede
0 siblings, 1 reply; 28+ messages in thread
From: Schmid, Carsten @ 2019-08-09 9:34 UTC (permalink / raw)
To: Greg KH, hdegoede
Cc: Alan Stern, Andrey Konovalov, Oliver Neukum, syzkaller-bugs,
syzbot, USB list, Hillf Danton
> -----Ursprüngliche Nachricht-----
> Von: Greg KH [mailto:gregkh@linuxfoundation.org]
> Gesendet: Freitag, 9. August 2019 09:56
> An: Schmid, Carsten <Carsten_Schmid@mentor.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>; Andrey Konovalov
> <andreyknvl@google.com>; Oliver Neukum <oneukum@suse.com>;
> syzkaller-bugs <syzkaller-bugs@googlegroups.com>; syzbot
> <syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com>; USB list
> <linux-usb@vger.kernel.org>; Hillf Danton <hdanton@sina.com>
> Betreff: Re: KASAN: use-after-free Read in usbhid_power
>
> On Fri, Aug 09, 2019 at 07:35:32AM +0000, Schmid, Carsten wrote:
> > Hi all having use-after-free issues in USB shutdowns:
> > I hunted for a similar case in the intel_xhci_usb_sw driver.
> > What i have found and proposed is (from yesterday):
> > ---
> > [PATCH] kernel/resource.c: invalidate parent when freed resource has
> childs
> >
> > When a resource is freed and has children, the childrens are
> > left without any hint that their parent is no more valid.
> > This caused at least one use-after-free in the xhci-hcd using
> > ext-caps driver when platform code released platform devices.
> >
> > Fix this by setting child's parent to zero and warn.
> >
> > Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
> > ---
> > Rationale:
> > When hunting for the root cause of a crash on a 4.14.86 kernel, i
> > have found the root cause and checked it being still present
> > upstream. Our case:
> > Having xhci-hcd and intel_xhci_usb_sw active we can see in
> > /proc/meminfo: (exceirpt)
> > b3c00000-b3c0ffff : 0000:00:15.0
> > b3c00000-b3c0ffff : xhci-hcd
> > b3c08070-b3c0846f : intel_xhci_usb_sw
> > intel_xhci_usb_sw being a child of xhci-hcd.
> >
> > Doing an unbind command
> > echo 0000:00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
> > leads to xhci-hcd being freed in __release_region.
> > The intel_xhci_usb_sw resource is accessed in platform code
> > in platform_device_del with
> > for (i = 0; i < pdev->num_resources; i++) {
> > struct resource *r = &pdev->resource[i];
> > if (r->parent)
> > release_resource(r);
> > }
> > as the resource's parent has not been updated, the release_resource
> > uses the parent:
> > p = &old->parent->child;
> > which is now invalid.
> > Fix this by marking the parent invalid in the child and give a warning
> > in dmesg.
> > ---
> > kernel/resource.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/kernel/resource.c b/kernel/resource.c
> > index 158f04ec1d4f..95340cb0b1c2 100644
> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent,
> resource_size_t start,
> > write_unlock(&resource_lock);
> > if (res->flags & IORESOURCE_MUXED)
> > wake_up(&muxed_resource_wait);
> > +
> > + write_lock(&resource_lock);
>
> Nit, should't this be written so that you only drop/get the lock if the
> above if statement is true?
>
What if some other async part invalidates the child while accessing it?
I wanted to make sure that the res->child is valid through the time it is accessed.
> > + if (res->child) {
> > + printk(KERN_WARNING "__release_region: %s has child
> %s,"
> > + "invalidating childs parent\n",
> > + res->name, res->child->name);
>
> What can userspace/anyone do about this if it triggers?
>
At least a platform driver developer can see he did something wrong.
I had a look at the code of Hans and did not see anything weird.
(platform driver is in drivers/usb/host/xhci-ext-caps.c)
The issue is very racy - what happens when the platform code accesses the
resource mainly depends on if the freed resource memory already has been
reused by someone.
It was hard to find that, and only a single core dump enabled me to find it.
A first attempt was to set resource count to 0 in Hans' driver in the unregister
pdev before calling platform_device_unregister. That worked.
But this is a dirty hack in my eyes. The framework should detect such issues,
so i decided to find the best place where to insert the check - and
found it is the place where the resource is freed and still has childrens.
> Can't we fix the root problem in the xhci driver to properly order how
> it tears things down? If it has intel_xhci_usb_driver as a "child"
> shouldn't that be unbound/freed before the parent is?
>
Hans, any idea?
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 9:34 ` AW: " Schmid, Carsten
@ 2019-08-09 10:20 ` Hans de Goede
2019-08-09 10:47 ` AW: " Schmid, Carsten
0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2019-08-09 10:20 UTC (permalink / raw)
To: Schmid, Carsten, Greg KH
Cc: Alan Stern, Andrey Konovalov, Oliver Neukum, syzkaller-bugs,
syzbot, USB list, Hillf Danton
Hi,
On 8/9/19 11:34 AM, Schmid, Carsten wrote:
>> -----Ursprüngliche Nachricht-----
>> Von: Greg KH [mailto:gregkh@linuxfoundation.org]
>> Gesendet: Freitag, 9. August 2019 09:56
>> An: Schmid, Carsten <Carsten_Schmid@mentor.com>
>> Cc: Alan Stern <stern@rowland.harvard.edu>; Andrey Konovalov
>> <andreyknvl@google.com>; Oliver Neukum <oneukum@suse.com>;
>> syzkaller-bugs <syzkaller-bugs@googlegroups.com>; syzbot
>> <syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com>; USB list
>> <linux-usb@vger.kernel.org>; Hillf Danton <hdanton@sina.com>
>> Betreff: Re: KASAN: use-after-free Read in usbhid_power
>>
>> On Fri, Aug 09, 2019 at 07:35:32AM +0000, Schmid, Carsten wrote:
>>> Hi all having use-after-free issues in USB shutdowns:
>>> I hunted for a similar case in the intel_xhci_usb_sw driver.
>>> What i have found and proposed is (from yesterday):
>>> ---
>>> [PATCH] kernel/resource.c: invalidate parent when freed resource has
>> childs
>>>
>>> When a resource is freed and has children, the childrens are
>>> left without any hint that their parent is no more valid.
>>> This caused at least one use-after-free in the xhci-hcd using
>>> ext-caps driver when platform code released platform devices.
>>>
>>> Fix this by setting child's parent to zero and warn.
>>>
>>> Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
>>> ---
>>> Rationale:
>>> When hunting for the root cause of a crash on a 4.14.86 kernel, i
>>> have found the root cause and checked it being still present
>>> upstream. Our case:
>>> Having xhci-hcd and intel_xhci_usb_sw active we can see in
>>> /proc/meminfo: (exceirpt)
>>> b3c00000-b3c0ffff : 0000:00:15.0
>>> b3c00000-b3c0ffff : xhci-hcd
>>> b3c08070-b3c0846f : intel_xhci_usb_sw
>>> intel_xhci_usb_sw being a child of xhci-hcd.
>>>
>>> Doing an unbind command
>>> echo 0000:00:15.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
>>> leads to xhci-hcd being freed in __release_region.
>>> The intel_xhci_usb_sw resource is accessed in platform code
>>> in platform_device_del with
>>> for (i = 0; i < pdev->num_resources; i++) {
>>> struct resource *r = &pdev->resource[i];
>>> if (r->parent)
>>> release_resource(r);
>>> }
>>> as the resource's parent has not been updated, the release_resource
>>> uses the parent:
>>> p = &old->parent->child;
>>> which is now invalid.
>>> Fix this by marking the parent invalid in the child and give a warning
>>> in dmesg.
>>> ---
>>> kernel/resource.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/kernel/resource.c b/kernel/resource.c
>>> index 158f04ec1d4f..95340cb0b1c2 100644
>>> --- a/kernel/resource.c
>>> +++ b/kernel/resource.c
>>> @@ -1200,6 +1200,15 @@ void __release_region(struct resource *parent,
>> resource_size_t start,
>>> write_unlock(&resource_lock);
>>> if (res->flags & IORESOURCE_MUXED)
>>> wake_up(&muxed_resource_wait);
>>> +
>>> + write_lock(&resource_lock);
>>
>> Nit, should't this be written so that you only drop/get the lock if the
>> above if statement is true?
>>
> What if some other async part invalidates the child while accessing it?
> I wanted to make sure that the res->child is valid through the time it is accessed.
>
>>> + if (res->child) {
>>> + printk(KERN_WARNING "__release_region: %s has child
>> %s,"
>>> + "invalidating childs parent\n",
>>> + res->name, res->child->name);
>>
>> What can userspace/anyone do about this if it triggers?
>>
> At least a platform driver developer can see he did something wrong.
> I had a look at the code of Hans and did not see anything weird.
> (platform driver is in drivers/usb/host/xhci-ext-caps.c)
> The issue is very racy - what happens when the platform code accesses the
> resource mainly depends on if the freed resource memory already has been
> reused by someone.
We are talking memory-mapped io here, so it cannot just be "re-used", it
is wat it is. I guess the PCI BAR could be released and then the physical
address the resource was at could be re-used for another piece of MMIo,
but AFAIK outside of PI=CI hotplug we never release BARs.
Maybe we need to ref-count resources and have the aprent free only be
a deref and not release the resource until the child resource also
is free-ed doing another deref?
I must say that to me it sometimes just seems like always allowing unbind
is a bad idea. Another example of this is things like virtio, where
we can have a filesystem based on virtio-block, but the virtio interface
between the hypervisor and the guest-kernel is a PCI-device and in theory
the user could unbind the virtio driver from that PCI-device, after which
the whole house comes crashing down.
I also know that the extcon framework in its current incarnaton
does not deal with unbind properly...
Maybe it is time that we allow drivers to block unbind instead of
trying to support unbind in really complex situations where normal
use-cases will never need it ?
I do realize unbind is very useful for driver developent without
rebooting.
> It was hard to find that, and only a single core dump enabled me to find it.
> A first attempt was to set resource count to 0 in Hans' driver in the unregister
> pdev before calling platform_device_unregister. That worked.
> But this is a dirty hack in my eyes. The framework should detect such issues,
> so i decided to find the best place where to insert the check - and
> found it is the place where the resource is freed and still has childrens.
>
>> Can't we fix the root problem in the xhci driver to properly order how
>> it tears things down? If it has intel_xhci_usb_driver as a "child"
>> shouldn't that be unbound/freed before the parent is?
>>
> Hans, any idea?
1) make resources refcounted, have child resources take a ref on the parent
2) Disallow unbind on devices with bound child-devices?
Regards,
Hans
^ permalink raw reply [flat|nested] 28+ messages in thread
* AW: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 10:20 ` Hans de Goede
@ 2019-08-09 10:47 ` Schmid, Carsten
2019-08-09 10:53 ` Hans de Goede
0 siblings, 1 reply; 28+ messages in thread
From: Schmid, Carsten @ 2019-08-09 10:47 UTC (permalink / raw)
To: Hans de Goede, Greg KH
Cc: Alan Stern, Andrey Konovalov, Oliver Neukum, syzkaller-bugs,
syzbot, USB list, Hillf Danton
>
> We are talking memory-mapped io here, so it cannot just be "re-used", it
> is wat it is. I guess the PCI BAR could be released and then the physical
> address the resource was at could be re-used for another piece of MMIo,
> but AFAIK outside of PI=CI hotplug we never release BARs.
>
> Maybe we need to ref-count resources and have the aprent free only be
> a deref and not release the resource until the child resource also
> is free-ed doing another deref?
>
> I must say that to me it sometimes just seems like always allowing unbind
> is a bad idea. Another example of this is things like virtio, where
> we can have a filesystem based on virtio-block, but the virtio interface
> between the hypervisor and the guest-kernel is a PCI-device and in theory
> the user could unbind the virtio driver from that PCI-device, after which
> the whole house comes crashing down.
>
> I also know that the extcon framework in its current incarnaton
> does not deal with unbind properly...
>
> Maybe it is time that we allow drivers to block unbind instead of
> trying to support unbind in really complex situations where normal
> use-cases will never need it ?
>
> I do realize unbind is very useful for driver developent without
> rebooting.
>
Hey, i did not want to trigger an eartquake in the basement of the kernel ;-)
My intention was to prevent some crashes, and help developers to find their bugs.
I think my patch exactly does this.
> 1) make resources refcounted, have child resources take a ref on the parent
> 2) Disallow unbind on devices with bound child-devices?
>
Exactly what i was thinking of in first attempts.
But i fear that would break even more use cases.
Hans, directly regarding the driver:
The problem i see is that the xhci_intel_unregister_pdev which is added
as an action with devm_add_action_or_reset() is called late by the framework,
later than the usb_hcd_pci_remove() in xhci_pci_remove.
Is there any chance to trigger this before?
This is what Greg meant with "right order".
Anyway, i really appreciate these discussions, thanks for all
your patience.
Best Regards
Carsten
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: AW: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 10:47 ` AW: " Schmid, Carsten
@ 2019-08-09 10:53 ` Hans de Goede
2019-08-09 12:38 ` AW: " Schmid, Carsten
0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2019-08-09 10:53 UTC (permalink / raw)
To: Schmid, Carsten, Greg KH
Cc: Alan Stern, Andrey Konovalov, Oliver Neukum, syzkaller-bugs,
syzbot, USB list, Hillf Danton
Hi,
On 8/9/19 12:47 PM, Schmid, Carsten wrote:
>>
>> We are talking memory-mapped io here, so it cannot just be "re-used", it
>> is wat it is. I guess the PCI BAR could be released and then the physical
>> address the resource was at could be re-used for another piece of MMIo,
>> but AFAIK outside of PI=CI hotplug we never release BARs.
>>
>> Maybe we need to ref-count resources and have the aprent free only be
>> a deref and not release the resource until the child resource also
>> is free-ed doing another deref?
>>
>> I must say that to me it sometimes just seems like always allowing unbind
>> is a bad idea. Another example of this is things like virtio, where
>> we can have a filesystem based on virtio-block, but the virtio interface
>> between the hypervisor and the guest-kernel is a PCI-device and in theory
>> the user could unbind the virtio driver from that PCI-device, after which
>> the whole house comes crashing down.
>>
>> I also know that the extcon framework in its current incarnaton
>> does not deal with unbind properly...
>>
>> Maybe it is time that we allow drivers to block unbind instead of
>> trying to support unbind in really complex situations where normal
>> use-cases will never need it ?
>>
>> I do realize unbind is very useful for driver developent without
>> rebooting.
>>
>
> Hey, i did not want to trigger an eartquake in the basement of the kernel ;-)
> My intention was to prevent some crashes, and help developers to find their bugs.
> I think my patch exactly does this.
Hehe, actually drivers not being able to block unbind has been bugging me for
a while now, because there are cases where this would be really helpful.
>> 1) make resources refcounted, have child resources take a ref on the parent
>> 2) Disallow unbind on devices with bound child-devices?
>>
>
> Exactly what i was thinking of in first attempts.
> But i fear that would break even more use cases.
>
> Hans, directly regarding the driver:
> The problem i see is that the xhci_intel_unregister_pdev which is added
> as an action with devm_add_action_or_reset() is called late by the framework,
> later than the usb_hcd_pci_remove() in xhci_pci_remove.
> Is there any chance to trigger this before?
> This is what Greg meant with "right order".
Ah, I missed that part, sure that should be easy, just stop using
devm_add_action_or_reset() and do the xhci_intel_unregister_pdev()
manually at the right time. The downside of this is that you also
need to make sure it happens at the right time from probe error-paths
but given the bug you are hitting, I guess that is probably
already a problem.
Regards,
Hans
^ permalink raw reply [flat|nested] 28+ messages in thread
* AW: AW: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 10:53 ` Hans de Goede
@ 2019-08-09 12:38 ` Schmid, Carsten
2019-08-09 12:54 ` Greg KH
2019-08-10 11:12 ` AW: " Hans de Goede
0 siblings, 2 replies; 28+ messages in thread
From: Schmid, Carsten @ 2019-08-09 12:38 UTC (permalink / raw)
To: Hans de Goede, Greg KH
Cc: Alan Stern, Andrey Konovalov, Oliver Neukum, syzkaller-bugs,
syzbot, USB list, Hillf Danton
Hi again,
>>
>> Hey, i did not want to trigger an eartquake in the basement of the kernel ;-)
>> My intention was to prevent some crashes, and help developers to find their bugs.
>> I think my patch exactly does this.
>
> Hehe, actually drivers not being able to block unbind has been bugging me
> for
> a while now, because there are cases where this would be really helpful.
>>> 1) make resources refcounted, have child resources take a ref on the parent
>>> 2) Disallow unbind on devices with bound child-devices?
>>>
>> Exactly what i was thinking of in first attempts.
>> But i fear that would break even more use cases.
>>
>> Hans, directly regarding the driver:
>> The problem i see is that the xhci_intel_unregister_pdev which is added
>> as an action with devm_add_action_or_reset() is called late by the framework,
>> later than the usb_hcd_pci_remove() in xhci_pci_remove.
>> Is there any chance to trigger this before?
>> This is what Greg meant with "right order".
>
> Ah, I missed that part, sure that should be easy, just stop using
> devm_add_action_or_reset() and do the xhci_intel_unregister_pdev()
> manually at the right time. The downside of this is that you also
> need to make sure it happens at the right time from probe error-paths
> but given the bug you are hitting, I guess that is probably
> already a problem.
>
@Hans:
Sure, will have a look at this. I think i have found where to do that,
but need to check how to get the pdev pointer there ....
@Greg:
I am still confident that my patch in __release_region should be taken in.
Situation now without my patch:
If we have a device driver (or whatever) releasing a resource, the owner of
the child will have no notification that the parent is gone.
Accessing the parent (at least this will happen when trying to free the resource)
might have changed memory at the parent location, and what happens might
be an access to unmapped memory, whatever - an oops and we don't know why.
That's what i experienced and hunting.
Situation with the patch applied:
The owner gets a notification (parent=NULL) and we have an indication in
the kernel log.
If an owner of the resource where the parent is gone checks for the parent,
we are fine. If he doesn't check: we have a NULL pointer deref with a warning
message pointing to the root cause.
Isn't it better to have a pointer to a crash rather than having unreliable
racy crashes in such a case?
Have a nice weekend.
Best regards
Carsten
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: AW: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 12:38 ` AW: " Schmid, Carsten
@ 2019-08-09 12:54 ` Greg KH
2019-08-09 13:00 ` AW: " Schmid, Carsten
2019-08-10 11:12 ` AW: " Hans de Goede
1 sibling, 1 reply; 28+ messages in thread
From: Greg KH @ 2019-08-09 12:54 UTC (permalink / raw)
To: Schmid, Carsten
Cc: Hans de Goede, Alan Stern, Andrey Konovalov, Oliver Neukum,
syzkaller-bugs, syzbot, USB list, Hillf Danton
On Fri, Aug 09, 2019 at 12:38:35PM +0000, Schmid, Carsten wrote:
> Hi again,
>
> >>
> >> Hey, i did not want to trigger an eartquake in the basement of the kernel ;-)
> >> My intention was to prevent some crashes, and help developers to find their bugs.
> >> I think my patch exactly does this.
> >
> > Hehe, actually drivers not being able to block unbind has been bugging me
> > for
> > a while now, because there are cases where this would be really helpful.
> >>> 1) make resources refcounted, have child resources take a ref on the parent
> >>> 2) Disallow unbind on devices with bound child-devices?
> >>>
> >> Exactly what i was thinking of in first attempts.
> >> But i fear that would break even more use cases.
> >>
> >> Hans, directly regarding the driver:
> >> The problem i see is that the xhci_intel_unregister_pdev which is added
> >> as an action with devm_add_action_or_reset() is called late by the framework,
> >> later than the usb_hcd_pci_remove() in xhci_pci_remove.
> >> Is there any chance to trigger this before?
> >> This is what Greg meant with "right order".
> >
> > Ah, I missed that part, sure that should be easy, just stop using
> > devm_add_action_or_reset() and do the xhci_intel_unregister_pdev()
> > manually at the right time. The downside of this is that you also
> > need to make sure it happens at the right time from probe error-paths
> > but given the bug you are hitting, I guess that is probably
> > already a problem.
> >
> @Hans:
> Sure, will have a look at this. I think i have found where to do that,
> but need to check how to get the pdev pointer there ....
>
> @Greg:
> I am still confident that my patch in __release_region should be taken in.
Ok, submit it in a "real" way and we can consider it :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* AW: AW: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 12:54 ` Greg KH
@ 2019-08-09 13:00 ` Schmid, Carsten
2019-08-09 13:38 ` Greg KH
0 siblings, 1 reply; 28+ messages in thread
From: Schmid, Carsten @ 2019-08-09 13:00 UTC (permalink / raw)
To: Greg KH
Cc: Hans de Goede, Alan Stern, Andrey Konovalov, Oliver Neukum,
syzkaller-bugs, syzbot, USB list, Hillf Danton
>>
>> @Greg:
>> I am still confident that my patch in __release_region should be taken in.
>
> Ok, submit it in a "real" way and we can consider it :)
>
> thanks,
>
> greg k-h
Already done, linux-kernel@vger.kernel.org, see
https://www.spinics.net/lists/kernel/msg3218180.html
Thanks, and have a nice weekend.
Best regards
Carsten
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: AW: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 13:00 ` AW: " Schmid, Carsten
@ 2019-08-09 13:38 ` Greg KH
0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2019-08-09 13:38 UTC (permalink / raw)
To: Schmid, Carsten
Cc: Hans de Goede, Alan Stern, Andrey Konovalov, Oliver Neukum,
syzkaller-bugs, syzbot, USB list, Hillf Danton
On Fri, Aug 09, 2019 at 01:00:25PM +0000, Schmid, Carsten wrote:
> >>
> >> @Greg:
> >> I am still confident that my patch in __release_region should be taken in.
> >
> > Ok, submit it in a "real" way and we can consider it :)
> >
> > thanks,
> >
> > greg k-h
>
> Already done, linux-kernel@vger.kernel.org, see
> https://www.spinics.net/lists/kernel/msg3218180.html
You didn't cc: any developer, that's a sure way to get a patch ignored
:(
Try resending it with at least the people who get_maintainer.pl says has
touched that file last in it.
Also, Linus is the unofficial resource.c maintainer. I think he has a
set of userspace testing scripts for changes somewhere, so you should
cc: him too. And might as well add me :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-08-08 19:37 ` Alan Stern
2019-08-09 7:35 ` AW: " Schmid, Carsten
@ 2019-08-09 17:36 ` Andrey Konovalov
2019-08-09 18:12 ` syzbot
1 sibling, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2019-08-09 17:36 UTC (permalink / raw)
To: Alan Stern; +Cc: Oliver Neukum, syzkaller-bugs, syzbot, USB list, Hillf Danton
[-- Attachment #1: Type: text/plain, Size: 2675 bytes --]
On Thu, Aug 8, 2019 at 9:37 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Thu, 8 Aug 2019, Andrey Konovalov wrote:
>
> > On Thu, Jul 25, 2019 at 5:09 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > On Thu, 25 Jul 2019, Oliver Neukum wrote:
> > >
> > > > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > > > >
> > > > > > drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> > > > > > 1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > > > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > > > int r = 0;
> > > > > >
> > > > > > + spin_lock_irq(&usbhid->lock);
> > > > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > > > + r = -ENODEV;
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + goto bail_out;
> > > > > > + } else {
> > > > > > + /* protect against disconnect */
> > > > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > > + }
> > > > > > +
> > > > > > switch (lvl) {
> > > > > > case PM_HINT_FULLON:
> > > > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > > > usb_autopm_put_interface(usbhid->intf);
> > > > > > break;
> > > > > > }
> > > > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > > > >
> > > > > > +bail_out:
> > > > > > return r;
> > > > > > }
>
> > This report looks like very similar to these two:
> >
> > https://syzkaller.appspot.com/bug?extid=b156665cf4d1b5e00c76
> > https://syzkaller.appspot.com/bug?extid=3cbe5cd105d2ad56a1df
>
> It also seems to resemble extids a7a6b9c609b9457c62c6,
> 62a1e04fd3ec2abf099e, and 75e6910bf03e266a277f, although this may be an
> illusion.
>
> > Maybe we should mark those two as duplicates.
> >
> > Hillf suggested a fix on one of them, but it looks different from what
> > you propose:
> >
> > https://groups.google.com/d/msg/syzkaller-bugs/xW7LvKfpyn0/SpKbs5ZLEAAJ
>
> Go ahead and try it out on all of them. I don't have a clear feeling
> about it, not having worked on usbhid in quite a while.
>
> Alan Stern
>
Let's try on this one first:
#syz test: https://github.com/google/kasan.git 6a3599ce
[-- Attachment #2: hid-core.patch --]
[-- Type: text/x-patch, Size: 351 bytes --]
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1410,6 +1410,7 @@ static void usbhid_disconnect(struct usb
spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
+ hid_hw_stop(hid);
hid_destroy_device(hid);
kfree(usbhid);
}
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-08-09 17:36 ` Andrey Konovalov
@ 2019-08-09 18:12 ` syzbot
2019-08-12 14:29 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: syzbot @ 2019-08-09 18:12 UTC (permalink / raw)
To: andreyknvl, hdanton, linux-usb, oneukum, stern, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger
crash:
Reported-and-tested-by:
syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
Tested on:
commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=17fcd52c600000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-08-08 18:54 ` Andrey Konovalov
2019-08-08 19:37 ` Alan Stern
@ 2019-08-09 20:26 ` Oliver Neukum
1 sibling, 0 replies; 28+ messages in thread
From: Oliver Neukum @ 2019-08-09 20:26 UTC (permalink / raw)
To: Andrey Konovalov, Alan Stern
Cc: syzkaller-bugs, Hillf Danton, syzbot, USB list
Am Donnerstag, den 08.08.2019, 20:54 +0200 schrieb Andrey Konovalov:
> On Thu, Jul 25, 2019 at 5:09 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Thu, 25 Jul 2019, Oliver Neukum wrote:
> >
> > > Am Mittwoch, den 24.07.2019, 17:02 -0400 schrieb Alan Stern:
> > > > On Wed, 24 Jul 2019, Oliver Neukum wrote:
> > > >
> > > > > drivers/hid/usbhid/hid-core.c | 13 +++++++++++++
> > > > > 1 file changed, 13 insertions(+)
> > > > >
> > > > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > > > > index c7bc9db5b192..98b996ecf4d3 100644
> > > > > --- a/drivers/hid/usbhid/hid-core.c
> > > > > +++ b/drivers/hid/usbhid/hid-core.c
> > > > > @@ -1229,6 +1229,17 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > > struct usbhid_device *usbhid = hid->driver_data;
> > > > > int r = 0;
> > > > >
> > > > > + spin_lock_irq(&usbhid->lock);
> > > > > + if (test_bit(HID_DISCONNECTED, &usbhid->iofl)) {
> > > > > + r = -ENODEV;
> > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > + goto bail_out;
> > > > > + } else {
> > > > > + /* protect against disconnect */
> > > > > + usb_get_dev(interface_to_usbdev(usbhid->intf));
> > > > > + spin_unlock_irq(&usbhid->lock);
> > > > > + }
> > > > > +
> > > > > switch (lvl) {
> > > > > case PM_HINT_FULLON:
> > > > > r = usb_autopm_get_interface(usbhid->intf);
> > > > > @@ -1238,7 +1249,9 @@ static int usbhid_power(struct hid_device *hid, int lvl)
> > > > > usb_autopm_put_interface(usbhid->intf);
> > > > > break;
> > > > > }
> > > > > + usb_put_dev(interface_to_usbdev(usbhid->intf));
> > > > >
> > > > > +bail_out:
> > > > > return r;
> > > > > }
> > > >
> > > > Isn't this treating the symptom instead of the cause?
> > >
> > > Sort of. Holding a reference for the whole time would have merit,
> > > but I doubt it is strictly necessary.
> >
> > Just to be crystal clear, I was talking about a device reference --
> > usb_{get,put}_dev or usb_{get,put}_intf -- not a runtime PM reference.
> >
> > (Incidentally, your patch could be simplified by using usb_get_intf
> > instead of usb_get_dev.)
> >
> > > > Shouldn't the hid_device hold a reference to usbhid->intf throughout
> > > > its lifetime? That way this sort of problem wouldn't arise in any
> > > > routine, not just usbhid_power().
> > >
> > > Unfortunately the semantics would still be wrong without the check
> > > in corner cases. In case disconnect() is called without a physical
> > > unplug, we must not touch the power state.
> > > I am indeed afraid that in that case my putative fix is still racy.
> > > But I don't to just introduce a mutex just for this. Any ideas?
> >
> > That's a separate issue. USB drivers -- indeed, all drivers -- are
> > required to balance their runtime PM gets and puts (although in the
> > case of a physical disconnection it doesn't matter). Are you asking
> > about the best way to do this?
> >
> > Normally a driver's release or disconnect routine will stop all
> > asynchronous accesses to the device (interrupt handlers, work queues,
> > URBs, and so on). At that point the only remaining runtime PM activity
> > will be whatever the routine itself does. So it can see if any extra
> > runtime PM gets or puts are needed, and do whatever is necessary.
> >
> > Does that answer your question? I can't tell for sure...
> >
> > Note: I did not try to track down the reason for the invalid access
> > reported by syzbot. It looked like a simple use-after-free, which
> > would normally be fixed by taking the appropriate reference. Which is
> > what your patch does, except that it holds the reference only for a
> > short time instead of over the entire lifetime of the private data
> > structure (the usbhid structure), which is what normally happens.
>
> This report looks like very similar to these two:
>
> https://syzkaller.appspot.com/bug?extid=b156665cf4d1b5e00c76
> https://syzkaller.appspot.com/bug?extid=3cbe5cd105d2ad56a1df
Yes, they stem from the same root cause most likely.
> Maybe we should mark those two as duplicates.
>
> Hillf suggested a fix on one of them, but it looks different from what
> you propose:
>
> https://groups.google.com/d/msg/syzkaller-bugs/xW7LvKfpyn0/SpKbs5ZLEAAJ
The fix may indeed be necessary, but it is incomplete. It does
not help keeping the PM counters consistent.
Regards
Oliver
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: AW: AW: AW: KASAN: use-after-free Read in usbhid_power
2019-08-09 12:38 ` AW: " Schmid, Carsten
2019-08-09 12:54 ` Greg KH
@ 2019-08-10 11:12 ` Hans de Goede
1 sibling, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2019-08-10 11:12 UTC (permalink / raw)
To: Schmid, Carsten, Greg KH
Cc: Alan Stern, Andrey Konovalov, Oliver Neukum, syzkaller-bugs,
syzbot, USB list, Hillf Danton
Hi,
On 09-08-19 14:38, Schmid, Carsten wrote:
> Hi again,
>
>>>
>>> Hey, i did not want to trigger an eartquake in the basement of the kernel ;-)
>>> My intention was to prevent some crashes, and help developers to find their bugs.
>>> I think my patch exactly does this.
>>
>> Hehe, actually drivers not being able to block unbind has been bugging me
>> for
>> a while now, because there are cases where this would be really helpful.
>>>> 1) make resources refcounted, have child resources take a ref on the parent
>>>> 2) Disallow unbind on devices with bound child-devices?
>>>>
>>> Exactly what i was thinking of in first attempts.
>>> But i fear that would break even more use cases.
>>>
>>> Hans, directly regarding the driver:
>>> The problem i see is that the xhci_intel_unregister_pdev which is added
>>> as an action with devm_add_action_or_reset() is called late by the framework,
>>> later than the usb_hcd_pci_remove() in xhci_pci_remove.
>>> Is there any chance to trigger this before?
>>> This is what Greg meant with "right order".
>>
>> Ah, I missed that part, sure that should be easy, just stop using
>> devm_add_action_or_reset() and do the xhci_intel_unregister_pdev()
>> manually at the right time. The downside of this is that you also
>> need to make sure it happens at the right time from probe error-paths
>> but given the bug you are hitting, I guess that is probably
>> already a problem.
>>
> @Hans:
> Sure, will have a look at this. I think i have found where to do that,
> but need to check how to get the pdev pointer there ....
You probably need to store the pdev pointer in one of the xhci driver's
private data structs.
Regards,
Hans
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: KASAN: use-after-free Read in usbhid_power
2019-08-09 18:12 ` syzbot
@ 2019-08-12 14:29 ` Andrey Konovalov
0 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2019-08-12 14:29 UTC (permalink / raw)
To: syzbot; +Cc: Hillf Danton, USB list, Oliver Neukum, Alan Stern, syzkaller-bugs
On Fri, Aug 9, 2019 at 8:12 PM syzbot
<syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger
> crash:
>
> Reported-and-tested-by:
> syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com
OK, I'm duping this BUG to the similar one that Hillf fixed:
#syz dup: general protection fault in __pm_runtime_resume
If there are more issues with PM counters, syzbot will rereport them
once the fix is in its tree.
>
> Tested on:
>
> commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git
> kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> patch: https://syzkaller.appspot.com/x/patch.diff?x=17fcd52c600000
>
> Note: testing is done by a robot and is best-effort only.
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000004de4e2058fb31c2b%40google.com.
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2019-08-12 14:29 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23 12:48 KASAN: use-after-free Read in usbhid_power syzbot
2019-07-24 14:17 ` Oliver Neukum
2019-07-24 14:24 ` Andrey Konovalov
2019-07-24 20:55 ` Oliver Neukum
2019-07-24 21:02 ` Alan Stern
2019-07-25 9:33 ` Oliver Neukum
2019-07-25 15:09 ` Alan Stern
2019-08-08 18:54 ` Andrey Konovalov
2019-08-08 19:37 ` Alan Stern
2019-08-09 7:35 ` AW: " Schmid, Carsten
2019-08-09 7:55 ` Greg KH
2019-08-09 9:34 ` AW: " Schmid, Carsten
2019-08-09 10:20 ` Hans de Goede
2019-08-09 10:47 ` AW: " Schmid, Carsten
2019-08-09 10:53 ` Hans de Goede
2019-08-09 12:38 ` AW: " Schmid, Carsten
2019-08-09 12:54 ` Greg KH
2019-08-09 13:00 ` AW: " Schmid, Carsten
2019-08-09 13:38 ` Greg KH
2019-08-10 11:12 ` AW: " Hans de Goede
2019-08-09 17:36 ` Andrey Konovalov
2019-08-09 18:12 ` syzbot
2019-08-12 14:29 ` Andrey Konovalov
2019-08-09 20:26 ` Oliver Neukum
2019-07-24 21:16 ` syzbot
2019-07-25 11:22 ` Andrey Konovalov
2019-07-25 12:15 ` Oliver Neukum
2019-07-25 12:27 ` syzbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).