linux-kernel - Re: [PATCH v5 00/16] Attack vector controls (part 1)

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: <20250422135907.GAaAegq5YUl1qQhjWM@renoirsky.local>
Date: Tue, 22 Apr 2025 15:59:07 +0200
From: Borislav Petkov <bp@...en8.de>
To: Ingo Molnar <mingo@...nel.org>
Cc: David Kaplan <david.kaplan@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H . Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 00/16] Attack vector controls (part 1)

On Tue, Apr 22, 2025 at 11:46:33AM +0200, Ingo Molnar wrote:
> The above pattern is not a big problem really, as the human brain has 
> no trouble ignoring well-structured syntactic repetitions on the left 
> side and will focus on the right side column.
> 
> Unlike the current status quo, which your reply didn't quote, so I'll 
> quote it again:
> 
>  static void __init spectre_v1_select_mitigation(void);
>  static void __init spectre_v2_select_mitigation(void);
>  static void __init retbleed_select_mitigation(void);
>  static void __init spectre_v2_user_select_mitigation(void);
>  static void __init ssb_select_mitigation(void);
>  static void __init l1tf_select_mitigation(void);
>  static void __init mds_select_mitigation(void);
>  static void __init md_clear_update_mitigation(void);
>  static void __init md_clear_select_mitigation(void);
>  static void __init taa_select_mitigation(void);
>  static void __init mmio_select_mitigation(void);
>  static void __init srbds_select_mitigation(void);
>  static void __init l1d_flush_select_mitigation(void);
>  static void __init srso_select_mitigation(void);
>  static void __init gds_select_mitigation(void);
> 
> Which is far more visually messy.

That's when those are written in a block together - there the human
brain can selectively ignore.

I mean when the functions are called in succession. See
cpu_select_mitigations(). All this function does is select mitigations.
So there's no need to state the selection of every single mitigation.

> What do I select? Some GDS detail? Or the main mitigation itself? 
> Nothing really tells me.

We're going to have _select(), _update() and apply() function per
mitigation. And this will be documented at the top of bugs.c

In the confines of this file, those functions are special and when you
select, update or apply something in bugs.c, it should always refer to
a mitigation.

We can make that decision here, in that file, for the sanity of everyone
who's looking at it.

> While with:
> 
> 	static void __init mitigation_select_gds(void)
> 	{
> 		u64 mcu_ctrl;
> 
> It's immediately clear that this is the main function that selects the 
> GDS mitigation.

Sorry:

$ git grep -o -i mitigation arch/x86/kernel/cpu/bugs.c | wc -l
714

That's just madness: this file has waaaaay too many "mitigation"s and it
impairs the reading. *Especially* if this file is *all* *about*
mitigations and only about that.

The new scheme is going to tip those scales over 1K...

> 3)
> 
> A proper namespace also makes it *much* easier to grep for specific 
> primitives.
> 
> With your suggested 'gds_select()' naming, if I want to search for all 
> gds_ primitives, I get:

Sorry, this is a bad example: if you want to search for a static
function's uses in the same file, you simply go in your favorite editor
and search for "word-under-cursor".

> Hierarchical lexicographic organization of function names is basically 
> a code organization 101 concept, and I didn't think it would be 
> particularly controversial. :-)

And yet the current naming ain't optimal, IMO, due to the too often
regurgitation of "mitigation". And that's particularly a problem if
it impairs reading of the code - lexicographic organization then is
secondary.

And as said before, you don't need a "mitigation_" prefix for static
functions.

And reading that code properly is the most important thing. *Especially*
*that* code.

So the goal of the bikeshedding here should be optimal reading. I.e.,
what function and variable naming allows for an optimal reading of the
code.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette