linux-kernel - Re: [PATCH] x86/fpu: Avoid writing LBR bit to IA32

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: <87sevet447.ffs@tglx>
Date: Fri, 09 Aug 2024 11:52:40 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Mitchell Levy via B4 Relay <devnull+levymitchell0.gmail.com@...nel.org>,
 Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave
 Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin"
 <hpa@...or.com>
Cc: stable@...r.kernel.org, Borislav Petkov <bp@...e.de>,
 linux-kernel@...r.kernel.org, Mitchell Levy <levymitchell0@...il.com>
Subject: Re: [PATCH] x86/fpu: Avoid writing LBR bit to IA32_XSS unless
 supported

On Thu, Aug 08 2024 at 16:30, Mitchell Levy via wrote:
> From: Mitchell Levy <levymitchell0@...il.com>
>
> When computing which xfeatures are available, make sure that LBR is only
> present if both LBR is supported in general, as well as by XSAVES.
>
> There are two distinct CPU features related to the use of XSAVES as it
> applies to LBR: whether LBR is itself supported (strictly speaking, I'm
> not sure that this is necessary to check though it's certainly a good
> sanity check), and whether XSAVES supports LBR (see sections 13.2 and
> 13.5.12 of the Intel 64 and IA-32 Architectures Software Developer's
> Manual, Volume 1). Currently, the LBR subsystem correctly checks both
> (see intel_pmu_arch_lbr_init), however the xstate initialization
> subsystem does not.
>
> When calculating what value to place in the IA32_XSS MSR,
> xfeatures_mask_independent only checks whether LBR support is present,
> not whether XSAVES supports LBR. If XSAVES does not support LBR, this
> write causes #GP, leaving the state of IA32_XSS unchanged (i.e., set to
> zero, as its not written with other values, and its default value is
> zero out of RESET per section 13.3 of the arch manual).
>
> Then, the next time XRSTORS is used to restore supervisor state, it will
> fail with #GP (because the RFBM has zero for all supervisor features,
> which does not match the XCOMP_BV field). In particular,
> XFEATURE_MASK_FPSTATE includes supervisor features, so setting up the FPU
> will cause a #GP. This results in a call to fpu_reset_from_exception_fixup,
> which by the same process results in another #GP. Eventually this causes
> the kernel to run out of stack space and #DF.

Cute.

> Fixes: d72c87018d00 ("x86/fpu/xstate: Move remaining xfeature helpers to core")

This is not the culprit/

> Cc: stable@...r.kernel.org
>
> Signed-off-by: Mitchell Levy <levymitchell0@...il.com>
> ---
>  arch/x86/kernel/fpu/xstate.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
> index 2ee0b9c53dcc..574d2c2ea227 100644
> --- a/arch/x86/kernel/fpu/xstate.h
> +++ b/arch/x86/kernel/fpu/xstate.h
> @@ -61,7 +61,8 @@ static inline u64 xfeatures_mask_supervisor(void)
>  
>  static inline u64 xfeatures_mask_independent(void)
>  {
> -	if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR))
> +	if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR) ||
> +	    (fpu_kernel_cfg.max_features & XFEATURE_MASK_LBR) != XFEATURE_MASK_LBR)

This is wrong because fpu_kernel_cfg.max_features never contains
XFEATURE_MASK_LBR. It only contains the bits which are managed by the
FPU subsystem.

Thanks,

        tglx