[PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[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".

Link: https://github.com/KSPP/linux/issues/368 [1]
Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: David Rientjes <rientjes@google.com>
Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
Signed-off-by: Kees Cook <kees@kernel.org>
---
 v2: use switch state to avoid missing states (pmladek)
 v1: https://lore.kernel.org/lkml/20250410174428.work.488-kees@kernel.org/
Cc: Petr Mladek <pmladek@suse.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Harry Yoo <harry.yoo@oracle.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Tamir Duberstein <tamird@gmail.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-mm@kvack.org
---
 .../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(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 76e538c77e31..d0fd9c745db9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -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.
 
diff --git a/include/linux/sprintf.h b/include/linux/sprintf.h
index 51cab2def9ec..521bb2cd2648 100644
--- a/include/linux/sprintf.h
+++ b/include/linux/sprintf.h
@@ -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);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 01699852f30c..22cbd75266ef 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -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);
 
 /*
diff --git a/mm/slub.c b/mm/slub.c
index b46f87662e71..f3d61b330a76 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -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;
-- 
2.34.1

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Petr Mladek]

On Tue 2025-04-15 10:02:33, Kees Cook wrote:
> 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".
> 
> Link: https://github.com/KSPP/linux/issues/368 [1]
> Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: David Rientjes <rientjes@google.com>
> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Kees Cook <kees@kernel.org>

Tested-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>

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 ;-)

> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -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);

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/Z_0AFjai6Bvg-YLD@pathway.suse.cz
was hairy another way.

We could always improve it when it causes troubles.

Best Regards,
Petr

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Kees Cook]

On Wed, Apr 16, 2025 at 02:06:21PM +0200, Petr Mladek wrote:
> On Tue 2025-04-15 10:02:33, Kees Cook wrote:
> > 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".
> > 
> > Link: https://github.com/KSPP/linux/issues/368 [1]
> > Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> > Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > Acked-by: David Rientjes <rientjes@google.com>
> > Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > Signed-off-by: Kees Cook <kees@kernel.org>
> 
> Tested-by: Petr Mladek <pmladek@suse.com>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> 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!

> 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.

> But the alternative solution proposed at
> https://lore.kernel.org/r/Z_0AFjai6Bvg-YLD@pathway.suse.cz
> 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

-- 
Kees Cook

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Kees Cook]

On Wed, Apr 16, 2025 at 02:06:21PM +0200, Petr Mladek wrote:
> On Tue 2025-04-15 10:02:33, Kees Cook wrote:
> > 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".
> > 
> > Link: https://github.com/KSPP/linux/issues/368 [1]
> > Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> > Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > Acked-by: David Rientjes <rientjes@google.com>
> > Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > Signed-off-by: Kees Cook <kees@kernel.org>
> 
> Tested-by: Petr Mladek <pmladek@suse.com>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> 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

-- 
Kees Cook

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Petr Mladek]

On Thu 2025-06-05 13:15:32, Kees Cook wrote:
> On Wed, Apr 16, 2025 at 02:06:21PM +0200, Petr Mladek wrote:
> > On Tue 2025-04-15 10:02:33, Kees Cook wrote:
> > > 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".
> > > 
> > > Link: https://github.com/KSPP/linux/issues/368 [1]
> > > Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> > > Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > > Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > > Acked-by: David Rientjes <rientjes@google.com>
> > > Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > > Signed-off-by: Kees Cook <kees@kernel.org>
> > 
> > Tested-by: Petr Mladek <pmladek@suse.com>
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> > 
> > 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

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Kees Cook]

On Mon, Jun 09, 2025 at 04:39:25PM +0200, Petr Mladek wrote:
> On Thu 2025-06-05 13:15:32, Kees Cook wrote:
> > On Wed, Apr 16, 2025 at 02:06:21PM +0200, Petr Mladek wrote:
> > > On Tue 2025-04-15 10:02:33, Kees Cook wrote:
> > > > 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".
> > > > 
> > > > Link: https://github.com/KSPP/linux/issues/368 [1]
> > > > Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> > > > Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > > > Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > > > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > > > Acked-by: David Rientjes <rientjes@google.com>
> > > > Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > 
> > > Tested-by: Petr Mladek <pmladek@suse.com>
> > > Reviewed-by: Petr Mladek <pmladek@suse.com>
> > > 
> > > 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.

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

-- 
Kees Cook

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Andy Shevchenko]

On Mon, Jun 09, 2025 at 08:24:47AM -0700, Kees Cook wrote:
> On Mon, Jun 09, 2025 at 04:39:25PM +0200, Petr Mladek wrote:

...

> 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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Petr Mladek]

On Mon 2025-06-09 23:12:38, Andy Shevchenko wrote:
> On Mon, Jun 09, 2025 at 08:24:47AM -0700, Kees Cook wrote:
> > On Mon, Jun 09, 2025 at 04:39:25PM +0200, Petr Mladek wrote:
> 
> ...
> 
> > 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

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Rafael Aquini]

On Tue, Apr 15, 2025 at 10:02:33AM -0700, Kees Cook wrote:
> 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".
> 
> Link: https://github.com/KSPP/linux/issues/368 [1]
> Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: David Rientjes <rientjes@google.com>
> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  v2: use switch state to avoid missing states (pmladek)
>  v1: https://lore.kernel.org/lkml/20250410174428.work.488-kees@kernel.org/
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Cc: Harry Yoo <harry.yoo@oracle.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Tamir Duberstein <tamird@gmail.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-mm@kvack.org
> ---
>  .../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(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 76e538c77e31..d0fd9c745db9 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -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.
>  
> diff --git a/include/linux/sprintf.h b/include/linux/sprintf.h
> index 51cab2def9ec..521bb2cd2648 100644
> --- a/include/linux/sprintf.h
> +++ b/include/linux/sprintf.h
> @@ -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);
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 01699852f30c..22cbd75266ef 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -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);
>  
>  /*
> diff --git a/mm/slub.c b/mm/slub.c
> index b46f87662e71..f3d61b330a76 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -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;
> -- 
> 2.34.1
> 
>

Thank you, Kees.

Acked-by: Rafael Aquini <raquini@redhat.com>

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Harry Yoo]

On Tue, Apr 15, 2025 at 10:02:33AM -0700, Kees Cook wrote:
> 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.

> 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".
> 
> Link: https://github.com/KSPP/linux/issues/368  [1]
> Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: David Rientjes <rientjes@google.com>
> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

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

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Kees Cook]

On Thu, Apr 17, 2025 at 11:15:11PM +0900, Harry Yoo wrote:
> On Tue, Apr 15, 2025 at 10:02:33AM -0700, Kees Cook wrote:
> > 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?

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4568572205ee..982e6511a225 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -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

> 
> > 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".
> > 
> > Link: https://github.com/KSPP/linux/issues/368  [1]
> > Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> > Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > Acked-by: David Rientjes <rientjes@google.com>
> > Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > Signed-off-by: Kees Cook <kees@kernel.org>
> > ---
> 
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
> 
> 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

-- 
Kees Cook

Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

[Harry Yoo]

On Fri, Apr 18, 2025 at 01:13:20PM -0700, Kees Cook wrote:
> On Thu, Apr 17, 2025 at 11:15:11PM +0900, Harry Yoo wrote:
> > On Tue, Apr 15, 2025 at 10:02:33AM -0700, Kees Cook wrote:
> > > 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?
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 4568572205ee..982e6511a225 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -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.

>  	slab_max_order= [MM]
>  			Determines the maximum allowed order for slabs.
>  			A high setting may cause OOMs due to memory
> 
> > 
> > > 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".
> > > 
> > > Link: https://urldefense.com/v3/__https://github.com/KSPP/linux/issues/368__;!!ACWV5N9M2RV99hQ!No7Nk2rrOwtdh-XVVxppszL38saNKUNOLWogxasxGNiGfiS7gTVkUmDrw8J_YVAsbujlEb3hfb30$   [1]
> > > Fixes: 792702911f58 ("slub: force on no_hash_pointers when slub_debug is enabled")
> > > Co-developed-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > > Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
> > > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > > Acked-by: David Rientjes <rientjes@google.com>
> > > Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > ---
> > 
> > Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
> > 
> > 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.)

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.

-- 
Cheers,
Harry / Hyeonggon