On Wed, Apr 21, 2021 at 10:01 AM David Edmondson <dme@xxxxxxx> 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@xxxxxxx> 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.
