Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* KASAN: use-after-free Read in __fput (3)
@ 2020-08-31 23:08 syzbot
       [not found] ` <20200901023547.14976-1-hdanton@sina.com>
  0 siblings, 1 reply; 3+ messages in thread
From: syzbot @ 2020-08-31 23:08 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, maz, syzkaller-bugs, viro

Hello,

syzbot found the following issue on:

HEAD commit:    15bc20c6 Merge tag 'tty-5.9-rc3' of git://git.kernel.org/p..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10b440d1900000
kernel config:  https://syzkaller.appspot.com/x/.config?x=978db74cb30aa994
dashboard link: https://syzkaller.appspot.com/bug?extid=c282923e5da93549fa27
compiler:       gcc (GCC) 10.1.0-syz 20200507
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=16a5452e900000

The issue was bisected to:

commit a9ed4a6560b8562b7e2e2bed9527e88001f7b682
Author: Marc Zyngier <maz@kernel.org>
Date:   Wed Aug 19 16:12:17 2020 +0000

    epoll: Keep a reference on files added to the check list

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=147a02f2900000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=167a02f2900000
console output: https://syzkaller.appspot.com/x/log.txt?x=127a02f2900000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+c282923e5da93549fa27@syzkaller.appspotmail.com
Fixes: a9ed4a6560b8 ("epoll: Keep a reference on files added to the check list")

==================================================================
BUG: KASAN: use-after-free in d_inode include/linux/dcache.h:522 [inline]
BUG: KASAN: use-after-free in fsnotify_parent include/linux/fsnotify.h:54 [inline]
BUG: KASAN: use-after-free in fsnotify_file include/linux/fsnotify.h:90 [inline]
BUG: KASAN: use-after-free in fsnotify_close include/linux/fsnotify.h:279 [inline]
BUG: KASAN: use-after-free in __fput+0x8ac/0x920 fs/file_table.c:267
Read of size 8 at addr ffff888087a020a8 by task syz-executor.2/11261

CPU: 0 PID: 11261 Comm: syz-executor.2 Not tainted 5.9.0-rc2-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+0x18f/0x20d lib/dump_stack.c:118
 print_address_description.constprop.0.cold+0xae/0x497 mm/kasan/report.c:383
 __kasan_report mm/kasan/report.c:513 [inline]
 kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
 d_inode include/linux/dcache.h:522 [inline]
 fsnotify_parent include/linux/fsnotify.h:54 [inline]
 fsnotify_file include/linux/fsnotify.h:90 [inline]
 fsnotify_close include/linux/fsnotify.h:279 [inline]
 __fput+0x8ac/0x920 fs/file_table.c:267
 task_work_run+0xdd/0x190 kernel/task_work.c:141
 tracehook_notify_resume include/linux/tracehook.h:188 [inline]
 exit_to_user_mode_loop kernel/entry/common.c:140 [inline]
 exit_to_user_mode_prepare+0x195/0x1c0 kernel/entry/common.c:167
 syscall_exit_to_user_mode+0x59/0x2b0 kernel/entry/common.c:242
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x416f01
Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 1b 00 00 c3 48 83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:00007fff215c6f90 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000004 RCX: 0000000000416f01
RDX: 00000000000f4240 RSI: 0000000000000081 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000001190358 R09: 0000000000000000
R10: 00007fff215c7070 R11: 0000000000000293 R12: 0000000001190360
R13: 0000000000000000 R14: ffffffffffffffff R15: 000000000118cf4c

Allocated by task 11262:
 kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
 kasan_set_track mm/kasan/common.c:56 [inline]
 __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:461
 slab_post_alloc_hook mm/slab.h:518 [inline]
 slab_alloc mm/slab.c:3312 [inline]
 kmem_cache_alloc+0x138/0x3a0 mm/slab.c:3482
 __d_alloc+0x2a/0x950 fs/dcache.c:1709
 d_alloc_pseudo+0x19/0x70 fs/dcache.c:1838
 alloc_file_pseudo+0xc6/0x250 fs/file_table.c:226
 sock_alloc_file+0x4f/0x190 net/socket.c:411
 sock_map_fd net/socket.c:435 [inline]
 __sys_socket+0x13d/0x200 net/socket.c:1524
 __do_sys_socket net/socket.c:1529 [inline]
 __se_sys_socket net/socket.c:1527 [inline]
 __x64_sys_socket+0x6f/0xb0 net/socket.c:1527
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Freed by task 11262:
 kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
 kasan_set_track+0x1c/0x30 mm/kasan/common.c:56
 kasan_set_free_info+0x1b/0x30 mm/kasan/generic.c:355
 __kasan_slab_free+0xd8/0x120 mm/kasan/common.c:422
 __cache_free mm/slab.c:3418 [inline]
 kmem_cache_free.part.0+0x67/0x1f0 mm/slab.c:3693
 __d_free fs/dcache.c:271 [inline]
 dentry_free+0xde/0x160 fs/dcache.c:348
 __dentry_kill+0x4c6/0x640 fs/dcache.c:593
 dentry_kill fs/dcache.c:705 [inline]
 dput+0x725/0xbc0 fs/dcache.c:878
 __fput+0x3ab/0x920 fs/file_table.c:294
 task_work_run+0xdd/0x190 kernel/task_work.c:141
 tracehook_notify_resume include/linux/tracehook.h:188 [inline]
 exit_to_user_mode_loop kernel/entry/common.c:140 [inline]
 exit_to_user_mode_prepare+0x195/0x1c0 kernel/entry/common.c:167
 syscall_exit_to_user_mode+0x59/0x2b0 kernel/entry/common.c:242
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Last call_rcu():
 kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
 kasan_record_aux_stack+0x82/0xb0 mm/kasan/generic.c:346
 __call_rcu kernel/rcu/tree.c:2894 [inline]
 call_rcu+0x14f/0x7e0 kernel/rcu/tree.c:2968
 dentry_free+0xc3/0x160 fs/dcache.c:350
 __dentry_kill+0x4c6/0x640 fs/dcache.c:593
 shrink_dentry_list+0x144/0x480 fs/dcache.c:1141
 shrink_dcache_parent+0x219/0x3c0 fs/dcache.c:1568
 d_invalidate fs/dcache.c:1677 [inline]
 d_invalidate+0x13f/0x280 fs/dcache.c:1662
 proc_invalidate_siblings_dcache+0x43b/0x600 fs/proc/inode.c:150
 release_task+0xc63/0x14d0 kernel/exit.c:221
 wait_task_zombie kernel/exit.c:1088 [inline]
 wait_consider_task+0x2fb3/0x3b20 kernel/exit.c:1315
 do_wait_thread kernel/exit.c:1378 [inline]
 do_wait+0x36a/0x9e0 kernel/exit.c:1449
 kernel_wait4+0x14c/0x260 kernel/exit.c:1621
 __do_sys_wait4+0x13f/0x150 kernel/exit.c:1649
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Second to last call_rcu():
 kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
 kasan_record_aux_stack+0x82/0xb0 mm/kasan/generic.c:346
 __call_rcu kernel/rcu/tree.c:2894 [inline]
 call_rcu+0x14f/0x7e0 kernel/rcu/tree.c:2968
 dentry_free+0xc3/0x160 fs/dcache.c:350
 __dentry_kill+0x4c6/0x640 fs/dcache.c:593
 dentry_kill fs/dcache.c:717 [inline]
 dput+0x635/0xbc0 fs/dcache.c:878
 handle_mounts fs/namei.c:1389 [inline]
 step_into+0xc43/0x1990 fs/namei.c:1690
 walk_component+0x171/0x6a0 fs/namei.c:1866
 link_path_walk.part.0+0x6b8/0xc20 fs/namei.c:2183
 link_path_walk fs/namei.c:2111 [inline]
 path_lookupat+0xb7/0x830 fs/namei.c:2332
 filename_lookup+0x19f/0x560 fs/namei.c:2366
 user_path_at include/linux/namei.h:59 [inline]
 do_faccessat+0x129/0x820 fs/open.c:423
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

The buggy address belongs to the object at ffff888087a02040
 which belongs to the cache dentry of size 312
The buggy address is located 104 bytes inside of
 312-byte region [ffff888087a02040, ffff888087a02178)
The buggy address belongs to the page:
page:00000000b4e25e7c refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x87a02
flags: 0xfffe0000000200(slab)
raw: 00fffe0000000200 ffffea000212a688 ffffea00021e6a48 ffff88821bc47f00
raw: 0000000000000000 ffff888087a02040 000000010000000a 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff888087a01f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888087a02000: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
>ffff888087a02080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                  ^
 ffff888087a02100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fc
 ffff888087a02180: fc fc fc fc fc fc fc 00 00 00 00 00 00 00 00 00
==================================================================


---
This report 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 issue. 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 issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: KASAN: use-after-free Read in __fput (3)
       [not found] ` <20200901023547.14976-1-hdanton@sina.com>
@ 2020-09-01  3:19   ` Al Viro
       [not found]   ` <20200901084309.9528-1-hdanton@sina.com>
  1 sibling, 0 replies; 3+ messages in thread
From: Al Viro @ 2020-09-01  3:19 UTC (permalink / raw)
  To: Hillf Danton; +Cc: syzbot, linux-fsdevel, linux-kernel, maz, syzkaller-bugs

On Tue, Sep 01, 2020 at 10:35:47AM +0800, Hillf Danton wrote:

> Any light on how a9ed4a6560b8 ends up with __fput called twice is
> highly appreciated.

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index e0decff22ae2..8107e06d7f6f 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1995,9 +1995,9 @@ static int ep_loop_check_proc(void *priv, void *cookie, int call_nests)
 			 * during ep_insert().
 			 */
 			if (list_empty(&epi->ffd.file->f_tfile_llink)) {
-				get_file(epi->ffd.file);
-				list_add(&epi->ffd.file->f_tfile_llink,
-					 &tfile_check_list);
+				if (get_file_rcu(epi->ffd.file))
+					list_add(&epi->ffd.file->f_tfile_llink,
+						 &tfile_check_list);
 			}
 		}
 	}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: KASAN: use-after-free Read in __fput (3)
       [not found]   ` <20200901084309.9528-1-hdanton@sina.com>
@ 2020-09-01 12:34     ` Al Viro
  0 siblings, 0 replies; 3+ messages in thread
From: Al Viro @ 2020-09-01 12:34 UTC (permalink / raw)
  To: Hillf Danton; +Cc: syzbot, linux-fsdevel, linux-kernel, maz, syzkaller-bugs

On Tue, Sep 01, 2020 at 04:43:09PM +0800, Hillf Danton wrote:

> Below is my 2c for making it safe to access the rbtree entry and the
> file tucked in it by bumping up the file count before adding epi into
> rbtree.

NAK.  epitems, by design, do *NOT* pin files down.  That's what
eventpoll_release() is about - those references are not counting and
epitems can be taken out by the final close.

It's a user-visible aspect of API.  And the problem Marc's patch was
trying to solve had nothing to do with that - at the root it's
the lack of suitable exclusion and the atrocious way loop prevention
and reverse path count checks are done on watch insertion.

What happens there is that files that would have paths added by
EPOLL_CTL_ADD (including those via several intermediate epoll files)
are collected into a temporary list and then checked for excessive
reverse paths.  The list is emptied before epoll_ctl() returns.

And that's _the_ list - if epoll_ctl decides to go there in the
first place, it grabs a system-wide mutex and holds it as long as
it's playing around.  Everything would've worked, if not for the
fact that
	* the bloody list goes through struct file instances.  It's
a bad decision, for a lot of reasons.
	* in particular, files can go away while epoll_ctl() is playing
around.  The same system-wide mutex would've blocked their freeing (modulo
a narrow race in eventpoll_release()) *IF* they remained attached to
epitems.  However, explicit EPOLL_CTL_DEL removing such a beast
in the middle of EPOLL_CTL_ADD checks will remove such epitems without
touching the same mutex.

Marc's patch tried to brute-force the protection of files in that
temporary list; what it had missed was the fact that a file on it could
have already been committed to getting killed - f_count already zero,
just hadn't gotten through the __fput() yet.  In such cases we don't
want to do any reverse path checks for that sucker - it *is* going
away, so there's no point considering it.  We can't blindly bump the
refcount, though, for obvious reasons.

That struct file can't get freed right under the code inserting it into
the list - the locks held at the moment (ep->mtx) are more than enough.
It's what can happen to it while on the list that is a problem.

Of course, as a long-term solution we want to have that crap with
temporary list going through struct file instances taken out and
moved to epitems themselves; the check does scan all epitems for
every file in the set and we bloody well could gather those into
the list at once.  Then we'd only need to protect the list walking
vs. removals (with a spinlock, for all we care).

It's too invasive a change for -stable, though, and I'm still digging
through fs/eventpoll.c locking - it's much too convoluted and it
got no attention for a decade ;-/

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-09-01 12:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31 23:08 KASAN: use-after-free Read in __fput (3) syzbot
     [not found] ` <20200901023547.14976-1-hdanton@sina.com>
2020-09-01  3:19   ` Al Viro
     [not found]   ` <20200901084309.9528-1-hdanton@sina.com>
2020-09-01 12:34     ` Al Viro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).