Skip to contentSkip to conversation

slab: Decouple slab_debug and no_hash_pointers

PATCH Superseded Kees Cook 5 participants 12 messages Apr 15, 2025 – Jun 10, 2025

Subsystems (5)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH v2 1/1 slab: Decouple slab_debug and no_hash_pointers

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

---
v2: use switch state to avoid missing states (pmladek)
v1: https://lore.kernel.org/lkml/[email protected]/
Cc: Petr Mladek <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Roman Gushchin <[email protected]>
Cc: Harry Yoo <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Tamir Duberstein <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Cc: Alice Ryhl <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
 .../admin-guide/kernel-parameters.txt         | 34 +++++++----
 include/linux/sprintf.h                       |  2 +-
 lib/vsprintf.c                                | 61 +++++++++++++++++--
 mm/slub.c                                     |  5 +-
 4 files changed, 82 insertions(+), 20 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +22 -12
index 76e538c77e31..d0fd9c745db9 100644
@@ -1798,6 +1798,27 @@
 			backtraces on all cpus.
 			Format: 0 | 1
 
+	hash_pointers=
+			[KNL,EARLY]
+			By default, when pointers are printed to the console
+			or buffers via the %p format string, that pointer is
+			"hashed", i.e. obscured by hashing the pointer value.
+			This is a security feature that hides actual kernel
+			addresses from unprivileged users, but it also makes
+			debugging the kernel more difficult since unequal
+			pointers can no longer be compared. The choices are:
+			Format: { auto | always | never }
+			Default: auto
+
+			auto   - Hash pointers unless slab_debug is enabled.
+			always - Always hash pointers (even if slab_debug is
+				 enabled).
+			never  - Never hash pointers. This option should only
+				 be specified when debugging the kernel. Do
+				 not use on production kernels. The boot
+				 param "no_hash_pointers" is an alias for
+				 this mode.
+
 	hashdist=	[KNL,NUMA] Large hashes allocated during boot
 			are distributed across NUMA nodes.  Defaults on
 			for 64-bit NUMA, off otherwise.
@@ -4120,18 +4141,7 @@
 
 	no_hash_pointers
 			[KNL,EARLY]
-			Force pointers printed to the console or buffers to be
-			unhashed.  By default, when a pointer is printed via %p
-			format string, that pointer is "hashed", i.e. obscured
-			by hashing the pointer value.  This is a security feature
-			that hides actual kernel addresses from unprivileged
-			users, but it also makes debugging the kernel more
-			difficult since unequal pointers can no longer be
-			compared.  However, if this command-line option is
-			specified, then all normal pointers will have their true
-			value printed. This option should only be specified when
-			debugging the kernel.  Please do not use on production
-			kernels.
+			Alias for "hash_pointers=never".
 
 	nohibernate	[HIBERNATION] Disable hibernation and resume.
 
include/linux/sprintf.h +1 -1
index 51cab2def9ec..521bb2cd2648 100644
@@ -22,7 +22,7 @@ __scanf(2, 0) int vsscanf(const char *, const char *, va_list);
 
 /* These are for specific cases, do not use without real need */
 extern bool no_hash_pointers;
-int no_hash_pointers_enable(char *str);
+void hash_pointers_finalize(bool slub_debug);
 
 /* Used for Rust formatting ('%pA') */
 char *rust_fmt_argument(char *buf, char *end, const void *ptr);
lib/vsprintf.c +57 -4

86 lines not on this page · raw message

mm/slub.c +2 -3
index b46f87662e71..f3d61b330a76 100644
@@ -6314,9 +6314,8 @@ void __init kmem_cache_init(void)
 	if (debug_guardpage_minorder())
 		slub_max_order = 0;
 
-	/* Print slub debugging pointers without hashing */
-	if (__slub_debug_enabled())
-		no_hash_pointers_enable(NULL);
+	/* Inform pointer hashing choice about slub debugging state. */
+	hash_pointers_finalize(__slub_debug_enabled());
 
 	kmem_cache_node = &boot_kmem_cache_node;
 	kmem_cache = &boot_kmem_cache;
Signature · 1 line
2.34.1
maintainer Tested-by Reviewed-by on v2 1/1
#1 lore raw
Quoted 11 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 8 lines

I am going to wait few more days for a potential feedback. I'll queue it for 6.16 unless anyone complains.

See a rant below ;-)

Quoted 58 lines
lib/vsprintf.c +37 -4
@@ -2271,12 +2285,23 @@ char *resource_or_range(const char *fmt, char *buf, char *end, void *ptr,
 	return resource_string(buf, end, ptr, spec, fmt);
 }
 
-int __init no_hash_pointers_enable(char *str)
+void __init hash_pointers_finalize(bool slub_debug)
 {
-	if (no_hash_pointers)
-		return 0;
+	switch (hash_pointers_mode) {
+	case HASH_PTR_ALWAYS:
+		no_hash_pointers = false;
+		break;
+	case HASH_PTR_NEVER:
+		no_hash_pointers = true;
+		break;
+	case HASH_PTR_AUTO:
+	default:
+		no_hash_pointers = slub_debug;
+		break;
+	}
 
-	no_hash_pointers = true;
+	if (!no_hash_pointers)
+		return;
 
 	pr_warn("**********************************************************\n");
 	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
@@ -2289,11 +2314,39 @@ int __init no_hash_pointers_enable(char *str)
 	pr_warn("** the kernel, report this immediately to your system   **\n");
 	pr_warn("** administrator!                                       **\n");
 	pr_warn("**                                                      **\n");
+	pr_warn("** Use hash_pointers=always to force this mode off      **\n");
+	pr_warn("**                                                      **\n");
 	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
 	pr_warn("**********************************************************\n");
+}
+
+static int __init hash_pointers_mode_parse(char *str)
+{
+	if (!str) {
+		pr_warn("Hash pointers mode empty; falling back to auto.\n");
+		hash_pointers_mode = HASH_PTR_AUTO;
+	} else if (strncmp(str, "auto", 4) == 0)   {
+		pr_info("Hash pointers mode set to auto.\n");
+		hash_pointers_mode = HASH_PTR_AUTO;
+	} else if (strncmp(str, "never", 5) == 0) {
+		pr_info("Hash pointers mode set to never.\n");
+		hash_pointers_mode = HASH_PTR_NEVER;
+	} else if (strncmp(str, "always", 6) == 0) {
+		pr_info("Hash pointers mode set to always.\n");
+		hash_pointers_mode = HASH_PTR_ALWAYS;
+	} else {
+		pr_warn("Unknown hash_pointers mode '%s' specified; assuming auto.\n", str);
+		hash_pointers_mode = HASH_PTR_AUTO;
+	}

lib/vsprintf.c int __init no_hash_pointers_enable(char *str) · line 2350

Quoted 10 lines
lib/vsprintf.c +6 -0
 
 	return 0;
 }
+early_param("hash_pointers", hash_pointers_mode_parse);
+
+static int __init no_hash_pointers_enable(char *str)
+{
+	return hash_pointers_mode_parse("never");
+}
 early_param("no_hash_pointers", no_hash_pointers_enable);

I personally do not like that these two parameters do not have the real effect until hash_pointers_finalize() is called at some "random" "unrelated" location, namely kmem_cache_init(). But I could live with it.

But the alternative solution proposed at
https://lore.kernel.org/r/[email protected]
was hairy another way.

We could always improve it when it causes troubles.

Best Regards,
Petr

author
#2 lore raw
Quoted 21 lines · Petr Mladek
Quoted 11 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 8 lines
Quoted 5 lines

I am going to wait few more days for a potential feedback. I'll queue it for 6.16 unless anyone complains.

Thanks very much!

Quoted 6 lines

See a rant below ;-)
[...]
I personally do not like that these two parameters do not have the real effect until hash_pointers_finalize() is called at some "random" "unrelated" location, namely kmem_cache_init(). But I could live with it.

Yeah, this is mainly due to slab_debug wanting to be able to control it. I'd prefer they weren't linked at all, but it's also not too unreasonable.

Quoted 5 lines

But the alternative solution proposed at
https://lore.kernel.org/r/[email protected]
was hairy another way.

We could always improve it when it causes troubles.

Right. My thinking is that if another subsystem comes along with the same compelling complaint as kmem, we can look at it again then. IMO, really only an allocator should be in charge of such a large knob -- its whole job is managing pointers. :)

-Kees

Signature · 1 line
Kees Cook
author
#3 lore raw
In reply to Petr Mladek
Quoted 21 lines · Petr Mladek
Quoted 11 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 8 lines
Quoted 5 lines

I am going to wait few more days for a potential feedback. I'll queue it for 6.16 unless anyone complains.

Hi,

Is this still planned for 6.16? I just noticed it wasn't in -next, and we're almost to the end of the merge window...

-Kees

Signature · 1 line
Kees Cook
maintainer
#4 lore raw
Quoted 25 lines · Kees Cook
Quoted 24 lines · Petr Mladek
Quoted 11 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 8 lines
Quoted 7 lines
Quoted 2 lines

I am going to wait few more days for a potential feedback. I'll queue it for 6.16 unless anyone complains.

Hi,

Is this still planned for 6.16? I just noticed it wasn't in -next, and we're almost to the end of the merge window...

I am terribly sorry. I have forgot this patch...

I have just pushed it into printk/linux.git, branch for-6.17-hash_pointers.

Also I updated the documentation, aka applied the diff from https://lore.kernel.org/r/202504181307.254F81843@keescook
See
https://web.git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git/commit/?h=for-6.17-hash_pointers&id=de1c831a7898f164c1c2703c6b2b9e4fb4bebefc

As the branch name suggests, I am going to push it for 6.17. If it is too late for you then please let me know.

Best Regards,
Petr

author
#5 lore raw
Quoted 39 lines · Petr Mladek
Quoted 25 lines · Kees Cook
Quoted 24 lines · Petr Mladek
Quoted 11 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 8 lines
Quoted 7 lines
Quoted 2 lines

I am going to wait few more days for a potential feedback. I'll queue it for 6.16 unless anyone complains.

Hi,

Is this still planned for 6.16? I just noticed it wasn't in -next, and we're almost to the end of the merge window...

I am terribly sorry. I have forgot this patch...

I have just pushed it into printk/linux.git, branch for-6.17-hash_pointers.

Quoted 7 lines

Also I updated the documentation, aka applied the diff from https://lore.kernel.org/r/202504181307.254F81843@keescook
See
https://web.git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git/commit/?h=for-6.17-hash_pointers&id=de1c831a7898f164c1c2703c6b2b9e4fb4bebefc

As the branch name suggests, I am going to push it for 6.17. If it is too late for you then please let me know.

As long as it's in -next and scheduled to land, I'm happy. I'd always like it earlier, but the less workflow disruption the better! :)

Thanks!

-Kees

Signature · 1 line
Kees Cook
#6 lore raw
Quoted 1 line · Kees Cook

On Mon, Jun 09, 2025 at 04:39:25PM +0200, Petr Mladek wrote:

...

Quoted 2 lines

As long as it's in -next and scheduled to land, I'm happy. I'd always like it earlier, but the less workflow disruption the better! :)

Hmm... The __diag patch series were in Linux Next for a few weeks and did not land in v6.16-rc1. Just saying that there is always chance to go into cracks.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#7 lore raw
Quoted 10 lines · Andy Shevchenko
Quoted 1 line · Kees Cook

On Mon, Jun 09, 2025 at 04:39:25PM +0200, Petr Mladek wrote:

...

Quoted 2 lines

As long as it's in -next and scheduled to land, I'm happy. I'd always like it earlier, but the less workflow disruption the better! :)

Hmm... The __diag patch series were in Linux Next for a few weeks and did not land in v6.16-rc1. Just saying that there is always chance to go into cracks.

Urgh, I am terribly sorry for this. I forgot to do it the other week. And I missed it in the merge window because it was in for-6.15* branch. I had a feeling that I have forgot something but I did not find it.

OK, I guess that I am getting older or going through some more distracting times. And have to update my process.

I have just created a list of branches which I have to merge for 6.17. I hope that it will do the trick.

Best Regards,
Petr

Acked-by on v2 1/1
#8 lore raw
In reply to Kees Cook
Quoted 228 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

---
v2: use switch state to avoid missing states (pmladek)
v1: https://lore.kernel.org/lkml/[email protected]/
Cc: Petr Mladek <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Roman Gushchin <[email protected]>
Cc: Harry Yoo <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Tamir Duberstein <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Cc: Alice Ryhl <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
 .../admin-guide/kernel-parameters.txt         | 34 +++++++----
 include/linux/sprintf.h                       |  2 +-
 lib/vsprintf.c                                | 61 +++++++++++++++++--
 mm/slub.c                                     |  5 +-
 4 files changed, 82 insertions(+), 20 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +22 -12
index 76e538c77e31..d0fd9c745db9 100644
@@ -1798,6 +1798,27 @@
 			backtraces on all cpus.
 			Format: 0 | 1
 
+	hash_pointers=
+			[KNL,EARLY]
+			By default, when pointers are printed to the console
+			or buffers via the %p format string, that pointer is
+			"hashed", i.e. obscured by hashing the pointer value.
+			This is a security feature that hides actual kernel
+			addresses from unprivileged users, but it also makes
+			debugging the kernel more difficult since unequal
+			pointers can no longer be compared. The choices are:
+			Format: { auto | always | never }
+			Default: auto
+
+			auto   - Hash pointers unless slab_debug is enabled.
+			always - Always hash pointers (even if slab_debug is
+				 enabled).
+			never  - Never hash pointers. This option should only
+				 be specified when debugging the kernel. Do
+				 not use on production kernels. The boot
+				 param "no_hash_pointers" is an alias for
+				 this mode.
+
 	hashdist=	[KNL,NUMA] Large hashes allocated during boot
 			are distributed across NUMA nodes.  Defaults on
 			for 64-bit NUMA, off otherwise.
@@ -4120,18 +4141,7 @@
 
 	no_hash_pointers
 			[KNL,EARLY]
-			Force pointers printed to the console or buffers to be
-			unhashed.  By default, when a pointer is printed via %p
-			format string, that pointer is "hashed", i.e. obscured
-			by hashing the pointer value.  This is a security feature
-			that hides actual kernel addresses from unprivileged
-			users, but it also makes debugging the kernel more
-			difficult since unequal pointers can no longer be
-			compared.  However, if this command-line option is
-			specified, then all normal pointers will have their true
-			value printed. This option should only be specified when
-			debugging the kernel.  Please do not use on production
-			kernels.
+			Alias for "hash_pointers=never".
 
 	nohibernate	[HIBERNATION] Disable hibernation and resume.
 
include/linux/sprintf.h +1 -1
index 51cab2def9ec..521bb2cd2648 100644
@@ -22,7 +22,7 @@ __scanf(2, 0) int vsscanf(const char *, const char *, va_list);
 
 /* These are for specific cases, do not use without real need */
 extern bool no_hash_pointers;
-int no_hash_pointers_enable(char *str);
+void hash_pointers_finalize(bool slub_debug);
 
 /* Used for Rust formatting ('%pA') */
 char *rust_fmt_argument(char *buf, char *end, const void *ptr);
lib/vsprintf.c +57 -4
index 01699852f30c..22cbd75266ef 100644
@@ -60,6 +60,20 @@
 bool no_hash_pointers __ro_after_init;
 EXPORT_SYMBOL_GPL(no_hash_pointers);
 
+/*
+ * Hashed pointers policy selected by "hash_pointers=..." boot param
+ *
+ * `auto`   - Hashed pointers enabled unless disabled by slub_debug_enabled=true
+ * `always` - Hashed pointers enabled unconditionally
+ * `never`  - Hashed pointers disabled unconditionally
+ */
+enum hash_pointers_policy {
+	HASH_PTR_AUTO = 0,
+	HASH_PTR_ALWAYS,
+	HASH_PTR_NEVER
+};
+static enum hash_pointers_policy hash_pointers_mode __initdata;
+
 noinline
 static unsigned long long simple_strntoull(const char *startp, char **endp, unsigned int base, size_t max_chars)
 {
@@ -2271,12 +2285,23 @@ char *resource_or_range(const char *fmt, char *buf, char *end, void *ptr,
 	return resource_string(buf, end, ptr, spec, fmt);
 }
 
-int __init no_hash_pointers_enable(char *str)
+void __init hash_pointers_finalize(bool slub_debug)
 {
-	if (no_hash_pointers)
-		return 0;
+	switch (hash_pointers_mode) {
+	case HASH_PTR_ALWAYS:
+		no_hash_pointers = false;
+		break;
+	case HASH_PTR_NEVER:
+		no_hash_pointers = true;
+		break;
+	case HASH_PTR_AUTO:
+	default:
+		no_hash_pointers = slub_debug;
+		break;
+	}
 
-	no_hash_pointers = true;
+	if (!no_hash_pointers)
+		return;
 
 	pr_warn("**********************************************************\n");
 	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
@@ -2289,11 +2314,39 @@ int __init no_hash_pointers_enable(char *str)
 	pr_warn("** the kernel, report this immediately to your system   **\n");
 	pr_warn("** administrator!                                       **\n");
 	pr_warn("**                                                      **\n");
+	pr_warn("** Use hash_pointers=always to force this mode off      **\n");
+	pr_warn("**                                                      **\n");
 	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
 	pr_warn("**********************************************************\n");
+}
+
+static int __init hash_pointers_mode_parse(char *str)
+{
+	if (!str) {
+		pr_warn("Hash pointers mode empty; falling back to auto.\n");
+		hash_pointers_mode = HASH_PTR_AUTO;
+	} else if (strncmp(str, "auto", 4) == 0)   {
+		pr_info("Hash pointers mode set to auto.\n");
+		hash_pointers_mode = HASH_PTR_AUTO;
+	} else if (strncmp(str, "never", 5) == 0) {
+		pr_info("Hash pointers mode set to never.\n");
+		hash_pointers_mode = HASH_PTR_NEVER;
+	} else if (strncmp(str, "always", 6) == 0) {
+		pr_info("Hash pointers mode set to always.\n");
+		hash_pointers_mode = HASH_PTR_ALWAYS;
+	} else {
+		pr_warn("Unknown hash_pointers mode '%s' specified; assuming auto.\n", str);
+		hash_pointers_mode = HASH_PTR_AUTO;
+	}
 
 	return 0;
 }
+early_param("hash_pointers", hash_pointers_mode_parse);
+
+static int __init no_hash_pointers_enable(char *str)
+{
+	return hash_pointers_mode_parse("never");
+}
 early_param("no_hash_pointers", no_hash_pointers_enable);
 
 /*
mm/slub.c +2 -3
index b46f87662e71..f3d61b330a76 100644
@@ -6314,9 +6314,8 @@ void __init kmem_cache_init(void)
 	if (debug_guardpage_minorder())
 		slub_max_order = 0;
 
-	/* Print slub debugging pointers without hashing */
-	if (__slub_debug_enabled())
-		no_hash_pointers_enable(NULL);
+	/* Inform pointer hashing choice about slub debugging state. */
+	hash_pointers_finalize(__slub_debug_enabled());
 
 	kmem_cache_node = &boot_kmem_cache_node;
 	kmem_cache = &boot_kmem_cache;
Signature · 1 line
2.34.1

Thank you, Kees.

Reviewed-by on v2 1/1
#9 lore raw
In reply to Kees Cook
Quoted 3 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Is this behavior documented somewhere or it's only in the code? I couldn't find anything other than the code.

Quoted 15 lines

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 1 line

---

By the way, while this patch does not change existing behavior of slub_debug implying no_hash_pointers, kmem_cache_init() is not the only place that enables slub_debug_enabled static key.

Maybe we should update __kmem_cache_create_args() too? (in a separate patch)

--
Cheers,
Harry / Hyeonggon

author
#10 lore raw
Quoted 7 lines · Harry Yoo
Quoted 3 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Is this behavior documented somewhere or it's only in the code? I couldn't find anything other than the code.

Hmm, that's an excellent point. I don't see any mention of it in kernel-parameters.txt. Perhaps this?

Documentation/admin-guide/kernel-parameters.txt +4 -0
index 4568572205ee..982e6511a225 100644
@@ -6483,6 +6483,10 @@
 			Documentation/mm/slub.rst.
 			(slub_debug legacy name also accepted for now)
 
+			Using this option implies the "no_hash_pointers"
+			option which can be undone by adding the
+			"hash_pointers=always" option.
+
 	slab_max_order= [MM]
 			Determines the maximum allowed order for slabs.
 			A high setting may cause OOMs due to memory
Quoted 18 lines
Quoted 16 lines

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 1 line

---

Quoted 8 lines

By the way, while this patch does not change existing behavior of slub_debug implying no_hash_pointers, kmem_cache_init() is not the only place that enables slub_debug_enabled static key.

Maybe we should update __kmem_cache_create_args() too? (in a separate patch)

The state of pointer hashing should not change after boot. (It is intentionally designed to use __ro_after_init.) Honestly, I'd prefer that slab_debug was not tied to no_hash_pointers at all...

-Kees

Signature · 1 line
Kees Cook
#11 lore raw
Quoted 17 lines · Kees Cook
Quoted 7 lines · Harry Yoo
Quoted 3 lines · Kees Cook

Some system owners use slab_debug=FPZ (or similar) as a hardening option, but do not want to be forced into having kernel addresses exposed due to the implicit "no_hash_pointers" boot param setting.[1]

Is this behavior documented somewhere or it's only in the code? I couldn't find anything other than the code.

Hmm, that's an excellent point. I don't see any mention of it in kernel-parameters.txt. Perhaps this?

Documentation/admin-guide/kernel-parameters.txt +0 -0
index 4568572205ee..982e6511a225 100644

Documentation/admin-guide/kernel-parameters.txt line 6489

Quoted 7 lines
Documentation/admin-guide/kernel-parameters.txt +4 -0
@@ -6483,6 +6483,10 @@
 			Documentation/mm/slub.rst.
 			(slub_debug legacy name also accepted for now)
 
+			Using this option implies the "no_hash_pointers"
+			option which can be undone by adding the
+			"hash_pointers=always" option.
+

Looks good to me as it correctly states current behavior.

Quoted 28 lines
Documentation/admin-guide/kernel-parameters.txt +0 -0
 	slab_max_order= [MM]
 			Determines the maximum allowed order for slabs.
 			A high setting may cause OOMs due to memory
Quoted 25 lines
Quoted 17 lines

Introduce the "hash_pointers" boot param, which defaults to "auto" (the current behavior), but also includes "always" (forcing on hashing even when "slab_debug=..." is defined), and "never". The existing "no_hash_pointers" boot param becomes an alias for "hash_pointers=never".

This makes it possible to boot with "slab_debug=FPZ hash_pointers=always".

Quoted 1 line

---

By the way, while this patch does not change existing behavior of slub_debug implying no_hash_pointers, kmem_cache_init() is not the only place that enables slub_debug_enabled static key.

Quoted 5 lines
Quoted 2 lines

Maybe we should update __kmem_cache_create_args() too? (in a separate patch)

The state of pointer hashing should not change after boot. (It is intentionally designed to use __ro_after_init.)

Okay. In security perspective that makes sense.

IIRC the only case that dynamically creates debug caches is rcutorture so probably doesn't matter that much.

Signature · 2 lines
Cheers,
Harry / Hyeonggon