Re: [BUG REPORT] USE_AFTER_FREE in complete_emulated_mmio found by KASAN/Syzkaller fuzz test (v5.10.0)

[Paolo Bonzini <pbonzini@redhat.com>]

> I've analyzed the Syzkaller output and the complete_emulated_mmio() code path.
> The buggy address is created in em_enter(), where it passes its local variable `ulong rbp` to emulate_push(), finally ends in emulator_read_write_onepage() putting the address into vcpu->mmio_fragments[].data .
> The bug happens when kvm guest executes an "enter" instruction, and top of the stack crosses the mem page.
> In that case, the em_enter() function cannot complete the instruction within itself, but leave the rest data (which is in the other page) to complete_emulated_mmio().
> When complete_emulated_mmio() starts, em_enter() has exited, so local variable `ulong rbp` is also released.
> Now complete_emulated_mmio() trys to access vcpu->mmio_fragments[].data , and the bug happened.
>
> any idea?

Ouch, the bug is certainly legit.  The easiest way to fix it is something
like this:

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c8e292e9a24d..1c8698139dd5 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1905,7 +1905,7 @@ static int em_enter(struct x86_emulate_ctxt *ctxt)
  	rbp = reg_read(ctxt, VCPU_REGS_RBP);
  	rc = emulate_push(ctxt, &rbp, stack_size(ctxt));
  	if (rc != X86EMUL_CONTINUE)
-		return rc;
+		return X86EMUL_UNHANDLEABLE;
  	assign_masked(reg_rmw(ctxt, VCPU_REGS_RBP), reg_read(ctxt, VCPU_REGS_RSP),
  		      stack_mask(ctxt));
   	assign_masked(reg_rmw(ctxt, VCPU_REGS_RSP),

The hard part is auditing all similar places that lead to passing a
local variable to emulator_read_write_onepage().

For example I found this one:

	write_segment_descriptor(ctxt, old_tss_sel, &curr_tss_desc);

which however is caught at kvm_task_switch() and changed to an
emulation error.

It may be a good idea to stick a defensive "vcpu->mmio_needed = false;"
in prepare_emulation_failure_exit(), as well as
"vcpu->arch.complete_userspace_io = NULL" both there and in
kvm_task_switch().

Paolo