On Thu, May 26, 2022, Kees Cook wrote: > On Wed, May 25, 2022 at 10:26:02PM +0000, Sean Christopherson wrote: > > Link: https://lore.kernel.org/all/YofQlBrlx18J7h9Y@xxxxxxxxxx > > Cc: Robert Dinse <nanook@xxxxxxxxxx> > > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> > > --- > > arch/x86/kvm/emulate.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > > index 7226a127ccb4..c58366ae4da2 100644 > > --- a/arch/x86/kvm/emulate.c > > +++ b/arch/x86/kvm/emulate.c > > @@ -247,6 +247,9 @@ enum x86_transfer_type { > > > > static ulong reg_read(struct x86_emulate_ctxt *ctxt, unsigned nr) > > { > > + if (WARN_ON_ONCE(nr >= 16)) > > + nr &= 16 - 1; > > Instead of doing a modulo here, what about forcing it into an "unused" > slot? > > i.e. define _regs as an array of [16 + 1], and: > > if (WARN_ON_ONCE(nr >= 16) > nr = 16; > > Then there is both no out-of-bounds access, but also no weird "actual" > register indexed? Eh, IMO it doesn't provide any meaningful value, and requires documenting why the emulator allocates an extra register. The guest is still going to experience data loss/corruption if KVM drops a write or reads zeros instead whatever register it was supposed to access. I.e. the guest is equally hosed either way. One idea along the lines of Vitaly's idea of KVM_BUG_ON() would be to add an emulator hook to bug the VM, e.g. #define KVM_EMULATOR_BUG_ON(cond, ctxt) \ ({ \ int __ret = (cond); \ \ if (WARN_ON_ONCE(__ret)) \ ctxt->ops->vm_bugged(ctxt); \ unlikely(__ret); \ }) to workaround not having access to the 'struct kvm_vcpu' in the emulator. The bad access will still go through, but the VM will be killed before the vCPU can re-enter the guest and do more damage.
