linux-kernel - [PATCH 1/2] xfs: Fix missing block calculations in xfs datadev fsmap

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: <20240826031005.2493150-2-wozizhi@huawei.com>
Date: Mon, 26 Aug 2024 11:10:04 +0800
From: Zizhi Wo <wozizhi@...wei.com>
To: <chandan.babu@...cle.com>, <djwong@...nel.org>, <dchinner@...hat.com>,
	<osandov@...com>, <john.g.garry@...cle.com>
CC: <linux-xfs@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<wozizhi@...wei.com>, <yangerkun@...wei.com>
Subject: [PATCH 1/2] xfs: Fix missing block calculations in xfs datadev fsmap

In xfs datadev fsmap query, I noticed a missing block calculation problem:
[root@...ora ~]# xfs_db -r -c "sb 0" -c "p" /dev/vdb
magicnum = 0x58465342
blocksize = 4096
dblocks = 5242880
......
[root@...ora ~]# xfs_io -c 'fsmap -vvvv' /mnt
...
30: 253:16 [31457384..41943031]: free space            3  (104..10485751)    10485648

(41943031 + 1) / 8 = 5242879 != 5242880
We missed one block in our fsmap calculation!

The root cause of the problem lies in __xfs_getfsmap_datadev(), where the
calculation of "end_fsb" requires a classification discussion. If "end_fsb"
is calculated based on "eofs", we need to add an extra sentinel node for
subsequent length calculations. Otherwise, one block will be missed. If
"end_fsb" is calculated based on "keys[1]", then there is no need to add an
extra node. Because "keys[1]" itself is unreachable, it cancels out one of
the additions. The diagram below illustrates this:

|0|1|2|3|4|5|6|7|8|9|10|11|12|13|14|15|16|-----eofs
|---------------|---------------------|
a       n       b         n+1         c

Assume that eofs is 16, the start address of the previous query is block n,
sector 0, and the length is 1, so the "info->next" is at point b, sector 8.
In the last query, suppose the "rm_startblock" calculated based on
"eofs - 1" is the last block n+1 at point b. All we get is the starting
address of the block, not the end. Therefore, an additional sentinel node
needs to be added to move it to point c. After that, subtracting one from
the other will yield the remaining 1.

Although we can now calculate the exact last query using "info->end_daddr",
we will still get an incorrect value if the device at this point is not the
boundary device specified by "keys[1]", as "end_daddr" is still the initial
value. Therefore, the eofs situation here needs to be corrected. The issue
is resolved by adding a sentinel node.

Fixes: e89c041338ed ("xfs: implement the GETFSMAP ioctl")
Signed-off-by: Zizhi Wo <wozizhi@...wei.com>
---
 fs/xfs/xfs_fsmap.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
index 85dbb46452ca..8a2dfe96dae7 100644
--- a/fs/xfs/xfs_fsmap.c
+++ b/fs/xfs/xfs_fsmap.c
@@ -596,12 +596,27 @@ __xfs_getfsmap_datadev(
 	xfs_agnumber_t			end_ag;
 	uint64_t			eofs;
 	int				error = 0;
+	int				sentinel = 0;

 	eofs = XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
 	if (keys[0].fmr_physical >= eofs)
 		return 0;
 	start_fsb = XFS_DADDR_TO_FSB(mp, keys[0].fmr_physical);
-	end_fsb = XFS_DADDR_TO_FSB(mp, min(eofs - 1, keys[1].fmr_physical));
+	/*
+	 * For the case of eofs, we need to add a sentinel node;
+	 * otherwise, one block will be missed when calculating the length
+	 * in the last query.
+	 * For the case of key[1], there is no need to add a sentinel node
+	 * because it already represents a value that cannot be reached.
+	 * For the case where key[1] after shifting is within the same
+	 * block as the starting address, it is resolved using end_daddr.
+	 */
+	if (keys[1].fmr_physical > eofs - 1) {
+		sentinel = 1;
+		end_fsb = XFS_DADDR_TO_FSB(mp, eofs - 1);
+	} else {
+		end_fsb = XFS_DADDR_TO_FSB(mp, keys[1].fmr_physical);
+	}

 	/*
 	 * Convert the fsmap low/high keys to AG based keys.  Initialize
@@ -649,7 +664,7 @@ __xfs_getfsmap_datadev(
 		info->pag = pag;
 		if (pag->pag_agno == end_ag) {
 			info->high.rm_startblock = XFS_FSB_TO_AGBNO(mp,
-					end_fsb);
+					end_fsb) + sentinel;
 			info->high.rm_offset = XFS_BB_TO_FSBT(mp,
 					keys[1].fmr_offset);
 			error = xfs_fsmap_owner_to_rmap(&info->high, &keys[1]);
-- 
2.39.2