linux-kernel - [PATCH] stackdepot: Rename pool_index to pool_index_plus

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: <20240402001500.53533-1-pcc@google.com>
Date: Mon,  1 Apr 2024 17:14:58 -0700
From: Peter Collingbourne <pcc@...gle.com>
To: Andrey Konovalov <andreyknvl@...il.com>, Oscar Salvador <osalvador@...e.de>
Cc: Peter Collingbourne <pcc@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
	Michal Hocko <mhocko@...e.com>, Vlastimil Babka <vbabka@...e.cz>, Marco Elver <elver@...gle.com>, 
	Alexander Potapenko <glider@...gle.com>, Omar Sandoval <osandov@...com>
Subject: [PATCH] stackdepot: Rename pool_index to pool_index_plus_1

Commit 3ee34eabac2a ("lib/stackdepot: fix first entry having a
0-handle") changed the meaning of the pool_index field to mean "the
pool index plus 1". This made the code accessing this field less
self-documenting, as well as causing debuggers such as drgn to not
be able to easily remain compatible with both old and new kernels,
because they typically do that by testing for presence of the new
field. Because stackdepot is a debugging tool, we should make sure
that it is debugger friendly. Therefore, give the field a different
name to improve readability as well as enabling debugger backwards
compatibility.

Signed-off-by: Peter Collingbourne <pcc@...gle.com>
Link: https://linux-review.googlesource.com/id/Ib3e70c36c1d230dd0a118dc22649b33e768b9f88
---
Although this technically isn't a bug fix in the kernel,
I would appreciate if this could be picked up for 6.9
to avoid temporarily introducing a silent regression in drgn
(loud regressions are fine).

 include/linux/stackdepot.h | 7 +++----
 lib/stackdepot.c           | 4 ++--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 3c6caa5abc7c..e9ec32fb97d4 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -44,10 +44,9 @@ typedef u32 depot_stack_handle_t;
 union handle_parts {
 	depot_stack_handle_t handle;
 	struct {
-		/* pool_index is offset by 1 */
-		u32 pool_index	: DEPOT_POOL_INDEX_BITS;
-		u32 offset	: DEPOT_OFFSET_BITS;
-		u32 extra	: STACK_DEPOT_EXTRA_BITS;
+		u32 pool_index_plus_1	: DEPOT_POOL_INDEX_BITS;
+		u32 offset		: DEPOT_OFFSET_BITS;
+		u32 extra		: STACK_DEPOT_EXTRA_BITS;
 	};
 };
 
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index af6cc19a2003..68c97387aa54 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -330,7 +330,7 @@ static struct stack_record *depot_pop_free_pool(void **prealloc, size_t size)
 	stack = current_pool + pool_offset;
 
 	/* Pre-initialize handle once. */
-	stack->handle.pool_index = pool_index + 1;
+	stack->handle.pool_index_plus_1 = pool_index + 1;
 	stack->handle.offset = pool_offset >> DEPOT_STACK_ALIGN;
 	stack->handle.extra = 0;
 	INIT_LIST_HEAD(&stack->hash_list);
@@ -441,7 +441,7 @@ static struct stack_record *depot_fetch_stack(depot_stack_handle_t handle)
 	const int pools_num_cached = READ_ONCE(pools_num);
 	union handle_parts parts = { .handle = handle };
 	void *pool;
-	u32 pool_index = parts.pool_index - 1;
+	u32 pool_index = parts.pool_index_plus_1 - 1;
 	size_t offset = parts.offset << DEPOT_STACK_ALIGN;
 	struct stack_record *stack;
 
-- 
2.44.0.478.gd926399ef9-goog