linux-kernel - Re: [PATCH v3 2/7] x86/cpu: Allow caps to be set arbitrarily early

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: <CAMj1kXHGJcn+BrchpFSKL8mykvYjhcSGEVrRwLSXHsu7jAFW8Q@mail.gmail.com>
Date: Thu, 15 May 2025 10:45:55 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Ingo Molnar <mingo@...nel.org>
Cc: "Kirill A. Shutemov" <kirill@...temov.name>, Ard Biesheuvel <ardb+git@...gle.com>, 
	linux-kernel@...r.kernel.org, x86@...nel.org, 
	Linus Torvalds <torvalds@...ux-foundation.org>, Brian Gerst <brgerst@...il.com>
Subject: Re: [PATCH v3 2/7] x86/cpu: Allow caps to be set arbitrarily early

On Thu, 15 May 2025 at 09:18, Ingo Molnar <mingo@...nel.org> wrote:
>
>
> * Kirill A. Shutemov <kirill@...temov.name> wrote:
>
> > On Thu, May 15, 2025 at 08:56:59AM +0200, Ingo Molnar wrote:
> > >
> > > * Ard Biesheuvel <ardb+git@...gle.com> wrote:
> > >
> > > > From: Ard Biesheuvel <ardb@...nel.org>
> > > >
> > > > cpu_feature_enabled() uses a ternary alternative, where the late variant
> > > > is based on code patching and the early variant accesses the capability
> > > > field in boot_cpu_data directly.
> > > >
> > > > This allows cpu_feature_enabled() to be called quite early, but it still
> > > > requires that the CPU feature detection code runs before being able to
> > > > rely on the return value of cpu_feature_enabled().
> > > >
> > > > This is a problem for the implementation of pgtable_l5_enabled(), which
> > > > is based on cpu_feature_enabled(X86_FEATURE_5LEVEL_PAGING), and may be
> > > > called extremely early. Currently, there is a hacky workaround where
> > > > some source files that may execute before (but also after) CPU feature
> > > > detection have a different version of pgtable_l5_enabled(), based on the
> > > > USE_EARLY_PGTABLE_L5 preprocessor macro.
> > > >
> > > > Instead, let's make it possible to set CPU feature arbitrarily early, so
> > > > that the X86_FEATURE_5LEVEL_PAGING capability can be set before even
> > > > entering C code.
> > > >
> > > > This involves relying on static initialization of boot_cpu_data and the
> > > > cpu_caps_set/cpu_caps_cleared arrays, so they all need to reside in
> > > > .data. This ensures that they won't be cleared along with the rest of
> > > > BSS.
> > > >
> > > > Note that forcing a capability involves setting it in both
> > > > boot_cpu_data.x86_capability[] and cpu_caps_set[].
> > > >
> > > > Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> > > > ---
> > > >  arch/x86/kernel/cpu/common.c | 9 +++------
> > > >  1 file changed, 3 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> > > > index 6f7827015834..f6f206743d6a 100644
> > > > --- a/arch/x86/kernel/cpu/common.c
> > > > +++ b/arch/x86/kernel/cpu/common.c
> > > > @@ -704,8 +704,8 @@ static const char *table_lookup_model(struct cpuinfo_x86 *c)
> > > >  }
> > > >
> > > >  /* Aligned to unsigned long to avoid split lock in atomic bitmap ops */
> > > > -__u32 cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
> > > > -__u32 cpu_caps_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
> > > > +__u32 __read_mostly cpu_caps_cleared[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
> > > > +__u32 __read_mostly cpu_caps_set[NCAPINTS + NBUGINTS] __aligned(sizeof(unsigned long));
> > >
> > > This change is not mentioned in the changelog AFAICS, but it should be
> > > in a separate patch anyway.
> >
> > And why not __ro_after_init?
>
> That's patch #7 :-)
>
> I got confused about that too.
>
> Patch #2 should not touch this line, and patch #7 should simply
> introduce __ro_after_init, and we are good I think.
>

This change is needed because it prevents these arrays from being
cleared along with the rest of BSS, which occurs after the startup
code executes.

So conceptually, moving these out of BSS is similar to dropping the
memset()s, and therefore this belongs in the same patch.

However, you are correct that moving these into __ro_after_init
achieves the same thing, so I will just reorder that patch with this
one, and clarify in the commit log that we are relying on the fact
that __ro_after_init is not cleared at boot time.