linux-kernel - Re: [patch v3 2/7] x86/smp: Dont access non-existing CPUID leaf

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]

Date:   Mon, 19 Jun 2023 12:02:19 -0500
From:   "Limonciello, Mario" <mario.limonciello@....com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     x86@...nel.org, Tom Lendacky <thomas.lendacky@....com>,
        Tony Battersby <tonyb@...ernetics.com>,
        Ashok Raj <ashok.raj@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Arjan van de Veen <arjan@...ux.intel.com>,
        Eric Biederman <ebiederm@...ssion.com>
Subject: Re: [patch v3 2/7] x86/smp: Dont access non-existing CPUID leaf


On 6/15/2023 3:33 PM, Thomas Gleixner wrote:
> From: Tony Battersby <tonyb@...ernetics.com>
>
> stop_this_cpu() tests CPUID leaf 0x8000001f::EAX unconditionally. CPUs
> return the content of the highest supported leaf when a non-existing leaf
> is read. So the result of the test is lottery except on AMD CPUs which
> support that leaf.
>
> While harmless it's incorrect and causes the conditional wbinvd() to be
> issued where not required.
>
> Check whether the leaf is supported before reading it.
>
> [ tglx: Adjusted changelog ]
>
> Fixes: 08f253ec3767 ("x86/cpu: Clear SME feature flag when not in use")

Thanks for this fix.
This particular patch should probably also CC to stable.

Reviewed-by: Mario Limonciello <mario.limonciello@....com>

> Signed-off-by: Tony Battersby <tonyb@...ernetics.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Link: https://lore.kernel.org/r/3817d810-e0f1-8ef8-0bbd-663b919ca49b@cybernetics.com
> ---
>   arch/x86/kernel/process.c |    5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -763,6 +763,7 @@ struct cpumask cpus_stop_mask;
>   
>   void __noreturn stop_this_cpu(void *dummy)
>   {
> +	struct cpuinfo_x86 *c = this_cpu_ptr(&cpu_info);
>   	unsigned int cpu = smp_processor_id();
>   
>   	local_irq_disable();
> @@ -777,7 +778,7 @@ void __noreturn stop_this_cpu(void *dumm
>   	 */
>   	set_cpu_online(cpu, false);
>   	disable_local_APIC();
> -	mcheck_cpu_clear(this_cpu_ptr(&cpu_info));
> +	mcheck_cpu_clear(c);
>   
>   	/*
>   	 * Use wbinvd on processors that support SME. This provides support
> @@ -791,7 +792,7 @@ void __noreturn stop_this_cpu(void *dumm
>   	 * Test the CPUID bit directly because the machine might've cleared
>   	 * X86_FEATURE_SME due to cmdline options.
>   	 */
> -	if (cpuid_eax(0x8000001f) & BIT(0))
> +	if (c->extended_cpuid_level >= 0x8000001f && (cpuid_eax(0x8000001f) & BIT(0)))
>   		native_wbinvd();
>   
>   	/*
>