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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191228033404.14654-1-yukuai3@huawei.com>
Date:   Sat, 28 Dec 2019 11:34:04 +0800
From:   yu kuai <yukuai3@...wei.com>
To:     <darrick.wong@...cle.com>, <bfoster@...hat.com>,
        <dchinner@...hat.com>, <sandeen@...deen.net>,
        <cmaiolino@...hat.com>, <hch@....de>
CC:     <linux-xfs@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <yukuai3@...wei.com>, <yi.zhang@...wei.com>,
        <zhengbin13@...wei.com>, <houtao1@...wei.com>
Subject: [PATCH V2] xfs: fix stale data exposure problem when punch hole, collapse range or zero range across a delalloc extent

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@...wei.com>
---

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