Re: [PATCH] x86/nospec: Unwreck the RSB stuffing

[Daniel Sneddon]

On 8/16/22 10:34, Daniel Sneddon wrote:
> On 8/16/22 05:28, Peter Zijlstra wrote:
> 
>>
>> Could you please test this; I've only compiled it.
>>
> When booting I get the following BUG:
> 
> ------------[ cut here ]------------
> kernel BUG at arch/x86/kernel/alternative.c:290!
> invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.0.0-rc1-00001-gb72b03c96999 #3
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1
> 04/01/2014
> RIP: 0010:apply_alternatives+0x287/0x2d0
> Code: 4c 29 f6 03 74 24 13 89 74 24 13 45 85 c0 0f 85 d2 41 e9 00 41 0f b6 02 83
> e0 fd 3c e9 0f 84 46 ff ff ff e9 5e fe ff ff 0f 0b <0f> 0b f7 c6 00 ff ff ff 0f
> 84 68 ff ff ff 8d 71 fb c6 44 24 12 e9
> RSP: 0000:ffffffff82c03d68 EFLAGS: 00010206
> RAX: 0000000000000000 RBX: ffffffff83728c24 RCX: 0000000000007fff
> RDX: 00000000ffffffea RSI: 0000000000000000 RDI: 000000000000ffff
> RBP: ffffffff82c03d7a R08: e800000010c4c749 R09: 0001e8cc00000001
> R10: 10c48348cc000000 R11: e8ae0feb75ccff49 R12: ffffffff8372fcf8
> R13: 0000000000000000 R14: ffffffff81001a68 R15: 000000000000001f
> FS:  0000000000000000(0000) GS:ffff88813bc00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff88813ffff000 CR3: 0000000002c0c001 CR4: 0000000000770ef0
> PKRU: 55555554
> Call Trace:
>  <TASK>
>  ? insn_get_opcode+0xef/0x1c0
>  ? ct_nmi_enter+0xb3/0x180
>  ? ct_nmi_exit+0xbe/0x1d0
>  ? irqentry_exit+0x2d/0x40
>  ? asm_common_interrupt+0x22/0x40
>  alternative_instructions+0x5b/0xf5
>  check_bugs+0xdaf/0xdef
>  start_kernel+0x66a/0x6a2
>  secondary_startup_64_no_verify+0xe0/0xeb
>  </TASK>
> Modules linked in:
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:apply_alternatives+0x287/0x2d0
> Code: 4c 29 f6 03 74 24 13 89 74 24 13 45 85 c0 0f 85 d2 41 e9 00 41 0f b6 02 83
> e0 fd 3c e9 0f 84 46 ff ff ff e9 5e fe ff ff 0f 0b <0f> 0b f7 c6 00 ff ff ff 0f
> 84 68 ff ff ff 8d 71 fb c6 44 24 12 e9
> RSP: 0000:ffffffff82c03d68 EFLAGS: 00010206
> RAX: 0000000000000000 RBX: ffffffff83728c24 RCX: 0000000000007fff
> RDX: 00000000ffffffea RSI: 0000000000000000 RDI: 000000000000ffff
> RBP: ffffffff82c03d7a R08: e800000010c4c749 R09: 0001e8cc00000001
> R10: 10c48348cc000000 R11: e8ae0feb75ccff49 R12: ffffffff8372fcf8
> R13: 0000000000000000 R14: ffffffff81001a68 R15: 000000000000001f
> FS:  0000000000000000(0000) GS:ffff88813bc00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff88813ffff000 CR3: 0000000002c0c001 CR4: 0000000000770ef0
> PKRU: 55555554
> Kernel panic - not syncing: Attempted to kill the idle task!
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
> 
> 
> 
I can get it to work with the below changes.  I had to change the -1 to 0x7FFF
because bit 15 is masked out in apply_alternatives().  Not sure if that's the
right way to fix it, but it boots and has the correct assembly code in
vmx_vmexit, so the alternative does get applied correctly there.

---
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 1a31ae6d758b..c5b55c9f2849 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -420,7 +420,7 @@
 #define X86_FEATURE_V_TSC_AUX          (19*32+ 9) /* "" Virtual TSC_AUX */
 #define X86_FEATURE_SME_COHERENT       (19*32+10) /* "" AMD hardware-enforced
cache coherency */

-#define X86_FEATURE_NEVER              (-1) /* "" Logical complement of ALWAYS */
+#define X86_FEATURE_NEVER              (0x7FFF) /* "" Logical complement of
ALWAYS */

 /*
  * BUG word(s)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 62f6b8b7c4a5..5c476b37b3bc 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -284,6 +284,9 @@ void __init_or_module noinline apply_alternatives(struct
alt_instr *start,
                /* Mask away "NOT" flag bit for feature to test. */
                u16 feature = a->cpuid & ~ALTINSTR_FLAG_INV;

+               if (feature == X86_FEATURE_NEVER)
+                       goto next;
+
                instr = (u8 *)&a->instr_offset + a->instr_offset;
                replacement = (u8 *)&a->repl_offset + a->repl_offset;
                BUG_ON(a->instrlen > sizeof(insn_buff));