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
