linux-kernel - Re: [PATCH] proc: Handle umounts cleanly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200415193612.7cmmbwfpof6pvsqv@wittgenstein>
Date:   Wed, 15 Apr 2020 21:36:12 +0200
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     syzbot <syzbot+d9ae59d4662c941e39c6@...kaller.appspotmail.com>,
        adobriyan@...il.com, akpm@...ux-foundation.org, avagin@...il.com,
        bernd.edlinger@...mail.de, christian@...uner.io, guro@...com,
        kent.overstreet@...il.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, mhocko@...e.com,
        syzkaller-bugs@...glegroups.com, tglx@...utronix.de,
        Alexey Gladkov <gladkov.alexey@...il.com>
Subject: Re: [PATCH] proc: Handle umounts cleanly

On Wed, Apr 15, 2020 at 01:28:24PM -0500, Eric W. Biederman wrote:
> syzbot writes:
> > KASAN: use-after-free Read in dput (2)
> >
> > proc_fill_super: allocate dentry failed
> > ==================================================================
> > BUG: KASAN: use-after-free in fast_dput fs/dcache.c:727 [inline]
> > BUG: KASAN: use-after-free in dput+0x53e/0xdf0 fs/dcache.c:846
> > Read of size 4 at addr ffff88808a618cf0 by task syz-executor.0/8426
> >
> > CPU: 0 PID: 8426 Comm: syz-executor.0 Not tainted 5.6.0-next-20200412-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/0x315 mm/kasan/report.c:382
> >  __kasan_report.cold+0x35/0x4d mm/kasan/report.c:511
> >  kasan_report+0x33/0x50 mm/kasan/common.c:625
> >  fast_dput fs/dcache.c:727 [inline]
> >  dput+0x53e/0xdf0 fs/dcache.c:846
> >  proc_kill_sb+0x73/0xf0 fs/proc/root.c:195
> >  deactivate_locked_super+0x8c/0xf0 fs/super.c:335
> >  vfs_get_super+0x258/0x2d0 fs/super.c:1212
> >  vfs_get_tree+0x89/0x2f0 fs/super.c:1547
> >  do_new_mount fs/namespace.c:2813 [inline]
> >  do_mount+0x1306/0x1b30 fs/namespace.c:3138
> >  __do_sys_mount fs/namespace.c:3347 [inline]
> >  __se_sys_mount fs/namespace.c:3324 [inline]
> >  __x64_sys_mount+0x18f/0x230 fs/namespace.c:3324
> >  do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295
> >  entry_SYSCALL_64_after_hwframe+0x49/0xb3
> > RIP: 0033:0x45c889
> > Code: ad b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> > RSP: 002b:00007ffc1930ec48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
> > RAX: ffffffffffffffda RBX: 0000000001324914 RCX: 000000000045c889
> > RDX: 0000000020000140 RSI: 0000000020000040 RDI: 0000000000000000
> > RBP: 000000000076bf00 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
> > R13: 0000000000000749 R14: 00000000004ca15a R15: 0000000000000013
> 
> Looking at the code now that it the internal mount of proc is no
> longer used it is possible to unmount proc.   If proc is unmounted
> the fields of the pid namespace that were used for filesystem
> specific state are not reinitialized.
> 
> Which means that proc_self and proc_thread_self can be pointers to
> already freed dentries.
> 
> The reported user after free appears to be from mounting and
> unmounting proc followed by mounting proc again and using error
> injection to cause the new root dentry allocation to fail.  This in
> turn results in proc_kill_sb running with proc_self and
> proc_thread_self still retaining their values from the previous mount
> of proc.  Then calling dput on either proc_self of proc_thread_self
> will result in double put.  Which KASAN sees as a use after free.
> 
> Solve this by always reinitializing the filesystem state stored
> in the struct pid_namespace, when proc is unmounted.
> 
> Reported-by: syzbot+72868dd424eb66c6b95f@...kaller.appspotmail.com
> Fixes: 69879c01a0c3 ("proc: Remove the now unnecessary internal mount of proc")
> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>

Was looking at that earlier right before eod briefly here as well.
Acked-by: Christian Brauner <christian.brauner@...ntu.com>

Thanks!
Christian