Message-Id: <20240613090033.2246907-1-yi.zhang@huaweicloud.com>
Date: Thu, 13 Jun 2024 17:00:25 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: linux-xfs@...r.kernel.org,
	linux-fsdevel@...r.kernel.org
Cc: linux-kernel@...r.kernel.org,
	djwong@...nel.org,
	hch@...radead.org,
	brauner@...nel.org,
	david@...morbit.com,
	chandanbabu@...nel.org,
	jack@...e.cz,
	yi.zhang@...wei.com,
	yi.zhang@...weicloud.com,
	chengzhihao1@...wei.com,
	yukuai3@...wei.com
Subject: [PATCH -next v5 0/8] iomap/xfs: fix stale data exposure when truncating realtime inodes

From: Zhang Yi <yi.zhang@...wei.com>

Changes since v4:
 - Drop the first patch in v4 "iomap: zeroing needs to be pagecache
   aware" since this series is not strongly depends on it, that patch
   still needs furtuer analyse and also should add to handle the case of
   a pending COW extent that extends over a data fork hole. This is a
   big job, so let's fix the exposure stale data issue and brings back
   the changes in iomap_write_end() first, don't block the ext4 buffered
   iomap conversion.
 - In patch 1, drop the 'ifndef rem_u64'.
 - In patch 4, factor out a helper xfs_setattr_truncate_data() to handle
   the zero out, update i_size, write back and drop pagecache on
   truncate.
 - In patch 5, switch to use xfs_inode_alloc_unitsize() in
   xfs_itruncate_extents_flags().
 - In patch 6, changes to reserve blocks for rtextsize > 1 realtime
   inodes on truncate down.
 - In patch 7, drop the unwritten convert threshold, always convert tail
   blocks to unwritten on truncate down realtime inodes.
 - Add patch 8 to bring back 'commit 943bc0882ceb ("iomap: don't
   increase i_size if it's not a write operation")'.

Changes since v3:
 - Factor out a new helper to get the remainder in math64.h as Darrick
   suggested.
 - Adjust the truncating order to prevent too much redundant blocking
   writes as Dave suggested.
 - Improve to convert the tail extent to unwritten when truncating down
   an inode with large rtextsize as Darrick and Dave suggested.

Since 'commit 943bc0882ceb ("iomap: don't increase i_size if it's not a
write operation")' merged, Chandan reported a stale data exposure issue
when running fstests generic/561 on xfs with realtime device [1]. This
issue has been fix on 6.10 by revert this commit through commit
'0841ea4a3b41 ("iomap: keep on increasing i_size in iomap_write_end()")',
but the real problem is xfs_setattr_size() doesn't zero out enough range
when truncate down a realtime inode. So this series fix this problem by
zeroing out allocation unitsize and convert the tail blocks to unwritten
when truncate down realtime inodes, finally we could bring commit
943bc0882ceb back.

Patch 1-3 modify iomap_truncate_page() and dax_truncate_page() to pass
filesystem identified blocksize, and drop the assumption of
i_blocksize() as Dave suggested.

Patch 4-5 refactor and adjust the truncating down processing order to
first zero out the tail aligned blocks, then write back and update
i_size, finally drop cache beyond aligned EOF. Fix the data exposure
issue by zeroing out the entire EOF extent.

Patch 6-7 improves truncate down performace on realtime inodes with
big rtextsize(>1 fsblock) by converting the tail unaligned extent to
unwritten.

Patch 8 reverts commit 0841ea4a3b41 and brings commit 943bc0882ceb back,
don't increase i_size on IOMAP_ZERO and IOMAP_UNSHARE.

I've tested this series on fstests (1) with reflink=0, (2) with
reflink=1, (3) with 28K RT device and (4) with dax, no new failures
detected, and it passed generic/561 on RT device over 1000+ rounds,
please let me know if it needs other tests.

[1] https://lore.kernel.org/linux-xfs/87ttj8ircu.fsf@debian-BULLSEYE-live-builder-AMD64/

Thanks,
Yi.

Zhang Yi (8):
  math64: add rem_u64() to just return the remainder
  iomap: pass blocksize to iomap_truncate_page()
  fsdax: pass blocksize to dax_truncate_page()
  xfs: refactor the truncating order
  xfs: correct the truncate blocksize of realtime inode
  xfs: reserve blocks for truncating large realtime inode
  xfs: speed up truncating down a big realtime inode
  iomap: don't increase i_size in iomap_write_end()

 fs/dax.c               |   8 +-
 fs/ext2/inode.c        |   4 +-
 fs/iomap/buffered-io.c |  61 +++++++-------
 fs/xfs/xfs_inode.c     |   9 ++-
 fs/xfs/xfs_iomap.c     |   5 +-
 fs/xfs/xfs_iomap.h     |   3 +-
 fs/xfs/xfs_iops.c      | 180 ++++++++++++++++++++++++++++-------------
 include/linux/dax.h    |   4 +-
 include/linux/iomap.h  |   4 +-
 include/linux/math64.h |  22 +++++
 10 files changed, 204 insertions(+), 96 deletions(-)

-- 
2.39.2

Powered by blists - more mailing lists