Message ID | 20240529095206.2568162-1-yi.zhang@huaweicloud.com (mailing list archive) |
---|---|
Headers | show |
Series | iomap/xfs: fix stale data exposure when truncating realtime inodes | expand |
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?
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.
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.
From: Zhang Yi <yi.zhang@huawei.com> 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(-)