On Fri, Jul 29, 2022, Michal Luczaj wrote:
The emulator mishandles LEA with register source operand. Even though such LEA is illegal, it can be encoded and fed to CPU. In which case real hardware throws #UD. The emulator, instead, returns address of x86_emulate_ctxt._regs. This info leak hurts host's kASLR.
Tell the decoder that illegal LEA is not to be emulated.
Signed-off-by: Michal Luczaj mhal@rbox.co
What the emulator does for LEA is simply: case 0x8d: /* lea r16/r32, m */ ctxt->dst.val = ctxt->src.addr.mem.ea; break;
And it makes sense if you assume that LEA's source operand is always memory. But because there is a race window between VM-exit and the decoder instruction fetch, emulator can be force fed an arbitrary opcode of choice. Including some that are simply illegal and would cause #UD in normal circumstances. Such as a LEA with a register-direct source operand -- for which the emulator sets `op->addr.reg`, but reads `op->addr.mem.ea`.
union { unsigned long *reg; struct segmented_address { ulong ea; unsigned seg; } mem; ... } addr;Because `reg` and `mem` are in union, emulator reveals address in host's memory.
I hope this patch is not considered an `instr_dual` abuse?
Nope, definitely not abuse. This is very similar to how both SVM and VMX usurped "reserved" Mod=3 (register) encodings from SGDT, SIDT, LGDT, LIDT, and INVLPG to implement virtualization instructions. I.e. if the Mod=3 encoding is ever repurposed, then using instr_dual will become necessary. I'm actually somewhat surprised that Mod=3 hasn't already been repurposed.
arch/x86/kvm/emulate.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index f8382abe22ff..7c14706372d0 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4566,6 +4566,10 @@ static const struct mode_dual mode_dual_63 = { N, I(DstReg | SrcMem32 | ModRM | Mov, em_movsxd) }; +static const struct instr_dual instr_dual_8d = {
- D(DstReg | SrcMem | ModRM | NoAccess), N
+};
static const struct opcode opcode_table[256] = { /* 0x00 - 0x07 */ F6ALU(Lock, em_add), @@ -4622,7 +4626,7 @@ static const struct opcode opcode_table[256] = { I2bv(DstMem | SrcReg | ModRM | Mov | PageTable, em_mov), I2bv(DstReg | SrcMem | ModRM | Mov, em_mov), I(DstMem | SrcNone | ModRM | Mov | PageTable, em_mov_rm_sreg),
- D(ModRM | SrcMem | NoAccess | DstReg),
- ID(0, &instr_dual_8d),
Somewhat tentatively because I'm terrible at reading the emulator's decoder, but this looks correct...
Reviewed-by: Sean Christopherson seanjc@google.com
I(ImplicitOps | SrcMem16 | ModRM, em_mov_sreg_rm), G(0, group1A), /* 0x90 - 0x97 */ -- 2.32.0