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-3-wozizhi@huawei.com>
Date: Mon, 26 Aug 2024 11:10:05 +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 2/2] xfs: Fix incorrect parameter calculation in rt fsmap

I noticed a bug related to xfs realtime device fsmap:
[root@...ora ~]# xfs_io -c 'fsmap -vvvv -r' /mnt
 EXT: DEV    BLOCK-RANGE         OWNER            FILE-OFFSET      AG AG-OFFSET          TOTAL
   0: 253:48 [0..7]:             unknown                                                     8
   1: 253:48 [8..1048575]:       free space                                            1048568
   2: 253:48 [1048576..1050623]: unknown                                                  2048
   3: 253:48 [1050624..2097151]: free space                                            1046528

Bug:
[root@...ora ~]# xfs_io -c 'fsmap -vvvv -r 1050621 1050621' /mnt
 EXT: DEV    BLOCK-RANGE         OWNER            FILE-OFFSET      AG AG-OFFSET        TOTAL
   0: 253:48 [1050621..1050623]: unknown                                                   3
   1: 253:48 [1050624..1050631]: free space                                                8
Normally, we should not get any results, but we do get two queries.

The root cause of this problem lies in the calculation of "end_rtb" in
xfs_getfsmap_rtdev_rtbitmap(), which uses XFS_BB_TO_FSB method (round up).
However, in the subsequent call to xfs_rtalloc_query_range(), "high_rec"
calculated based on "end_rtb" has a semantic meaning of being reachable
within the loop. The first round of the loop in xfs_rtalloc_query_range()
doesn't find any free extents. But after incrementing "rtstart" by 1, start
still does not exceed "high_key", and the second round of the loop entered.
It finds free extent and obtains the first unknown extent by subtracting it
from "info->next_daddr". Even though we can accurately handle it through
"info->end_daddr", two incorrect extents has already been returned before
the last query. The main call stack is as follows:

xfs_getfsmap_rtdev_rtbitmap
  // rounded up
  end_rtb = XFS_BB_TO_FSB(..., keys[1].fmr_physical)
  ahigh.ar_startext = xfs_rtb_to_rtxup(mp, end_rtb)
  xfs_rtalloc_query_range
    // high_key is calculated based on end_rtb
    high_key = min(high_rec->ar_startext, ...)
    while (rtstart <= high_key)
      // First loop, doesn't find free extent
      xfs_rtcheck_range
      rtstart = rtend + 1
      // Second loop, the free extent outside the query interval is found
      xfs_getfsmap_rtdev_rtbitmap_helper
        // unknown and free were printed out together in the second round
        xfs_getfsmap_helper

The issue is resolved by adjusting the relevant calculations. Both the loop
exit condition in the xfs_rtalloc_query_range() and the length calculation
condition (high_key - start + 1) in the xfs_rtfind_forw() reflect the open
interval semantics of "high_key". Therefore, when calculating "end_rtb",
XFS_BB_TO_FSBT is used. In addition, in order to satisfy the close interval
semantics, "key[1].fmr_physical" needs to be decremented by 1. For the
non-eofs case, there is no need to worry about over-counting because we can
accurately count the block number through "info->end_daddr".

After applying this patch, the above problem have been solved:
[root@...ora ~]# xfs_io -c 'fsmap -vvvv -r 1050621 1050621' /mnt
[root@...ora ~]#

Fixes: 4c934c7dd60c ("xfs: report realtime space information via the rtbitmap")
Signed-off-by: Zizhi Wo <wozizhi@...wei.com>
---
 fs/xfs/libxfs/xfs_rtbitmap.c |  4 +---
 fs/xfs/xfs_fsmap.c           | 20 +++++++++++++++++---
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index 386b672c5058..7af4e7afda7d 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -1034,8 +1034,7 @@ xfs_rtalloc_query_range(
 
 	if (low_rec->ar_startext > high_rec->ar_startext)
 		return -EINVAL;
-	if (low_rec->ar_startext >= mp->m_sb.sb_rextents ||
-	    low_rec->ar_startext == high_rec->ar_startext)
+	if (low_rec->ar_startext >= mp->m_sb.sb_rextents)
 		return 0;
 
 	high_key = min(high_rec->ar_startext, mp->m_sb.sb_rextents - 1);
@@ -1057,7 +1056,6 @@ xfs_rtalloc_query_range(
 		if (is_free) {
 			rec.ar_startext = rtstart;
 			rec.ar_extcount = rtend - rtstart + 1;
-
 			error = fn(mp, tp, &rec, priv);
 			if (error)
 				break;
diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
index 8a2dfe96dae7..42c4b94b0493 100644
--- a/fs/xfs/xfs_fsmap.c
+++ b/fs/xfs/xfs_fsmap.c
@@ -515,11 +515,20 @@ xfs_getfsmap_rtdev_rtbitmap(
 	int				error;
 
 	eofs = XFS_FSB_TO_BB(mp, xfs_rtx_to_rtb(mp, mp->m_sb.sb_rextents));
-	if (keys[0].fmr_physical >= eofs)
+	if (keys[0].fmr_physical >= eofs ||
+		keys[0].fmr_physical == keys[1].fmr_physical)
 		return 0;
 	start_rtb = XFS_BB_TO_FSBT(mp,
 				keys[0].fmr_physical + keys[0].fmr_length);
-	end_rtb = XFS_BB_TO_FSB(mp, min(eofs - 1, keys[1].fmr_physical));
+	/*
+	 * The passed keys[1] is an unreachable value, while "end_rtb" is used
+	 * to calculate "ahigh.ar_startext", serving as an input parameter for
+	 * xfs_rtalloc_query_range(), which is a value that can be reached.
+	 * Therefore, it is necessary to use "keys[1].fmr_physical - 1" here.
+	 * And because of the semantics of "end_rtb", it needs to be
+	 * supplemented by 1 in the last calculation.
+	 */
+	end_rtb = XFS_BB_TO_FSBT(mp, min(eofs - 1, keys[1].fmr_physical - 1));
 
 	info->missing_owner = XFS_FMR_OWN_UNKNOWN;
 
@@ -549,9 +558,14 @@ xfs_getfsmap_rtdev_rtbitmap(
 	/*
 	 * Report any gaps at the end of the rtbitmap by simulating a null
 	 * rmap starting at the block after the end of the query range.
+	 * For the boundary case of eofs, we need to increment the count
+	 * by 1 to prevent omission in block statistics.
+	 * For the boundary case of non-eofs, even if incrementing by 1
+	 * may lead to over-counting, it doesn't matter because it is
+	 * handled by "info->end_daddr" in this situation, not "ahigh".
 	 */
 	info->last = true;
-	ahigh.ar_startext = min(mp->m_sb.sb_rextents, ahigh.ar_startext);
+	ahigh.ar_startext = min(mp->m_sb.sb_rextents, ahigh.ar_startext + 1);
 
 	error = xfs_getfsmap_rtdev_rtbitmap_helper(mp, tp, &ahigh, info);
 	if (error)
-- 
2.39.2


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ