• Sean Christopherson's avatar
    KVM: x86: Harden _regs accesses to guard against buggy input · dfe21e6b
    Sean Christopherson authored
    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: default avatarRobert Dinse <nanook@eskimo.com>
    Reported-by: default avatarKees Cook <keescook@chromium.org>
    Reviewed-by: default avatarKees Cook <keescook@chromium.org>
    Signed-off-by: default avatarSean Christopherson <seanjc@google.com>
    Message-Id: <20220526210817.3428868-3-seanjc@google.com>
    Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
    dfe21e6b
emulate.c 149 KB