Re: [PATCH -next v5 04/32] ext4: skip ordered I/O wait when zeroing beyond i_disksize block

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Torn read of the 64-bit `i_disksize` on 32-bit architectures can cause the kernel to skip ordered I/O, leading to stale data exposure.
--

--- Patch [4]: [PATCH -next v5 04/32] ext4: skip ordered I/O wait when zeroing beyond i_disksize block ---
commit bb84de10d990ded9aab4107d18dd1616342efccd
Author: Zhang Yi <[email protected]>

ext4: skip ordered I/O wait when zeroing beyond i_disksize block

This commit optimizes ext4_block_zero_eof() by skipping the ordered I/O
wait when the zeroed range is entirely beyond the block containing
i_disksize. Since data beyond i_disksize hasn't been written to disk yet,
the ordered I/O wait is unnecessary and can be bypassed.

> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
[ ... ]
> @@ -4241,9 +4241,16 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
>  	 * truncating up or performing an append write, because there might be
>  	 * exposing stale on-disk data which may caused by concurrent post-EOF
>  	 * mmap write during folio writeback.
> +	 *
> +	 * Ordered I/O is required only when zeroing the tail of a block that
> +	 * overlaps with i_disksize. If the zeroed range falls outside that
> +	 * block, the zeroed data lies beyond the existing on-disk data. It
> +	 * will be written out before i_disksize is later extended past
> +	 * i_size, so no stale data can be exposed.
>  	 */
>  	if (ext4_should_order_data(inode) &&
> -	    did_zero && zero_written && !IS_DAX(inode)) {
> +	    did_zero && zero_written && !IS_DAX(inode) &&
> +	    from < round_up(READ_ONCE(EXT4_I(inode)->i_disksize), blocksize)) {

[Severity: High]
Could this READ_ONCE() on the 64-bit i_disksize result in a torn read on
32-bit architectures?

If a background writeback thread concurrently updates i_disksize under
i_data_sem but without holding i_rwsem, and the file size is extending
across a 4GB boundary (for example, transitioning from 0x00000000FFFFFFFF
to 0x0000000100000000), a torn read might fetch 0 if the lower 32 bits are
zeroed first.

If 0 is returned here, round_up(0, blocksize) evaluates to 0, which makes
the condition 'from < 0' false. Would this incorrectly bypass the critical
ordered I/O guarantee and potentially expose stale data in the event of a
crash?

>  		handle_t *handle;
>  
>  		handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4