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 <sperez...@gmail.com>
> Signed-off-by: Sergio Perez Gonzalez <sperez...@gmail.com>
> Acked-by: Vlastimil Babka <vba...@suse.cz>
> Acked-by: David Rientjes <rient...@google.com>
> Reviewed-by: Bagas Sanjaya <bagasdo...@gmail.com>
> Signed-off-by: Kees Cook <k...@kernel.org>
Tested-by: Petr Mladek <pmla...@suse.com>
Reviewed-by: Petr Mladek <pmla...@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-...@pathway.suse.cz
was hairy another way.
We could always improve it when it causes troubles.
Best Regards,
Petr
