linux-ext4 - Re: [PATCH v2] generic: add testcase to test fallocate & f{data,}sync

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: <20171115114208.GT17339@eguan.usersys.redhat.com>
Date:   Wed, 15 Nov 2017 19:42:08 +0800
From:   Eryu Guan <eguan@...hat.com>
To:     linux-ext4@...r.kernel.org
Cc:     fstests@...r.kernel.org, jaegeuk@...nel.org, chao@...nel.org,
        yuchao0@...wei.com
Subject: Re: [PATCH v2] generic: add testcase to test fallocate & f{data,}sync

On Wed, Nov 15, 2017 at 07:37:28PM +0800, Eryu Guan wrote:
> [adding ext4 list]
> 
> On Wed, Nov 15, 2017 at 04:58:33PM +0800, Chao Yu wrote:
> > f2fs can skip isize updating in fsync(), since during mount, f2fs tries
> > to recovery isize according to valid block address or preallocated flag
> > in last fsynced dnode block.
> > 
> > However, fallocate() breaks our rule with setting FALLOC_FL_KEEP_SIZE
> > flag, since it can preallocated block cross EOF, once the file is fsynced,
> > in POR, we will recover isize incorrectly based on these fallocated
> > blocks.
> > 
> > This patch adds a new testcase to test fallocate, in order to verify
> > whether filesystem will do correct recovery on isize.
> > 
> > Signed-off-by: Chao Yu <yuchao0@...wei.com>
> 
> This test fails on ext4 as
> 
>  ==== falloc -k 1024 test with fdatasync ====
> +Before:  "b: 8216 s: 4202496"
> +After :  "b: 8208 s: 4202496"
>  ==== falloc -k 4096 test with fdatasync ====
> +Before:  "b: 8216 s: 4202496"
> +After :  "b: 8208 s: 4202496"
>  ==== falloc -k 104857600 test with fdatasync ====
> +Before:  "b: 213008 s: 4202496"
> +After :  "b: 8208 s: 4202496"
> 
> So the block counts are changed after fdatasync & fs shutdown in the
> fallocate(2) KEEP_SIZE case. Looks like a real ext4 bug, fdatasync
> failed to flush the preallocated blocks beyond EOF to disk, but I want
> to confirm first with ext4 people. If it turns out to be a case issue,
> we'd like to update test case accordingly before merging it.

I think I've found the problem, ext4 fallocate(2) path is missing a
ext4_update_inode_fsync_trans(handle, inode, 1) call, as what commit
("67a7d5f561f4 ext4: fix fdatasync(2) after extent manipulation
operations") did.

I'll send an ext4 patch soon.

Thanks,
Eryu