FAILED: patch "[PATCH] x86/nospec: Fix i386 RSB stuffing" failed to apply to 5.10-stable tree

PATCH [email protected] 3 participants 14 messages Aug 29, 2022 – Sep 13, 2022

Subsystems (3)

Stable branch x86 x86 hardware vulnerabilities

lore

Share & tools

lore

[email protected]

author maintainer

2022-08-29 08:30 UTC #0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 332924973725e8cdcc783c175f68cf7e162cb9e5 Mon Sep 17 00:00:00 2001
From: Peter Zijlstra <[email protected]>
Date: Fri, 19 Aug 2022 13:01:35 +0200
Subject: [PATCH] x86/nospec: Fix i386 RSB stuffing

Turns out that i386 doesn't unconditionally have LFENCE, as such the loop in __FILL_RETURN_BUFFER isn't actually speculation safe on such chips.

Fixes ba6e31af2be9 ("x86/speculation: Add LFENCE to RSB fill sequence")

Reported-by Ben Hutchings <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Link https://lkml.kernel.org/r/[email protected] open here

arch/x86/include/asm/nospec-branch.h +12 -0

index 10731ccfed37..c936ce9f0c47 100644
@@ -50,6 +50,7 @@
  * the optimal version - two calls, each with their own speculation
  * trap should their return address end up getting used, in a loop.
  */
+#ifdef CONFIG_X86_64
 #define __FILL_RETURN_BUFFER(reg, nr)			\
 	mov	$(nr/2), reg;				\
 771:							\

@@ -60,6 +61,17 @@
 	jnz	771b;					\
 	/* barrier for jnz misprediction */		\
 	lfence;
+#else
+/*
+ * i386 doesn't unconditionally have LFENCE, as such it can't
+ * do a loop.
+ */
+#define __FILL_RETURN_BUFFER(reg, nr)			\
+	.rept nr;					\
+	__FILL_RETURN_SLOT;				\
+	.endr;						\
+	add	$(BITS_PER_LONG/8) * nr, %_ASM_SP;
+#endif
 
 /*
  * Stuff a single RSB slot.

Ben Hutchings

2022-08-29 14:05 UTC #1 lore raw

Quoted 5 lines · On Mon, 2022-08-29 at 10:30 +0200, [email protected] wrote:

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

You need commit 4e3aa9238277 "x86/nospec: Unwreck the RSB stuffing" before this one. I've attached the backport of that for 5.10. I haven't checked the older branches.

Ben.

Quoted 51 lines

thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 332924973725e8cdcc783c175f68cf7e162cb9e5 Mon Sep 17 00:00:00 2001
From: Peter Zijlstra <[email protected]>
Date: Fri, 19 Aug 2022 13:01:35 +0200
Subject: [PATCH] x86/nospec: Fix i386 RSB stuffing
Turns out that i386 doesn't unconditionally have LFENCE, as such the loop in __FILL_RETURN_BUFFER isn't actually speculation safe on such chips.
Fixes ba6e31af2be9 ("x86/speculation: Add LFENCE to RSB fill sequence")
Reported-by Ben Hutchings <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Link https://lkml.kernel.org/r/[email protected] open here
arch/x86/include/asm/nospec-branch.h +12 -0
index 10731ccfed37..c936ce9f0c47 100644
@@ -50,6 +50,7 @@
  * the optimal version - two calls, each with their own speculation
  * trap should their return address end up getting used, in a loop.
  */
+#ifdef CONFIG_X86_64
 #define __FILL_RETURN_BUFFER(reg, nr)			\
 	mov	$(nr/2), reg;				\
 771:							\
@@ -60,6 +61,17 @@
 	jnz	771b;					\
 	/* barrier for jnz misprediction */		\
 	lfence;
+#else
+/*
+ * i386 doesn't unconditionally have LFENCE, as such it can't
+ * do a loop.
+ */
+#define __FILL_RETURN_BUFFER(reg, nr)			\
+	.rept nr;					\
+	__FILL_RETURN_SLOT;				\
+	.endr;						\
+	add	$(BITS_PER_LONG/8) * nr, %_ASM_SP;
+#endif
 
 /*
  * Stuff a single RSB slot.

Signature · 3 lines

Ben Hutchings
Make three consecutive correct guesses and you will be considered
an expert.

From: Peter Zijlstra <[email protected]>
Date: Tue, 16 Aug 2022 14:28:36 +0200
Subject: x86/nospec: Unwreck the RSB stuffing

commit 4e3aa9238277597c6c7624f302d81a7b568b6f2d upstream.

Commit 2b1299322016 ("x86/speculation: Add RSB VM Exit protections") made a right mess of the RSB stuffing, rewrite the whole thing to not suck.

Thanks to Andrew for the enlightening comment about Post-Barrier RSB things so we can make this code less magical.

Cc [email protected]

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Link https://lkml.kernel.org/r/[email protected] open here

[bwh: Backported to 5.10: adjust context]

Signed-off-by Ben Hutchings <[email protected]>

---
 arch/x86/include/asm/nospec-branch.h | 80 ++++++++++++++--------------
 1 file changed, 39 insertions(+), 41 deletions(-)

arch/x86/include/asm/nospec-branch.h +39 ~~-41~~

@@ -35,33 +35,44 @@
 #define RSB_CLEAR_LOOPS		32	/* To forcibly overwrite all entries */
 
 /*
+ * Common helper for __FILL_RETURN_BUFFER and __FILL_ONE_RETURN.
+ */
+#define __FILL_RETURN_SLOT			\
+	ANNOTATE_INTRA_FUNCTION_CALL;		\
+	call	772f;				\
+	int3;					\
+772:
+
+/*
+ * Stuff the entire RSB.
+ *
  * Google experimented with loop-unrolling and this turned out to be
  * the optimal version — two calls, each with their own speculation
  * trap should their return address end up getting used, in a loop.
  */
-#define __FILL_RETURN_BUFFER(reg, nr, sp)	\
-	mov	$(nr/2), reg;			\
-771:						\
-	ANNOTATE_INTRA_FUNCTION_CALL;		\
-	call	772f;				\
-773:	/* speculation trap */			\
-	UNWIND_HINT_EMPTY;			\
-	pause;					\
-	lfence;					\
-	jmp	773b;				\
-772:						\
-	ANNOTATE_INTRA_FUNCTION_CALL;		\
-	call	774f;				\
-775:	/* speculation trap */			\
-	UNWIND_HINT_EMPTY;			\
-	pause;					\
-	lfence;					\
-	jmp	775b;				\
-774:						\
-	add	$(BITS_PER_LONG/8) * 2, sp;	\
-	dec	reg;				\
-	jnz	771b;				\
-	/* barrier for jnz misprediction */	\
+#define __FILL_RETURN_BUFFER(reg, nr)			\
+	mov	$(nr/2), reg;				\
+771:							\
+	__FILL_RETURN_SLOT				\
+	__FILL_RETURN_SLOT				\
+	add	$(BITS_PER_LONG/8) * 2, %_ASM_SP;	\
+	dec	reg;					\
+	jnz	771b;					\
+	/* barrier for jnz misprediction */		\
+	lfence;
+
+/*
+ * Stuff a single RSB slot.
+ *
+ * To mitigate Post-Barrier RSB speculation, one CALL instruction must be
+ * forced to retire before letting a RET instruction execute.
+ *
+ * On PBRSB-vulnerable CPUs, it is not safe for a RET to be executed
+ * before this point.
+ */
+#define __FILL_ONE_RETURN				\
+	__FILL_RETURN_SLOT				\
+	add	$(BITS_PER_LONG/8), %_ASM_SP;		\
 	lfence;
 
 #ifdef __ASSEMBLY__

@@ -120,28 +131,15 @@
 #endif
 .endm
 
-.macro ISSUE_UNBALANCED_RET_GUARD
-	ANNOTATE_INTRA_FUNCTION_CALL
-	call .Lunbalanced_ret_guard_\@
-	int3
-.Lunbalanced_ret_guard_\@:
-	add $(BITS_PER_LONG/8), %_ASM_SP
-	lfence
-.endm
-
  /*
   * A simpler FILL_RETURN_BUFFER macro. Don't make people use the CPP
   * monstrosity above, manually.
   */
-.macro FILL_RETURN_BUFFER reg:req nr:req ftr:req ftr2
-.ifb \ftr2
-	ALTERNATIVE "jmp .Lskip_rsb_\@", "", \ftr
-.else
-	ALTERNATIVE_2 "jmp .Lskip_rsb_\@", "", \ftr, "jmp .Lunbalanced_\@", \ftr2
-.endif
-	__FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP)
-.Lunbalanced_\@:
-	ISSUE_UNBALANCED_RET_GUARD
+.macro FILL_RETURN_BUFFER reg:req nr:req ftr:req ftr2=ALT_NOT(X86_FEATURE_ALWAYS)
+	ALTERNATIVE_2 "jmp .Lskip_rsb_\@", \
+		__stringify(__FILL_RETURN_BUFFER(\reg,\nr)), \ftr, \
+		__stringify(__FILL_ONE_RETURN), \ftr2
+
 .Lskip_rsb_\@:
 .endm

Greg KH

author maintainer

2022-09-01 09:43 UTC #2 lore raw

Quoted 10 lines · Ben Hutchings

Quoted 5 lines · On Mon, 2022-08-29 at 10:30 +0200, [email protected] wrote:
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
You need commit 4e3aa9238277 "x86/nospec: Unwreck the RSB stuffing" before this one. I've attached the backport of that for 5.10. I haven't checked the older branches.

Great, thanks, this worked. But the backport did not apply to 4.19, so I will need that in order to take this one as well.

thanks,

greg k-h

Ben Hutchings

2022-09-02 14:59 UTC #3 lore raw

Quoted 8 lines · Greg KH

Quoted 7 lines · Ben Hutchings
Quoted 5 lines · On Mon, 2022-08-29 at 10:30 +0200, [email protected] wrote:
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Quoted 6 lines

Quoted 3 lines
You need commit 4e3aa9238277 "x86/nospec: Unwreck the RSB stuffing" before this one. I've attached the backport of that for 5.10. I haven't checked the older branches.
Great, thanks, this worked. But the backport did not apply to 4.19, so I will need that in order to take this one as well.

I've had a look at 5.4, and it's sufficiently different from upstream that I don't see how to move forward.

However, I also found that the PBRSB mitigation seems broken, as commit fc02735b14ff "KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS" was not backported (and would be hard to add).

So, perhaps it would be best to revert the backports of:

2b1299322016 x86/speculation: Add RSB VM Exit protections ba6e31af2be9 x86/speculation: Add LFENCE to RSB fill sequence

in stable branches older than 5.10.

Ben.

Signature · 3 lines

Ben Hutchings
Lowery's Law:
        If it jams, force it. If it breaks, it needed replacing anyway.

Greg KH

author maintainer

2022-09-06 12:08 UTC #4 lore raw

Quoted 23 lines · Ben Hutchings

Quoted 8 lines · Greg KH
Quoted 7 lines · Ben Hutchings
Quoted 5 lines · On Mon, 2022-08-29 at 10:30 +0200, [email protected] wrote:
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Quoted 6 lines
Quoted 3 lines
You need commit 4e3aa9238277 "x86/nospec: Unwreck the RSB stuffing" before this one. I've attached the backport of that for 5.10. I haven't checked the older branches.
Great, thanks, this worked. But the backport did not apply to 4.19, so I will need that in order to take this one as well.
I've had a look at 5.4, and it's sufficiently different from upstream that I don't see how to move forward.
However, I also found that the PBRSB mitigation seems broken, as commit fc02735b14ff "KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS" was not backported (and would be hard to add).

Quoted 6 lines

So, perhaps it would be best to revert the backports of:
2b1299322016 x86/speculation: Add RSB VM Exit protections ba6e31af2be9 x86/speculation: Add LFENCE to RSB fill sequence
in stable branches older than 5.10.

Why? Is it because they do not work at all there, or are they causing problems?

thanks,

greg k-h

Ben Hutchings

2022-09-06 17:19 UTC #5 lore raw

Quoted 26 lines · Greg KH

Quoted 25 lines · Ben Hutchings
Quoted 8 lines · Greg KH
Quoted 7 lines · Ben Hutchings
Quoted 5 lines · On Mon, 2022-08-29 at 10:30 +0200, [email protected] wrote:
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Quoted 6 lines
Quoted 3 lines
You need commit 4e3aa9238277 "x86/nospec: Unwreck the RSB stuffing" before this one. I've attached the backport of that for 5.10. I haven't checked the older branches.
Great, thanks, this worked. But the backport did not apply to 4.19, so I will need that in order to take this one as well.
I've had a look at 5.4, and it's sufficiently different from upstream that I don't see how to move forward.
However, I also found that the PBRSB mitigation seems broken, as commit fc02735b14ff "KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS" was not backported (and would be hard to add).
So, perhaps it would be best to revert the backports of:

Quoted 7 lines

Quoted 4 lines
2b1299322016 x86/speculation: Add RSB VM Exit protections ba6e31af2be9 x86/speculation: Add LFENCE to RSB fill sequence
in stable branches older than 5.10.
Why? Is it because they do not work at all there, or are they causing problems?

- They both add unconditional LFENCE instructions, which are not implemented on older 32-bit CPUs and will therefore result in a crash.

- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.

Ben.

Signature · 2 lines

Ben Hutchings
friends: People who know you well, but like you anyway.

Pawan Gupta

reviewer

2022-09-06 21:20 UTC #6 lore raw

Hi Ben,

Quoted 30 lines · Ben Hutchings

Quoted 29 lines · Greg KH
Quoted 28 lines · Ben Hutchings
Quoted 8 lines · Greg KH
Quoted 7 lines · Ben Hutchings
Quoted 5 lines · On Mon, 2022-08-29 at 10:30 +0200, [email protected] wrote:
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Quoted 6 lines
Quoted 3 lines
You need commit 4e3aa9238277 "x86/nospec: Unwreck the RSB stuffing" before this one. I've attached the backport of that for 5.10. I haven't checked the older branches.
Great, thanks, this worked. But the backport did not apply to 4.19, so I will need that in order to take this one as well.
I've had a look at 5.4, and it's sufficiently different from upstream that I don't see how to move forward.
However, I also found that the PBRSB mitigation seems broken, as commit fc02735b14ff "KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS" was not backported (and would be hard to add).
So, perhaps it would be best to revert the backports of:
2b1299322016 x86/speculation: Add RSB VM Exit protections ba6e31af2be9 x86/speculation: Add LFENCE to RSB fill sequence

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
in stable branches older than 5.10.
Why? Is it because they do not work at all there, or are they causing problems?
- They both add unconditional LFENCE instructions, which are not implemented on older 32-bit CPUs and will therefore result in a crash.

Backporting commit 332924973725 ("x86/nospec: Fix i386 RSB stuffing") should fix this?

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=332924973725e8cdcc783c175f68cf7e162cb9e5

Quoted 3 lines

- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.

I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?

Thanks,
Pawan

Ben Hutchings

2022-09-07 00:24 UTC #7 lore raw

Quoted 35 lines · Pawan Gupta

Hi Ben,
Quoted 32 lines · Ben Hutchings
Quoted 31 lines · Greg KH
Quoted 29 lines · Ben Hutchings
Quoted 8 lines · Greg KH
Quoted 7 lines · Ben Hutchings
Quoted 5 lines · On Mon, 2022-08-29 at 10:30 +0200, [email protected] wrote:
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Quoted 6 lines
Quoted 3 lines
You need commit 4e3aa9238277 "x86/nospec: Unwreck the RSB stuffing" before this one. I've attached the backport of that for 5.10. I haven't checked the older branches.
Great, thanks, this worked. But the backport did not apply to 4.19, so I will need that in order to take this one as well.
I've had a look at 5.4, and it's sufficiently different from upstream that I don't see how to move forward.
However, I also found that the PBRSB mitigation seems broken, as commit fc02735b14ff "KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS" was not backported (and would be hard to add).
So, perhaps it would be best to revert the backports of:
2b1299322016 x86/speculation: Add RSB VM Exit protections ba6e31af2be9 x86/speculation: Add LFENCE to RSB fill sequence
in stable branches older than 5.10.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Why? Is it because they do not work at all there, or are they causing problems?
- They both add unconditional LFENCE instructions, which are not implemented on older 32-bit CPUs and will therefore result in a crash.
Backporting commit 332924973725 ("x86/nospec: Fix i386 RSB stuffing") should fix this?

That's where this thread started. The problem is that it seems to depend on a lot of other changes.

Quoted 9 lines

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=332924973725e8cdcc783c175f68cf7e162cb9e5
Quoted 3 lines
- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.
I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?

Yes, you're right. The backported versions avoid this problem. They are quite different from the upstream commit - and I would have appreciated some explanation of this in their commit messages.

So, let's try again to move forward. I've attached a backport for 4.19 and 5.4 (only tested with the latter so far).

Ben.

Signature · 3 lines

Ben Hutchings
Hoare's Law of Large Problems:
   Inside every large problem is a small problem struggling to get out.

From e751e8a43c78e045a20819eb6b598039c3027449 Mon Sep 17 00:00:00 2001
From: Peter Zijlstra <[email protected]>
Date: Fri, 19 Aug 2022 13:01:35 +0200
Subject: [PATCH] x86/nospec: Fix i386 RSB stuffing

commit 332924973725e8cdcc783c175f68cf7e162cb9e5 upstream.

Turns out that i386 doesn't unconditionally have LFENCE, as such the loop in __FILL_RETURN_BUFFER isn't actually speculation safe on such chips.

Fixes ba6e31af2be9 ("x86/speculation: Add LFENCE to RSB fill sequence")

Reported-by Ben Hutchings <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Link https://lkml.kernel.org/r/[email protected] open here

[bwh: Backported to 4.19/5.4:
- __FILL_RETURN_BUFFER takes an sp parameter
- Open-code __FILL_RETURN_SLOT]

Signed-off-by Ben Hutchings <[email protected]>

---
 arch/x86/include/asm/nospec-branch.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/x86/include/asm/nospec-branch.h +14 -0

index 1e5df3ccdd5c..a1ee1a760c3e 100644
@@ -44,6 +44,7 @@
  * the optimal version — two calls, each with their own speculation
  * trap should their return address end up getting used, in a loop.
  */
+#ifdef CONFIG_X86_64
 #define __FILL_RETURN_BUFFER(reg, nr, sp)	\
 	mov	$(nr/2), reg;			\
 771:						\

@@ -64,6 +65,19 @@
 	add	$(BITS_PER_LONG/8) * nr, sp;	\
 	/* barrier for jnz misprediction */	\
 	lfence;
+#else
+/*
+ * i386 doesn't unconditionally have LFENCE, as such it can't
+ * do a loop.
+ */
+#define __FILL_RETURN_BUFFER(reg, nr, sp)	\
+	.rept nr;				\
+	call	772f;				\
+	int3;					\
+772:;						\
+	.endr;					\
+	add	$(BITS_PER_LONG/8) * nr, sp;
+#endif
 
 #define __ISSUE_UNBALANCED_RET_GUARD(sp)	\
 	call	881f;				\

Pawan Gupta

reviewer

2022-09-08 06:09 UTC #8 lore raw

Quoted 11 lines · Ben Hutchings

Quoted 7 lines
Quoted 3 lines
- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.
I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?
Yes, you're right. The backported versions avoid this problem. They are quite different from the upstream commit - and I would have appreciated some explanation of this in their commit messages.

Ahh right, I will keep in mind next time.

Quoted 2 lines

So, let's try again to move forward. I've attached a backport for 4.19 and 5.4 (only tested with the latter so far).

I am not understanding why lfence in single-entry-fill sequence is okay on 32-bit kernels?

#define __FILL_ONE_RETURN                               \
        __FILL_RETURN_SLOT                              \
        add     $(BITS_PER_LONG/8), %_ASM_SP;           \
        lfence;

Ben Hutchings

2022-09-08 12:45 UTC #9 lore raw

Quoted 18 lines · Pawan Gupta

Quoted 11 lines · Ben Hutchings
Quoted 7 lines
Quoted 3 lines
- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.
I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?
Yes, you're right. The backported versions avoid this problem. They are quite different from the upstream commit - and I would have appreciated some explanation of this in their commit messages.
Ahh right, I will keep in mind next time.
Quoted 2 lines
So, let's try again to move forward. I've attached a backport for 4.19 and 5.4 (only tested with the latter so far).

Quoted 7 lines

I am not understanding why lfence in single-entry-fill sequence is okay on 32-bit kernels?
#define __FILL_ONE_RETURN                               \
        __FILL_RETURN_SLOT                              \
        add     $(BITS_PER_LONG/8), %_ASM_SP;           \
        lfence;

This isn't exactly about whether the kernel is 32-bit vs 64-bit, it's about whether the code may run on a processor that lacks support for LFENCE (part of SSE2).

- SSE2 is architectural on x86_64, so 64-bit kernels can use LFENCE unconditionally.
- PBRSB doesn't affect any of those old processors, so its mitigation can use LFENCE unconditionally. (Those procesors don't support VMX either.)

Ben.

Signature · 2 lines

Ben Hutchings
For every action, there is an equal and opposite criticism. - Harrison

Greg KH

author maintainer

2022-09-11 05:47 UTC #10 lore raw

Quoted 31 lines · Ben Hutchings

Quoted 18 lines · Pawan Gupta
Quoted 11 lines · Ben Hutchings
Quoted 7 lines
Quoted 3 lines
- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.
I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?
Yes, you're right. The backported versions avoid this problem. They are quite different from the upstream commit - and I would have appreciated some explanation of this in their commit messages.
Ahh right, I will keep in mind next time.
Quoted 2 lines
So, let's try again to move forward. I've attached a backport for 4.19 and 5.4 (only tested with the latter so far).
Quoted 7 lines
I am not understanding why lfence in single-entry-fill sequence is okay on 32-bit kernels?
#define __FILL_ONE_RETURN                               \
        __FILL_RETURN_SLOT                              \
        add     $(BITS_PER_LONG/8), %_ASM_SP;           \
        lfence;
This isn't exactly about whether the kernel is 32-bit vs 64-bit, it's about whether the code may run on a processor that lacks support for LFENCE (part of SSE2).

Quoted 5 lines

- SSE2 is architectural on x86_64, so 64-bit kernels can use LFENCE unconditionally.
- PBRSB doesn't affect any of those old processors, so its mitigation can use LFENCE unconditionally. (Those procesors don't support VMX either.)

Ok, it seems that I need to take Ben's patch to resolve this. Pawan, if you object, please let us know.

thanks,

greg k-h

Pawan Gupta

reviewer

2022-09-12 21:56 UTC #11 lore raw

Quoted 32 lines · Greg KH

Quoted 31 lines · Ben Hutchings
Quoted 18 lines · Pawan Gupta
Quoted 11 lines · Ben Hutchings
Quoted 7 lines
Quoted 3 lines
- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.
I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?
Yes, you're right. The backported versions avoid this problem. They are quite different from the upstream commit - and I would have appreciated some explanation of this in their commit messages.
Ahh right, I will keep in mind next time.
Quoted 2 lines
So, let's try again to move forward. I've attached a backport for 4.19 and 5.4 (only tested with the latter so far).
Quoted 7 lines
I am not understanding why lfence in single-entry-fill sequence is okay on 32-bit kernels?
#define __FILL_ONE_RETURN                               \
        __FILL_RETURN_SLOT                              \
        add     $(BITS_PER_LONG/8), %_ASM_SP;           \
        lfence;
This isn't exactly about whether the kernel is 32-bit vs 64-bit, it's about whether the code may run on a processor that lacks support for LFENCE (part of SSE2).

Quoted 8 lines

Quoted 5 lines
- SSE2 is architectural on x86_64, so 64-bit kernels can use LFENCE unconditionally.
- PBRSB doesn't affect any of those old processors, so its mitigation can use LFENCE unconditionally. (Those procesors don't support VMX either.)
Ok, it seems that I need to take Ben's patch to resolve this. Pawan, if you object, please let us know.

I don't see any issue taking Ben's patch to resolve this.

Backport for 5.4 didn't apply cleanly on 4.19 and needed a minor change.

Attaching the patch for 4.19. It built fine with CONFIG_64BIT=n.

I don't see LFENCE in the i386 version of FILL_RETURN_BUFFER:

Dump of assembler code for function __switch_to_asm:

   0xc1d63e00 <+0>:     push   %ebp
   0xc1d63e01 <+1>:     push   %ebx
   0xc1d63e02 <+2>:     push   %edi
   0xc1d63e03 <+3>:     push   %esi
   0xc1d63e04 <+4>:     pushf
   0xc1d63e05 <+5>:     mov    %esp,0x69c(%eax)
   0xc1d63e0b <+11>:    mov    0x69c(%edx),%esp
   0xc1d63e11 <+17>:    mov    0x378(%edx),%ebx
   0xc1d63e17 <+23>:    mov    %ebx,%fs:0xc23b0e74
   0xc1d63e1e <+30>:    call   0xc1d63e24 <__switch_to_asm+36>     ---> //FILL_RETURN_BUFFER
   0xc1d63e23 <+35>:    int3
   0xc1d63e24 <+36>:    call   0xc1d63e2a <__switch_to_asm+42>
   0xc1d63e29 <+41>:    int3
   0xc1d63e2a <+42>:    call   0xc1d63e30 <__switch_to_asm+48>
   0xc1d63e2f <+47>:    int3
   0xc1d63e30 <+48>:    call   0xc1d63e36 <__switch_to_asm+54>
   0xc1d63e35 <+53>:    int3
   0xc1d63e36 <+54>:    call   0xc1d63e3c <__switch_to_asm+60>
   0xc1d63e3b <+59>:    int3
   0xc1d63e3c <+60>:    call   0xc1d63e42 <__switch_to_asm+66>

[...]

   0xc1d63ecc <+204>:   call   0xc1d63ed2 <__switch_to_asm+210>
   0xc1d63ed1 <+209>:   int3
   0xc1d63ed2 <+210>:   call   0xc1d63ed8 <__switch_to_asm+216>
   0xc1d63ed7 <+215>:   int3
   0xc1d63ed8 <+216>:   call   0xc1d63ede <__switch_to_asm+222>
   0xc1d63edd <+221>:   int3
   0xc1d63ede <+222>:   add    $0x80,%esp

From 60487cc8dd7eda1f8aafc75a51cff179e1d95e83 Mon Sep 17 00:00:00 2001
Message-Id: <60487cc8dd7eda1f8aafc75a51cff179e1d95e83.1663018922.git.pawan.kumar.gupta@linux.intel.com>
From: Pawan Gupta <[email protected]>
Date: Fri, 19 Aug 2022 13:01:35 +0200
Subject: [PATCH] x86/nospec: Fix i386 RSB stuffing

From: Peter Zijlstra <[email protected]>

commit 332924973725e8cdcc783c175f68cf7e162cb9e5 upstream.

Turns out that i386 doesn't unconditionally have LFENCE, as such the loop in __FILL_RETURN_BUFFER isn't actually speculation safe on such chips.

Fixes ba6e31af2be9 ("x86/speculation: Add LFENCE to RSB fill sequence")

Reported-by Ben Hutchings <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Link https://lkml.kernel.org/r/[email protected] open here

[bwh: Backported to 4.19/5.4:
- __FILL_RETURN_BUFFER takes an sp parameter
- Open-code __FILL_RETURN_SLOT]

Signed-off-by Ben Hutchings <[email protected]>

Signed-off-by Pawan Gupta <[email protected]>

---
 arch/x86/include/asm/nospec-branch.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/x86/include/asm/nospec-branch.h +14 -0

index 747549934fe3..17a236a8b237 100644
@@ -35,6 +35,7 @@
  * the optimal version — two calls, each with their own speculation
  * trap should their return address end up getting used, in a loop.
  */
+#ifdef CONFIG_X86_64
 #define __FILL_RETURN_BUFFER(reg, nr, sp)	\
 	mov	$(nr/2), reg;			\
 771:						\

@@ -55,6 +56,19 @@
 	add	$(BITS_PER_LONG/8) * nr, sp;	\
 	/* barrier for jnz misprediction */	\
 	lfence;
+#else
+/*
+ * i386 doesn't unconditionally have LFENCE, as such it can't
+ * do a loop.
+ */
+#define __FILL_RETURN_BUFFER(reg, nr, sp)	\
+	.rept nr;				\
+	call	772f;				\
+	int3;					\
+772:;						\
+	.endr;					\
+	add	$(BITS_PER_LONG/8) * nr, sp;
+#endif
 
 /* Sequence to mitigate PBRSB on eIBRS CPUs */
 #define __ISSUE_UNBALANCED_RET_GUARD(sp)	\

base-commit: 41b46409f97a703ab1dd9227c40e76a0d3eeea1c

Signature · 1 line

2.37.2

Greg KH

author maintainer

2022-09-13 11:27 UTC #12 lore raw

Quoted 39 lines · Pawan Gupta

Quoted 38 lines · Greg KH
Quoted 36 lines · Ben Hutchings
Quoted 18 lines · Pawan Gupta
Quoted 11 lines · Ben Hutchings
Quoted 7 lines
Quoted 3 lines
- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.
I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?
Yes, you're right. The backported versions avoid this problem. They are quite different from the upstream commit - and I would have appreciated some explanation of this in their commit messages.
Ahh right, I will keep in mind next time.
Quoted 2 lines
So, let's try again to move forward. I've attached a backport for 4.19 and 5.4 (only tested with the latter so far).
Quoted 7 lines
I am not understanding why lfence in single-entry-fill sequence is okay on 32-bit kernels?
#define __FILL_ONE_RETURN                               \
        __FILL_RETURN_SLOT                              \
        add     $(BITS_PER_LONG/8), %_ASM_SP;           \
        lfence;
This isn't exactly about whether the kernel is 32-bit vs 64-bit, it's about whether the code may run on a processor that lacks support for LFENCE (part of SSE2).
- SSE2 is architectural on x86_64, so 64-bit kernels can use LFENCE unconditionally.
- PBRSB doesn't affect any of those old processors, so its mitigation can use LFENCE unconditionally. (Those procesors don't support VMX either.)

Quoted 8 lines

Quoted 2 lines
Ok, it seems that I need to take Ben's patch to resolve this. Pawan, if you object, please let us know.
I don't see any issue taking Ben's patch to resolve this.
Backport for 5.4 didn't apply cleanly on 4.19 and needed a minor change.
Attaching the patch for 4.19. It built fine with CONFIG_64BIT=n.

We already had a 4.19 patch, but I'll add your signed-off-by to it :)

thanks,

greg k-h

Pawan Gupta

reviewer

2022-09-13 00:53 UTC #13 lore raw

In reply to Ben Hutchings

Quoted 31 lines · Ben Hutchings

Quoted 18 lines · Pawan Gupta
Quoted 11 lines · Ben Hutchings
Quoted 7 lines
Quoted 3 lines
- The added mitigation, for PBRSB, requires removing any RET instructions executed between VM exit and the RSB filling. In these older branches that hasn't been done, so the mitigation doesn't work.
I checked 4.19 and 5.4, I don't see any RET between VM-exit and RSB filling. Could you please point me to any specific instance you are seeing?
Yes, you're right. The backported versions avoid this problem. They are quite different from the upstream commit - and I would have appreciated some explanation of this in their commit messages.
Ahh right, I will keep in mind next time.
Quoted 2 lines
So, let's try again to move forward. I've attached a backport for 4.19 and 5.4 (only tested with the latter so far).
Quoted 7 lines
I am not understanding why lfence in single-entry-fill sequence is okay on 32-bit kernels?
#define __FILL_ONE_RETURN                               \
        __FILL_RETURN_SLOT                              \
        add     $(BITS_PER_LONG/8), %_ASM_SP;           \
        lfence;
This isn't exactly about whether the kernel is 32-bit vs 64-bit, it's about whether the code may run on a processor that lacks support for LFENCE (part of SSE2).

Quoted 5 lines

- SSE2 is architectural on x86_64, so 64-bit kernels can use LFENCE unconditionally.
- PBRSB doesn't affect any of those old processors, so its mitigation can use LFENCE unconditionally. (Those procesors don't support VMX either.)

Thanks Ben.