* KASAN: use-after-free Read in dev_uevent
@ 2020-06-19 5:48 syzbot
2022-02-20 17:19 ` [syzbot] " syzbot
0 siblings, 1 reply; 26+ messages in thread
From: syzbot @ 2020-06-19 5:48 UTC (permalink / raw)
To: gregkh, linux-kernel, rafael, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 7ae77150 Merge tag 'powerpc-5.8-1' of git://git.kernel.org..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=169fa049100000
kernel config: https://syzkaller.appspot.com/x/.config?x=be4578b3f1083656
dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in dev_uevent+0x30a/0x580 drivers/base/core.c:1486
Read of size 8 at addr ffff888098662098 by task systemd-udevd/29958
CPU: 0 PID: 29958 Comm: systemd-udevd Not tainted 5.7.0-syzkaller #0
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+0x1e9/0x30e lib/dump_stack.c:118
print_address_description+0x66/0x5a0 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report+0x132/0x1d0 mm/kasan/report.c:530
dev_uevent+0x30a/0x580 drivers/base/core.c:1486
uevent_show+0x1b2/0x2f0 drivers/base/core.c:1557
dev_attr_show+0x50/0xc0 drivers/base/core.c:1261
sysfs_kf_seq_show+0x30e/0x4e0 fs/sysfs/file.c:60
seq_read+0x41a/0xce0 fs/seq_file.c:208
__vfs_read+0x9c/0x6d0 fs/read_write.c:426
vfs_read+0x1c4/0x400 fs/read_write.c:462
ksys_read+0x11b/0x220 fs/read_write.c:588
do_syscall_64+0xf3/0x1b0 arch/x86/entry/common.c:295
entry_SYSCALL_64_after_hwframe+0x49/0xb3
RIP: 0033:0x7f28fc677910
Code: b6 fe ff ff 48 8d 3d 0f be 08 00 48 83 ec 08 e8 06 db 01 00 66 0f 1f 44 00 00 83 3d f9 2d 2c 00 00 75 10 b8 00 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 de 9b 01 00 48 89 04 24
RSP: 002b:00007ffe3053dd18 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 00005562a17eb920 RCX: 00007f28fc677910
RDX: 0000000000001000 RSI: 00005562a17fe8c0 RDI: 0000000000000007
RBP: 00007f28fc932440 R08: 00007f28fc9361e8 R09: 0000000000001010
R10: 00005562a17eb920 R11: 0000000000000246 R12: 0000000000001000
R13: 0000000000000d68 R14: 00005562a17fe8c0 R15: 00007f28fc931900
Allocated by task 29904:
save_stack mm/kasan/common.c:48 [inline]
set_track mm/kasan/common.c:56 [inline]
__kasan_kmalloc+0x103/0x140 mm/kasan/common.c:494
kmem_cache_alloc_trace+0x234/0x300 mm/slab.c:3551
kmalloc include/linux/slab.h:555 [inline]
kzalloc include/linux/slab.h:669 [inline]
dev_new drivers/usb/gadget/legacy/raw_gadget.c:182 [inline]
raw_open+0x87/0x500 drivers/usb/gadget/legacy/raw_gadget.c:372
misc_open+0x346/0x3c0 drivers/char/misc.c:141
chrdev_open+0x498/0x580 fs/char_dev.c:414
do_dentry_open+0x808/0x1020 fs/open.c:828
do_open fs/namei.c:3229 [inline]
path_openat+0x2790/0x38b0 fs/namei.c:3346
do_filp_open+0x191/0x3a0 fs/namei.c:3373
do_sys_openat2+0x463/0x770 fs/open.c:1179
do_sys_open fs/open.c:1195 [inline]
ksys_open include/linux/syscalls.h:1388 [inline]
__do_sys_open fs/open.c:1201 [inline]
__se_sys_open fs/open.c:1199 [inline]
__x64_sys_open+0x1af/0x1e0 fs/open.c:1199
do_syscall_64+0xf3/0x1b0 arch/x86/entry/common.c:295
entry_SYSCALL_64_after_hwframe+0x49/0xb3
Freed by task 29956:
save_stack mm/kasan/common.c:48 [inline]
set_track mm/kasan/common.c:56 [inline]
kasan_set_free_info mm/kasan/common.c:316 [inline]
__kasan_slab_free+0x114/0x170 mm/kasan/common.c:455
__cache_free mm/slab.c:3426 [inline]
kfree+0x10a/0x220 mm/slab.c:3757
raw_release+0x130/0x1e0 drivers/usb/gadget/legacy/raw_gadget.c:411
__fput+0x2ed/0x750 fs/file_table.c:281
task_work_run+0x147/0x1d0 kernel/task_work.c:123
exit_task_work include/linux/task_work.h:22 [inline]
do_exit+0x5ef/0x1f80 kernel/exit.c:806
do_group_exit+0x15e/0x2c0 kernel/exit.c:904
get_signal+0x13cf/0x1d60 kernel/signal.c:2739
do_signal+0x33/0x610 arch/x86/kernel/signal.c:810
exit_to_usermode_loop arch/x86/entry/common.c:161 [inline]
prepare_exit_to_usermode+0x32a/0x600 arch/x86/entry/common.c:196
entry_SYSCALL_64_after_hwframe+0x49/0xb3
The buggy address belongs to the object at ffff888098662000
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 152 bytes inside of
4096-byte region [ffff888098662000, ffff888098663000)
The buggy address belongs to the page:
page:ffffea0002619880 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 head:ffffea0002619880 order:1 compound_mapcount:0
flags: 0xfffe0000010200(slab|head)
raw: 00fffe0000010200 ffffea00021d0908 ffffea0002a5a808 ffff8880aa402000
raw: 0000000000000000 ffff888098662000 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888098661f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888098662000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888098662080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888098662100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888098662180: 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.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2020-06-19 5:48 KASAN: use-after-free Read in dev_uevent syzbot
@ 2022-02-20 17:19 ` syzbot
2022-02-23 6:51 ` Zhang, Qiang1
2022-02-23 11:17 ` Zhang, Qiang1
0 siblings, 2 replies; 26+ messages in thread
From: syzbot @ 2022-02-20 17:19 UTC (permalink / raw)
To: gregkh, linux-kernel, rafael, syzkaller-bugs
syzbot has found a reproducer for the following issue on:
HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320
Read of size 8 at addr ffff88802b934098 by task udevd/3689
CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255
__kasan_report mm/kasan/report.c:442 [inline]
kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
dev_uevent+0x712/0x780 drivers/base/core.c:2320
uevent_show+0x1b8/0x380 drivers/base/core.c:2391
dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241
call_read_iter include/linux/fs.h:2068 [inline]
new_sync_read+0x429/0x6e0 fs/read_write.c:400
vfs_read+0x35c/0x600 fs/read_write.c:481
ksys_read+0x12d/0x250 fs/read_write.c:619
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f964cc558fe
Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68
</TASK>
Allocated by task 4316:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38
kasan_set_track mm/kasan/common.c:45 [inline]
set_alloc_info mm/kasan/common.c:436 [inline]
____kasan_kmalloc mm/kasan/common.c:515 [inline]
____kasan_kmalloc mm/kasan/common.c:474 [inline]
__kasan_kmalloc+0xa6/0xd0 mm/kasan/common.c:524
kasan_kmalloc include/linux/kasan.h:270 [inline]
kmem_cache_alloc_trace+0x1ea/0x4a0 mm/slab.c:3567
kmalloc include/linux/slab.h:581 [inline]
kzalloc include/linux/slab.h:715 [inline]
dev_new drivers/usb/gadget/legacy/raw_gadget.c:183 [inline]
raw_open+0x8d/0x4c0 drivers/usb/gadget/legacy/raw_gadget.c:373
misc_open+0x372/0x4a0 drivers/char/misc.c:141
chrdev_open+0x266/0x770 fs/char_dev.c:414
do_dentry_open+0x4b9/0x1250 fs/open.c:824
do_open fs/namei.c:3476 [inline]
path_openat+0x1c9e/0x2940 fs/namei.c:3609
do_filp_open+0x1aa/0x400 fs/namei.c:3636
do_sys_openat2+0x16d/0x4d0 fs/open.c:1214
do_sys_open fs/open.c:1230 [inline]
__do_sys_openat fs/open.c:1246 [inline]
__se_sys_openat fs/open.c:1241 [inline]
__x64_sys_openat+0x13f/0x1f0 fs/open.c:1241
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
Freed by task 4315:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38
kasan_set_track+0x21/0x30 mm/kasan/common.c:45
kasan_set_free_info+0x20/0x30 mm/kasan/generic.c:370
____kasan_slab_free mm/kasan/common.c:366 [inline]
____kasan_slab_free+0xff/0x140 mm/kasan/common.c:328
kasan_slab_free include/linux/kasan.h:236 [inline]
__cache_free mm/slab.c:3437 [inline]
kfree+0xf8/0x2b0 mm/slab.c:3794
kref_put include/linux/kref.h:65 [inline]
raw_release+0x218/0x290 drivers/usb/gadget/legacy/raw_gadget.c:412
__fput+0x286/0x9f0 fs/file_table.c:317
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
tracehook_notify_resume include/linux/tracehook.h:188 [inline]
exit_to_user_mode_loop kernel/entry/common.c:175 [inline]
exit_to_user_mode_prepare+0x27e/0x290 kernel/entry/common.c:207
__syscall_exit_to_user_mode_work kernel/entry/common.c:289 [inline]
syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:300
do_syscall_64+0x42/0xb0 arch/x86/entry/common.c:86
entry_SYSCALL_64_after_hwframe+0x44/0xae
The buggy address belongs to the object at ffff88802b934000
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 152 bytes inside of
4096-byte region [ffff88802b934000, ffff88802b935000)
The buggy address belongs to the page:
page:ffffea0000ae4d00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x2b934
head:ffffea0000ae4d00 order:1 compound_mapcount:0
flags: 0xfff00000010200(slab|head|node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000010200 ffffea00008be908 ffffea0000612d08 ffff888010c40900
raw: 0000000000000000 ffff88802b934000 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 1, migratetype Unmovable, gfp_mask 0x2420c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_COMP|__GFP_THISNODE), pid 4316, ts 254636955499, free_ts 240714313612
prep_new_page mm/page_alloc.c:2434 [inline]
get_page_from_freelist+0xa72/0x2f50 mm/page_alloc.c:4165
__alloc_pages_slowpath.constprop.0+0x2eb/0x20d0 mm/page_alloc.c:4934
__alloc_pages+0x412/0x500 mm/page_alloc.c:5402
__alloc_pages_node include/linux/gfp.h:572 [inline]
kmem_getpages mm/slab.c:1378 [inline]
cache_grow_begin+0x75/0x390 mm/slab.c:2584
cache_alloc_refill+0x27f/0x380 mm/slab.c:2957
____cache_alloc mm/slab.c:3040 [inline]
____cache_alloc mm/slab.c:3023 [inline]
__do_cache_alloc mm/slab.c:3267 [inline]
slab_alloc mm/slab.c:3308 [inline]
kmem_cache_alloc_trace+0x380/0x4a0 mm/slab.c:3565
kmalloc include/linux/slab.h:581 [inline]
kzalloc include/linux/slab.h:715 [inline]
dev_new drivers/usb/gadget/legacy/raw_gadget.c:183 [inline]
raw_open+0x8d/0x4c0 drivers/usb/gadget/legacy/raw_gadget.c:373
misc_open+0x372/0x4a0 drivers/char/misc.c:141
chrdev_open+0x266/0x770 fs/char_dev.c:414
do_dentry_open+0x4b9/0x1250 fs/open.c:824
do_open fs/namei.c:3476 [inline]
path_openat+0x1c9e/0x2940 fs/namei.c:3609
do_filp_open+0x1aa/0x400 fs/namei.c:3636
do_sys_openat2+0x16d/0x4d0 fs/open.c:1214
do_sys_open fs/open.c:1230 [inline]
__do_sys_openat fs/open.c:1246 [inline]
__se_sys_openat fs/open.c:1241 [inline]
__x64_sys_openat+0x13f/0x1f0 fs/open.c:1241
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
page last free stack trace:
reset_page_owner include/linux/page_owner.h:24 [inline]
free_pages_prepare mm/page_alloc.c:1352 [inline]
free_pcp_prepare+0x374/0x870 mm/page_alloc.c:1404
free_unref_page_prepare mm/page_alloc.c:3325 [inline]
free_unref_page+0x19/0x690 mm/page_alloc.c:3404
slab_destroy mm/slab.c:1630 [inline]
slabs_destroy+0x89/0xc0 mm/slab.c:1650
cache_flusharray mm/slab.c:3410 [inline]
___cache_free+0x303/0x600 mm/slab.c:3472
qlink_free mm/kasan/quarantine.c:157 [inline]
qlist_free_all+0x50/0x1a0 mm/kasan/quarantine.c:176
kasan_quarantine_reduce+0x180/0x200 mm/kasan/quarantine.c:283
__kasan_slab_alloc+0x97/0xb0 mm/kasan/common.c:446
kasan_slab_alloc include/linux/kasan.h:260 [inline]
slab_post_alloc_hook mm/slab.h:732 [inline]
slab_alloc_node mm/slab.c:3253 [inline]
kmem_cache_alloc_node+0x2ea/0x590 mm/slab.c:3591
__alloc_skb+0x215/0x340 net/core/skbuff.c:414
alloc_skb include/linux/skbuff.h:1158 [inline]
alloc_skb_with_frags+0x93/0x620 net/core/skbuff.c:5956
sock_alloc_send_pskb+0x793/0x920 net/core/sock.c:2586
unix_dgram_sendmsg+0x414/0x1a10 net/unix/af_unix.c:1896
sock_sendmsg_nosec net/socket.c:705 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:725
__sys_sendto+0x21c/0x320 net/socket.c:2040
__do_sys_sendto net/socket.c:2052 [inline]
__se_sys_sendto net/socket.c:2048 [inline]
__x64_sys_sendto+0xdd/0x1b0 net/socket.c:2048
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
Memory state around the buggy address:
ffff88802b933f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88802b934000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88802b934080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88802b934100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88802b934180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-20 17:19 ` [syzbot] " syzbot
@ 2022-02-23 6:51 ` Zhang, Qiang1
2022-02-23 7:13 ` gregkh
2022-02-23 11:17 ` Zhang, Qiang1
1 sibling, 1 reply; 26+ messages in thread
From: Zhang, Qiang1 @ 2022-02-23 6:51 UTC (permalink / raw)
To: syzbot, gregkh, linux-kernel, rafael, syzkaller-bugs, balbi
HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by task udevd/3689
CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
dev_uevent+0x712/0x780 drivers/base/core.c:2320
uevent_show+0x1b8/0x380 drivers/base/core.c:2391
dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241 call_read_iter include/linux/fs.h:2068 [inline]
new_sync_read+0x429/0x6e0 fs/read_write.c:400
vfs_read+0x35c/0x600 fs/read_write.c:481
ksys_read+0x12d/0x250 fs/read_write.c:619
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f964cc558fe
Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68 </TASK>
Hi All
This should be because when the raw_dev is released, the 'driver' address has expired,
although the usb_gadget_remove_driver() empty 'dev.driver ' NULL, but UAF cannot be avoided.
static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env) {
.....
if (dev->driver)
2320 add_uevent_var(env, "DRIVER=%s", dev->driver->name);
.....
}
Whether protection can be added when operating 'dev->driver'?
Thanks,
Zqiang
Allocated by task 4316:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38 kasan_set_track mm/kasan/common.c:45 [inline] set_alloc_info mm/kasan/common.c:436 [inline] ____kasan_kmalloc mm/kasan/common.c:515 [inline] ____kasan_kmalloc mm/kasan/common.c:474 [inline]
__kasan_kmalloc+0xa6/0xd0 mm/kasan/common.c:524 kasan_kmalloc include/linux/kasan.h:270 [inline]
kmem_cache_alloc_trace+0x1ea/0x4a0 mm/slab.c:3567 kmalloc include/linux/slab.h:581 [inline] kzalloc include/linux/slab.h:715 [inline] dev_new drivers/usb/gadget/legacy/raw_gadget.c:183 [inline]
raw_open+0x8d/0x4c0 drivers/usb/gadget/legacy/raw_gadget.c:373
misc_open+0x372/0x4a0 drivers/char/misc.c:141
chrdev_open+0x266/0x770 fs/char_dev.c:414
do_dentry_open+0x4b9/0x1250 fs/open.c:824 do_open fs/namei.c:3476 [inline]
path_openat+0x1c9e/0x2940 fs/namei.c:3609
do_filp_open+0x1aa/0x400 fs/namei.c:3636
do_sys_openat2+0x16d/0x4d0 fs/open.c:1214 do_sys_open fs/open.c:1230 [inline] __do_sys_openat fs/open.c:1246 [inline] __se_sys_openat fs/open.c:1241 [inline]
__x64_sys_openat+0x13f/0x1f0 fs/open.c:1241
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
Freed by task 4315:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38
kasan_set_track+0x21/0x30 mm/kasan/common.c:45
kasan_set_free_info+0x20/0x30 mm/kasan/generic.c:370 ____kasan_slab_free mm/kasan/common.c:366 [inline]
____kasan_slab_free+0xff/0x140 mm/kasan/common.c:328 kasan_slab_free include/linux/kasan.h:236 [inline] __cache_free mm/slab.c:3437 [inline]
kfree+0xf8/0x2b0 mm/slab.c:3794
kref_put include/linux/kref.h:65 [inline]
raw_release+0x218/0x290 drivers/usb/gadget/legacy/raw_gadget.c:412
__fput+0x286/0x9f0 fs/file_table.c:317
task_work_run+0xdd/0x1a0 kernel/task_work.c:164 tracehook_notify_resume include/linux/tracehook.h:188 [inline] exit_to_user_mode_loop kernel/entry/common.c:175 [inline]
exit_to_user_mode_prepare+0x27e/0x290 kernel/entry/common.c:207 __syscall_exit_to_user_mode_work kernel/entry/common.c:289 [inline]
syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:300
do_syscall_64+0x42/0xb0 arch/x86/entry/common.c:86 entry_SYSCALL_64_after_hwframe+0x44/0xae
The buggy address belongs to the object at ffff88802b934000 which belongs to the cache kmalloc-4k of size 4096 The buggy address is located 152 bytes inside of 4096-byte region [ffff88802b934000, ffff88802b935000) The buggy address belongs to the page:
page:ffffea0000ae4d00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x2b934
head:ffffea0000ae4d00 order:1 compound_mapcount:0
flags: 0xfff00000010200(slab|head|node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000010200 ffffea00008be908 ffffea0000612d08 ffff888010c40900
raw: 0000000000000000 ffff88802b934000 0000000100000001 0000000000000000 page dumped because: kasan: bad access detected page_owner tracks the page as allocated page last allocated via order 1, migratetype Unmovable, gfp_mask 0x2420c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_COMP|__GFP_THISNODE), pid 4316, ts 254636955499, free_ts 240714313612 prep_new_page mm/page_alloc.c:2434 [inline]
get_page_from_freelist+0xa72/0x2f50 mm/page_alloc.c:4165
__alloc_pages_slowpath.constprop.0+0x2eb/0x20d0 mm/page_alloc.c:4934
__alloc_pages+0x412/0x500 mm/page_alloc.c:5402 __alloc_pages_node include/linux/gfp.h:572 [inline] kmem_getpages mm/slab.c:1378 [inline]
cache_grow_begin+0x75/0x390 mm/slab.c:2584
cache_alloc_refill+0x27f/0x380 mm/slab.c:2957 ____cache_alloc mm/slab.c:3040 [inline] ____cache_alloc mm/slab.c:3023 [inline] __do_cache_alloc mm/slab.c:3267 [inline] slab_alloc mm/slab.c:3308 [inline]
kmem_cache_alloc_trace+0x380/0x4a0 mm/slab.c:3565 kmalloc include/linux/slab.h:581 [inline] kzalloc include/linux/slab.h:715 [inline] dev_new drivers/usb/gadget/legacy/raw_gadget.c:183 [inline]
raw_open+0x8d/0x4c0 drivers/usb/gadget/legacy/raw_gadget.c:373
misc_open+0x372/0x4a0 drivers/char/misc.c:141
chrdev_open+0x266/0x770 fs/char_dev.c:414
do_dentry_open+0x4b9/0x1250 fs/open.c:824 do_open fs/namei.c:3476 [inline]
path_openat+0x1c9e/0x2940 fs/namei.c:3609
do_filp_open+0x1aa/0x400 fs/namei.c:3636
do_sys_openat2+0x16d/0x4d0 fs/open.c:1214 do_sys_open fs/open.c:1230 [inline] __do_sys_openat fs/open.c:1246 [inline] __se_sys_openat fs/open.c:1241 [inline]
__x64_sys_openat+0x13f/0x1f0 fs/open.c:1241
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
page last free stack trace:
reset_page_owner include/linux/page_owner.h:24 [inline] free_pages_prepare mm/page_alloc.c:1352 [inline]
free_pcp_prepare+0x374/0x870 mm/page_alloc.c:1404 free_unref_page_prepare mm/page_alloc.c:3325 [inline]
free_unref_page+0x19/0x690 mm/page_alloc.c:3404 slab_destroy mm/slab.c:1630 [inline]
slabs_destroy+0x89/0xc0 mm/slab.c:1650
cache_flusharray mm/slab.c:3410 [inline]
___cache_free+0x303/0x600 mm/slab.c:3472 qlink_free mm/kasan/quarantine.c:157 [inline]
qlist_free_all+0x50/0x1a0 mm/kasan/quarantine.c:176
kasan_quarantine_reduce+0x180/0x200 mm/kasan/quarantine.c:283
__kasan_slab_alloc+0x97/0xb0 mm/kasan/common.c:446 kasan_slab_alloc include/linux/kasan.h:260 [inline] slab_post_alloc_hook mm/slab.h:732 [inline] slab_alloc_node mm/slab.c:3253 [inline]
kmem_cache_alloc_node+0x2ea/0x590 mm/slab.c:3591
__alloc_skb+0x215/0x340 net/core/skbuff.c:414 alloc_skb include/linux/skbuff.h:1158 [inline]
alloc_skb_with_frags+0x93/0x620 net/core/skbuff.c:5956
sock_alloc_send_pskb+0x793/0x920 net/core/sock.c:2586
unix_dgram_sendmsg+0x414/0x1a10 net/unix/af_unix.c:1896 sock_sendmsg_nosec net/socket.c:705 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:725
__sys_sendto+0x21c/0x320 net/socket.c:2040 __do_sys_sendto net/socket.c:2052 [inline] __se_sys_sendto net/socket.c:2048 [inline]
__x64_sys_sendto+0xdd/0x1b0 net/socket.c:2048
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
Memory state around the buggy address:
ffff88802b933f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88802b934000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88802b934080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88802b934100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88802b934180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ==================================================================
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 6:51 ` Zhang, Qiang1
@ 2022-02-23 7:13 ` gregkh
2022-02-23 8:08 ` Zhang, Qiang1
0 siblings, 1 reply; 26+ messages in thread
From: gregkh @ 2022-02-23 7:13 UTC (permalink / raw)
To: Zhang, Qiang1; +Cc: syzbot, linux-kernel, rafael, syzkaller-bugs, balbi
On Wed, Feb 23, 2022 at 06:51:10AM +0000, Zhang, Qiang1 wrote:
>
> HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
> kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
> dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by task udevd/3689
>
> CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
> dev_uevent+0x712/0x780 drivers/base/core.c:2320
> uevent_show+0x1b8/0x380 drivers/base/core.c:2391
> dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
> sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
> seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
> kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241 call_read_iter include/linux/fs.h:2068 [inline]
> new_sync_read+0x429/0x6e0 fs/read_write.c:400
> vfs_read+0x35c/0x600 fs/read_write.c:481
> ksys_read+0x12d/0x250 fs/read_write.c:619
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7f964cc558fe
> Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
> RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
> RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
> RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
> R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
> R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68 </TASK>
>
>
> Hi All
>
> This should be because when the raw_dev is released, the 'driver' address has expired,
> although the usb_gadget_remove_driver() empty 'dev.driver ' NULL, but UAF cannot be avoided.
>
> static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env) {
> .....
> if (dev->driver)
> 2320 add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> .....
> }
>
> Whether protection can be added when operating 'dev->driver'?
When the driver structure is unbound, this should be set to NULL. Why
isn't that happening?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 7:13 ` gregkh
@ 2022-02-23 8:08 ` Zhang, Qiang1
0 siblings, 0 replies; 26+ messages in thread
From: Zhang, Qiang1 @ 2022-02-23 8:08 UTC (permalink / raw)
To: gregkh; +Cc: syzbot, linux-kernel, rafael, syzkaller-bugs, balbi
On Wed, Feb 23, 2022 at 06:51:10AM +0000, Zhang, Qiang1 wrote:
>
> HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
> git tree: upstream
> console output:
> https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
> kernel config:
> https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
> dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in dev_uevent+0x712/0x780
> drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by
> task udevd/3689
>
> CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> print_address_description.constprop.0.cold+0x8d/0x303
> mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline]
> kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
> dev_uevent+0x712/0x780 drivers/base/core.c:2320
> uevent_show+0x1b8/0x380 drivers/base/core.c:2391
> dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
> sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
> seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
> kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241 call_read_iter
> include/linux/fs.h:2068 [inline]
> new_sync_read+0x429/0x6e0 fs/read_write.c:400
> vfs_read+0x35c/0x600 fs/read_write.c:481
> ksys_read+0x12d/0x250 fs/read_write.c:619
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7f964cc558fe
> Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f
> 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d
> 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
> RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
> RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
> RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
> R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
> R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68
> </TASK>
>
>
> Hi All
>
> This should be because when the raw_dev is released, the 'driver'
> address has expired, although the usb_gadget_remove_driver() empty 'dev.driver ' NULL, but UAF cannot be avoided.
>
> static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env
> *env) { .....
> if (dev->driver)
> 2320 add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> .....
> }
>
> Whether protection can be added when operating 'dev->driver'?
>
>When the driver structure is unbound, this should be set to NULL. Why isn't that happening?
Yes, it should be set NULL at:
close
raw_release
->usb_gadget_unregister_driver
-> usb_gadget_remove_driver
{
......
udc->driver = NULL;
udc->dev.driver = NULL;
udc->gadget->dev.driver = NULL;
}
kref_put(&dev->count, dev_free)
-> dev_free
kfree(dev) <------------------------------------------ raw_dev be freed
if the udev process not meet it. The UAF maybe can trigger.
Did I miss something?
thanks,
Zqiang
>
>thanks,
>
>greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-20 17:19 ` [syzbot] " syzbot
2022-02-23 6:51 ` Zhang, Qiang1
@ 2022-02-23 11:17 ` Zhang, Qiang1
2022-02-23 11:23 ` Pavel Skripkin
2022-02-23 11:29 ` gregkh
1 sibling, 2 replies; 26+ messages in thread
From: Zhang, Qiang1 @ 2022-02-23 11:17 UTC (permalink / raw)
To: syzbot, gregkh, linux-kernel, rafael, syzkaller-bugs, balbi, stern
syzbot has found a reproducer for the following issue on:
HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by task udevd/3689
CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
dev_uevent+0x712/0x780 drivers/base/core.c:2320
uevent_show+0x1b8/0x380 drivers/base/core.c:2391
dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241 call_read_iter include/linux/fs.h:2068 [inline]
new_sync_read+0x429/0x6e0 fs/read_write.c:400
vfs_read+0x35c/0x600 fs/read_write.c:481
ksys_read+0x12d/0x250 fs/read_write.c:619
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f964cc558fe
Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68 </TASK>
Cc: Alan Stern
Felipe Balbi
Hello syzbot, Please try it:
From 574d45ff924e2d2f9b9f5cc3e846f8004498a811 Mon Sep 17 00:00:00 2001
From: Zqiang <qiang1.zhang@intel.com>
Date: Wed, 23 Feb 2022 18:18:22 +0800
Subject: [PATCH] driver core: Fix use-after-free in dev_uevent()
In dev_uevent(), if the "dev->driver" is valid, the "dev->driver->name"
be accessed, there may be a window period between these two operations.
in this window period if the "dev->driver" is set to null
(in usb_gadget_unregister_driver function), when the "dev->driver->name"
is accessed again, invalid address will be accessed. fix it by checking
"dev->driver" again.
Signed-off-by: Zqiang <qiang1.zhang@intel.com>
---
drivers/base/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3d6430eb0c6a..a45b927ee76e 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2317,7 +2317,7 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
if (dev->driver)
- add_uevent_var(env, "DRIVER=%s", dev->driver->name);
+ add_uevent_var(env, "DRIVER=%s", dev_driver_string(dev));
/* Add common DT information about the device */
of_device_uevent(dev, env);
--
2.25.1
Thanks,
Zqiang
Allocated by task 4316:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38 kasan_set_track mm/kasan/common.c:45 [inline] set_alloc_info mm/kasan/common.c:436 [inline] ____kasan_kmalloc mm/kasan/common.c:515 [inline] ____kasan_kmalloc mm/kasan/common.c:474 [inline]
__kasan_kmalloc+0xa6/0xd0 mm/kasan/common.c:524 kasan_kmalloc include/linux/kasan.h:270 [inline]
kmem_cache_alloc_trace+0x1ea/0x4a0 mm/slab.c:3567 kmalloc include/linux/slab.h:581 [inline] kzalloc include/linux/slab.h:715 [inline] dev_new drivers/usb/gadget/legacy/raw_gadget.c:183 [inline]
raw_open+0x8d/0x4c0 drivers/usb/gadget/legacy/raw_gadget.c:373
misc_open+0x372/0x4a0 drivers/char/misc.c:141
chrdev_open+0x266/0x770 fs/char_dev.c:414
do_dentry_open+0x4b9/0x1250 fs/open.c:824 do_open fs/namei.c:3476 [inline]
path_openat+0x1c9e/0x2940 fs/namei.c:3609
do_filp_open+0x1aa/0x400 fs/namei.c:3636
do_sys_openat2+0x16d/0x4d0 fs/open.c:1214 do_sys_open fs/open.c:1230 [inline] __do_sys_openat fs/open.c:1246 [inline] __se_sys_openat fs/open.c:1241 [inline]
__x64_sys_openat+0x13f/0x1f0 fs/open.c:1241
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
Freed by task 4315:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38
kasan_set_track+0x21/0x30 mm/kasan/common.c:45
kasan_set_free_info+0x20/0x30 mm/kasan/generic.c:370 ____kasan_slab_free mm/kasan/common.c:366 [inline]
____kasan_slab_free+0xff/0x140 mm/kasan/common.c:328 kasan_slab_free include/linux/kasan.h:236 [inline] __cache_free mm/slab.c:3437 [inline]
kfree+0xf8/0x2b0 mm/slab.c:3794
kref_put include/linux/kref.h:65 [inline]
raw_release+0x218/0x290 drivers/usb/gadget/legacy/raw_gadget.c:412
__fput+0x286/0x9f0 fs/file_table.c:317
task_work_run+0xdd/0x1a0 kernel/task_work.c:164 tracehook_notify_resume include/linux/tracehook.h:188 [inline] exit_to_user_mode_loop kernel/entry/common.c:175 [inline]
exit_to_user_mode_prepare+0x27e/0x290 kernel/entry/common.c:207 __syscall_exit_to_user_mode_work kernel/entry/common.c:289 [inline]
syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:300
do_syscall_64+0x42/0xb0 arch/x86/entry/common.c:86 entry_SYSCALL_64_after_hwframe+0x44/0xae
The buggy address belongs to the object at ffff88802b934000 which belongs to the cache kmalloc-4k of size 4096 The buggy address is located 152 bytes inside of 4096-byte region [ffff88802b934000, ffff88802b935000) The buggy address belongs to the page:
page:ffffea0000ae4d00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x2b934
head:ffffea0000ae4d00 order:1 compound_mapcount:0
flags: 0xfff00000010200(slab|head|node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000010200 ffffea00008be908 ffffea0000612d08 ffff888010c40900
raw: 0000000000000000 ffff88802b934000 0000000100000001 0000000000000000 page dumped because: kasan: bad access detected page_owner tracks the page as allocated page last allocated via order 1, migratetype Unmovable, gfp_mask 0x2420c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_COMP|__GFP_THISNODE), pid 4316, ts 254636955499, free_ts 240714313612 prep_new_page mm/page_alloc.c:2434 [inline]
get_page_from_freelist+0xa72/0x2f50 mm/page_alloc.c:4165
__alloc_pages_slowpath.constprop.0+0x2eb/0x20d0 mm/page_alloc.c:4934
__alloc_pages+0x412/0x500 mm/page_alloc.c:5402 __alloc_pages_node include/linux/gfp.h:572 [inline] kmem_getpages mm/slab.c:1378 [inline]
cache_grow_begin+0x75/0x390 mm/slab.c:2584
cache_alloc_refill+0x27f/0x380 mm/slab.c:2957 ____cache_alloc mm/slab.c:3040 [inline] ____cache_alloc mm/slab.c:3023 [inline] __do_cache_alloc mm/slab.c:3267 [inline] slab_alloc mm/slab.c:3308 [inline]
kmem_cache_alloc_trace+0x380/0x4a0 mm/slab.c:3565 kmalloc include/linux/slab.h:581 [inline] kzalloc include/linux/slab.h:715 [inline] dev_new drivers/usb/gadget/legacy/raw_gadget.c:183 [inline]
raw_open+0x8d/0x4c0 drivers/usb/gadget/legacy/raw_gadget.c:373
misc_open+0x372/0x4a0 drivers/char/misc.c:141
chrdev_open+0x266/0x770 fs/char_dev.c:414
do_dentry_open+0x4b9/0x1250 fs/open.c:824 do_open fs/namei.c:3476 [inline]
path_openat+0x1c9e/0x2940 fs/namei.c:3609
do_filp_open+0x1aa/0x400 fs/namei.c:3636
do_sys_openat2+0x16d/0x4d0 fs/open.c:1214 do_sys_open fs/open.c:1230 [inline] __do_sys_openat fs/open.c:1246 [inline] __se_sys_openat fs/open.c:1241 [inline]
__x64_sys_openat+0x13f/0x1f0 fs/open.c:1241
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
page last free stack trace:
reset_page_owner include/linux/page_owner.h:24 [inline] free_pages_prepare mm/page_alloc.c:1352 [inline]
free_pcp_prepare+0x374/0x870 mm/page_alloc.c:1404 free_unref_page_prepare mm/page_alloc.c:3325 [inline]
free_unref_page+0x19/0x690 mm/page_alloc.c:3404 slab_destroy mm/slab.c:1630 [inline]
slabs_destroy+0x89/0xc0 mm/slab.c:1650
cache_flusharray mm/slab.c:3410 [inline]
___cache_free+0x303/0x600 mm/slab.c:3472 qlink_free mm/kasan/quarantine.c:157 [inline]
qlist_free_all+0x50/0x1a0 mm/kasan/quarantine.c:176
kasan_quarantine_reduce+0x180/0x200 mm/kasan/quarantine.c:283
__kasan_slab_alloc+0x97/0xb0 mm/kasan/common.c:446 kasan_slab_alloc include/linux/kasan.h:260 [inline] slab_post_alloc_hook mm/slab.h:732 [inline] slab_alloc_node mm/slab.c:3253 [inline]
kmem_cache_alloc_node+0x2ea/0x590 mm/slab.c:3591
__alloc_skb+0x215/0x340 net/core/skbuff.c:414 alloc_skb include/linux/skbuff.h:1158 [inline]
alloc_skb_with_frags+0x93/0x620 net/core/skbuff.c:5956
sock_alloc_send_pskb+0x793/0x920 net/core/sock.c:2586
unix_dgram_sendmsg+0x414/0x1a10 net/unix/af_unix.c:1896 sock_sendmsg_nosec net/socket.c:705 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:725
__sys_sendto+0x21c/0x320 net/socket.c:2040 __do_sys_sendto net/socket.c:2052 [inline] __se_sys_sendto net/socket.c:2048 [inline]
__x64_sys_sendto+0xdd/0x1b0 net/socket.c:2048
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
Memory state around the buggy address:
ffff88802b933f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88802b934000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88802b934080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88802b934100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88802b934180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ==================================================================
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 11:17 ` Zhang, Qiang1
@ 2022-02-23 11:23 ` Pavel Skripkin
2022-02-23 11:27 ` Pavel Skripkin
2022-02-23 11:29 ` gregkh
1 sibling, 1 reply; 26+ messages in thread
From: Pavel Skripkin @ 2022-02-23 11:23 UTC (permalink / raw)
To: Zhang, Qiang1, syzbot, gregkh, linux-kernel, rafael,
syzkaller-bugs, balbi, stern
Hi Qiang1,
On 2/23/22 14:17, Zhang, Qiang1 wrote:
>
> Cc: Alan Stern
> Felipe Balbi
>
> Hello syzbot, Please try it:
>
> From 574d45ff924e2d2f9b9f5cc3e846f8004498a811 Mon Sep 17 00:00:00 2001
> From: Zqiang <qiang1.zhang@intel.com>
> Date: Wed, 23 Feb 2022 18:18:22 +0800
> Subject: [PATCH] driver core: Fix use-after-free in dev_uevent()
>
> In dev_uevent(), if the "dev->driver" is valid, the "dev->driver->name"
> be accessed, there may be a window period between these two operations.
> in this window period if the "dev->driver" is set to null
> (in usb_gadget_unregister_driver function), when the "dev->driver->name"
> is accessed again, invalid address will be accessed. fix it by checking
> "dev->driver" again.
>
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---
> drivers/base/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3d6430eb0c6a..a45b927ee76e 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2317,7 +2317,7 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
> add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
>
> if (dev->driver)
> - add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> + add_uevent_var(env, "DRIVER=%s", dev_driver_string(dev));
>
> /* Add common DT information about the device */
> of_device_uevent(dev, env);
> --
> 2.25.1
>
you should use '#syz test' command to ask syzbot to test the patch.
Basic syntax is '#syz test: <git tree> <branch or sha>' and syzbot will
apply attached patch (if you have attached it)
More about syzbot interactions here [1].
[1]
https://github.com/google/syzkaller/blob/15439f1624735bde5ae3f3b66c1b964a98
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 11:23 ` Pavel Skripkin
@ 2022-02-23 11:27 ` Pavel Skripkin
0 siblings, 0 replies; 26+ messages in thread
From: Pavel Skripkin @ 2022-02-23 11:27 UTC (permalink / raw)
To: Zhang, Qiang1, syzbot, gregkh, linux-kernel, rafael,
syzkaller-bugs, balbi, stern
On 2/23/22 14:23, Pavel Skripkin wrote:
> you should use '#syz test' command to ask syzbot to test the patch.
> Basic syntax is '#syz test: <git tree> <branch or sha>' and syzbot will
> apply attached patch (if you have attached it)
>
>
> More about syzbot interactions here [1].
>
> [1]
> https://github.com/google/syzkaller/blob/15439f1624735bde5ae3f3b66c1b964a98
^^^^^^
I've copy-pasted something weird... Sorry about that. Here is actual link
https://github.com/google/syzkaller/blob/master/docs/syzbot.md
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 11:17 ` Zhang, Qiang1
2022-02-23 11:23 ` Pavel Skripkin
@ 2022-02-23 11:29 ` gregkh
2022-02-23 14:38 ` stern
2022-03-05 18:58 ` stern
1 sibling, 2 replies; 26+ messages in thread
From: gregkh @ 2022-02-23 11:29 UTC (permalink / raw)
To: Zhang, Qiang1; +Cc: syzbot, linux-kernel, rafael, syzkaller-bugs, balbi, stern
On Wed, Feb 23, 2022 at 11:17:07AM +0000, Zhang, Qiang1 wrote:
>
> syzbot has found a reproducer for the following issue on:
>
> HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
> kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
> dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by task udevd/3689
>
> CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
> dev_uevent+0x712/0x780 drivers/base/core.c:2320
> uevent_show+0x1b8/0x380 drivers/base/core.c:2391
> dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
> sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
> seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
> kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241 call_read_iter include/linux/fs.h:2068 [inline]
> new_sync_read+0x429/0x6e0 fs/read_write.c:400
> vfs_read+0x35c/0x600 fs/read_write.c:481
> ksys_read+0x12d/0x250 fs/read_write.c:619
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7f964cc558fe
> Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
> RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
> RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
> RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
> R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
> R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68 </TASK>
>
> Cc: Alan Stern
> Felipe Balbi
>
> Hello syzbot, Please try it:
>
> From 574d45ff924e2d2f9b9f5cc3e846f8004498a811 Mon Sep 17 00:00:00 2001
> From: Zqiang <qiang1.zhang@intel.com>
> Date: Wed, 23 Feb 2022 18:18:22 +0800
> Subject: [PATCH] driver core: Fix use-after-free in dev_uevent()
>
> In dev_uevent(), if the "dev->driver" is valid, the "dev->driver->name"
> be accessed, there may be a window period between these two operations.
There should not be any such window period. The bus locks should
prevent this, unless some driver is doing odd things with the pointers
that it should not be doing.
> in this window period if the "dev->driver" is set to null
> (in usb_gadget_unregister_driver function), when the "dev->driver->name"
> is accessed again, invalid address will be accessed. fix it by checking
> "dev->driver" again.
>
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---
> drivers/base/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3d6430eb0c6a..a45b927ee76e 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2317,7 +2317,7 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
> add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
>
> if (dev->driver)
> - add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> + add_uevent_var(env, "DRIVER=%s", dev_driver_string(dev));
What's to prevent the "window" from happening in the middle of the
dev_driver_string() call?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 11:29 ` gregkh
@ 2022-02-23 14:38 ` stern
2022-02-23 16:00 ` gregkh
2022-03-05 18:58 ` stern
1 sibling, 1 reply; 26+ messages in thread
From: stern @ 2022-02-23 14:38 UTC (permalink / raw)
To: gregkh; +Cc: Zhang, Qiang1, syzbot, linux-kernel, rafael, syzkaller-bugs, balbi
On Wed, Feb 23, 2022 at 12:29:03PM +0100, gregkh@linuxfoundation.org wrote:
> On Wed, Feb 23, 2022 at 11:17:07AM +0000, Zhang, Qiang1 wrote:
> >
> > syzbot has found a reproducer for the following issue on:
> >
> > HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
> > dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
> > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by task udevd/3689
> >
> > CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
> > <TASK>
> > __dump_stack lib/dump_stack.c:88 [inline]
> > dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> > print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
> > dev_uevent+0x712/0x780 drivers/base/core.c:2320
> > uevent_show+0x1b8/0x380 drivers/base/core.c:2391
> > dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
> > sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
> > seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
> > kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241 call_read_iter include/linux/fs.h:2068 [inline]
> > new_sync_read+0x429/0x6e0 fs/read_write.c:400
> > vfs_read+0x35c/0x600 fs/read_write.c:481
> > ksys_read+0x12d/0x250 fs/read_write.c:619
> > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
> > RIP: 0033:0x7f964cc558fe
> > Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
> > RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> > RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
> > RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
> > RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
> > R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
> > R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68 </TASK>
> >
> > Cc: Alan Stern
> > Felipe Balbi
> >
> > Hello syzbot, Please try it:
> >
> > From 574d45ff924e2d2f9b9f5cc3e846f8004498a811 Mon Sep 17 00:00:00 2001
> > From: Zqiang <qiang1.zhang@intel.com>
> > Date: Wed, 23 Feb 2022 18:18:22 +0800
> > Subject: [PATCH] driver core: Fix use-after-free in dev_uevent()
> >
> > In dev_uevent(), if the "dev->driver" is valid, the "dev->driver->name"
> > be accessed, there may be a window period between these two operations.
>
> There should not be any such window period. The bus locks should
> prevent this, unless some driver is doing odd things with the pointers
> that it should not be doing.
Which bus locks are you referring to? I'm not aware of any locks that
synchronize dev_uevent() with anything (in particular, with driver
unbinding).
And as far as I know, usb_gadget_remove_driver() doesn't play any odd
tricks with pointers.
> > in this window period if the "dev->driver" is set to null
> > (in usb_gadget_unregister_driver function), when the "dev->driver->name"
> > is accessed again, invalid address will be accessed. fix it by checking
> > "dev->driver" again.
> >
> > Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> > ---
> > drivers/base/core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 3d6430eb0c6a..a45b927ee76e 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -2317,7 +2317,7 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
> > add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
> >
> > if (dev->driver)
> > - add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> > + add_uevent_var(env, "DRIVER=%s", dev_driver_string(dev));
>
> What's to prevent the "window" from happening in the middle of the
> dev_driver_string() call?
Nothing prevents it. But if you read the code for dev_driver_string(),
you will see that it doesn't matter if dev->driver gets set to NULL
while the routine is running.
(Of course, there is still the possibility that the driver structure
itself might get deallocated while dev_driver_string() is running.
This whole area could perhaps use a little careful thought. Driver
unbinding might be a good application for SRCU.)
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 14:38 ` stern
@ 2022-02-23 16:00 ` gregkh
2022-02-23 16:34 ` stern
0 siblings, 1 reply; 26+ messages in thread
From: gregkh @ 2022-02-23 16:00 UTC (permalink / raw)
To: stern; +Cc: Zhang, Qiang1, syzbot, linux-kernel, rafael, syzkaller-bugs, balbi
On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote:
> On Wed, Feb 23, 2022 at 12:29:03PM +0100, gregkh@linuxfoundation.org wrote:
> > On Wed, Feb 23, 2022 at 11:17:07AM +0000, Zhang, Qiang1 wrote:
> > >
> > > syzbot has found a reproducer for the following issue on:
> > >
> > > HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
> > > git tree: upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
> > > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
> > >
> > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
> > >
> > > ==================================================================
> > > BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by task udevd/3689
> > >
> > > CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
> > > <TASK>
> > > __dump_stack lib/dump_stack.c:88 [inline]
> > > dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> > > print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
> > > dev_uevent+0x712/0x780 drivers/base/core.c:2320
> > > uevent_show+0x1b8/0x380 drivers/base/core.c:2391
> > > dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
> > > sysfs_kf_seq_show+0x219/0x3d0 fs/sysfs/file.c:59
> > > seq_read_iter+0x4f5/0x1280 fs/seq_file.c:230
> > > kernfs_fop_read_iter+0x514/0x6f0 fs/kernfs/file.c:241 call_read_iter include/linux/fs.h:2068 [inline]
> > > new_sync_read+0x429/0x6e0 fs/read_write.c:400
> > > vfs_read+0x35c/0x600 fs/read_write.c:481
> > > ksys_read+0x12d/0x250 fs/read_write.c:619
> > > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > > do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae
> > > RIP: 0033:0x7f964cc558fe
> > > Code: c0 e9 e6 fe ff ff 50 48 8d 3d 0e c7 09 00 e8 c9 cf 01 00 66 0f 1f 84 00 00 00 00 00 64 8b 04 25 18 00 00 00 85 c0 75 14 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 28
> > > RSP: 002b:00007ffc0133d258 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> > > RAX: ffffffffffffffda RBX: 000056497b21a140 RCX: 00007f964cc558fe
> > > RDX: 0000000000001000 RSI: 000056497b218650 RDI: 0000000000000008
> > > RBP: 00007f964cd22380 R08: 0000000000000008 R09: 00007f964cd25a60
> > > R10: 0000000000000008 R11: 0000000000000246 R12: 000056497b21a140
> > > R13: 0000000000000d68 R14: 00007f964cd21780 R15: 0000000000000d68 </TASK>
> > >
> > > Cc: Alan Stern
> > > Felipe Balbi
> > >
> > > Hello syzbot, Please try it:
> > >
> > > From 574d45ff924e2d2f9b9f5cc3e846f8004498a811 Mon Sep 17 00:00:00 2001
> > > From: Zqiang <qiang1.zhang@intel.com>
> > > Date: Wed, 23 Feb 2022 18:18:22 +0800
> > > Subject: [PATCH] driver core: Fix use-after-free in dev_uevent()
> > >
> > > In dev_uevent(), if the "dev->driver" is valid, the "dev->driver->name"
> > > be accessed, there may be a window period between these two operations.
> >
> > There should not be any such window period. The bus locks should
> > prevent this, unless some driver is doing odd things with the pointers
> > that it should not be doing.
>
> Which bus locks are you referring to? I'm not aware of any locks that
> synchronize dev_uevent() with anything (in particular, with driver
> unbinding).
The locks in the driver core that handle the binding and unbinding of
drivers to devices.
> And as far as I know, usb_gadget_remove_driver() doesn't play any odd
> tricks with pointers.
Ah, I never noticed that this is doing a "fake" bus and does the
bind/unbind itself outside of the driver core. It should just be a
normal bus type and have the core do the work for it, but oh well.
And there is a lock that should serialize all of this already, so it's
odd that this is able to be triggered at all.
Unless the device is being removed at the same time it was manually
unbound from the driver? If so, then this really should be fixed up to
use the driver core logic instead...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 16:00 ` gregkh
@ 2022-02-23 16:34 ` stern
2022-02-24 1:44 ` Zhang, Qiang1
0 siblings, 1 reply; 26+ messages in thread
From: stern @ 2022-02-23 16:34 UTC (permalink / raw)
To: gregkh; +Cc: Zhang, Qiang1, syzbot, linux-kernel, rafael, syzkaller-bugs, balbi
On Wed, Feb 23, 2022 at 05:00:12PM +0100, gregkh@linuxfoundation.org wrote:
> On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote:
> > Which bus locks are you referring to? I'm not aware of any locks that
> > synchronize dev_uevent() with anything (in particular, with driver
> > unbinding).
>
> The locks in the driver core that handle the binding and unbinding of
> drivers to devices.
>
> > And as far as I know, usb_gadget_remove_driver() doesn't play any odd
> > tricks with pointers.
>
> Ah, I never noticed that this is doing a "fake" bus and does the
> bind/unbind itself outside of the driver core. It should just be a
> normal bus type and have the core do the work for it, but oh well.
>
> And there is a lock that should serialize all of this already, so it's
> odd that this is able to be triggered at all.
I guess at a minimum the UDC core should hold the device lock when it
registers, unregisters, binds, or unbinds UDC and gadget devices.
Would that be enough to fix the problem? I really don't understand how
sysfs file access gets synchronized with device removal.
> Unless the device is being removed at the same time it was manually
> unbound from the driver? If so, then this really should be fixed up to
> use the driver core logic instead...
Device removal does of course trigger unbinding, but they always take
place in the same thread so it isn't an issue.
Probably part of the reason people don't want to use the driver core
here is so that they can specify which UDC a gadget driver should bind
to. The driver core would always bind each new gadget to the first
registered gadget driver.
When Dave Brownell originally wrote the gadget subsystem, I believe he
didn't bother to integrate it with the driver core because it was a
"bus" with only a single device and a single driver. The ability to
have multiple UDCs in the system was added later.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 16:34 ` stern
@ 2022-02-24 1:44 ` Zhang, Qiang1
2022-02-24 3:14 ` Zhang, Qiang1
0 siblings, 1 reply; 26+ messages in thread
From: Zhang, Qiang1 @ 2022-02-24 1:44 UTC (permalink / raw)
To: stern, gregkh; +Cc: syzbot, linux-kernel, rafael, syzkaller-bugs, balbi
On Wed, Feb 23, 2022 at 05:00:12PM +0100, gregkh@linuxfoundation.org wrote:
> On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote:
> > Which bus locks are you referring to? I'm not aware of any locks
> > that synchronize dev_uevent() with anything (in particular, with
> > driver unbinding).
>
> The locks in the driver core that handle the binding and unbinding of
> drivers to devices.
>
> > And as far as I know, usb_gadget_remove_driver() doesn't play any
> > odd tricks with pointers.
>
> Ah, I never noticed that this is doing a "fake" bus and does the
> bind/unbind itself outside of the driver core. It should just be a
> normal bus type and have the core do the work for it, but oh well.
>
> And there is a lock that should serialize all of this already, so it's
> odd that this is able to be triggered at all.
>>I guess at a minimum the UDC core should hold the device lock when it registers, unregisters, binds, or unbinds UDC and gadget devices.
>>Would that be enough to fix the problem? I really don't understand how sysfs file access gets synchronized with device removal.
Agree with you, in usb_gadget_remove_driver() function, the udc->dev.driver and udc->gadget->dev.driver be set to null
without any protection, so when the udevd accessed the dev->driver, this address may be invalid at this time.
maybe the operation of dev->driver can be protected by device_lock().
Thanks,
Zqiang
> Unless the device is being removed at the same time it was manually
> unbound from the driver? If so, then this really should be fixed up
> to use the driver core logic instead...
>>
>>Device removal does of course trigger unbinding, but they always take place in the same thread so it isn't an issue.
>>
>>Probably part of the reason people don't want to use the driver core here is so that they can specify which UDC a gadget driver should bind to. The driver core would always bind each new gadget to the first registered gadget driver.
>>
>>When Dave Brownell originally wrote the gadget subsystem, I believe he didn't bother to integrate it with the driver core because it was a "bus" with only a single device and a single driver. The ability to have multiple UDCs in the system was added later.
>>
>>Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-24 1:44 ` Zhang, Qiang1
@ 2022-02-24 3:14 ` Zhang, Qiang1
2022-02-24 21:23 ` stern
0 siblings, 1 reply; 26+ messages in thread
From: Zhang, Qiang1 @ 2022-02-24 3:14 UTC (permalink / raw)
To: stern, gregkh; +Cc: syzbot, linux-kernel, rafael, syzkaller-bugs, balbi
On Wed, Feb 23, 2022 at 05:00:12PM +0100, gregkh@linuxfoundation.org wrote:
> On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote:
> > Which bus locks are you referring to? I'm not aware of any locks
> > that synchronize dev_uevent() with anything (in particular, with
> > driver unbinding).
>
> The locks in the driver core that handle the binding and unbinding of
> drivers to devices.
>
> > And as far as I know, usb_gadget_remove_driver() doesn't play any
> > odd tricks with pointers.
>
> Ah, I never noticed that this is doing a "fake" bus and does the
> bind/unbind itself outside of the driver core. It should just be a
> normal bus type and have the core do the work for it, but oh well.
>
> And there is a lock that should serialize all of this already, so it's
> odd that this is able to be triggered at all.
>>I guess at a minimum the UDC core should hold the device lock when it registers, unregisters, binds, or unbinds UDC and gadget devices.
>>Would that be enough to fix the problem? I really don't understand how sysfs file access gets synchronized with device removal.
>>>
>>>Agree with you, in usb_gadget_remove_driver() function, the udc->dev.driver and udc->gadget->dev.driver be set to null without any protection, so when the udevd accessed the dev->driver, this address may be invalid at this time.
>>>maybe the operation of dev->driver can be protected by device_lock().
>>>
Is it enough that we just need to protect "dev.driver" ?
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3d6430eb0c6a..9bd2624973d7 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2316,8 +2316,10 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
if (dev->type && dev->type->name)
add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
+ device_lock(dev);
if (dev->driver)
add_uevent_var(env, "DRIVER=%s", dev->driver->name);
+ device_unlock(dev);
/* Add common DT information about the device */
of_device_uevent(dev, env);
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 568534a0d17c..7877142397d3 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1436,8 +1436,14 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
usb_gadget_udc_stop(udc);
udc->driver = NULL;
+
+ device_lock(&udc->dev);
udc->dev.driver = NULL;
+ device_unlock(&udc->dev);
+
+ device_lock(&udc->gadget->dev);
udc->gadget->dev.driver = NULL;
+ device_unlock(&udc->gadget->dev);
}
/**
@@ -1498,8 +1504,14 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
driver->function);
udc->driver = driver;
+
+ device_lock(&udc->dev);
udc->dev.driver = &driver->driver;
+ device_unlock(&udc->dev);
+
+ device_lock(&udc->gadget->dev);
udc->gadget->dev.driver = &driver->driver;
+ device_unlock(&udc->gadget->dev);
usb_gadget_udc_set_speed(udc, driver->max_speed);
@@ -1521,8 +1533,14 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
dev_err(&udc->dev, "failed to start %s: %d\n",
udc->driver->function, ret);
udc->driver = NULL;
+
+ device_lock(&udc->dev);
udc->dev.driver = NULL;
+ device_unlock(&udc->dev);
+
+ device_lock(&udc->gadget->dev);
udc->gadget->dev.driver = NULL;
+ device_unlock(&udc->gadget->dev);
return ret;
}
Thanks,
Zqiang
>>>Thanks,
>>>Zqiang
> Unless the device is being removed at the same time it was manually
> unbound from the driver? If so, then this really should be fixed up
> to use the driver core logic instead...
>>
>>Device removal does of course trigger unbinding, but they always take place in the same thread so it isn't an issue.
>>
>>Probably part of the reason people don't want to use the driver core here is so that they can specify which UDC a gadget driver should bind to. The driver core would always bind each new gadget to the first registered gadget driver.
>>
>>When Dave Brownell originally wrote the gadget subsystem, I believe he didn't bother to integrate it with the driver core because it was a "bus" with only a single device and a single driver. The ability to have multiple UDCs in the system was added later.
>>
>>Alan Stern
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-24 3:14 ` Zhang, Qiang1
@ 2022-02-24 21:23 ` stern
2022-02-24 22:37 ` gregkh
2022-02-25 1:45 ` Zhang, Qiang1
0 siblings, 2 replies; 26+ messages in thread
From: stern @ 2022-02-24 21:23 UTC (permalink / raw)
To: Zhang, Qiang1, Tejun Heo
Cc: gregkh, syzbot, linux-kernel, syzkaller-bugs, balbi
On Thu, Feb 24, 2022 at 03:14:54AM +0000, Zhang, Qiang1 wrote:
>
> On Wed, Feb 23, 2022 at 05:00:12PM +0100, gregkh@linuxfoundation.org wrote:
> > On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote:
> > > Which bus locks are you referring to? I'm not aware of any locks
> > > that synchronize dev_uevent() with anything (in particular, with
> > > driver unbinding).
> >
> > The locks in the driver core that handle the binding and unbinding of
> > drivers to devices.
> >
> > > And as far as I know, usb_gadget_remove_driver() doesn't play any
> > > odd tricks with pointers.
> >
> > Ah, I never noticed that this is doing a "fake" bus and does the
> > bind/unbind itself outside of the driver core. It should just be a
> > normal bus type and have the core do the work for it, but oh well.
> >
> > And there is a lock that should serialize all of this already, so it's
> > odd that this is able to be triggered at all.
>
> >>I guess at a minimum the UDC core should hold the device lock when it registers, unregisters, binds, or unbinds UDC and gadget devices.
> >>Would that be enough to fix the problem? I really don't understand how sysfs file access gets synchronized with device removal.
>
> >>>
> >>>Agree with you, in usb_gadget_remove_driver() function, the udc->dev.driver and udc->gadget->dev.driver be set to null without any protection, so when the udevd accessed the dev->driver, this address may be invalid at this time.
> >>>maybe the operation of dev->driver can be protected by device_lock().
> >>>
>
> Is it enough that we just need to protect "dev.driver" ?
I don't know, although I doubt it. The right way to fix it is to make
sure that the existing protections, which apply to drivers that are
registered in the driver core, can also work properly with gadgets. But
I don't know what those protections are or how they work.
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3d6430eb0c6a..9bd2624973d7 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2316,8 +2316,10 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
> if (dev->type && dev->type->name)
> add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
>
> + device_lock(dev);
> if (dev->driver)
> add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> + device_unlock(dev);
You probably should not do this. Unless there's a serious bug, the
driver core already takes all the locks it needs. Doing this might
cause a deadlock (because the caller may already hold the device lock).
>
> /* Add common DT information about the device */
> of_device_uevent(dev, env);
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 568534a0d17c..7877142397d3 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -1436,8 +1436,14 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
> usb_gadget_udc_stop(udc);
>
> udc->driver = NULL;
> +
> + device_lock(&udc->dev);
> udc->dev.driver = NULL;
> + device_unlock(&udc->dev);
> +
> + device_lock(&udc->gadget->dev);
> udc->gadget->dev.driver = NULL;
> + device_unlock(&udc->gadget->dev);
> }
These are reasonable things to do, but I don't know if they will fix the
problem.
We really should ask advice from somebody who understands how this stuff
is supposed to work. I'm not sure who to ask, though. Tejun Heo,
perhaps (CC'ed).
Tejun: The USB Gadget core binds and unbinds drivers without using the
normal driver core facilities (see the code in
usb_gadget_remove_driver() above). As a result, unbinding races with
uevent generation, which can lead to a NULL pointer dereference as found
by syzbot testing. In particular, dev->driver can become NULL between
the times when dev_uevent() tests it and uses it (see above).
Can you tell us how this should be fixed?
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-24 21:23 ` stern
@ 2022-02-24 22:37 ` gregkh
2022-02-25 2:06 ` stern
2022-02-25 1:45 ` Zhang, Qiang1
1 sibling, 1 reply; 26+ messages in thread
From: gregkh @ 2022-02-24 22:37 UTC (permalink / raw)
To: stern
Cc: Zhang, Qiang1, Tejun Heo, syzbot, linux-kernel, syzkaller-bugs, balbi
On Thu, Feb 24, 2022 at 04:23:26PM -0500, stern@rowland.harvard.edu wrote:
> On Thu, Feb 24, 2022 at 03:14:54AM +0000, Zhang, Qiang1 wrote:
> >
> > On Wed, Feb 23, 2022 at 05:00:12PM +0100, gregkh@linuxfoundation.org wrote:
> > > On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote:
> > > > Which bus locks are you referring to? I'm not aware of any locks
> > > > that synchronize dev_uevent() with anything (in particular, with
> > > > driver unbinding).
> > >
> > > The locks in the driver core that handle the binding and unbinding of
> > > drivers to devices.
> > >
> > > > And as far as I know, usb_gadget_remove_driver() doesn't play any
> > > > odd tricks with pointers.
> > >
> > > Ah, I never noticed that this is doing a "fake" bus and does the
> > > bind/unbind itself outside of the driver core. It should just be a
> > > normal bus type and have the core do the work for it, but oh well.
> > >
> > > And there is a lock that should serialize all of this already, so it's
> > > odd that this is able to be triggered at all.
> >
> > >>I guess at a minimum the UDC core should hold the device lock when it registers, unregisters, binds, or unbinds UDC and gadget devices.
> > >>Would that be enough to fix the problem? I really don't understand how sysfs file access gets synchronized with device removal.
> >
> > >>>
> > >>>Agree with you, in usb_gadget_remove_driver() function, the udc->dev.driver and udc->gadget->dev.driver be set to null without any protection, so when the udevd accessed the dev->driver, this address may be invalid at this time.
> > >>>maybe the operation of dev->driver can be protected by device_lock().
> > >>>
> >
> > Is it enough that we just need to protect "dev.driver" ?
>
> I don't know, although I doubt it. The right way to fix it is to make
> sure that the existing protections, which apply to drivers that are
> registered in the driver core, can also work properly with gadgets. But
> I don't know what those protections are or how they work.
>
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 3d6430eb0c6a..9bd2624973d7 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -2316,8 +2316,10 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
> > if (dev->type && dev->type->name)
> > add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
> >
> > + device_lock(dev);
> > if (dev->driver)
> > add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> > + device_unlock(dev);
>
> You probably should not do this. Unless there's a serious bug, the
> driver core already takes all the locks it needs. Doing this might
> cause a deadlock (because the caller may already hold the device lock).
>
> >
> > /* Add common DT information about the device */
> > of_device_uevent(dev, env);
> > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> > index 568534a0d17c..7877142397d3 100644
> > --- a/drivers/usb/gadget/udc/core.c
> > +++ b/drivers/usb/gadget/udc/core.c
> > @@ -1436,8 +1436,14 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
> > usb_gadget_udc_stop(udc);
> >
> > udc->driver = NULL;
> > +
> > + device_lock(&udc->dev);
> > udc->dev.driver = NULL;
> > + device_unlock(&udc->dev);
> > +
> > + device_lock(&udc->gadget->dev);
> > udc->gadget->dev.driver = NULL;
> > + device_unlock(&udc->gadget->dev);
> > }
>
> These are reasonable things to do, but I don't know if they will fix the
> problem.
>
> We really should ask advice from somebody who understands how this stuff
> is supposed to work. I'm not sure who to ask, though. Tejun Heo,
> perhaps (CC'ed).
>
> Tejun: The USB Gadget core binds and unbinds drivers without using the
> normal driver core facilities (see the code in
> usb_gadget_remove_driver() above). As a result, unbinding races with
> uevent generation, which can lead to a NULL pointer dereference as found
> by syzbot testing. In particular, dev->driver can become NULL between
> the times when dev_uevent() tests it and uses it (see above).
>
> Can you tell us how this should be fixed?
It should be fixed by properly using the driver core to bind/unbind the
driver to devices like I mentioned previously :)
That will be more work, but it's the correct fix here. Otherwise it
needs to take the same bus locks that the device lives on to keep things
in sync, like the driver core would do if it were managing these things.
That could be the "short term" fix if no one wants to do the real work
needed here. Nothing should be needed to change in the driver core
itself, it is rightfully thinking it owns the device and can free it
when needed.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-24 21:23 ` stern
2022-02-24 22:37 ` gregkh
@ 2022-02-25 1:45 ` Zhang, Qiang1
1 sibling, 0 replies; 26+ messages in thread
From: Zhang, Qiang1 @ 2022-02-25 1:45 UTC (permalink / raw)
To: stern, Tejun Heo; +Cc: gregkh, syzbot, linux-kernel, syzkaller-bugs, balbi
On Thu, Feb 24, 2022 at 03:14:54AM +0000, Zhang, Qiang1 wrote:
>
> On Wed, Feb 23, 2022 at 05:00:12PM +0100, gregkh@linuxfoundation.org wrote:
> > On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote:
> > > Which bus locks are you referring to? I'm not aware of any locks
> > > that synchronize dev_uevent() with anything (in particular, with
> > > driver unbinding).
> >
> > The locks in the driver core that handle the binding and unbinding
> > of drivers to devices.
> >
> > > And as far as I know, usb_gadget_remove_driver() doesn't play any
> > > odd tricks with pointers.
> >
> > Ah, I never noticed that this is doing a "fake" bus and does the
> > bind/unbind itself outside of the driver core. It should just be a
> > normal bus type and have the core do the work for it, but oh well.
> >
> > And there is a lock that should serialize all of this already, so
> > it's odd that this is able to be triggered at all.
>
> >>I guess at a minimum the UDC core should hold the device lock when it registers, unregisters, binds, or unbinds UDC and gadget devices.
> >>Would that be enough to fix the problem? I really don't understand how sysfs file access gets synchronized with device removal.
>
> >>>
> >>>Agree with you, in usb_gadget_remove_driver() function, the udc->dev.driver and udc->gadget->dev.driver be set to null without any protection, so when the udevd accessed the dev->driver, this address may be invalid at this time.
> >>>maybe the operation of dev->driver can be protected by device_lock().
> >>>
>
> Is it enough that we just need to protect "dev.driver" ?
I don't know, although I doubt it. The right way to fix it is to make sure that the existing protections, which apply to drivers that are registered in the driver core, can also work properly with gadgets. But I don't know what those protections are or how they work.
> diff --git a/drivers/base/core.c b/drivers/base/core.c index
> 3d6430eb0c6a..9bd2624973d7 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2316,8 +2316,10 @@ static int dev_uevent(struct kobject *kobj, struct kobj_uevent_env *env)
> if (dev->type && dev->type->name)
> add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
>
> + device_lock(dev);
> if (dev->driver)
> add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> + device_unlock(dev);
>>You probably should not do this. Unless there's a serious bug, the driver core already takes all the locks it needs. Doing this might cause a deadlock (because the caller may already hold the device lock).
Sorry, yes it causes recursive locks.
>
> /* Add common DT information about the device */
> of_device_uevent(dev, env);
> diff --git a/drivers/usb/gadget/udc/core.c
> b/drivers/usb/gadget/udc/core.c index 568534a0d17c..7877142397d3
> 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -1436,8 +1436,14 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
> usb_gadget_udc_stop(udc);
>
> udc->driver = NULL;
> +
> + device_lock(&udc->dev);
> udc->dev.driver = NULL;
> + device_unlock(&udc->dev);
> +
> + device_lock(&udc->gadget->dev);
> udc->gadget->dev.driver = NULL;
> + device_unlock(&udc->gadget->dev);
> }
>>These are reasonable things to do, but I don't know if they will fix the problem.
>>
>>We really should ask advice from somebody who understands how this stuff is supposed to work. I'm not sure who to ask, though. Tejun Heo, perhaps (CC'ed).
>>
>>Tejun: The USB Gadget core binds and unbinds drivers without using the normal driver core facilities (see the code in
>>usb_gadget_remove_driver() above). As a result, unbinding races with uevent generation, which can lead to a NULL pointer dereference as found by syzbot testing. In particular, dev->driver can become NULL between the times when dev_uevent() tests it and uses it (see above).
CPU0 (task 4316) CPU1 (udevd task 3689)
usb_gadget_remove_driver() dev_uevent()
........... if (dev->driver)
udc->dev.driver = NULL; add_uevent_var(env, "DRIVER=%s", dev->driver->name)
udc->gadget->dev.driver = NULL;
Thanks,
Zqiang
>>
>>Can you tell us how this should be fixed?
>>
>>Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-24 22:37 ` gregkh
@ 2022-02-25 2:06 ` stern
2022-02-25 8:53 ` gregkh
0 siblings, 1 reply; 26+ messages in thread
From: stern @ 2022-02-25 2:06 UTC (permalink / raw)
To: gregkh
Cc: Zhang, Qiang1, Tejun Heo, syzbot, linux-kernel, syzkaller-bugs, balbi
On Thu, Feb 24, 2022 at 11:37:39PM +0100, gregkh@linuxfoundation.org wrote:
> On Thu, Feb 24, 2022 at 04:23:26PM -0500, stern@rowland.harvard.edu wrote:
> > Can you tell us how this should be fixed?
>
> It should be fixed by properly using the driver core to bind/unbind the
> driver to devices like I mentioned previously :)
This would involve creating a "gadget" bus_type (or should it be a
device_type under the platform bus?) and registering the gadgets
on it, right?. Similarly, the gadget drivers would be registered on
this bus. I suppose we can control which drivers get bound to which
gadgets with careful matching code.
> That will be more work, but it's the correct fix here. Otherwise it
> needs to take the same bus locks that the device lives on to keep things
> in sync, like the driver core would do if it were managing these things.
> That could be the "short term" fix if no one wants to do the real work
> needed here. Nothing should be needed to change in the driver core
> itself, it is rightfully thinking it owns the device and can free it
> when needed.
All right, thanks. I'll think about implementing it.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-25 2:06 ` stern
@ 2022-02-25 8:53 ` gregkh
2022-02-25 15:51 ` stern
0 siblings, 1 reply; 26+ messages in thread
From: gregkh @ 2022-02-25 8:53 UTC (permalink / raw)
To: stern
Cc: Zhang, Qiang1, Tejun Heo, syzbot, linux-kernel, syzkaller-bugs, balbi
On Thu, Feb 24, 2022 at 09:06:13PM -0500, stern@rowland.harvard.edu wrote:
> On Thu, Feb 24, 2022 at 11:37:39PM +0100, gregkh@linuxfoundation.org wrote:
> > On Thu, Feb 24, 2022 at 04:23:26PM -0500, stern@rowland.harvard.edu wrote:
> > > Can you tell us how this should be fixed?
> >
> > It should be fixed by properly using the driver core to bind/unbind the
> > driver to devices like I mentioned previously :)
>
> This would involve creating a "gadget" bus_type (or should it be a
> device_type under the platform bus?) and registering the gadgets
> on it, right?.
Yes. Or you can use the aux bus for this, which might be easier.
> Similarly, the gadget drivers would be registered on
> this bus. I suppose we can control which drivers get bound to which
> gadgets with careful matching code.
The aux bus might make this easier:
Documentation/driver-api/auxiliary_bus.rst
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-25 8:53 ` gregkh
@ 2022-02-25 15:51 ` stern
2022-02-26 9:07 ` gregkh
0 siblings, 1 reply; 26+ messages in thread
From: stern @ 2022-02-25 15:51 UTC (permalink / raw)
To: gregkh
Cc: Zhang, Qiang1, Tejun Heo, syzbot, linux-kernel, syzkaller-bugs, balbi
On Fri, Feb 25, 2022 at 09:53:35AM +0100, gregkh@linuxfoundation.org wrote:
> On Thu, Feb 24, 2022 at 09:06:13PM -0500, stern@rowland.harvard.edu wrote:
> > On Thu, Feb 24, 2022 at 11:37:39PM +0100, gregkh@linuxfoundation.org wrote:
> > > On Thu, Feb 24, 2022 at 04:23:26PM -0500, stern@rowland.harvard.edu wrote:
> > > > Can you tell us how this should be fixed?
> > >
> > > It should be fixed by properly using the driver core to bind/unbind the
> > > driver to devices like I mentioned previously :)
> >
> > This would involve creating a "gadget" bus_type (or should it be a
> > device_type under the platform bus?) and registering the gadgets
> > on it, right?.
>
> Yes. Or you can use the aux bus for this, which might be easier.
>
> > Similarly, the gadget drivers would be registered on
> > this bus. I suppose we can control which drivers get bound to which
> > gadgets with careful matching code.
>
> The aux bus might make this easier:
> Documentation/driver-api/auxiliary_bus.rst
Won't this end up changing the user-visible filenames and directories in
sysfs for gadgets and gadget drivers?
For instance, currently gadgets get registered under their UDC driver
name, like "net2280" or "at91". If we put them on the aux bus then they
will have to get registered under a name looking something like
"udc.gadget.0", i.e., module name, generic device name, and ID number.
We will be forced to use a generic device name because the aux bus does
matching based on it, and we want every gadget driver to be able to
match every UDC. We don't want some gadget drivers restricted to
net2280 gadgets, others restricted to fotg210 gadgets, and so on.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-25 15:51 ` stern
@ 2022-02-26 9:07 ` gregkh
2022-03-02 19:10 ` stern
0 siblings, 1 reply; 26+ messages in thread
From: gregkh @ 2022-02-26 9:07 UTC (permalink / raw)
To: stern
Cc: Zhang, Qiang1, Tejun Heo, syzbot, linux-kernel, syzkaller-bugs, balbi
On Fri, Feb 25, 2022 at 10:51:48AM -0500, stern@rowland.harvard.edu wrote:
> On Fri, Feb 25, 2022 at 09:53:35AM +0100, gregkh@linuxfoundation.org wrote:
> > On Thu, Feb 24, 2022 at 09:06:13PM -0500, stern@rowland.harvard.edu wrote:
> > > On Thu, Feb 24, 2022 at 11:37:39PM +0100, gregkh@linuxfoundation.org wrote:
> > > > On Thu, Feb 24, 2022 at 04:23:26PM -0500, stern@rowland.harvard.edu wrote:
> > > > > Can you tell us how this should be fixed?
> > > >
> > > > It should be fixed by properly using the driver core to bind/unbind the
> > > > driver to devices like I mentioned previously :)
> > >
> > > This would involve creating a "gadget" bus_type (or should it be a
> > > device_type under the platform bus?) and registering the gadgets
> > > on it, right?.
> >
> > Yes. Or you can use the aux bus for this, which might be easier.
> >
> > > Similarly, the gadget drivers would be registered on
> > > this bus. I suppose we can control which drivers get bound to which
> > > gadgets with careful matching code.
> >
> > The aux bus might make this easier:
> > Documentation/driver-api/auxiliary_bus.rst
>
> Won't this end up changing the user-visible filenames and directories in
> sysfs for gadgets and gadget drivers?
>
> For instance, currently gadgets get registered under their UDC driver
> name, like "net2280" or "at91". If we put them on the aux bus then they
> will have to get registered under a name looking something like
> "udc.gadget.0", i.e., module name, generic device name, and ID number.
Ah, yeah, that isn't good.
> We will be forced to use a generic device name because the aux bus does
> matching based on it, and we want every gadget driver to be able to
> match every UDC. We don't want some gadget drivers restricted to
> net2280 gadgets, others restricted to fotg210 gadgets, and so on.
So yes, I guess it does need to be a "real" bus, sorry.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-26 9:07 ` gregkh
@ 2022-03-02 19:10 ` stern
2022-03-02 21:36 ` gregkh
0 siblings, 1 reply; 26+ messages in thread
From: stern @ 2022-03-02 19:10 UTC (permalink / raw)
To: gregkh
Cc: Zhang, Qiang1, Tejun Heo, syzbot, linux-kernel, syzkaller-bugs, balbi
On Sat, Feb 26, 2022 at 10:07:02AM +0100, gregkh@linuxfoundation.org wrote:
> On Fri, Feb 25, 2022 at 10:51:48AM -0500, stern@rowland.harvard.edu wrote:
> > On Fri, Feb 25, 2022 at 09:53:35AM +0100, gregkh@linuxfoundation.org wrote:
> > > The aux bus might make this easier:
> > > Documentation/driver-api/auxiliary_bus.rst
> >
> > Won't this end up changing the user-visible filenames and directories in
> > sysfs for gadgets and gadget drivers?
> >
> > For instance, currently gadgets get registered under their UDC driver
> > name, like "net2280" or "at91". If we put them on the aux bus then they
> > will have to get registered under a name looking something like
> > "udc.gadget.0", i.e., module name, generic device name, and ID number.
>
> Ah, yeah, that isn't good.
>
> > We will be forced to use a generic device name because the aux bus does
> > matching based on it, and we want every gadget driver to be able to
> > match every UDC. We don't want some gadget drivers restricted to
> > net2280 gadgets, others restricted to fotg210 gadgets, and so on.
>
> So yes, I guess it does need to be a "real" bus, sorry.
It turns out not to be so bad. In fact there are only five
gadget drivers (that is, instances of struct usb_gadget_driver) in the
kernel:
composite_driver_template (gadget/composite.c)
configfs_driver_template (gadget/configfs.c)
gadgetfs_driver (gadget/legacy/inode.c)
driver (gadget/legacy/raw_gadget.c)
dbgp_driver (gadget/legacy/dbgp.c)
Everything else is implemented as a "function" driver. So the gadget
driver's name doesn't mean very much to the user anyway.
The interaction between the gadget subsystem and the device core is
rather peculiar. Each UDC controller is represented by a pair of device
structures: the .dev fields in struct usb_udc and struct usb_gadget.
These two are siblings -- they always have the same parent (see
usb_add_gadget() in gadget/udc/core.c). Furthermore, they have the same
driver; that is, udc->dev.driver is always the same as
gadget->dev.driver (see udc_bind_to_driver()). Which is doubly odd,
because gadget drivers manage only gadget devices, not udc devices. The
major difference between them is that the usb_udc is a class device
whereas the usb_gadget is a "real" device.
Currently neither the udc device nor the gadget device is registered on
any bus. IMO it would make sense to leave udc->dev.driver always set to
NULL, keep the udc devices bus-less, and put the gadget devices on the
aux bus.
Does that sound reasonable? I'll work on a patch to do it.
Alan Stern
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-03-02 19:10 ` stern
@ 2022-03-02 21:36 ` gregkh
0 siblings, 0 replies; 26+ messages in thread
From: gregkh @ 2022-03-02 21:36 UTC (permalink / raw)
To: stern
Cc: Zhang, Qiang1, Tejun Heo, syzbot, linux-kernel, syzkaller-bugs, balbi
On Wed, Mar 02, 2022 at 02:10:15PM -0500, stern@rowland.harvard.edu wrote:
> On Sat, Feb 26, 2022 at 10:07:02AM +0100, gregkh@linuxfoundation.org wrote:
> > On Fri, Feb 25, 2022 at 10:51:48AM -0500, stern@rowland.harvard.edu wrote:
> > > On Fri, Feb 25, 2022 at 09:53:35AM +0100, gregkh@linuxfoundation.org wrote:
> > > > The aux bus might make this easier:
> > > > Documentation/driver-api/auxiliary_bus.rst
> > >
> > > Won't this end up changing the user-visible filenames and directories in
> > > sysfs for gadgets and gadget drivers?
> > >
> > > For instance, currently gadgets get registered under their UDC driver
> > > name, like "net2280" or "at91". If we put them on the aux bus then they
> > > will have to get registered under a name looking something like
> > > "udc.gadget.0", i.e., module name, generic device name, and ID number.
> >
> > Ah, yeah, that isn't good.
> >
> > > We will be forced to use a generic device name because the aux bus does
> > > matching based on it, and we want every gadget driver to be able to
> > > match every UDC. We don't want some gadget drivers restricted to
> > > net2280 gadgets, others restricted to fotg210 gadgets, and so on.
> >
> > So yes, I guess it does need to be a "real" bus, sorry.
>
> It turns out not to be so bad. In fact there are only five
> gadget drivers (that is, instances of struct usb_gadget_driver) in the
> kernel:
>
> composite_driver_template (gadget/composite.c)
> configfs_driver_template (gadget/configfs.c)
> gadgetfs_driver (gadget/legacy/inode.c)
> driver (gadget/legacy/raw_gadget.c)
> dbgp_driver (gadget/legacy/dbgp.c)
I would really love to drop the gadget/legacy/ stuff if at all possible
entirely. I wonder if that's possible. I know that Android has moved
off of this, and that used to be the largest user (in the billions), so
we might be ok to possibly just delete these entirely.
> Everything else is implemented as a "function" driver. So the gadget
> driver's name doesn't mean very much to the user anyway.
That's good to know.
> The interaction between the gadget subsystem and the device core is
> rather peculiar. Each UDC controller is represented by a pair of device
> structures: the .dev fields in struct usb_udc and struct usb_gadget.
> These two are siblings -- they always have the same parent (see
> usb_add_gadget() in gadget/udc/core.c). Furthermore, they have the same
> driver; that is, udc->dev.driver is always the same as
> gadget->dev.driver (see udc_bind_to_driver()). Which is doubly odd,
> because gadget drivers manage only gadget devices, not udc devices. The
> major difference between them is that the usb_udc is a class device
> whereas the usb_gadget is a "real" device.
>
> Currently neither the udc device nor the gadget device is registered on
> any bus. IMO it would make sense to leave udc->dev.driver always set to
> NULL, keep the udc devices bus-less, and put the gadget devices on the
> aux bus.
>
> Does that sound reasonable? I'll work on a patch to do it.
That's odd, but it might work, so sure, let's see how it turns out.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-02-23 11:29 ` gregkh
2022-02-23 14:38 ` stern
@ 2022-03-05 18:58 ` stern
2022-03-05 19:15 ` syzbot
1 sibling, 1 reply; 26+ messages in thread
From: stern @ 2022-03-05 18:58 UTC (permalink / raw)
To: syzbot; +Cc: Zhang, Qiang1, syzkaller-bugs, USB mailing list
On Wed, Feb 23, 2022 at 12:29:03PM +0100, gregkh@linuxfoundation.org wrote:
> On Wed, Feb 23, 2022 at 11:17:07AM +0000, Zhang, Qiang1 wrote:
> >
> > syzbot has found a reproducer for the following issue on:
> >
> > HEAD commit: 4f12b742eb2b Merge tag 'nfs-for-5.17-3' of git://git.linux..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=110a6df2700000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
> > dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
> > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12377296700000
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320 Read of size 8 at addr ffff88802b934098 by task udevd/3689
> >
> > CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace:
> > <TASK>
> > __dump_stack lib/dump_stack.c:88 [inline]
> > dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> > print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255 __kasan_report mm/kasan/report.c:442 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
> > dev_uevent+0x712/0x780 drivers/base/core.c:2320
> > uevent_show+0x1b8/0x380 drivers/base/core.c:2391
> > dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
If the uevent file being read was for the gadget rather than the udc,
this should fix the problem.
Alan Stern
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ v5.17-rc4
Index: usb-devel/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/core.c
+++ usb-devel/drivers/usb/gadget/udc/core.c
@@ -1436,7 +1436,6 @@ static void usb_gadget_remove_driver(str
usb_gadget_udc_stop(udc);
udc->driver = NULL;
- udc->dev.driver = NULL;
udc->gadget->dev.driver = NULL;
}
@@ -1498,7 +1497,6 @@ static int udc_bind_to_driver(struct usb
driver->function);
udc->driver = driver;
- udc->dev.driver = &driver->driver;
udc->gadget->dev.driver = &driver->driver;
usb_gadget_udc_set_speed(udc, driver->max_speed);
@@ -1521,7 +1519,6 @@ err1:
dev_err(&udc->dev, "failed to start %s: %d\n",
udc->driver->function, ret);
udc->driver = NULL;
- udc->dev.driver = NULL;
udc->gadget->dev.driver = NULL;
return ret;
}
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [syzbot] KASAN: use-after-free Read in dev_uevent
2022-03-05 18:58 ` stern
@ 2022-03-05 19:15 ` syzbot
2022-03-06 2:47 ` [PATCH] usb: gadget: Fix use-after-free bug by not setting udc->dev.driver Alan Stern
0 siblings, 1 reply; 26+ messages in thread
From: syzbot @ 2022-03-05 19:15 UTC (permalink / raw)
To: linux-usb, qiang1.zhang, stern, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
Tested on:
commit: 754e0b0e Linux 5.17-rc4
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ v5.17-rc4
kernel config: https://syzkaller.appspot.com/x/.config?x=f6a069ed94a1ed1d
dashboard link: https://syzkaller.appspot.com/bug?extid=348b571beb5eeb70a582
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
patch: https://syzkaller.appspot.com/x/patch.diff?x=12191281700000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] usb: gadget: Fix use-after-free bug by not setting udc->dev.driver
2022-03-05 19:15 ` syzbot
@ 2022-03-06 2:47 ` Alan Stern
0 siblings, 0 replies; 26+ messages in thread
From: Alan Stern @ 2022-03-06 2:47 UTC (permalink / raw)
To: Greg KH, Zhang, Qiang1; +Cc: USB mailing list, syzkaller-bugs
The syzbot fuzzer found a use-after-free bug:
BUG: KASAN: use-after-free in dev_uevent+0x712/0x780 drivers/base/core.c:2320
Read of size 8 at addr ffff88802b934098 by task udevd/3689
CPU: 2 PID: 3689 Comm: udevd Not tainted 5.17.0-rc4-syzkaller-00229-g4f12b742eb2b #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description.constprop.0.cold+0x8d/0x303 mm/kasan/report.c:255
__kasan_report mm/kasan/report.c:442 [inline]
kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
dev_uevent+0x712/0x780 drivers/base/core.c:2320
uevent_show+0x1b8/0x380 drivers/base/core.c:2391
dev_attr_show+0x4b/0x90 drivers/base/core.c:2094
Although the bug manifested in the driver core, the real cause was a
race with the gadget core. dev_uevent() does:
if (dev->driver)
add_uevent_var(env, "DRIVER=%s", dev->driver->name);
and between the test and the dereference of dev->driver, the gadget
core sets dev->driver to NULL.
The race wouldn't occur if the gadget core registered its devices on
a real bus, using the standard synchronization techniques of the
driver core. However, it's not necessary to make such a large change
in order to fix this bug; all we need to do is make sure that
udc->dev.driver is always NULL.
In fact, there is no reason for udc->dev.driver ever to be set to
anything, let alone to the value it currently gets: the address of the
gadget's driver. After all, a gadget driver only knows how to manage
a gadget, not how to manage a UDC.
This patch simply removes the statements in the gadget core that touch
udc->dev.driver.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com
Fixes: 2ccea03a8f7e ("usb: gadget: introduce UDC Class")
CC: <stable@vger.kernel.org>
---
[as1974]
drivers/usb/gadget/udc/core.c | 3 ---
1 file changed, 3 deletions(-)
Index: usb-devel/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/core.c
+++ usb-devel/drivers/usb/gadget/udc/core.c
@@ -1436,7 +1436,6 @@ static void usb_gadget_remove_driver(str
usb_gadget_udc_stop(udc);
udc->driver = NULL;
- udc->dev.driver = NULL;
udc->gadget->dev.driver = NULL;
}
@@ -1498,7 +1497,6 @@ static int udc_bind_to_driver(struct usb
driver->function);
udc->driver = driver;
- udc->dev.driver = &driver->driver;
udc->gadget->dev.driver = &driver->driver;
usb_gadget_udc_set_speed(udc, driver->max_speed);
@@ -1521,7 +1519,6 @@ err1:
dev_err(&udc->dev, "failed to start %s: %d\n",
udc->driver->function, ret);
udc->driver = NULL;
- udc->dev.driver = NULL;
udc->gadget->dev.driver = NULL;
return ret;
}
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2022-03-06 2:47 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-19 5:48 KASAN: use-after-free Read in dev_uevent syzbot
2022-02-20 17:19 ` [syzbot] " syzbot
2022-02-23 6:51 ` Zhang, Qiang1
2022-02-23 7:13 ` gregkh
2022-02-23 8:08 ` Zhang, Qiang1
2022-02-23 11:17 ` Zhang, Qiang1
2022-02-23 11:23 ` Pavel Skripkin
2022-02-23 11:27 ` Pavel Skripkin
2022-02-23 11:29 ` gregkh
2022-02-23 14:38 ` stern
2022-02-23 16:00 ` gregkh
2022-02-23 16:34 ` stern
2022-02-24 1:44 ` Zhang, Qiang1
2022-02-24 3:14 ` Zhang, Qiang1
2022-02-24 21:23 ` stern
2022-02-24 22:37 ` gregkh
2022-02-25 2:06 ` stern
2022-02-25 8:53 ` gregkh
2022-02-25 15:51 ` stern
2022-02-26 9:07 ` gregkh
2022-03-02 19:10 ` stern
2022-03-02 21:36 ` gregkh
2022-02-25 1:45 ` Zhang, Qiang1
2022-03-05 18:58 ` stern
2022-03-05 19:15 ` syzbot
2022-03-06 2:47 ` [PATCH] usb: gadget: Fix use-after-free bug by not setting udc->dev.driver Alan Stern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.