2022-03-17 12:53:56

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On Thu, Mar 17, 2022 at 10:37:56AM +0100, Borislav Petkov wrote:
> Jamie, I'd appreciate testing this one too, pls, just in case.

Here's a version against -rc8 - the previous one was against tip/master
and had other contextual changes in it.

---
From: Borislav Petkov <[email protected]>

The commit in Fixes started adding INT3 after RETs as a mitigation
against straight-line speculation.

The fastop SETcc implementation in kvm's insn emulator uses macro magic
to generate all possible SETcc functions and to jump to them when
emulating the respective instruction.

However, it hardcodes the size and alignment of those functions to 4: a
three-byte SETcc insn and a single-byte RET. BUT, with SLS, there's an
INT3 that gets slapped after the RET, which brings the whole scheme out
of alignment:

15: 0f 90 c0 seto %al
18: c3 ret
19: cc int3
1a: 0f 1f 00 nopl (%rax)
1d: 0f 91 c0 setno %al
20: c3 ret
21: cc int3
22: 0f 1f 00 nopl (%rax)
25: 0f 92 c0 setb %al
28: c3 ret
29: cc int3

and this explodes like this:

int3: 0000 [#1] PREEMPT SMP PTI
CPU: 0 PID: 2435 Comm: qemu-system-x86 Not tainted 5.17.0-rc8-sls #1
Hardware name: Dell Inc. Precision WorkStation T3400 /0TP412, BIOS A14 04/30/2012
RIP: 0010:setc+0x5/0x8 [kvm]
Code: 00 00 0f 1f 00 0f b6 05 43 24 06 00 c3 cc 0f 1f 80 00 00 00 00 0f 90 c0 c3 cc 0f 1f 00 0f 91 c0 c3 cc 0f 1f 00 0f 92 c0 c3 cc <0f> 1f 00 0f 93 c0 c3 cc 0f 1f 00 0f 94 c0 c3 cc 0f 1f 00 0f 95 c0
Call Trace:
<TASK>
? x86_emulate_insn [kvm]
? x86_emulate_instruction [kvm]
? vmx_handle_exit [kvm_intel]
? kvm_arch_vcpu_ioctl_run [kvm]
? kvm_vcpu_ioctl [kvm]
? __x64_sys_ioctl
? do_syscall_64+0x40/0xa0
? entry_SYSCALL_64_after_hwframe+0x44/0xae
</TASK>

Raise the alignment value when SLS is enabled and use a macro for that
instead of hard-coding naked numbers.

Fixes: e463a09af2f0 ("x86: Add straight-line-speculation mitigation")
Reported-by: Jamie Heilman <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kvm/emulate.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 5719d8cfdbd9..f321abb9a4a8 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -429,8 +429,11 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);
FOP_END

/* Special case for SETcc - 1 instruction per cc */
+
+#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
+
#define FOP_SETCC(op) \
- ".align 4 \n\t" \
+ ".align " __stringify(SETCC_ALIGN) " \n\t" \
".type " #op ", @function \n\t" \
#op ": \n\t" \
#op " %al \n\t" \
@@ -1047,7 +1050,7 @@ static int em_bsr_c(struct x86_emulate_ctxt *ctxt)
static __always_inline u8 test_cc(unsigned int condition, unsigned long flags)
{
u8 rc;
- void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf);
+ void (*fop)(void) = (void *)em_setcc + SETCC_ALIGN * (condition & 0xf);

flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
asm("push %[flags]; popf; " CALL_NOSPEC
--
2.29.2

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette


2022-03-17 17:37:52

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On Thu, Mar 17, 2022 at 11:52:33AM +0100, Borislav Petkov wrote:

> From: Borislav Petkov <[email protected]>
>
> The commit in Fixes started adding INT3 after RETs as a mitigation
> against straight-line speculation.
>
> The fastop SETcc implementation in kvm's insn emulator uses macro magic
> to generate all possible SETcc functions and to jump to them when
> emulating the respective instruction.
>
> However, it hardcodes the size and alignment of those functions to 4: a
> three-byte SETcc insn and a single-byte RET. BUT, with SLS, there's an
> INT3 that gets slapped after the RET, which brings the whole scheme out
> of alignment:
>
> 15: 0f 90 c0 seto %al
> 18: c3 ret
> 19: cc int3
> 1a: 0f 1f 00 nopl (%rax)
> 1d: 0f 91 c0 setno %al
> 20: c3 ret
> 21: cc int3
> 22: 0f 1f 00 nopl (%rax)
> 25: 0f 92 c0 setb %al
> 28: c3 ret
> 29: cc int3
>
> and this explodes like this:
>
> int3: 0000 [#1] PREEMPT SMP PTI
> CPU: 0 PID: 2435 Comm: qemu-system-x86 Not tainted 5.17.0-rc8-sls #1
> Hardware name: Dell Inc. Precision WorkStation T3400 /0TP412, BIOS A14 04/30/2012
> RIP: 0010:setc+0x5/0x8 [kvm]
> Code: 00 00 0f 1f 00 0f b6 05 43 24 06 00 c3 cc 0f 1f 80 00 00 00 00 0f 90 c0 c3 cc 0f 1f 00 0f 91 c0 c3 cc 0f 1f 00 0f 92 c0 c3 cc <0f> 1f 00 0f 93 c0 c3 cc 0f 1f 00 0f 94 c0 c3 cc 0f 1f 00 0f 95 c0
> Call Trace:
> <TASK>
> ? x86_emulate_insn [kvm]
> ? x86_emulate_instruction [kvm]
> ? vmx_handle_exit [kvm_intel]
> ? kvm_arch_vcpu_ioctl_run [kvm]
> ? kvm_vcpu_ioctl [kvm]
> ? __x64_sys_ioctl
> ? do_syscall_64+0x40/0xa0
> ? entry_SYSCALL_64_after_hwframe+0x44/0xae
> </TASK>
>
> Raise the alignment value when SLS is enabled and use a macro for that
> instead of hard-coding naked numbers.
>
> Fixes: e463a09af2f0 ("x86: Add straight-line-speculation mitigation")
> Reported-by: Jamie Heilman <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]

Acked-by: Peter Zijlstra (Intel) <[email protected]>

Depending on what Paolo wants, it might make sense to merge this into
tip/x86/urgent such that we can then resolve the merge conflict vs
tip/x86/core with something like the below:

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 113fd5c1b874..06dfbe9adcdb 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -24,6 +24,7 @@
#include <linux/stringify.h>
#include <asm/debugreg.h>
#include <asm/nospec-branch.h>
+#include <asm/ibt.h>

#include "x86.h"
#include "tss.h"
@@ -431,7 +432,19 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);

/* Special case for SETcc - 1 instruction per cc */

-#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
+/*
+ * Depending on .config the SETcc functions look like:
+ *
+ * setcc:
+ * +0 ENDBR [CONFIG_X86_KERNEL_IBT]
+ * +4 SETcc %al
+ * +7 RET
+ * +8 INT3 [CONFIG_SLS]
+ *
+ * Which gives possible sizes: 4, 5, 8, 9 which when rounded up to the
+ * next power-of-two alignment become: 4, 8, 16.
+ */
+#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)) * (1 + HAS_KERNEL_IBT))

#define FOP_SETCC(op) \
".align " __stringify(SETCC_ALIGN) " \n\t" \

2022-03-17 19:59:19

by Jamie Heilman

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

Borislav Petkov wrote:
> On Thu, Mar 17, 2022 at 10:37:56AM +0100, Borislav Petkov wrote:
> > Jamie, I'd appreciate testing this one too, pls, just in case.
>
> Here's a version against -rc8 - the previous one was against tip/master
> and had other contextual changes in it.

You can add my Tested-by: Jamie Heilman <[email protected]>

This still works great with CONFIG_SLS=Y or CONFIG_SLS=N.

> ---
> From: Borislav Petkov <[email protected]>
>
> The commit in Fixes started adding INT3 after RETs as a mitigation
> against straight-line speculation.
>
> The fastop SETcc implementation in kvm's insn emulator uses macro magic
> to generate all possible SETcc functions and to jump to them when
> emulating the respective instruction.
>
> However, it hardcodes the size and alignment of those functions to 4: a
> three-byte SETcc insn and a single-byte RET. BUT, with SLS, there's an
> INT3 that gets slapped after the RET, which brings the whole scheme out
> of alignment:
>
> 15: 0f 90 c0 seto %al
> 18: c3 ret
> 19: cc int3
> 1a: 0f 1f 00 nopl (%rax)
> 1d: 0f 91 c0 setno %al
> 20: c3 ret
> 21: cc int3
> 22: 0f 1f 00 nopl (%rax)
> 25: 0f 92 c0 setb %al
> 28: c3 ret
> 29: cc int3
>
> and this explodes like this:
>
> int3: 0000 [#1] PREEMPT SMP PTI
> CPU: 0 PID: 2435 Comm: qemu-system-x86 Not tainted 5.17.0-rc8-sls #1
> Hardware name: Dell Inc. Precision WorkStation T3400 /0TP412, BIOS A14 04/30/2012
> RIP: 0010:setc+0x5/0x8 [kvm]
> Code: 00 00 0f 1f 00 0f b6 05 43 24 06 00 c3 cc 0f 1f 80 00 00 00 00 0f 90 c0 c3 cc 0f 1f 00 0f 91 c0 c3 cc 0f 1f 00 0f 92 c0 c3 cc <0f> 1f 00 0f 93 c0 c3 cc 0f 1f 00 0f 94 c0 c3 cc 0f 1f 00 0f 95 c0
> Call Trace:
> <TASK>
> ? x86_emulate_insn [kvm]
> ? x86_emulate_instruction [kvm]
> ? vmx_handle_exit [kvm_intel]
> ? kvm_arch_vcpu_ioctl_run [kvm]
> ? kvm_vcpu_ioctl [kvm]
> ? __x64_sys_ioctl
> ? do_syscall_64+0x40/0xa0
> ? entry_SYSCALL_64_after_hwframe+0x44/0xae
> </TASK>
>
> Raise the alignment value when SLS is enabled and use a macro for that
> instead of hard-coding naked numbers.
>
> Fixes: e463a09af2f0 ("x86: Add straight-line-speculation mitigation")
> Reported-by: Jamie Heilman <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> ---
> arch/x86/kvm/emulate.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 5719d8cfdbd9..f321abb9a4a8 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -429,8 +429,11 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);
> FOP_END
>
> /* Special case for SETcc - 1 instruction per cc */
> +
> +#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
> +
> #define FOP_SETCC(op) \
> - ".align 4 \n\t" \
> + ".align " __stringify(SETCC_ALIGN) " \n\t" \
> ".type " #op ", @function \n\t" \
> #op ": \n\t" \
> #op " %al \n\t" \
> @@ -1047,7 +1050,7 @@ static int em_bsr_c(struct x86_emulate_ctxt *ctxt)
> static __always_inline u8 test_cc(unsigned int condition, unsigned long flags)
> {
> u8 rc;
> - void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf);
> + void (*fop)(void) = (void *)em_setcc + SETCC_ALIGN * (condition & 0xf);
>
> flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
> asm("push %[flags]; popf; " CALL_NOSPEC
> --
> 2.29.2
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

--
Jamie Heilman http://audible.transient.net/~jamie/

2022-03-20 01:52:16

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On Sat, Mar 19, 2022 at 02:41:20PM +0100, Paolo Bonzini wrote:
> Nah, don't worry. I'll take care of it, I'm still not 100% on top of things
> but I can handle one patch. :)

Well, if you take it, then you'll have to give us an immutable branch,
please, to merge it into x86/core so that peterz can do his IBT fix
ontop before he sends the stuff during the merge window.

In any case, here's the final version (did some commit message fixups +
added tags).

Thx.

---
From: Borislav Petkov <[email protected]>
Date: Wed, 16 Mar 2022 22:05:52 +0100
Subject: [PATCH] kvm/emulate: Fix SETcc emulation function offsets with SLS

The commit in Fixes started adding INT3 after RETs as a mitigation
against straight-line speculation.

The fastop SETcc implementation in kvm's insn emulator uses macro magic
to generate all possible SETcc functions and to jump to them when
emulating the respective instruction.

However, it hardcodes the size and alignment of those functions to 4: a
three-byte SETcc insn and a single-byte RET. BUT, with SLS, there's an
INT3 that gets slapped after the RET, which brings the whole scheme out
of alignment:

15: 0f 90 c0 seto %al
18: c3 ret
19: cc int3
1a: 0f 1f 00 nopl (%rax)
1d: 0f 91 c0 setno %al
20: c3 ret
21: cc int3
22: 0f 1f 00 nopl (%rax)
25: 0f 92 c0 setb %al
28: c3 ret
29: cc int3

and this explodes like this:

int3: 0000 [#1] PREEMPT SMP PTI
CPU: 0 PID: 2435 Comm: qemu-system-x86 Not tainted 5.17.0-rc8-sls #1
Hardware name: Dell Inc. Precision WorkStation T3400 /0TP412, BIOS A14 04/30/2012
RIP: 0010:setc+0x5/0x8 [kvm]
Code: 00 00 0f 1f 00 0f b6 05 43 24 06 00 c3 cc 0f 1f 80 00 00 00 00 0f 90 c0 c3 cc 0f \
1f 00 0f 91 c0 c3 cc 0f 1f 00 0f 92 c0 c3 cc <0f> 1f 00 0f 93 c0 c3 cc 0f 1f 00 \
0f 94 c0 c3 cc 0f 1f 00 0f 95 c0
Call Trace:
<TASK>
? x86_emulate_insn [kvm]
? x86_emulate_instruction [kvm]
? vmx_handle_exit [kvm_intel]
? kvm_arch_vcpu_ioctl_run [kvm]
? kvm_vcpu_ioctl [kvm]
? __x64_sys_ioctl
? do_syscall_64
? entry_SYSCALL_64_after_hwframe
</TASK>

Raise the alignment value when SLS is enabled and use a macro for that
instead of hard-coding naked numbers.

Fixes: e463a09af2f0 ("x86: Add straight-line-speculation mitigation")
Reported-by: Jamie Heilman <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Tested-by: Jamie Heilman <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kvm/emulate.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 5719d8cfdbd9..f321abb9a4a8 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -429,8 +429,11 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);
FOP_END

/* Special case for SETcc - 1 instruction per cc */
+
+#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
+
#define FOP_SETCC(op) \
- ".align 4 \n\t" \
+ ".align " __stringify(SETCC_ALIGN) " \n\t" \
".type " #op ", @function \n\t" \
#op ": \n\t" \
#op " %al \n\t" \
@@ -1047,7 +1050,7 @@ static int em_bsr_c(struct x86_emulate_ctxt *ctxt)
static __always_inline u8 test_cc(unsigned int condition, unsigned long flags)
{
u8 rc;
- void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf);
+ void (*fop)(void) = (void *)em_setcc + SETCC_ALIGN * (condition & 0xf);

flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
asm("push %[flags]; popf; " CALL_NOSPEC
--
2.29.2

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-03-21 06:20:29

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On 3/19/22 14:36, Borislav Petkov wrote:
> On Sat, Mar 19, 2022 at 02:24:06PM +0100, Paolo Bonzini wrote:
>> Sorry for responding late, I was sick the past few days. Go ahead and apply
>> it to tip/x86/core with the rest of the SLS and IBT patches. If you place
>> it in front of the actual insertion of the INT3 it will even be bisectable,
>> but I'm not sure if your commit hashes are already frozen.
>
> I think they are and we need this fix in 5.17 where the SLS stuff went
> in. I'll send it to Linus tomorrow.

Nah, don't worry. I'll take care of it, I'm still not 100% on top of
things but I can handle one patch. :)

Paolo

>> Just one thing:
>
> Yeah, peterz can then do this ontop, before sending the IBT pile.
>
> Thx for letting us know!
>

2022-03-21 07:33:55

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On 3/17/22 12:04, Peter Zijlstra wrote:
> Acked-by: Peter Zijlstra (Intel) <[email protected]>
>
> Depending on what Paolo wants, it might make sense to merge this into
> tip/x86/urgent such that we can then resolve the merge conflict vs
> tip/x86/core with something like the below:

Sorry for responding late, I was sick the past few days. Go ahead and
apply it to tip/x86/core with the rest of the SLS and IBT patches. If
you place it in front of the actual insertion of the INT3 it will even
be bisectable, but I'm not sure if your commit hashes are already frozen.

Just one thing:

> -#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
> +/*
> + * Depending on .config the SETcc functions look like:
> + *
> + * setcc:
> + * +0 ENDBR [CONFIG_X86_KERNEL_IBT]
> + * +4 SETcc %al
> + * +7 RET
> + * +8 INT3 [CONFIG_SLS]
> + *
> + * Which gives possible sizes: 4, 5, 8, 9 which when rounded up to the
> + * next power-of-two alignment become: 4, 8, 16.
> + */
> +#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)) * (1 + HAS_KERNEL_IBT))

This might be slightly nicer as (4 << IS_ENABLED(CONFIG_SLS) <<
HAS_KERNEL_IBT. Or maybe not, depends on your taste.

It might also be worth doing:

#define SETCC_LENGTH (4 + IS_ENABLED(CONFIG_SLS) + 4 * HAS_KERNEL_IBT)
#define SETCC_ALIGN (4 << IS_ENABLED(CONFIG_SLS) << HAS_KERNEL_IBT)
BUILD_BUG_ON(SETCC_LENGTH <= SETCC_ALIGN);

Thanks,

Paolo

2022-03-21 14:43:47

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On March 20, 2022 2:04:02 PM UTC, Paolo Bonzini <[email protected]> wrote:
>So this is what I squashed in:
>
>diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>index f321abb9a4a8..e86d610dc6b7 100644
>--- a/arch/x86/kvm/emulate.c
>+++ b/arch/x86/kvm/emulate.c
>@@ -430,7 +430,19 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);
>
> /* Special case for SETcc - 1 instruction per cc */
>
>-#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
>+/*
>+ * Depending on .config the SETcc functions look like:
>+ *
>+ * SETcc %al [3 bytes]
>+ * RET [1 byte]
>+ * INT3 [1 byte; CONFIG_SLS]
>+ *
>+ * Which gives possible sizes 4 or 5. When rounded up to the
>+ * next power-of-two alignment they become 4 or 8.
>+ */
>+#define SETCC_LENGTH (4 + IS_ENABLED(CONFIG_SLS))
>+#define SETCC_ALIGN (4 << IS_ENABLED(CONFIG_SLS))
>+static_assert(SETCC_LENGTH <= SETCC_ALIGN);
>
> #define FOP_SETCC(op) \
> ".align " __stringify(SETCC_ALIGN) " \n\t" \
>
>Paolo


Ack.

Thanks.

--
Sent from a small device: formatting sux and brevity is inevitable.

2022-03-21 20:29:46

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On 3/19/22 14:50, Borislav Petkov wrote:
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 5719d8cfdbd9..f321abb9a4a8 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -429,8 +429,11 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);
> FOP_END
>
> /* Special case for SETcc - 1 instruction per cc */
> +
> +#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
> +
> #define FOP_SETCC(op) \
> - ".align 4 \n\t" \
> + ".align " __stringify(SETCC_ALIGN) " \n\t" \
> ".type " #op ", @function \n\t" \
> #op ": \n\t" \
> #op " %al \n\t" \
> @@ -1047,7 +1050,7 @@ static int em_bsr_c(struct x86_emulate_ctxt *ctxt)
> static __always_inline u8 test_cc(unsigned int condition, unsigned long flags)
> {
> u8 rc;
> - void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf);
> + void (*fop)(void) = (void *)em_setcc + SETCC_ALIGN * (condition & 0xf);
>
> flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
> asm("push %[flags]; popf; " CALL_NOSPEC

So this is what I squashed in:

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f321abb9a4a8..e86d610dc6b7 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -430,7 +430,19 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);

/* Special case for SETcc - 1 instruction per cc */

-#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)))
+/*
+ * Depending on .config the SETcc functions look like:
+ *
+ * SETcc %al [3 bytes]
+ * RET [1 byte]
+ * INT3 [1 byte; CONFIG_SLS]
+ *
+ * Which gives possible sizes 4 or 5. When rounded up to the
+ * next power-of-two alignment they become 4 or 8.
+ */
+#define SETCC_LENGTH (4 + IS_ENABLED(CONFIG_SLS))
+#define SETCC_ALIGN (4 << IS_ENABLED(CONFIG_SLS))
+static_assert(SETCC_LENGTH <= SETCC_ALIGN);

#define FOP_SETCC(op) \
".align " __stringify(SETCC_ALIGN) " \n\t" \

Paolo

2022-03-21 23:19:43

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

On Sat, Mar 19, 2022 at 02:24:06PM +0100, Paolo Bonzini wrote:
> Sorry for responding late, I was sick the past few days. Go ahead and apply
> it to tip/x86/core with the rest of the SLS and IBT patches. If you place
> it in front of the actual insertion of the INT3 it will even be bisectable,
> but I'm not sure if your commit hashes are already frozen.

I think they are and we need this fix in 5.17 where the SLS stuff went
in. I'll send it to Linus tomorrow.

> Just one thing:

Yeah, peterz can then do this ontop, before sending the IBT pile.

Thx for letting us know!

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette