linux-kernel - Re: general protection fault in do_move

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20190509063034.GQ23075@ZenIV.linux.org.uk>
Date:   Thu, 9 May 2019 07:30:34 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     syzbot <syzbot+494c7ddf66acac0ad747@...kaller.appspotmail.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        syzkaller-bugs@...glegroups.com,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: general protection fault in do_move_mount

On Wed, May 08, 2019 at 10:40:06PM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    80f23212 Merge git://git.kernel.org/pub/scm/linux/kernel/g..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=11ab8dd0a00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=40a58b399941db7e
> dashboard link: https://syzkaller.appspot.com/bug?extid=494c7ddf66acac0ad747
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> 
> 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+494c7ddf66acac0ad747@...kaller.appspotmail.com

*Ugh*

That's a bloody dumb leftover from very old variant of that thing;
the following should fix it.

do_move_mount(): fix an unsafe use of is_anon_ns()

What triggers it is a race between mount --move and umount -l
of the source; we should reject it (the source is parentless *and*
not the root of anon namespace at that), but the check for namespace
being an anon one is broken in that case - is_anon_ns() needs
ns to be non-NULL.  Better fixed here than in is_anon_ns(), since
the rest of the callers is guaranteed to get a non-NULL argument...

Reported-by: syzbot+494c7ddf66acac0ad747@...kaller.appspotmail.com
Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 3357c3d65475..ffb13f0562b0 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2599,7 +2599,7 @@ static int do_move_mount(struct path *old_path, struct path *new_path)
 	if (attached && !check_mnt(old))
 		goto out;
 
-	if (!attached && !is_anon_ns(ns))
+	if (!attached && !(ns && is_anon_ns(ns)))
 		goto out;
 
 	if (old->mnt.mnt_flags & MNT_LOCKED)