2024-05-29 02:10:13

by Zhang Yi

[permalink] [raw]
Subject: [RFC PATCH v4 0/8] iomap/xfs: fix stale data exposure when truncating realtime inodes

From: Zhang Yi <[email protected]>

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.

This series fix a stale data exposure issue reported by Chandan when
running fstests generic/561 on xfs with realtime device[1]. The real
problem is xfs_setattr_size() doesn't zero out enough range when
truncating a realtime inode, please see the patch 6 or [1] for
details.

Patch 1 is from Dave, it improves truncate down performace by changing
iomap_zero_iter() to aware dirty pages on unwritten extents, but for the
case of the zeroing range that contains a cow mapping over a hole still
needs to be handled.

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

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

Patch 8-9 add a rtextsize threshold (64k), improves truncate down performace
on realtime inode with large rtextsize (beyonds this threshold) by
converting the tail unaligned extent to unwritten.

I've tested this series on fstests (1) with reflink=0, (2) with 28K RT
device and (3) with 96K RT device (beyonds rtextsize threshold), no new
failures detected. This series still needs to do furtuer tests with
reflink=1 after Patch 1 covers the cow mapping over a hole case.

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

Thanks,
Yi.

Dave Chinner (1):
iomap: zeroing needs to be pagecache aware

Zhang Yi (7):
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 realtime inode
xfs: improve truncate on a realtime inode with huge extsize

fs/dax.c | 8 +--
fs/ext2/inode.c | 4 +-
fs/iomap/buffered-io.c | 50 ++++++++++++++--
fs/xfs/xfs_inode.c | 3 +
fs/xfs/xfs_inode.h | 12 ++++
fs/xfs/xfs_iomap.c | 5 +-
fs/xfs/xfs_iomap.h | 3 +-
fs/xfs/xfs_iops.c | 133 +++++++++++++++++++++++++----------------
include/linux/dax.h | 4 +-
include/linux/iomap.h | 4 +-
include/linux/math64.h | 24 ++++++++
11 files changed, 179 insertions(+), 71 deletions(-)

--
2.39.2



2024-05-31 12:27:00

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC PATCH v4 0/8] iomap/xfs: fix stale data exposure when truncating realtime inodes

Procedural question before I get into the actual review: given we
are close to -rc3 and there is no user of the iomap change yet,
should we revert that for 6.10 and instead try again in 6.11 when
the XFS bits are sorted out?


2024-06-01 07:38:57

by Zhang Yi

[permalink] [raw]
Subject: Re: [RFC PATCH v4 0/8] iomap/xfs: fix stale data exposure when truncating realtime inodes

On 2024/5/31 20:26, Christoph Hellwig wrote:
> Procedural question before I get into the actual review: given we
> are close to -rc3 and there is no user of the iomap change yet,
> should we revert that for 6.10 and instead try again in 6.11 when
> the XFS bits are sorted out?
>

Okay, fine, it looks we still need some time to fix this issue. I
will send out a patch to revert the commit '943bc0882ceb ("iomap:
don't increase i_size if it's not a write operation")' soon, other
commits in my previous series looks harmless, so I think we can
keep them.

Thanks,
Yi.


2024-06-01 07:40:43

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC PATCH v4 0/8] iomap/xfs: fix stale data exposure when truncating realtime inodes

On Sat, Jun 01, 2024 at 03:38:37PM +0800, Zhang Yi wrote:
> will send out a patch to revert the commit '943bc0882ceb ("iomap:
> don't increase i_size if it's not a write operation")' soon, other
> commits in my previous series looks harmless, so I think we can
> keep them.

Agreed and thanks! For 6.11 we'll also need to figure out how to
keep the related changes together. I suspect we should just re-merge
that iomap change through the XFS tree, but we can talk about that
later.