linux-kernel - [PATCH 2/2] 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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191226134721.43797-3-yukuai3@huawei.com>
Date:   Thu, 26 Dec 2019 21:47:21 +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>, <zhengbin13@...wei.com>,
        <yi.zhang@...wei.com>, <houtao1@...wei.com>
Subject: [PATCH 2/2] 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>
---
 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..5398102feec9 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -786,6 +786,50 @@ xfs_break_layouts(
 
 	return error;
 }
+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_da_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_da_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