Skip to contentSkip to conversation

x86/fpu: Fix AMD X86_BUG_FXSAVE_LEAK fixup

PATCH Applied Uros Bizjak 2 participants 2 messages Mar 15, 2024 – Mar 19, 2024 x86

lore
Share & tools
lore

Uros Bizjak

author
#0 lore raw

PATCH 1/1 x86/fpu: Fix AMD X86_BUG_FXSAVE_LEAK fixup

The assembly snippet in restore_fpregs_from_fpstate() that implements X86_BUG_FXSAVE_LEAK fixup loads the value from a random variable, preferably the one that is already in the L1 cache.

However, the access to fpinit_state via *fpstate pointer is not implemented correctly. The "m" asm constraint requires dereferenced pointer variable, otherwise the compiler just reloads the value via temporary stack slot. The current asm code reflects this:

     mov    %rdi,(%rsp)
     ...
     fildl  (%rsp)

With dereferenced pointer variable, the code does what the comment above the asm snippet says:

fildl (%rdi)

Also, remove the pointless %P operand modifier. The modifier is ineffective on non-symbolic references - it was used to prevent %rip-relative addresses in .altinstr sections, but FILDL in the .text section can use %rip-relative addresses without problems.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Signed-off-by Uros Bizjak <[email protected]>
---
 arch/x86/kernel/fpu/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/fpu/core.c +2 -2
index 520deb411a70..1209c7aebb21 100644
@@ -145,8 +145,8 @@ void restore_fpregs_from_fpstate(struct fpstate *fpstate, u64 mask)
 		asm volatile(
 			"fnclex\n\t"
 			"emms\n\t"
-			"fildl %P[addr]"	/* set F?P to defined value */
-			: : [addr] "m" (fpstate));
+			"fildl %[addr]"	/* set F?P to defined value */
+			: : [addr] "m" (*fpstate));
 	}
 
 	if (use_xsave()) {
Signature · 1 line
2.44.0
merge bot Applied
#1 lore raw
The following commit has been merged into the x86/fpu branch of tip:

The following commit has been merged into the x86/fpu branch of tip:

Commit-ID:     5d31174f3c8c465d9dbe88f6b9d1fe5716f44981
Gitweb:        https://git.kernel.org/tip/5d31174f3c8c465d9dbe88f6b9d1fe5716f44981
Author:        Uros Bizjak <[email protected]>
AuthorDate:    Fri, 15 Mar 2024 09:18:23 +01:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 19 Mar 2024 14:02:29 +01:00

x86/fpu: Fix AMD X86_BUG_FXSAVE_LEAK fixup

The assembly snippet in restore_fpregs_from_fpstate() that implements X86_BUG_FXSAVE_LEAK fixup loads the value from a random variable, preferably the one that is already in the L1 cache.

However, the access to fpinit_state via *fpstate pointer is not implemented correctly. The "m" asm constraint requires dereferenced pointer variable, otherwise the compiler just reloads the value via temporary stack slot. The current asm code reflects this:

     mov    %rdi,(%rsp)
     ...
     fildl  (%rsp)

With dereferenced pointer variable, the code does what the comment above the asm snippet says:

fildl (%rdi)

Also, remove the pointless %P operand modifier. The modifier is ineffective on non-symbolic references - it was used to prevent %rip-relative addresses in .altinstr sections, but FILDL in the .text section can use %rip-relative addresses without problems.

Signed-off-by Uros Bizjak <[email protected]>
Signed-off-by Ingo Molnar <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Linus Torvalds <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
---
 arch/x86/kernel/fpu/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/fpu/core.c +2 -2
index 520deb4..1209c7a 100644
@@ -145,8 +145,8 @@ void restore_fpregs_from_fpstate(struct fpstate *fpstate, u64 mask)
 		asm volatile(
 			"fnclex\n\t"
 			"emms\n\t"
-			"fildl %P[addr]"	/* set F?P to defined value */
-			: : [addr] "m" (fpstate));
+			"fildl %[addr]"	/* set F?P to defined value */
+			: : [addr] "m" (*fpstate));
 	}
 
 	if (use_xsave()) {