linux-hardening - Re: [PATCH v2] slab: Decouple slab_debug and no_hash

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z_-dPcdiGW0fo8Ji@pathway.suse.cz>
Date: Wed, 16 Apr 2025 14:06:21 +0200
From: Petr Mladek <pmladek@...e.com>
To: Kees Cook <kees@...nel.org>
Cc: Sergio Perez Gonzalez <sperezglz@...il.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	David Rientjes <rientjes@...gle.com>,
	Bagas Sanjaya <bagasdotme@...il.com>,
	Jonathan Corbet <corbet@....net>,
	Steven Rostedt <rostedt@...dmis.org>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Roman Gushchin <roman.gushchin@...ux.dev>,
	Harry Yoo <harry.yoo@...cle.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Randy Dunlap <rdunlap@...radead.org>,
	Tamir Duberstein <tamird@...il.com>,
	Miguel Ojeda <ojeda@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
	linux-doc@...r.kernel.org, linux-mm@...ck.org,
	Thomas Huth <thuth@...hat.com>,
	"Borislav Petkov (AMD)" <bp@...en8.de>,
	Ard Biesheuvel <ardb@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Stephen Boyd <swboyd@...omium.org>, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] slab: Decouple slab_debug and no_hash_pointers

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@...il.com>
> Signed-off-by: Sergio Perez Gonzalez <sperezglz@...il.com>
> Acked-by: Vlastimil Babka <vbabka@...e.cz>
> Acked-by: David Rientjes <rientjes@...gle.com>
> Reviewed-by: Bagas Sanjaya <bagasdotme@...il.com>
> Signed-off-by: Kees Cook <kees@...nel.org>

Tested-by: Petr Mladek <pmladek@...e.com>
Reviewed-by: Petr Mladek <pmladek@...e.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