Re: [syzbot] [sysv?] [vfs?] WARNING in invalidate_bh_lru

[Theodore Ts'o]

On Mon, May 01, 2023 at 11:18:28AM +0200, Aleksandr Nogikh wrote:
> >
> > There are two reproducers, one that mounts a sysv file system, and the
> > other which mounts a udf file system.  There is no mention of ext4 in
> > the stack trace, and yet syzbot has assigned this to the ext4
> > subsystem for some unknown reason.
> 
> In this particular case, there were two ext4-related crashes as well:
> 
> https://syzkaller.appspot.com/text?tag=CrashReport&x=14c7dd1b480000
> https://syzkaller.appspot.com/text?tag=CrashReport&x=1153a07f480000
> 
> I think syzbot picked a too generic frame as a bug title and it just
> got confused by crashes belonging to different filesystems.
> Maybe we need to display subsystems for individual crashes as well, so
> that it's easier for a human to understand what's going on..

Hmm.... the the two ext4-related crashes have very different stack
traces.  In the first, there was some ext4 functions on the stack
trace, but those are not relevant, since we take an interrupt, and it
was apparently an arm64 IPI, which then results in the call to
invalidate_bh_lrus(), which then triggers the warning.  So this
*might* be related to ext4, but it could be any other file system that
could have been mounted at the same time, since syzbot runs multiple
streams of system calls in parallel.

In the second stack trace, we were in the middle of unmounting the
file system, and ext4_but_super() called invalidate_bdev(), which in
turn called invalidate_bh_lrus(), and that's when the buffer head
layer finds a buffer head whose refcount is zero, and that triggers
the:

VFS: brelse: Trying to free free buffer

... which then leads to the WARNING in invalidate_bh_lru().

So yes, the problem is that syzbot chose generic description based on
the warning.  An analogy might be a medical examiner in a police
procedural TV show[1] having seen a dozen corpses that were shot in
the torso, leading to them bleeding out and dying, erroneously
concluding that all six homocides *must* be related.  In fact, the
size of the bullet might be different, and who might have been holding
the gun might be very different.

[1] https://www.youtube.com/watch?v=lMalvNeJFLk

Worse, even if we did a better job disambiguating based on the stack
trace, without a reliable reproducer, the stack trace is of very
limited utility for trying to track down this kind of bug.  Going back
to the crime scene analogy, the stack trace basically tells us where
the body was found.  It doesn't tell us much about who might have been
fired the bullet.  That is, we need to know who called brelse() or
put_bh() on the buffer_head one too many times, leading to the report
that there was an attempt to free a free buffer.

(Fortunately, such bugs a relatively rare, although as this syzbot
report demonstrates, some still exist --- if I had to guess, the bugs
are on some error handling path which is very rarely hit.)

Perhaps syzbot could special case handling for those objects (like
struct buffer_head) which have a refcount and where the refcount can
go negative, and treat functions that manipulate such objects much
like how syzbot handles KASAN errors?  The tricky bit is the immediate
caller of the invalidate_bh_lrus() is not necessarily the best one to
use.  For example, invalidate_bdev() calls invalidate_bh_lrus(), and
it's the caller of invalidate_bdev() which is interesting in this case
--- e.g., ext4_put_super().

In the first stack trace, nothing in the stack trace is helpful, so
there may not much we can do debug this unless we can get a reproducer
that reproduces this particular stack trace.

In general, though, syzbot should disregard any functions after the
functions indicating that an interrupt had taken place, since which
code was interrupted when the IRQ came in is just an innocent victim.

For now, given that we *do* have a two reproducers, one involving sysv
and the other udf file systems, it's probably best that we try to let
the maintainers of those file systems try to figure out what be going
on.

     	 	     	      	       	  - Ted