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=120b26c1100000 kernel config: https://syzkaller.appspot.com/x/.config?x=d195fe572fb15312 dashboard link: https://syzkaller.appspot.com/bug?extid=7d2debdcdb3cb93c1e5e compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1724b246100000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14ceb3de100000
The bug was bisected to:
commit 76313c70c52f930af4afd21684509ca52297ea71 Author: Eric W. Biederman ebiederm@xmission.com Date: Wed Feb 19 16:37:15 2020 +0000
uml: Create a private mount of proc for mconsole
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=117c4912100000 final crash: https://syzkaller.appspot.com/x/report.txt?x=137c4912100000 console output: https://syzkaller.appspot.com/x/log.txt?x=157c4912100000
IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+7d2debdcdb3cb93c1e5e@syzkaller.appspotmail.com Fixes: 76313c70c52f ("uml: Create a private mount of proc for mconsole")
================================================================== BUG: KASAN: use-after-free in atomic64_inc include/asm-generic/atomic-instrumented.h:1049 [inline] BUG: KASAN: use-after-free in atomic_long_inc include/asm-generic/atomic-long.h:160 [inline] BUG: KASAN: use-after-free in fsnotify_detach_connector_from_object+0x25e/0x380 fs/notify/mark.c:185 Write of size 8 at addr ffff88809fd7e7c0 by task syz-executor972/8021
CPU: 1 PID: 8021 Comm: syz-executor972 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+0x188/0x20d lib/dump_stack.c:118 print_address_description.constprop.0.cold+0xd3/0x413 mm/kasan/report.c:383 __kasan_report mm/kasan/report.c:513 [inline] kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530 check_memory_region_inline mm/kasan/generic.c:186 [inline] check_memory_region+0x141/0x190 mm/kasan/generic.c:192 atomic64_inc include/asm-generic/atomic-instrumented.h:1049 [inline] atomic_long_inc include/asm-generic/atomic-long.h:160 [inline] fsnotify_detach_connector_from_object+0x25e/0x380 fs/notify/mark.c:185 fsnotify_put_mark+0x367/0x580 fs/notify/mark.c:250 fsnotify_clear_marks_by_group+0x33f/0x490 fs/notify/mark.c:764 fsnotify_destroy_group+0xc9/0x300 fs/notify/group.c:61 inotify_release+0x33/0x40 fs/notify/inotify/inotify_user.c:271 __fput+0x33e/0x880 fs/file_table.c:281 task_work_run+0xf4/0x1b0 kernel/task_work.c:123 exit_task_work include/linux/task_work.h:22 [inline] do_exit+0xb3f/0x2de0 kernel/exit.c:806 do_group_exit+0x125/0x340 kernel/exit.c:904 __do_sys_exit_group kernel/exit.c:915 [inline] __se_sys_exit_group kernel/exit.c:913 [inline] __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:913 do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295 entry_SYSCALL_64_after_hwframe+0x49/0xb3 RIP: 0033:0x445448 Code: Bad RIP value. RSP: 002b:00007ffe48521018 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000445448 RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000 RBP: 00000000004cca90 R08: 00000000000000e7 R09: ffffffffffffffd0 R10: 00007ffe48521060 R11: 0000000000000246 R12: 0000000000000001 R13: 00000000006e0340 R14: 0000000000000007 R15: 000000000000002d
Allocated by task 8026: save_stack+0x1b/0x40 mm/kasan/common.c:48 set_track mm/kasan/common.c:56 [inline] __kasan_kmalloc mm/kasan/common.c:494 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:467 kmem_cache_alloc_trace+0x153/0x7d0 mm/slab.c:3551 kmalloc include/linux/slab.h:555 [inline] kzalloc include/linux/slab.h:669 [inline] alloc_super+0x52/0x9d0 fs/super.c:203 sget_fc+0x13f/0x790 fs/super.c:530 vfs_get_super+0x6d/0x2d0 fs/super.c:1186 vfs_get_tree+0x89/0x2f0 fs/super.c:1547 do_new_mount fs/namespace.c:2874 [inline] do_mount+0x1306/0x1b40 fs/namespace.c:3199 __do_sys_mount fs/namespace.c:3409 [inline] __se_sys_mount fs/namespace.c:3386 [inline] __x64_sys_mount+0x18f/0x230 fs/namespace.c:3386 do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295 entry_SYSCALL_64_after_hwframe+0x49/0xb3
Freed by task 23: save_stack+0x1b/0x40 mm/kasan/common.c:48 set_track mm/kasan/common.c:56 [inline] kasan_set_free_info mm/kasan/common.c:316 [inline] __kasan_slab_free+0xf7/0x140 mm/kasan/common.c:455 __cache_free mm/slab.c:3426 [inline] kfree+0x109/0x2b0 mm/slab.c:3757 process_one_work+0x965/0x16a0 kernel/workqueue.c:2268 worker_thread+0x96/0xe20 kernel/workqueue.c:2414 kthread+0x388/0x470 kernel/kthread.c:268 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:351
The buggy address belongs to the object at ffff88809fd7e000 which belongs to the cache kmalloc-4k of size 4096 The buggy address is located 1984 bytes inside of 4096-byte region [ffff88809fd7e000, ffff88809fd7f000) The buggy address belongs to the page: page:ffffea00027f5f80 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 head:ffffea00027f5f80 order:1 compound_mapcount:0 flags: 0xfffe0000010200(slab|head) raw: 00fffe0000010200 ffffea000247aa88 ffffea000242ef08 ffff8880aa002000 raw: 0000000000000000 ffff88809fd7e000 0000000100000001 0000000000000000 page dumped because: kasan: bad access detected
Memory state around the buggy address: ffff88809fd7e680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff88809fd7e700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88809fd7e780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^ ffff88809fd7e800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff88809fd7e880: 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. For information about bisection process see: https://goo.gl/tpsmEJ#bisection syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
syzbot syzbot+7d2debdcdb3cb93c1e5e@syzkaller.appspotmail.com writes:
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=120b26c1100000 kernel config: https://syzkaller.appspot.com/x/.config?x=d195fe572fb15312 dashboard link: https://syzkaller.appspot.com/bug?extid=7d2debdcdb3cb93c1e5e compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1724b246100000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14ceb3de100000
The bug was bisected to:
That bisection can not be correct. The commit only added code, and the code that was added is not in any of the call traces. Further the failure on the final commit was different than the other commits in your bisection.
I will believe commit 69879c01a0c3f70e0887cfb4d9ff439814361e46 ("proc: Remove the now unnecessary internal mount of proc") is the point at which things start failing for your reproducer. That is the change that makes it possible to actually unmount proc, and for it's super block to be freed.
Now I don't know why fsnotify is holding on after a filesystem has been unmounted. At first glance this looks like a bug in inotify.
It looks like your reproducer is doing:
mkdir ./file mount -t proc ./file inotify_init() inotify_add_watch(./file, ...); umount(./file) ... exit(0); <kaboom>
Then after the exit inotify is falling over because the filesystem has already been unmounted.
Can anyone who is more familiar with inotify/fsnotify give a clue why the unmount of the filesystem is not clearing the watch?
Is it a generic bug or is there something proc is not doing?
Eric
commit 76313c70c52f930af4afd21684509ca52297ea71 Author: Eric W. Biederman ebiederm@xmission.com Date: Wed Feb 19 16:37:15 2020 +0000
uml: Create a private mount of proc for mconsolebisection log: https://syzkaller.appspot.com/x/bisect.txt?x=117c4912100000 final crash: https://syzkaller.appspot.com/x/report.txt?x=137c4912100000 console output: https://syzkaller.appspot.com/x/log.txt?x=157c4912100000
IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+7d2debdcdb3cb93c1e5e@syzkaller.appspotmail.com Fixes: 76313c70c52f ("uml: Create a private mount of proc for mconsole")
================================================================== BUG: KASAN: use-after-free in atomic64_inc include/asm-generic/atomic-instrumented.h:1049 [inline] BUG: KASAN: use-after-free in atomic_long_inc include/asm-generic/atomic-long.h:160 [inline] BUG: KASAN: use-after-free in fsnotify_detach_connector_from_object+0x25e/0x380 fs/notify/mark.c:185 Write of size 8 at addr ffff88809fd7e7c0 by task syz-executor972/8021
CPU: 1 PID: 8021 Comm: syz-executor972 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+0x188/0x20d lib/dump_stack.c:118 print_address_description.constprop.0.cold+0xd3/0x413 mm/kasan/report.c:383 __kasan_report mm/kasan/report.c:513 [inline] kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530 check_memory_region_inline mm/kasan/generic.c:186 [inline] check_memory_region+0x141/0x190 mm/kasan/generic.c:192 atomic64_inc include/asm-generic/atomic-instrumented.h:1049 [inline] atomic_long_inc include/asm-generic/atomic-long.h:160 [inline] fsnotify_detach_connector_from_object+0x25e/0x380 fs/notify/mark.c:185 fsnotify_put_mark+0x367/0x580 fs/notify/mark.c:250 fsnotify_clear_marks_by_group+0x33f/0x490 fs/notify/mark.c:764 fsnotify_destroy_group+0xc9/0x300 fs/notify/group.c:61 inotify_release+0x33/0x40 fs/notify/inotify/inotify_user.c:271 __fput+0x33e/0x880 fs/file_table.c:281 task_work_run+0xf4/0x1b0 kernel/task_work.c:123 exit_task_work include/linux/task_work.h:22 [inline] do_exit+0xb3f/0x2de0 kernel/exit.c:806 do_group_exit+0x125/0x340 kernel/exit.c:904 __do_sys_exit_group kernel/exit.c:915 [inline] __se_sys_exit_group kernel/exit.c:913 [inline] __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:913 do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295 entry_SYSCALL_64_after_hwframe+0x49/0xb3 RIP: 0033:0x445448 Code: Bad RIP value. RSP: 002b:00007ffe48521018 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000445448 RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000 RBP: 00000000004cca90 R08: 00000000000000e7 R09: ffffffffffffffd0 R10: 00007ffe48521060 R11: 0000000000000246 R12: 0000000000000001 R13: 00000000006e0340 R14: 0000000000000007 R15: 000000000000002d
Allocated by task 8026: save_stack+0x1b/0x40 mm/kasan/common.c:48 set_track mm/kasan/common.c:56 [inline] __kasan_kmalloc mm/kasan/common.c:494 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:467 kmem_cache_alloc_trace+0x153/0x7d0 mm/slab.c:3551 kmalloc include/linux/slab.h:555 [inline] kzalloc include/linux/slab.h:669 [inline] alloc_super+0x52/0x9d0 fs/super.c:203 sget_fc+0x13f/0x790 fs/super.c:530 vfs_get_super+0x6d/0x2d0 fs/super.c:1186 vfs_get_tree+0x89/0x2f0 fs/super.c:1547 do_new_mount fs/namespace.c:2874 [inline] do_mount+0x1306/0x1b40 fs/namespace.c:3199 __do_sys_mount fs/namespace.c:3409 [inline] __se_sys_mount fs/namespace.c:3386 [inline] __x64_sys_mount+0x18f/0x230 fs/namespace.c:3386 do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295 entry_SYSCALL_64_after_hwframe+0x49/0xb3
Freed by task 23: save_stack+0x1b/0x40 mm/kasan/common.c:48 set_track mm/kasan/common.c:56 [inline] kasan_set_free_info mm/kasan/common.c:316 [inline] __kasan_slab_free+0xf7/0x140 mm/kasan/common.c:455 __cache_free mm/slab.c:3426 [inline] kfree+0x109/0x2b0 mm/slab.c:3757 process_one_work+0x965/0x16a0 kernel/workqueue.c:2268 worker_thread+0x96/0xe20 kernel/workqueue.c:2414 kthread+0x388/0x470 kernel/kthread.c:268 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:351
The buggy address belongs to the object at ffff88809fd7e000 which belongs to the cache kmalloc-4k of size 4096 The buggy address is located 1984 bytes inside of 4096-byte region [ffff88809fd7e000, ffff88809fd7f000) The buggy address belongs to the page: page:ffffea00027f5f80 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 head:ffffea00027f5f80 order:1 compound_mapcount:0 flags: 0xfffe0000010200(slab|head) raw: 00fffe0000010200 ffffea000247aa88 ffffea000242ef08 ffff8880aa002000 raw: 0000000000000000 ffff88809fd7e000 0000000100000001 0000000000000000 page dumped because: kasan: bad access detected
Memory state around the buggy address: ffff88809fd7e680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff88809fd7e700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88809fd7e780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^ffff88809fd7e800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff88809fd7e880: 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. For information about bisection process see: https://goo.gl/tpsmEJ#bisection syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Recently syzbot reported that unmounting proc when there is an ongoing inotify watch on the root directory of proc could result in a use after free when the watch is removed after the unmount of proc when the watcher exits.
Commit 69879c01a0c3 ("proc: Remove the now unnecessary internal mount of proc") made it easier to unmount proc and allowed syzbot to see the problem, but looking at the code it has been around for a long time.
Looking at the code the fsnotify watch should have been removed by fsnotify_sb_delete in generic_shutdown_super. Unfortunately the inode was allocated with new_inode_pseudo instead of new_inode so the inode was not on the sb->s_inodes list. Which prevented fsnotify_unmount_inodes from finding the inode and removing the watch as well as made it so the "VFS: Busy inodes after unmount" warning could not find the inodes to warn about them.
Make all of the inodes in proc visible to generic_shutdown_super, and fsnotify_sb_delete by using new_inode instead of new_inode_pseudo. The only functional difference is that new_inode places the inodes on the sb->s_inodes list.
I wrote a small test program and I can verify that without changes it can trigger this issue, and by replacing new_inode_pseudo with new_inode the issues goes away.
Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/000000000000d788c905a7dfa3f4@google.com Reported-by: syzbot+7d2debdcdb3cb93c1e5e@syzkaller.appspotmail.com Fixes: 0097875bd415 ("proc: Implement /proc/thread-self to point at the directory of the current thread") Fixes: 021ada7dff22 ("procfs: switch /proc/self away from proc_dir_entry") Fixes: 51f0885e5415 ("vfs,proc: guarantee unique inodes in /proc") Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com --- fs/proc/inode.c | 2 +- fs/proc/self.c | 2 +- fs/proc/thread_self.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c index f40c2532c057..28d6105e908e 100644 --- a/fs/proc/inode.c +++ b/fs/proc/inode.c @@ -617,7 +617,7 @@ const struct inode_operations proc_link_inode_operations = {
struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de) { - struct inode *inode = new_inode_pseudo(sb); + struct inode *inode = new_inode(sb);
if (inode) { inode->i_ino = de->low_ino; diff --git a/fs/proc/self.c b/fs/proc/self.c index ca5158fa561c..72cd69bcaf4a 100644 --- a/fs/proc/self.c +++ b/fs/proc/self.c @@ -43,7 +43,7 @@ int proc_setup_self(struct super_block *s) inode_lock(root_inode); self = d_alloc_name(s->s_root, "self"); if (self) { - struct inode *inode = new_inode_pseudo(s); + struct inode *inode = new_inode(s); if (inode) { inode->i_ino = self_inum; inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode); diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c index ac284f409568..a553273fbd41 100644 --- a/fs/proc/thread_self.c +++ b/fs/proc/thread_self.c @@ -43,7 +43,7 @@ int proc_setup_thread_self(struct super_block *s) inode_lock(root_inode); thread_self = d_alloc_name(s->s_root, "thread-self"); if (thread_self) { - struct inode *inode = new_inode_pseudo(s); + struct inode *inode = new_inode(s); if (inode) { inode->i_ino = thread_self_inum; inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
On Fri 12-06-20 14:15:51, Eric W. Biederman wrote:
Recently syzbot reported that unmounting proc when there is an ongoing inotify watch on the root directory of proc could result in a use after free when the watch is removed after the unmount of proc when the watcher exits.
Commit 69879c01a0c3 ("proc: Remove the now unnecessary internal mount of proc") made it easier to unmount proc and allowed syzbot to see the problem, but looking at the code it has been around for a long time.
Looking at the code the fsnotify watch should have been removed by fsnotify_sb_delete in generic_shutdown_super. Unfortunately the inode was allocated with new_inode_pseudo instead of new_inode so the inode was not on the sb->s_inodes list. Which prevented fsnotify_unmount_inodes from finding the inode and removing the watch as well as made it so the "VFS: Busy inodes after unmount" warning could not find the inodes to warn about them.
Make all of the inodes in proc visible to generic_shutdown_super, and fsnotify_sb_delete by using new_inode instead of new_inode_pseudo. The only functional difference is that new_inode places the inodes on the sb->s_inodes list.
I wrote a small test program and I can verify that without changes it can trigger this issue, and by replacing new_inode_pseudo with new_inode the issues goes away.
Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/000000000000d788c905a7dfa3f4@google.com Reported-by: syzbot+7d2debdcdb3cb93c1e5e@syzkaller.appspotmail.com Fixes: 0097875bd415 ("proc: Implement /proc/thread-self to point at the directory of the current thread") Fixes: 021ada7dff22 ("procfs: switch /proc/self away from proc_dir_entry") Fixes: 51f0885e5415 ("vfs,proc: guarantee unique inodes in /proc") Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Thanks for analysing this! I agree with the analysis and the patch looks good to me. You can add:
Reviewed-by: Jan Kara jack@suse.cz
Honza
fs/proc/inode.c | 2 +- fs/proc/self.c | 2 +- fs/proc/thread_self.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c index f40c2532c057..28d6105e908e 100644 --- a/fs/proc/inode.c +++ b/fs/proc/inode.c @@ -617,7 +617,7 @@ const struct inode_operations proc_link_inode_operations = {
struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de) {
- struct inode *inode = new_inode_pseudo(sb);
struct inode *inode = new_inode(sb);
if (inode) { inode->i_ino = de->low_ino;
diff --git a/fs/proc/self.c b/fs/proc/self.c index ca5158fa561c..72cd69bcaf4a 100644 --- a/fs/proc/self.c +++ b/fs/proc/self.c @@ -43,7 +43,7 @@ int proc_setup_self(struct super_block *s) inode_lock(root_inode); self = d_alloc_name(s->s_root, "self"); if (self) {
struct inode *inode = new_inode_pseudo(s);
struct inode *inode = new_inode(s);diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c index ac284f409568..a553273fbd41 100644 --- a/fs/proc/thread_self.c +++ b/fs/proc/thread_self.c @@ -43,7 +43,7 @@ int proc_setup_thread_self(struct super_block *s) inode_lock(root_inode); thread_self = d_alloc_name(s->s_root, "thread-self"); if (thread_self) {
struct inode *inode = new_inode_pseudo(s);
struct inode *inode = new_inode(s);-- 2.20.1
b.a.t.m.a.n@lists.open-mesh.org
-
ebiederm＠xmission.com -
Jan Kara -
syzbot