linux-kernel - Re: [PATCH] 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] [day] [month] [year] [list]

Message-ID: <202504150956.9AFF9545@keescook>
Date: Tue, 15 Apr 2025 10:00:00 -0700
From: Kees Cook <kees@...nel.org>
To: Petr Mladek <pmladek@...e.com>
Cc: Vlastimil Babka <vbabka@...e.cz>,
	Sergio Perez Gonzalez <sperezglz@...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>,
	David Rientjes <rientjes@...gle.com>,
	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] slab: Decouple slab_debug and no_hash_pointers

On Mon, Apr 14, 2025 at 02:31:42PM +0200, Petr Mladek wrote:
> On Thu 2025-04-10 10:44:31, 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".
> 
> The idea makes sense. But it seems that the patch did not handle
> the "always" mode correctly, see below.

Actually, it was the "never" mode that was being ignored. Whoops! (The
double negation language is a little odd.) I've fixed this for v2 with
an explicit switch statement.

> > -int __init no_hash_pointers_enable(char *str)
> > +void __init hash_pointers_finalize(bool slub_debug)
> >  {
> > -	if (no_hash_pointers)
> > -		return 0;
> > +	if (hash_pointers_mode == HASH_PTR_AUTO && slub_debug)
> > +		no_hash_pointers = true;
> >  
> > -	no_hash_pointers = true;
> > +	if (!no_hash_pointers)
> > +		return;
> >  
> >  	pr_warn("**********************************************************\n");
> >  	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
> 
> 
> The mode/policy is generic but this function is ready to be called
> only once. And we might actually want to call it twice, see below.

I'd like to keep it a single call. I feel this simplifies the reporting
logic, keeps the selection logic in one place, and allows us to
trivially examine that it is safe to use with __init.

Thanks for the review!

-- 
Kees Cook