Re: general protection fault in do_move_mount (2)

[Eric Biggers]

[Moved most people to Bcc to avoid spamming them with yet another syzkaller
discussion.  The actual kernel bug here was already fixed in mainline.]

On Fri, Jul 05, 2019 at 02:17:04PM +0200, Dmitry Vyukov wrote:
> On Mon, Jul 1, 2019 at 5:18 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> >
> > 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.
> 
> Then I think the reason for the bizarre reproducer is what I described above.

I'm asking why such a bizarre reproducer was generated in the first place, not
why syzkaller couldn't minimize the bizarre reproducer it already generated.

Look at each repro on the syzbot dashboard for this bug
(https://syzkaller.appspot.com/bug?id=bbf8823c8407719f089da130f341ae11d20d1622)

There are 6 repros.  All of them exploit some multithreaded race condition to
pass an empty string to sys_move_mount(), when the literal program text is
passing an nonempty string.

It seems really statistically unlikely that that's the most likely way to
encounter this bug.  It should be *much* more likely for syzkaller to find this
by generating the 2 syscalls sequentially needed to reproduce it.

So I suspect that something is wrong with the syscall definition for
move_mount().  Does syzkaller know the path(s) can be empty, and that the file
descriptor doesn't necessarily have to be a directory?  Or maybe something is
wrong with the algorithms that syzkaller uses to generate programs.

- Eric