Re: possible deadlock in dquot_commit

[Theodore Ts'o]

>From: Theodore Ts'o <tytso@xxxxxxx>

On Fri, Feb 12, 2021 at 12:01:51PM +0100, Dmitry Vyukov wrote:
> > >
> > > There is a reproducer for 4.19 available on the dashboard. Maybe it will help.
> > > I don't why it did not pop up on upstream yet, there lots of potential
> > > reasons for this.
> >
> > The 4.19 version of the syzbot report has a very different stack
> > trace.  Instead of it being related to an apparent write to the quota
> > file, it is apparently caused by a call to rmdir:
> >
>
> The 4.19 reproducer may reproducer something else, you know better. I
> just want to answer points re syzkaller reproducers. FTR the 4.19
> reproducer/reproducer is here:
> https://syzkaller.appspot.com/bug?id=b6cacc9fa48fea07154b8797236727de981c1e02

Yes, I know.  That was my point.  I don't think it's useful for
debugging the upstream dquot_commit syzbot report (for which we don't
have a reproducer yet).

> > there is never any attempt to run rmdir() on the corrupted file system that is mounted.
> 
> Recursive rmdir happens as part of test cleanup implicitly, you can
> see rmdir call in remove_dir function in the C reproducer:
> https://syzkaller.appspot.com/text?tag=ReproC&x=12caea37900000

That rmdir() removes the mountpoint, which is *not* the fuzzed file
system which has the quota feature enabled.

> > procid never gets incremented, so all of the threads only operate on /dev/loop0
> 
> This is intentional. procid is supposed to "isolate" parallel test
> processes (if any). This reproducer does not use parallel test
> processes, thus procid has constant value.

Um... yes it does:

int main(void)
{
  syscall(__NR_mmap, 0x1ffff000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
  syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
  syscall(__NR_mmap, 0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
  use_temporary_dir();
  loop();
  return 0;
}

and what is loop?

static void loop(void)
{
  int iter = 0;
  for (;; iter++) {
  	...
    reset_loop();
    int pid = fork();
    if (pid < 0)
      exit(1);
    if (pid == 0) {
      if (chdir(cwdbuf))
        exit(1);
      setup_test();
      execute_one();
      exit(0);
    }
    ...	
    remove_dir(cwdbuf);
  }
}

> > Am I correct in understanding that when syzbot is running, it uses the syzbot repro, and not the C repro?
> 
> It tries both. If first tries to interpret "syzkaller program" as it
> was done when the bug was triggered during fuzzing. But then it tries
> to convert it to a corresponding stand-alone C program and confirms
> that it still triggers the bug. If it provides a C reproducer, it
> means that it did trigger the bug using this exact C program on a
> freshly booted kernel (and the provided kernel oops is the
> corresponding oops obtained on this exact program).
> If it fails to reproduce the bug with a C reproducer, then it provides
> only the "syzkaller program" to not mislead developers.

Well, looking at the C reproducer, it doesn't reproduce on upstream,
and the stack trace makes no sense to me.  The rmdir() executes at the
end of the test, as part of the cleanup, and looking at the syzkaller
console, the stack trace involving rmdir happens *early* while test
threads are still trying to mount the file system.

	    	  	    	      - Ted