[PATCH V2] xfs: fix stale data exposure problem when punch hole, collapse range or zero range across a delalloc extent

From: yu kuai
Date: Fri Dec 27 2019 - 22:34:59 EST


In xfs_file_fallocate, when punch hole, zero range or collapse range is
performed, xfs_fulsh_unmap_range() need to be called first. However,
xfs_map_blocks will convert the whole extent to real, even if there are
some blocks not related. Furthermore, the unrelated blocks will hold stale
data since xfs_fulsh_unmap_range didn't flush the correspond dirty pages
to disk.

In this case, if user shutdown file system through xfsioctl with cmd
'XFS_IOC_GOINGDOWN' and arg 'XFS_FSOP_GOING_FLAGS_LOGFLUSH'. All the
completed transactions will be flushed to disk, while dirty pages will
never be flushed to disk. And after remount, the file will hold stale
data.

Fix the problem by spliting delalloc extent before xfs_flush_unmap_range
is called.

Signed-off-by: yu kuai <yukuai3@xxxxxxxxxx>
---

Changes in V2:
I thought no transaction need to commit when we split a da extent. However,
kernel test robot found that it will cause xfs/011 failed:
XFS: Assertion failed: XFS_FORCED_SHUTDOWN(mp) || percpu_counter_sum(
&mp->m_delalloc_blks) == 0, file: fs/xfs/xfs_super.c, line: 1037
see details in https://patchwork.kernel.org/patch/11310513/

Delete the patch "xfs: introduce xfs_bmap_split_da_extent" and use
xfs_bmap_split_extent instead.

fs/xfs/xfs_file.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index c93250108952..e53da982ca7a 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -786,6 +786,50 @@ xfs_break_layouts(

return error;
}
+static int
+try_split_da_extent(
+ struct xfs_inode *ip,
+ loff_t offset,
+ loff_t len)
+{
+ struct xfs_mount *mp = ip->i_mount;
+ xfs_fileoff_t start = XFS_B_TO_FSBT(mp, offset);
+ xfs_fileoff_t end = XFS_B_TO_FSBT(mp, offset + len - 1);
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
+ struct xfs_iext_cursor cur;
+ struct xfs_bmbt_irec imap;
+ int error;
+
+ /*
+ * if start belong to a delalloc extent and it's not the first block,
+ * split the extent at start.
+ */
+ if (xfs_iext_lookup_extent(ip, ifp, start, &cur, &imap) &&
+ imap.br_startblock != HOLESTARTBLOCK &&
+ isnullstartblock(imap.br_startblock) &&
+ start > imap.br_startoff) {
+ error = xfs_bmap_split_extent(ip, start);
+ if (error)
+ return error;
+ ip->i_d.di_nextents--;
+ }
+
+ /*
+ * if end + 1 belong to a delalloc extent and it's not the first block,
+ * split the extent at end + 1.
+ */
+ if (xfs_iext_lookup_extent(ip, ifp, end + 1, &cur, &imap) &&
+ imap.br_startblock != HOLESTARTBLOCK &&
+ isnullstartblock(imap.br_startblock) &&
+ end + 1 > imap.br_startoff) {
+ error = xfs_bmap_split_extent(ip, end + 1);
+ if (error)
+ return error;
+ ip->i_d.di_nextents--;
+ }
+
+ return 0;
+}

#define XFS_FALLOC_FL_SUPPORTED \
(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | \
@@ -842,6 +886,9 @@ xfs_file_fallocate(
*/
if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE |
FALLOC_FL_COLLAPSE_RANGE)) {
+ error = try_split_da_extent(ip, offset, len);
+ if (error)
+ goto out_unlock;
error = xfs_flush_unmap_range(ip, offset, len);
if (error)
goto out_unlock;
--
2.17.2