linux-ext4 - Re: [4.14-rc1 bug] fstests generic/441 failure on ext2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170919145727.GA5540@quack2.suse.cz>
Date:   Tue, 19 Sep 2017 16:57:27 +0200
From:   Jan Kara <jack@...e.cz>
To:     Jeff Layton <jlayton@...hat.com>
Cc:     Eryu Guan <eguan@...hat.com>, linux-fsdevel@...r.kernel.org,
        linux-ext4@...r.kernel.org, Jan Kara <jack@...e.com>
Subject: Re: [4.14-rc1 bug] fstests generic/441 failure on ext2

On Mon 18-09-17 08:10:24, Jeff Layton wrote:
> On Mon, 2017-09-18 at 19:23 +0800, Eryu Guan wrote:
> > Hi all,
> > 
> > With ext2 driven by ext4 module (or ext4 without journal, I haven't
> > tested ext2 module, but I guess the result is the same), v4.14-rc1
> > kernel starts to fail fstests generic/441 as:
> > 
> > +First fsync after reopen of fd[0] failed: Input/output error
> > 
> > git bisect shows that this is uncovered by commit ffb959bbdf92 ("mm:
> > remove optimizations based on i_size in mapping writeback waits"), which
> > removed (i_size == 0) check in filemap_fdatawait().
> > 
> > I say "uncovered" because test fails with 4.13 kernel too if we re-open
> > the test file without O_TRUNC flag in src/fsync-err.c (so file size is
> > not zero, and fails the i_size == 0 check).
> > 
> > The EIO was returned by sync_inode_metadata() in __generic_file_fsync(),
> > the call trace is like:
> > 
> > do_fsync
> >  vfs_fsync_range
> >   ext4_sync_file
> >    __generic_file_fsync
> >     sync_inode_metadata
> >      writeback_single_inode
> >       __writeback_single_inode
> >        filemap_fdatawait  => EIO here
> > 
> > Thanks,
> > Eryu
> 
> (cc'ing Jan and linux-fsdevel)
> 
> Thanks for the bug report. The analysis looks spot-on.
> 
> So yeah...we have this "legacy" filemap_fdatawait call in
> __writeback_single_inode, and that is returning -EIO, likely because
> AS_EIO was set on the inode from the earlier wb errors.
> 
> That error return is pretty sketchy since it could be cleared at any
> time, and pretty much everything we care about here is now using
> errseq_t for error reporting at fsync. I don't think we really care too
> much about that flag in this codepath anymore.

So I agree fsync(2) path is covered but that fdatawait() call is also
responsible for reporting error e.g. for write_inode_now() calls and there
we still have some unconverted users. So for now I don't have a better
solution than to live with this additional somewhat stale EIO error. Or
possibly we can have truncate to 0 clear writeback error which would mask
the problem again and kind of makes sense...

								Honza

-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR