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]

Date:   Mon, 1 Jul 2019 08:18:08 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        syzbot <syzbot+6004acbaa1893ad013f0@...kaller.appspotmail.com>,
        Arnd Bergmann <arnd@...db.de>, Jens Axboe <axboe@...nel.dk>,
        Borislav Petkov <bp@...en8.de>,
        Catalin Marinas <catalin.marinas@....com>,
        Christian Brauner <christian@...uner.io>,
        David Howells <dhowells@...hat.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Hannes Reinecke <hare@...e.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        the arch/x86 maintainers <x86@...nel.org>
Subject: Re: general protection fault in do_move_mount (2)

On Mon, Jul 01, 2019 at 04:59:04PM +0200, 'Dmitry Vyukov' via syzkaller-bugs wrote:
> >
> > Dmitry, any idea why syzbot found such a bizarre reproducer for this?
> > This is actually reproducible by a simple single threaded program:
> >
> >     #include <unistd.h>
> >
> >     #define __NR_move_mount         429
> >     #define MOVE_MOUNT_F_EMPTY_PATH 0x00000004
> >
> >     int main()
> >     {
> >         int fds[2];
> >
> >         pipe(fds);
> >         syscall(__NR_move_mount, fds[0], "", -1, "/", MOVE_MOUNT_F_EMPTY_PATH);
> >     }
> 
> 
> There is no pipe in the reproducer, so it could not theoretically come
> up with the reproducer with the pipe. During minimization syzkaller
> only tries to remove syscalls and simplify arguments and execution
> mode.
> What would be the simplest reproducer expressed as further
> minimization of this reproducer?
> https://syzkaller.appspot.com/x/repro.syz?x=154e8c2aa00000
> I assume one of the syscalls is still move_mount, but what is the
> other one? If it's memfd_create, or open of the procfs file, then it
> seems that [ab]used heavy threading and syscall colliding as way to do
> an arbitrary mutation of the program. Per se results of
> memfd_create/procfs are not passed to move_mount. But by abusing races
> it probably managed to do so in small percent of cases. It would also
> explain why it's hard to reproduce.

To be clear, memfd_create() works just as well:

	#define _GNU_SOURCE
	#include <sys/mman.h>
	#include <unistd.h>

	#define __NR_move_mount         429
	#define MOVE_MOUNT_F_EMPTY_PATH 0x00000004

	int main()
	{
		int fd = memfd_create("foo", 0);

		syscall(__NR_move_mount, fd, "", -1, "/", MOVE_MOUNT_F_EMPTY_PATH);
	}

I just changed it to pipe() in my example, because pipe() is less obscure.

> 
> 
> > FYI, it also isn't really appropriate for syzbot to bisect all bugs in new
> > syscalls to wiring them up to x86, and then blame all the x86 maintainers.
> > Normally such bugs will be in the syscall itself, regardless of architecture.
> 
> Agree. Do you think it's something worth handling automatically
> (stands out of the long tail of other inappropriate cases)? If so, how
> could we detect such cases? It seems that some of these predicates are
> quite hard to program. Similar things happen with introduction of new
> bug detection tools and checks, wiring any functionality to new access
> points and similar things.
> 

Yes, this case could easily be automatically detected (most of the time) by
listing the filenames changed in the commit, and checking whether they all match
the pattern syscall.*\.tbl.  Sure, it's not common, but it could be alongside
other similar straightforward checks like checking for merge commits and
checking for commits that only modify Documentation/.

I'm not even asking for more correct bisection results at this point, I'm just
asking for fewer bad bisection results.

- Eric