[PATCH] ext4: skip eof zeroing when the zero range covers the eof block

[PATCH] ext4: skip eof zeroing when the zero range covers the eof block

[Baokun Li]

ext4_zero_range() drops the page cache of the range before allocating,
to avoid exposing stale cached data.  However, the dropped page can be
read back by the eof zeroing:

  fallocate(fd, FALLOC_FL_ZERO_RANGE, 0, 4096)
    ext4_zero_range()
      ext4_truncate_page_cache_block_range(0, 4096)
        truncate_pagecache_range(0, 4095)
          truncate_inode_pages_range()
            delete_from_page_cache_batch()  # drops the page
      ext4_alloc_file_blocks(..., 4096)
        ext4_block_zero_eof(512, LLONG_MAX) # not block aligned
          ext4_load_tail_bh()
             filemap_lock_folio(0)    # reads dropped page back
             # zeroes [512,1024), marks the buffer dirty
        ext4_map_blocks(...)          # zeroes the blocks on disk
      ext4_update_inode_size(inode, 4096)

The bytes below the old EOF keep the stale disk content.  The page is
left dirty, so read() returns old data while O_DIRECT reads zeroes, and
a write into the same block is not converted and is lost until the page
is dropped:

  page [0, 4096), 1k block size, file ends at 512
   0            512          1024                      4096
   |--- data ----|-- tail ---|---- zero range ----------|
   | old data    |           |                          |

For example:

  $ xfs_io -f -c "pwrite -S 1 0 512" f
  $ sync
  $ fallocate -z -o 0 -l 4096 f
  $ xfs_io -c "pread -v 0 16" f  # old data, not zeroes

Since the whole eof block is converted to unwritten, zeroing it here is
redundant.  Therefore, skip the zeroing when this allocation covers the
eof block entirely.

Keep the zeroing otherwise: the block is still written at that point,
so touching the page cache is safe.

Fixes: c7fc0366c656 ("ext4: partial zero eof block on unaligned inode size extension")
Cc: stable@vger.kernel.org
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 fs/ext4/extents.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ad0d80e33f60..2ce33fd3b912 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4639,8 +4639,15 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
 	credits = ext4_chunk_trans_blocks(inode, len_lblk);
 	depth = ext_depth(inode);
 
-	/* Zero to the end of the block containing i_size */
-	if (new_size > old_size) {
+	/*
+	 * Zero to the end of the block containing i_size.
+	 * Loading the eof block back before it is converted to unwritten
+	 * may leave the page cache inconsistent with the disk, and zeroing
+	 * it is redundant, so skip it.
+	 */
+	if (new_size > old_size &&
+	    (!(flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN) ||
+	     !in_range(old_size >> blkbits, map.m_lblk, map.m_len))) {
 		ret = ext4_block_zero_eof(inode, old_size, LLONG_MAX);
 		if (ret)
 			return ret;
-- 
2.43.7