Re: [syzbot] WARNING in do_mkdirat

[Theodore Ts'o]

On Thu, Dec 29, 2022 at 01:17:31PM -0800, Eric Biggers wrote:
> Thanks Aleksandr.  From what I can see, the fix is working for new filesystem
> bugs: the filesystem(s) involved get added to the title and the recipients.
> 
> One question: what happens to all the open bugs, like this one ("WARNING in
> do_mkdirat") that were reported before the syzbot fix?  Are they going to be
> re-reported correctly?  Perhaps any bug whose reproducer includes
> "syz_mount_image" and was reported before the date of this fix should be
> invalidated more aggressively than usual, so that it can be re-reported?

As a related request/wish, it would be nice if those dashboard pages
that were created before the new-style reporting which includes the
file system image, strace otuput, etc., could get regenerated.  For example:

https://syzkaller.appspot.com/bug?id=be6e90ce70987950e6deb3bac8418344ca8b96cd

Even if someone has already submitted a proposed fix, I often like to
double-check that the fix is really fixing the true root cause of the
problem, as opposed to just making a superficial change that blocks
the current syzbot reproducer, but which will eventually be tripped
again because code is still vulnerable.  (For example, we might block
a straightforward reproducer by adding a check at mount time, but if
the superblocks get corrupted during the journal replay, we'd still be
vulnerable.)  And having access to the corrupted file system image,
and other associated reporting data, is often super-helpful in that
regard.

Also, can we at some point have the C reproducer actually using proper
C strings instead of hex digits?  It will make the reproducer much
more human understandable, as well making it easier to edit the string
when the developer is trying to do a better job minimizing the test
case than syzbot.  For example:

  memcpy(
      (void*)0x20000000,
      "\x6e\x6f\x75\x73\x65\x72\x5f\x78\x61\x74\x74\x72\x2c\x61\x63\x6c\x2c\x64"
      "\x65\x62\x75\x67\x5f\x77\x61\x6e\x74\x5f\x65\x78\x74\x72\x61\x5f\x69\x73"
      "\x69\x7a\x65\x3d\x30\x78\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30"
      "\x30\x30\x38\x30\x2c\x6c\x61\x7a\x79\x74\x69\x6d\x65\x2c\x6e\x6f\x62\x68"
      "\x2c\x71\x75\x6f\x74\x61\x2c\x00\x3d\x93\x09\x61\x36\x5d\x73\x58\x9c",
      89);

Would be *much* more understable if it were:

  memcpy(
      (void*)0x20000000,
      "nouser_xattr,acl,debug_want_extra_isize=0x0000000000000080,lazytime,nobh,quota,",
      80);

Of course, something like:

   char mount_options[] = "nouser_xattr,acl,debug_want_extra_isize=0x0000000000000080,lazytime,nobh,quota,";

Would be even better (and more portable) than using random hex
addresses, but just simply using ASCII strings would be a good first
step.

Of course, filling in C structures instead of just a random memcpy of
hex garbage would be even *more* awesome, bunt I'll take what I can
get.  :-)

Another opportunity for improvement is to try minimizing mount
options, so it becomes more obvious which ones are required.  For
example, in the above example, a minimized mount option string would
have been:

  memcpy((void*)0x20000000, "debug_want_extra_isize=0x80,lazytime," 38);

Having a more minimized reproducer would improve the reliability of
the bisect, as well as making it easier for the developer to figure
out the true root cause of the problem.

Cheers,

					- Ted