Re: [PATCH -next] ext4: fix warning in ext4_handle_inode_extension

["Theodore Ts'o" <tytso@mit.edu>]

On Thu, Apr 14, 2022 at 10:46:36AM +0800, yebin wrote:
> To be honest, I don't know syzkaller how to inject the NOMEM
> fault. If syzkaller rely on the memory fault injection mode provided
> by the kernel, should report null pointer access. Anyway, If inject
> a single point of IO fault, we still have to face the same
> situation.

Was this patch in response to a syzkaller report?  There wasn't a
Reported-by tag indicating that this came from syzkaller.  If it did,
and it came from syzkaller run by the syzkaller team (e.g., at
https://syzkaller.appspot.com/upstream) could you include a reference
to the syzkaller report?

> On 2022/3/30 21:30, Jan Kara wrote:
> > > Do you mean call jbd2_abort in ext4_reserve_inode_write() ?
> > Yes.
> > 
> > > If we abort journal when metadata is not guaranteed to be consistent. The
> > > mode of ‘errors=continue’ is unnecessary.
> > Well, firstly, errors=continue was always the best effort. There are no
> > guarantees which failures we are able to withstand and which not.

That's true; however, in general, if we can back out and recover from
the error, we should, so that errors=continue can work.  If we think
that continuing will result in far more file system corruption and/or
the error is from the journalling infrastructure itself, then aborting
the journal makes sense.

> > There are
> > almost 80 callsites of ext4_mark_inode_dirty() and honestly I suspect that
> > e.g. inconsistent states resulting from extent tree manipulations being
> > aborted in the middle due to ext4_ext_dirty() failing due to ENOMEM will
> > also trigger all sorts of "interesting" behavior. So that's why I'd rather
> > abort the journal than try to continue when we almost certainly now we
> > cannot.

It would not be a bad thing for us to audit all of the callers of
ext4_mark_inode_dirty() and ext4_reserve_inode_write().  We are
getting things right in at least some of the callers (for example:
ext4_mkdir).

In any case, I'll take this patch, but if this was in response to a
syzkaller report, please let me know with the syzkaller ID is, so I
can update the commit before I send a pull request to Linus.

Thanks!

							- Ted