linux-kernel - [PATCH] btrfs: simplify copy_to_sk() by removing unnecessary min key check

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>] [day] [month] [year] [list]

Message-ID: <20250227073509.1764-1-sunk67188@gmail.com>
Date: Thu, 27 Feb 2025 15:35:04 +0800
From: Sun YangKai <sunk67188@...il.com>
To: dsterba@...e.com
Cc: Sun YangKai <sunk67188@...il.com>,
	Chris Mason <clm@...com>,
	Josef Bacik <josef@...icpanda.com>,
	linux-btrfs@...r.kernel.org (open list:BTRFS FILE SYSTEM),
	linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] btrfs: simplify copy_to_sk() by removing unnecessary min key check

The copy_to_sk() function, which is used solely by
search_ioctl(), previously relied on the helper function
key_in_sk() to evaluate whether a key fell within
the range defined by the search key (sk). This function
checked the key against both the minimum and maximum
bounds in the search key.

However, within search_ioctl(), the btrfs_search_forward()
ensures that the search starts at an item whose key
is already no less than min_key. This guarantees that
no keys below the minimum threshold will be encountered
during the traversal. As a result, the check against
the minimum key is redundant and can be safely removed.

This patch:

* Removes the key_in_sk() helper function, as
  it is no longer needed.
* Refactors copy_to_sk() to directly compare keysi
  with the max_key from the search key. If a key
  exceeds max_key, the traversal stops early since
  all relevant items have been processed.
* Remove the inline mark for copy_to_sk(), since
  it is only called once in search_ioctl().
* Updates the comments in copy_to_sk() to clarify
  the reasoning behind removing the min_key check
  and to document the new logic.

By removing the redundant check and simplifying the
traversal logic, this patch improves both readability
and runtime efficiency for this code path.

Signed-off-by: Sun YangKai <sunk67188@...il.com>
---
 fs/btrfs/ioctl.c | 57 ++++++++++++++++++++----------------------------
 1 file changed, 24 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 61c4c6ac8994..abf1ce7e9208 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1447,31 +1447,10 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file,
 	return ret;
 }
 
-static noinline int key_in_sk(struct btrfs_key *key,
-			      struct btrfs_ioctl_search_key *sk)
-{
-	struct btrfs_key test;
-	int ret;
-
-	test.objectid = sk->min_objectid;
-	test.type = sk->min_type;
-	test.offset = sk->min_offset;
-
-	ret = btrfs_comp_cpu_keys(key, &test);
-	if (ret < 0)
-		return 0;
-
-	test.objectid = sk->max_objectid;
-	test.type = sk->max_type;
-	test.offset = sk->max_offset;
-
-	ret = btrfs_comp_cpu_keys(key, &test);
-	if (ret > 0)
-		return 0;
-	return 1;
-}
-
-static noinline int copy_to_sk(struct btrfs_path *path,
+/*
+ * This is a helper function only used by search_ioctl()
+ */
+static int copy_to_sk(struct btrfs_path *path,
 			       struct btrfs_key *key,
 			       struct btrfs_ioctl_search_key *sk,
 			       u64 *buf_size,
@@ -1490,6 +1469,9 @@ static noinline int copy_to_sk(struct btrfs_path *path,
 	int slot;
 	int ret = 0;
 
+	test.objectid = sk->max_objectid;
+	test.type = sk->max_type;
+	test.offset = sk->max_offset;
 	leaf = path->nodes[0];
 	slot = path->slots[0];
 	nritems = btrfs_header_nritems(leaf);
@@ -1505,8 +1487,22 @@ static noinline int copy_to_sk(struct btrfs_path *path,
 		item_len = btrfs_item_size(leaf, i);
 
 		btrfs_item_key_to_cpu(leaf, key, i);
-		if (!key_in_sk(key, sk))
-			continue;
+		/*
+		 * The btrfs_search_forward() ensures that the key of
+		 * the returned slot should be no less than the min_key.
+		 * This guarantees that no keys below the minimum threshold
+		 * will be encountered during the traversal.
+		 * So we only need to check the max key here.
+		 *
+		 * If a key greater than max_key is found,
+		 * no more keys in range can be found in following slots
+		 * and all keys in range have been found and copied to
+		 * the buffer. So we should return 1 here.
+		 */
+		if (btrfs_comp_cpu_keys(key, &test) > 0) {
+			ret = 1;
+			goto out;
+		}
 
 		if (sizeof(sh) + item_len > *buf_size) {
 			if (*num_found) {
@@ -1575,12 +1571,7 @@ static noinline int copy_to_sk(struct btrfs_path *path,
 	}
 advance_key:
 	ret = 0;
-	test.objectid = sk->max_objectid;
-	test.type = sk->max_type;
-	test.offset = sk->max_offset;
-	if (btrfs_comp_cpu_keys(key, &test) >= 0)
-		ret = 1;
-	else if (key->offset < (u64)-1)
+	if (key->offset < (u64)-1)
 		key->offset++;
 	else if (key->type < (u8)-1) {
 		key->offset = 0;
-- 
2.48.1