[PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2085 bytes --]

From: Ben Hutchings <benh@debian.org>

The mitigation for PBRSB includes adding LFENCE instructions to the
RSB filling sequence.  However, RSB filling is done on some older CPUs
that don't support the LFENCE instruction.

Define and use a BARRIER_NOSPEC macro which makes the LFENCE
conditional on X86_FEATURE_LFENCE_RDTSC, like the barrier_nospec()
macro defined for C code in <asm/barrier.h>.

Reported-by: Martin-Éric Racine <martin-eric.racine@iki.fi>
References: https://bugs.debian.org/1017425
Cc: stable@vger.kernel.org
Cc: regressions@lists.linux.dev
Cc: Daniel Sneddon <daniel.sneddon@linux.intel.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Fixes: 2b1299322016 ("x86/speculation: Add RSB VM Exit protections")
Fixes: ba6e31af2be9 ("x86/speculation: Add LFENCE to RSB fill sequence")
Signed-off-by: Ben Hutchings <benh@debian.org>
---
Re-sending this with properly matched From address and server.
Apologies if you got 2 copies.

Ben.

 arch/x86/include/asm/nospec-branch.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index e64fd20778b6..b1029fd88474 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -34,6 +34,11 @@
 
 #define RSB_CLEAR_LOOPS		32	/* To forcibly overwrite all entries */
 
+#ifdef __ASSEMBLY__
+
+/* Prevent speculative execution past this barrier. */
+#define BARRIER_NOSPEC ALTERNATIVE "", "lfence", X86_FEATURE_LFENCE_RDTSC
+
 /*
  * Google experimented with loop-unrolling and this turned out to be
  * the optimal version - two calls, each with their own speculation
@@ -62,9 +67,7 @@
 	dec	reg;				\
 	jnz	771b;				\
 	/* barrier for jnz misprediction */	\
-	lfence;
-
-#ifdef __ASSEMBLY__
+	BARRIER_NOSPEC;
 
 /*
  * This should be used immediately before an indirect jump/call. It tells
@@ -138,7 +141,7 @@
 	int3
 .Lunbalanced_ret_guard_\@:
 	add $(BITS_PER_LONG/8), %_ASM_SP
-	lfence
+	BARRIER_NOSPEC
 .endm
 
  /*

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Peter Zijlstra]

On Fri, Aug 19, 2022 at 02:33:08AM +0200, Ben Hutchings wrote:
> From: Ben Hutchings <benh@debian.org>
> 
> The mitigation for PBRSB includes adding LFENCE instructions to the
> RSB filling sequence.  However, RSB filling is done on some older CPUs
> that don't support the LFENCE instruction.
> 

Wait; what? There are chips that enable the RSB mitigations and DONT
have LFENCE ?!?

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Peter Zijlstra]

On Fri, Aug 19, 2022 at 10:47:21AM +0200, Peter Zijlstra wrote:
> On Fri, Aug 19, 2022 at 02:33:08AM +0200, Ben Hutchings wrote:
> > From: Ben Hutchings <benh@debian.org>
> > 
> > The mitigation for PBRSB includes adding LFENCE instructions to the
> > RSB filling sequence.  However, RSB filling is done on some older CPUs
> > that don't support the LFENCE instruction.
> > 
> 
> Wait; what? There are chips that enable the RSB mitigations and DONT
> have LFENCE ?!?

So I gave in and clicked on the horrible bugzilla thing. Apparently this
is P3/Athlon64 era crud.

Anyway, the added LFENCE isn't because of retbleed; it is because you
can steer the jnz and terminate the loop early and then not actually
complete the RSB stuffing.

New insights etc.. So it's a geniune fix for the existing rsb stuffing.

I'm not entirly sure what to do here. On the one hand, it's 32bit, so
who gives a crap, otoh we shouldn't break these ancient chips either I
suppose.

How's something like so then? It goes on top of my other patch cleaning
up this RSB mess:

  https://lkml.kernel.org/r/Yv9m%2FhuNJLuyviIn%40worktop.programming.kicks-ass.net

---
Subject: 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 <ben@decadent.org.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---

--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -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.

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On Fri, 2022-08-19 at 10:47 +0200, Peter Zijlstra wrote:
> On Fri, Aug 19, 2022 at 02:33:08AM +0200, Ben Hutchings wrote:
> > From: Ben Hutchings <benh@debian.org>
> > 
> > The mitigation for PBRSB includes adding LFENCE instructions to the
> > RSB filling sequence.  However, RSB filling is done on some older CPUs
> > that don't support the LFENCE instruction.
> > 
> 
> Wait; what? There are chips that enable the RSB mitigations and DONT
> have LFENCE ?!?

Yes, X86_FEATURE_RSB_CTXSW is enabled if any other Spectre v2
mitigation is enabled.  And all Intel family 6 (except some early
Atoms) and AMD family 5+ get Spectre v2 mitigation by default.

Ben.

-- 
Ben Hutchings
Beware of bugs in the above code;
I have only proved it correct, not tried it. - Donald Knuth

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Martin-Éric Racine]

On Fri, Aug 19, 2022 at 2:01 PM Peter Zijlstra <peterz@infradead.org> wrote:
> I'm not entirly sure what to do here. On the one hand, it's 32bit, so
> who gives a crap, otoh we shouldn't break these ancient chips either I
> suppose.

This is something that I've repeatedly had to bring up, whenever
something breaks because someone meant well by enabling more security
bells and whistles:

x86-32 is by definition legacy hardware. Enabling more bells and
whistles essentially kills support for all but the very latest
variants of the x86-32 family. This is the wrong approach. The right
approach is to accept that building for x86-32 inherently means
building for older and thus less secure architectures.

Martin-Éric

[tip: x86/urgent] x86/nospec: Fix i386 RSB stuffing

[tip-bot2 for Peter Zijlstra]

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

Commit-ID:     332924973725e8cdcc783c175f68cf7e162cb9e5
Gitweb:        https://git.kernel.org/tip/332924973725e8cdcc783c175f68cf7e162cb9e5
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Fri, 19 Aug 2022 13:01:35 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 19 Aug 2022 13:24:33 +02:00

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 <ben@decadent.org.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/Yv9tj9vbQ9nNlXoY@worktop.programming.kicks-ass.net
---
 arch/x86/include/asm/nospec-branch.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 10731cc..c936ce9 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -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.

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

On Fri, 2022-08-19 at 13:01 +0200, Peter Zijlstra wrote:
> On Fri, Aug 19, 2022 at 10:47:21AM +0200, Peter Zijlstra wrote:
> > On Fri, Aug 19, 2022 at 02:33:08AM +0200, Ben Hutchings wrote:
> > > From: Ben Hutchings <benh@debian.org>
> > > 
> > > The mitigation for PBRSB includes adding LFENCE instructions to the
> > > RSB filling sequence.  However, RSB filling is done on some older CPUs
> > > that don't support the LFENCE instruction.
> > > 
> > 
> > Wait; what? There are chips that enable the RSB mitigations and DONT
> > have LFENCE ?!?
> 
> So I gave in and clicked on the horrible bugzilla thing. Apparently this
> is P3/Athlon64 era crud.
> 
> Anyway, the added LFENCE isn't because of retbleed; it is because you
> can steer the jnz and terminate the loop early and then not actually
> complete the RSB stuffing.

I know, I corrected that further down.

> New insights etc.. So it's a geniune fix for the existing rsb stuffing.
> 
> I'm not entirly sure what to do here. On the one hand, it's 32bit, so
> who gives a crap, otoh we shouldn't break these ancient chips either I
> suppose.
> 
> How's something like so then? It goes on top of my other patch cleaning
> up this RSB mess:
> 
>   https://lkml.kernel.org/r/Yv9m%2FhuNJLuyviIn%40worktop.programming.kicks-ass.net
[...]

That should be:
https://lore.kernel.org/all/Yv9m%2FhuNJLuyviIn@worktop.programming.kicks-ass.net/
(the redirector unescapes the URL-escaped /).

So that puts the whole __FILL_RETURN_BUFFER inside an alternative, and
we can't have nested alternatives.  That's unfortunate.

Ben.

-- 
Ben Hutchings
Beware of bugs in the above code;
I have only proved it correct, not tried it. - Donald Knuth

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Peter Zijlstra]

On Fri, Aug 19, 2022 at 01:38:27PM +0200, Ben Hutchings wrote:

> So that puts the whole __FILL_RETURN_BUFFER inside an alternative, and
> we can't have nested alternatives.  That's unfortunate.

Well, both alternatives end with the LFENCE instruction, so I could pull
it out and do two consequtive ALTs, but unrolling the loop for i386 is
a better solution in that the sequence, while larger, removes the need
for the LFENCE.

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Martin-Éric Racine]

Greetings,

On Fri, Aug 19, 2022 at 3:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Aug 19, 2022 at 01:38:27PM +0200, Ben Hutchings wrote:
>
> > So that puts the whole __FILL_RETURN_BUFFER inside an alternative, and
> > we can't have nested alternatives.  That's unfortunate.
>
> Well, both alternatives end with the LFENCE instruction, so I could pull
> it out and do two consequtive ALTs, but unrolling the loop for i386 is
> a better solution in that the sequence, while larger, removes the need
> for the LFENCE.

Have we reached a definitive conclusion on to how to fix this?

Martin-Éric

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Peter Zijlstra]

On Tue, Aug 30, 2022 at 02:42:04PM +0300, Martin-Éric Racine wrote:
> Greetings,
> 
> On Fri, Aug 19, 2022 at 3:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, Aug 19, 2022 at 01:38:27PM +0200, Ben Hutchings wrote:
> >
> > > So that puts the whole __FILL_RETURN_BUFFER inside an alternative, and
> > > we can't have nested alternatives.  That's unfortunate.
> >
> > Well, both alternatives end with the LFENCE instruction, so I could pull
> > it out and do two consequtive ALTs, but unrolling the loop for i386 is
> > a better solution in that the sequence, while larger, removes the need
> > for the LFENCE.
> 
> Have we reached a definitive conclusion on to how to fix this?

https://git.kernel.org/tip/332924973725e8cdcc783c175f68cf7e162cb9e5

Re: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Martin-Éric Racine]

On Tue, Aug 30, 2022 at 3:00 PM Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Aug 30, 2022 at 02:42:04PM +0300, Martin-Éric Racine wrote:
> > On Fri, Aug 19, 2022 at 3:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Fri, Aug 19, 2022 at 01:38:27PM +0200, Ben Hutchings wrote:
> > >
> > > > So that puts the whole __FILL_RETURN_BUFFER inside an alternative, and
> > > > we can't have nested alternatives.  That's unfortunate.
> > >
> > > Well, both alternatives end with the LFENCE instruction, so I could pull
> > > it out and do two consequtive ALTs, but unrolling the loop for i386 is
> > > a better solution in that the sequence, while larger, removes the need
> > > for the LFENCE.
> >
> > Have we reached a definitive conclusion on to how to fix this?
>
> https://git.kernel.org/tip/332924973725e8cdcc783c175f68cf7e162cb9e5

Thanks.

Ben: When can we expect an updated kernel to security-updates at Debian?

Martin-Éric

Re: Bug#1017425: [PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Salvatore Bonaccorso]

Hi Martin,

On Tue, Aug 30, 2022 at 03:18:51PM +0300, Martin-??ric Racine wrote:
> On Tue, Aug 30, 2022 at 3:00 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, Aug 30, 2022 at 02:42:04PM +0300, Martin-??ric Racine wrote:
> > > On Fri, Aug 19, 2022 at 3:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > On Fri, Aug 19, 2022 at 01:38:27PM +0200, Ben Hutchings wrote:
> > > >
> > > > > So that puts the whole __FILL_RETURN_BUFFER inside an alternative, and
> > > > > we can't have nested alternatives.  That's unfortunate.
> > > >
> > > > Well, both alternatives end with the LFENCE instruction, so I could pull
> > > > it out and do two consequtive ALTs, but unrolling the loop for i386 is
> > > > a better solution in that the sequence, while larger, removes the need
> > > > for the LFENCE.
> > >
> > > Have we reached a definitive conclusion on to how to fix this?
> >
> > https://git.kernel.org/tip/332924973725e8cdcc783c175f68cf7e162cb9e5
> 
> Thanks.
> 
> Ben: When can we expect an updated kernel to security-updates at Debian?

When the update is ready to go. Likely the update for the next point
release for bullseye will contain the fix for this issue.

Regards,
Salvatore

[PATCH] x86/speculation: Avoid LFENCE in FILL_RETURN_BUFFER on CPUs that lack it

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

The mitigation for PBRSB includes adding LFENCE instructions to the
RSB filling sequence.  However, RSB filling is done on some older CPUs
that don't support the LFENCE instruction.

Define and use a BARRIER_NOSPEC macro which makes the LFENCE
conditional on X86_FEATURE_LFENCE_RDTSC, like the barrier_nospec()
macro defined for C code in <asm/barrier.h>.

Reported-by: Martin-Éric Racine <martin-eric.racine@iki.fi>
References: https://bugs.debian.org/1017425
Cc: stable@vger.kernel.org
Cc: regressions@lists.linux.dev
Cc: Daniel Sneddon <daniel.sneddon@linux.intel.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Fixes: 2b1299322016 ("x86/speculation: Add RSB VM Exit protections")
Fixes: ba6e31af2be9 ("x86/speculation: Add LFENCE to RSB fill sequence")
Signed-off-by: Ben Hutchings <benh@debian.org>
---
 arch/x86/include/asm/nospec-branch.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index e64fd20778b6..b1029fd88474 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -34,6 +34,11 @@
 
 #define RSB_CLEAR_LOOPS		32	/* To forcibly overwrite all entries */
 
+#ifdef __ASSEMBLY__
+
+/* Prevent speculative execution past this barrier. */
+#define BARRIER_NOSPEC ALTERNATIVE "", "lfence", X86_FEATURE_LFENCE_RDTSC
+
 /*
  * Google experimented with loop-unrolling and this turned out to be
  * the optimal version - two calls, each with their own speculation
@@ -62,9 +67,7 @@
 	dec	reg;				\
 	jnz	771b;				\
 	/* barrier for jnz misprediction */	\
-	lfence;
-
-#ifdef __ASSEMBLY__
+	BARRIER_NOSPEC;
 
 /*
  * This should be used immediately before an indirect jump/call. It tells
@@ -138,7 +141,7 @@
 	int3
 .Lunbalanced_ret_guard_\@:
 	add $(BITS_PER_LONG/8), %_ASM_SP
-	lfence
+	BARRIER_NOSPEC
 .endm
 
  /*

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]