Re: [syzbot] WARNING in do_mkdirat

[Randy Dunlap]

On 12/31/22 08:57, Theodore Ts'o wrote:
> 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.

Amen to all of that. for so many good reasons.
Thanks.

-- 
~Randy