kvm: vmx: segment limit check: use access length

There is an imperfection in get_vmx_mem_address(): access length is ignored
when checking the limit. To fix this, pass access length as a function argument.
The access length is usually obvious since it is used by callers after
get_vmx_mem_address() call, but for vmread/vmwrite it depends on the
state of 64-bit mode.

Signed-off-by: Eugene Korenevsky <ekorenevsky@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 018790c1b47f7d64f1b3ec0fcde53bfa8efaad29..c92349e2f621e16493b7870711c9b71e143a4b31 100644 (file)
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -4008,7 +4008,7 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
  * #UD or #GP.
  */
 int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
-                       u32 vmx_instruction_info, bool wr, gva_t *ret)
+                       u32 vmx_instruction_info, bool wr, int len, gva_t *ret)
 {
        gva_t off;
        bool exn;
@@ -4115,7 +4115,7 @@ int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
                 */
                if (!(s.base == 0 && s.limit == 0xffffffff &&
                     ((s.type & 8) || !(s.type & 4))))
-                       exn = exn || ((u64)off + sizeof(u64) - 1 > s.limit);
+                       exn = exn || ((u64)off + len - 1 > s.limit);
        }
        if (exn) {
                kvm_queue_exception_e(vcpu,
@@ -4134,7 +4134,8 @@ static int nested_vmx_get_vmptr(struct kvm_vcpu *vcpu, gpa_t *vmpointer)
        struct x86_exception e;
 
        if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
-                       vmcs_read32(VMX_INSTRUCTION_INFO), false, &gva))
+                               vmcs_read32(VMX_INSTRUCTION_INFO), false,
+                               sizeof(*vmpointer), &gva))
                return 1;
 
        if (kvm_read_guest_virt(vcpu, gva, vmpointer, sizeof(*vmpointer), &e)) {
@@ -4386,6 +4387,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
        u64 field_value;
        unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
        u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
+       int len;
        gva_t gva = 0;
        struct vmcs12 *vmcs12;
 
@@ -4423,12 +4425,12 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
                kvm_register_writel(vcpu, (((vmx_instruction_info) >> 3) & 0xf),
                        field_value);
        } else {
+               len = is_64_bit_mode(vcpu) ? 8 : 4;
                if (get_vmx_mem_address(vcpu, exit_qualification,
-                               vmx_instruction_info, true, &gva))
+                               vmx_instruction_info, true, len, &gva))
                        return 1;
                /* _system ok, nested_vmx_check_permission has verified cpl=0 */
-               kvm_write_guest_virt_system(vcpu, gva, &field_value,
-                                           (is_long_mode(vcpu) ? 8 : 4), NULL);
+               kvm_write_guest_virt_system(vcpu, gva, &field_value, len, NULL);
        }
 
        return nested_vmx_succeed(vcpu);
@@ -4438,6 +4440,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
 static int handle_vmwrite(struct kvm_vcpu *vcpu)
 {
        unsigned long field;
+       int len;
        gva_t gva;
        struct vcpu_vmx *vmx = to_vmx(vcpu);
        unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
@@ -4463,11 +4466,11 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu)
                field_value = kvm_register_readl(vcpu,
                        (((vmx_instruction_info) >> 3) & 0xf));
        else {
+               len = is_64_bit_mode(vcpu) ? 8 : 4;
                if (get_vmx_mem_address(vcpu, exit_qualification,
-                               vmx_instruction_info, false, &gva))
+                               vmx_instruction_info, false, len, &gva))
                        return 1;
-               if (kvm_read_guest_virt(vcpu, gva, &field_value,
-                                       (is_64_bit_mode(vcpu) ? 8 : 4), &e)) {
+               if (kvm_read_guest_virt(vcpu, gva, &field_value, len, &e)) {
                        kvm_inject_page_fault(vcpu, &e);
                        return 1;
                }
@@ -4615,7 +4618,8 @@ static int handle_vmptrst(struct kvm_vcpu *vcpu)
        if (unlikely(to_vmx(vcpu)->nested.hv_evmcs))
                return 1;
 
-       if (get_vmx_mem_address(vcpu, exit_qual, instr_info, true, &gva))
+       if (get_vmx_mem_address(vcpu, exit_qual, instr_info,
+                               true, sizeof(gpa_t), &gva))
                return 1;
        /* *_system ok, nested_vmx_check_permission has verified cpl=0 */
        if (kvm_write_guest_virt_system(vcpu, gva, (void *)&current_vmptr,
@@ -4661,7 +4665,7 @@ static int handle_invept(struct kvm_vcpu *vcpu)
         * operand is read even if it isn't needed (e.g., for type==global)
         */
        if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
-                       vmx_instruction_info, false, &gva))
+                       vmx_instruction_info, false, sizeof(operand), &gva))
                return 1;
        if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
                kvm_inject_page_fault(vcpu, &e);
@@ -4723,7 +4727,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
         * operand is read even if it isn't needed (e.g., for type==global)
         */
        if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
-                       vmx_instruction_info, false, &gva))
+                       vmx_instruction_info, false, sizeof(operand), &gva))
                return 1;
        if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
                kvm_inject_page_fault(vcpu, &e);

diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index e847ff1019a29628d9e2e3fb71e3344e1f51d3b2..29d205bb4e4fefd6783d5e09aeb25bdf94db73fc 100644 (file)
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -21,7 +21,7 @@ void nested_sync_from_vmcs12(struct kvm_vcpu *vcpu);
 int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
 int vmx_get_vmx_msr(struct nested_vmx_msrs *msrs, u32 msr_index, u64 *pdata);
 int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
-                       u32 vmx_instruction_info, bool wr, gva_t *ret);
+                       u32 vmx_instruction_info, bool wr, int len, gva_t *ret);
 
 static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
 {

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f91323d527be3cc8c4c29eb6f82e7ec9409492bd..cccf73a91e883c448e8218d71c66a735c8f38165 100644 (file)
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5345,7 +5345,8 @@ static int handle_invpcid(struct kvm_vcpu *vcpu)
         * is read even if it isn't needed (e.g., for type==all)
         */
        if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
-                               vmx_instruction_info, false, &gva))
+                               vmx_instruction_info, false,
+                               sizeof(operand), &gva))
                return 1;
 
        if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {