| Message ID | 20250415170232.it.467-kees@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] slab: Decouple slab_debug and no_hash_pointers | expand |
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
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
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>
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
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
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;
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(-)