Re: KASAN: use-after-free Read in __fput (3)
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 ;-/
Re: KASAN: use-after-free Read in __fput (3)
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(>ffd.file->f_tfile_llink)) {
- get_file(epi->ffd.file);
- list_add(>ffd.file->f_tfile_llink,
-_check_list);
+ if (get_file_rcu(epi->ffd.file))
+ list_add(>ffd.file->f_tfile_llink,
+_check_list);
}
}
}
KASAN: use-after-free Read in __fput (3)
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=10b440d190
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=16a5452e90
The issue was bisected to:
commit a9ed4a6560b8562b7e2e2bed9527e88001f7b682
Author: Marc Zyngier
Date: Wed Aug 19 16:12:17 2020 +
epoll: Keep a reference on files added to the check list
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=147a02f290
final oops: https://syzkaller.appspot.com/x/report.txt?x=167a02f290
console output: https://syzkaller.appspot.com/x/log.txt?x=127a02f290
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+c282923e5da93549f...@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 888087a020a8 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:7fff215c6f90 EFLAGS: 0293 ORIG_RAX: 0003
RAX: RBX: 0004 RCX: 00416f01
RDX: 000f4240 RSI: 0081 RDI: 0003
RBP: R08: 01190358 R09:
R10: 7fff215c7070 R11: 0293 R12: 01190360
R13: R14: R15: 0118cf4c
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
