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

From: Daniel Sneddon
Date: Tue Aug 16 2022 - 14:05:46 EST


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));