Re: [PATCH 1/2] kvm: x86: Allow userspace to handle emulation errors

Jim Mattson
2021-04-21T17:29:02Z

On Wed, Apr 21, 2021 at 10:01 AM David Edmondson <dme@dme.org> wrote:
>
> On Wednesday, 2021-04-21 at 09:26:34 -07, Jim Mattson wrote:
>
> > On Wed, Apr 21, 2021 at 1:39 AM David Edmondson <dme@dme.org> wrote:
> >>
> >> On Tuesday, 2021-04-20 at 18:34:48 UTC, Sean Christopherson wrote:
> >>
> >> > On Fri, Apr 16, 2021, Aaron Lewis wrote:
> >> >> + KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES;
> >> >> + vcpu->run->emulation_failure.insn_size = insn_size;
> >> >> + memcpy(vcpu->run->emulation_failure.insn_bytes,
> >> >> + ctxt->fetch.data, sizeof(ctxt->fetch.data));
> >> >
> >> > Doesn't truly matter, but I think it's less confusing to copy over insn_size
> >> > bytes.
> >
> >> And zero out the rest?
> >
> > Why zero? Since we're talking about an instruction stream, wouldn't
> > 0x90 make more sense than zero?
>
> I'm not sure if you are serious or not.
>
> Zero-ing out the rest was intended to be to avoid leaking any previous
> emulated instruction stream. If the user-level code wants to start
> looking for instructions after insn_bytes[insn_size], they get what they
> deserve.

If we limit the copy to insn_size bytes, the only leak is what was
already present in the vcpu->run structure before this, which may or
may not be a previous instruction stream, and which has been readable
by userspace all along.