linux-ext4 - Re: [PATCH] ext4: Fix fsync for non-directories

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: <ZG6Lyq4iq/HnBvPu@debian-BULLSEYE-live-builder-AMD64>
Date:   Wed, 24 May 2023 18:12:26 -0400
From:   Eric Whitney <enwlinux@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org,
        Eric Whitney <enwlinux@...il.com>,
        Linux kernel regressions list <regressions@...ts.linux.dev>
Subject: Re: [PATCH] ext4: Fix fsync for non-directories

* Jan Kara <jack@...e.cz>:
> Commit e360c6ed7274 ("ext4: Drop special handling of journalled data
> from ext4_sync_file()") simplified ext4_sync_file() by dropping special
> handling of journalled data mode as it was not needed anymore. However
> that branch was also used for directories and symlinks and since the
> fastcommit code does not track metadata changes to non-regular files, the
> change has caused e.g. fsync(2) on directories to not commit transaction
> as it should. Fix the problem by adding handling for non-regular files.
> 
> Fixes: e360c6ed7274 ("ext4: Drop special handling of journalled data from ext4_sync_file()")
> Reported-by: Eric Whitney <enwlinux@...il.com>
> Link: https://lore.kernel.org/all/ZFqO3xVnmhL7zv1x@debian-BULLSEYE-live-builder-AMD64
> Signed-off-by: Jan Kara <jack@...e.cz>
> ---
>  fs/ext4/fsync.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
> index f65fdb27ce14..2a143209aa0c 100644
> --- a/fs/ext4/fsync.c
> +++ b/fs/ext4/fsync.c
> @@ -108,6 +108,13 @@ static int ext4_fsync_journal(struct inode *inode, bool datasync,
>  	journal_t *journal = EXT4_SB(inode->i_sb)->s_journal;
>  	tid_t commit_tid = datasync ? ei->i_datasync_tid : ei->i_sync_tid;
>  
> +	/*
> +	 * Fastcommit does not really support fsync on directories or other
> +	 * special files. Force a full commit.
> +	 */
> +	if (!S_ISREG(inode->i_mode))
> +		return ext4_force_commit(inode->i_sb);
> +
>  	if (journal->j_flags & JBD2_BARRIER &&
>  	    !jbd2_trans_will_send_data_barrier(journal, commit_tid))
>  		*needs_barrier = true;
> -- 
> 2.35.3
>

Hi Jan:

100/100 trials of both the original test regressions - generic/065 and
generic/535 - passed when I used kvm-xfstests to run them on a 6.4-rc3 kernel
modified with this patch.  A complete run of the adv test case also passed
without new regressions.

So,
Tested-by: Eric Whitney <enwlinux@...il.com>

Thanks!
Eric