Date: Tue, 18 Jun 2024 22:21:10 +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 v6 0/2] iomap/xfs: fix stale data exposure when truncating realtime inodes

Changes since v5:
 - Drop all the code about zeroing out the whole allocation unitsize
   on truncate down in xfs_setattr_size() as Christoph suggested, let's
   just fix this issue for RT file by converting tail blocks to
   unwritten now, and we could think about forced aligned extent and
   atomic write later until it needs, so only pick patch 6 and 8 in
   previous version, do some minor git log changes.

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 in 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
just converting the tail blocks to unwritten when truncate down realtime
inodes, then we could bring commit 943bc0882ceb back.

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

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

Thanks,
Yi.

Zhang Yi (2):
  xfs: reserve blocks for truncating large realtime inode
  iomap: don't increase i_size in iomap_write_end()

 fs/iomap/buffered-io.c | 53 +++++++++++++++++++++++-------------------
 fs/xfs/xfs_iops.c      | 15 +++++++++++-
 2 files changed, 43 insertions(+), 25 deletions(-)

-- 
2.39.2

Powered by blists - more mailing lists