1. 10 6月, 2022 7 次提交
    • S
      KVM: x86: Bug the VM if the emulator generates a bogus exception vector · 49a1431d
      Sean Christopherson 提交于
      Bug the VM if KVM's emulator attempts to inject a bogus exception vector.
      The guest is likely doomed even if KVM continues on, and propagating a
      bad vector to the rest of KVM runs the risk of breaking other assumptions
      in KVM and thus triggering a more egregious bug.
      
      All existing users of emulate_exception() have hardcoded vector numbers
      (__load_segment_descriptor() uses a few different vectors, but they're
      all hardcoded), and future users are likely to follow suit, i.e. the
      change to emulate_exception() is a glorified nop.
      
      As for the ctxt->exception.vector check in x86_emulate_insn(), the few
      known times the WARN has been triggered in the past is when the field was
      not set when synthesizing a fault, i.e. for all intents and purposes the
      check protects against consumption of uninitialized data.
      Signed-off-by: NSean Christopherson <seanjc@google.com>
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Reviewed-by: NVitaly Kuznetsov <vkuznets@redhat.com>
      Message-Id: <20220526210817.3428868-8-seanjc@google.com>
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      49a1431d
    • S
      KVM: x86: Bug the VM if the emulator accesses a non-existent GPR · 1cca2f8c
      Sean Christopherson 提交于
      Bug the VM, i.e. kill it, if the emulator accesses a non-existent GPR,
      i.e. generates an out-of-bounds GPR index.  Continuing on all but
      gaurantees some form of data corruption in the guest, e.g. even if KVM
      were to redirect to a dummy register, KVM would be incorrectly read zeros
      and drop writes.
      
      Note, bugging the VM doesn't completely prevent data corruption, e.g. the
      current round of emulation will complete before the vCPU bails out to
      userspace.  But, the very act of killing the guest can also cause data
      corruption, e.g. due to lack of file writeback before termination, so
      taking on additional complexity to cleanly bail out of the emulator isn't
      justified, the goal is purely to stem the bleeding and alert userspace
      that something has gone horribly wrong, i.e. to avoid _silent_ data
      corruption.
      Signed-off-by: NSean Christopherson <seanjc@google.com>
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Reviewed-by: NVitaly Kuznetsov <vkuznets@redhat.com>
      Message-Id: <20220526210817.3428868-7-seanjc@google.com>
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      1cca2f8c
    • S
      KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM · b443183a
      Sean Christopherson 提交于
      Reduce the number of GPRs emulated by 32-bit KVM from 16 to 8.  KVM does
      not support emulating 64-bit mode on 32-bit host kernels, and so should
      never generate accesses to R8-15.
      
      Opportunistically use NR_EMULATOR_GPRS in rsm_load_state_{32,64}() now
      that it is precise and accurate for both flavors.
      
      Wrap the definition with full #ifdef ugliness; sadly, IS_ENABLED()
      doesn't guarantee a compile-time constant as far as BUILD_BUG_ON() is
      concerned.
      Signed-off-by: NSean Christopherson <seanjc@google.com>
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Message-Id: <20220526210817.3428868-6-seanjc@google.com>
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      b443183a
    • S
      KVM: x86: Use 16-bit fields to track dirty/valid emulator GPRs · 0cbc60d4
      Sean Christopherson 提交于
      Use a u16 instead of a u32 to track the dirty/valid status of GPRs in the
      emulator.  Unlike struct kvm_vcpu_arch, x86_emulate_ctxt tracks only the
      "true" GPRs, i.e. doesn't include RIP in its array, and so only needs to
      track 16 registers.
      
      Note, maxing out at 16 GPRs is a fundamental property of x86-64 and will
      not change barring a massive architecture update.  Legacy x86 ModRM and
      SIB encodings use 3 bits for GPRs, i.e. support 8 registers.  x86-64 uses
      a single bit in the REX prefix for each possible reference type to double
      the number of supported GPRs to 16 registers (4 bits).
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Signed-off-by: NSean Christopherson <seanjc@google.com>
      Message-Id: <20220526210817.3428868-5-seanjc@google.com>
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      0cbc60d4
    • S
      KVM: x86: Omit VCPU_REGS_RIP from emulator's _regs array · a5ba67b4
      Sean Christopherson 提交于
      Omit RIP from the emulator's _regs array, which is used only for GPRs,
      i.e. registers that can be referenced via ModRM and/or SIB bytes.  The
      emulator uses the dedicated _eip field for RIP, and manually reads from
      _eip to handle RIP-relative addressing.
      
      To avoid an even bigger, slightly more dangerous change, hardcode the
      number of GPRs to 16 for the time being even though 32-bit KVM's emulator
      technically should only have 8 GPRs.  Add a TODO to address that in a
      future commit.
      
      See also the comments above the read_gpr() and write_gpr() declarations,
      and obviously the handling in writeback_registers().
      
      No functional change intended.
      Signed-off-by: NSean Christopherson <seanjc@google.com>
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Message-Id: <20220526210817.3428868-4-seanjc@google.com>
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      a5ba67b4
    • S
      KVM: x86: Harden _regs accesses to guard against buggy input · dfe21e6b
      Sean Christopherson 提交于
      WARN and truncate the incoming GPR number/index when reading/writing GPRs
      in the emulator to guard against KVM bugs, e.g. to avoid out-of-bounds
      accesses to ctxt->_regs[] if KVM generates a bogus index.  Truncate the
      index instead of returning e.g. zero, as reg_write() returns a pointer
      to the register, i.e. returning zero would result in a NULL pointer
      dereference.  KVM could also force the index to any arbitrary GPR, but
      that's no better or worse, just different.
      
      Open code the restriction to 16 registers; RIP is handled via _eip and
      should never be accessed through reg_read() or reg_write().  See the
      comments above the declarations of reg_read() and reg_write(), and the
      behavior of writeback_registers().  The horrific open coded mess will be
      cleaned up in a future commit.
      
      There are no such bugs known to exist in the emulator, but determining
      that KVM is bug-free is not at all simple and requires a deep dive into
      the emulator.  The code is so convoluted that GCC-12 with the recently
      enable -Warray-bounds spits out a false-positive due to a GCC bug:
      
        arch/x86/kvm/emulate.c:254:27: warning: array subscript 32 is above array
                                       bounds of 'long unsigned int[17]' [-Warray-bounds]
          254 |         return ctxt->_regs[nr];
              |                ~~~~~~~~~~~^~~~
        In file included from arch/x86/kvm/emulate.c:23:
        arch/x86/kvm/kvm_emulate.h: In function 'reg_rmw':
        arch/x86/kvm/kvm_emulate.h:366:23: note: while referencing '_regs'
          366 |         unsigned long _regs[NR_VCPU_REGS];
              |                       ^~~~~
      
      Link: https://lore.kernel.org/all/YofQlBrlx18J7h9Y@google.com
      Link: https://bugzilla.kernel.org/show_bug.cgi?id=216026
      Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679Reported-and-tested-by: NRobert Dinse <nanook@eskimo.com>
      Reported-by: NKees Cook <keescook@chromium.org>
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Signed-off-by: NSean Christopherson <seanjc@google.com>
      Message-Id: <20220526210817.3428868-3-seanjc@google.com>
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      dfe21e6b
    • S
      KVM: x86: Grab regs_dirty in local 'unsigned long' · 61d9c412
      Sean Christopherson 提交于
      Capture ctxt->regs_dirty in a local 'unsigned long' instead of casting it
      to an 'unsigned long *' for use in for_each_set_bit().  The bitops helpers
      really do read the entire 'unsigned long', even though the walking of the
      read value is capped at the specified size.  I.e. 64-bit KVM is reading
      memory beyond ctxt->regs_dirty, which is a u32 and thus 4 bytes, whereas
      an unsigned long is 8 bytes.  Functionally it's not an issue because
      regs_dirty is in the middle of x86_emulate_ctxt, i.e. KVM is just reading
      its own memory, but relying on that coincidence is gross and unsafe.
      Reviewed-by: NVitaly Kuznetsov <vkuznets@redhat.com>
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Signed-off-by: NSean Christopherson <seanjc@google.com>
      Message-Id: <20220526210817.3428868-2-seanjc@google.com>
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      61d9c412
  2. 09 6月, 2022 33 次提交