[PATCH] x86/HVM: XSETBV intercept needs to check CPL on SVM only

[PATCH] x86/HVM: XSETBV intercept needs to check CPL on SVM only

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]

VMX doesn't need a software CPL check on the XSETBV intercept, and
SVM can do that check without resorting to hvm_get_segment_register().

Clean up what is left of hvm_handle_xsetbv(), namely make it return a
proper error code.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3141,22 +3141,15 @@ out:
 
 int hvm_handle_xsetbv(u32 index, u64 new_bv)
 {
-    struct segment_register sreg;
-    struct vcpu *curr = current;
+    int rc;
 
-    hvm_get_segment_register(curr, x86_seg_ss, &sreg);
-    if ( sreg.attr.fields.dpl != 0 )
-        goto err;
+    hvm_event_crX(XCR0, new_bv, current->arch.xcr0);
 
-    hvm_event_crX(XCR0, new_bv, curr->arch.xcr0);
+    rc = handle_xsetbv(index, new_bv);
+    if ( rc )
+        hvm_inject_hw_exception(TRAP_gp_fault, 0);
 
-    if ( handle_xsetbv(index, new_bv) )
-        goto err;
-
-    return 0;
-err:
-    hvm_inject_hw_exception(TRAP_gp_fault, 0);
-    return -1;
+    return rc;
 }
 
 int hvm_set_efer(uint64_t value)
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2609,10 +2609,11 @@ void svm_vmexit_handler(struct cpu_user_
         break;
 
     case VMEXIT_XSETBV:
-        if ( (inst_len = __get_instruction_length(current, INSTR_XSETBV))==0 )
-            break;
-        if ( hvm_handle_xsetbv(regs->ecx,
-                               (regs->rdx << 32) | regs->_eax) == 0 )
+        if ( vmcb_get_cpl(vmcb) )
+            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+        else if ( (inst_len = __get_instruction_length(v, INSTR_XSETBV)) &&
+                  hvm_handle_xsetbv(regs->ecx,
+                                    (regs->rdx << 32) | regs->_eax) == 0 )
             __update_guest_eip(regs, inst_len);
         break;
 




[-- Attachment #2: x86-XSETBV-no-CPL-check-on-VMX.patch --]
[-- Type: text/plain, Size: 1894 bytes --]

x86/HVM: XSETBV intercept needs to check CPL on SVM only

VMX doesn't need a software CPL check on the XSETBV intercept, and
SVM can do that check without resorting to hvm_get_segment_register().

Clean up what is left of hvm_handle_xsetbv(), namely make it return a
proper error code.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3141,22 +3141,15 @@ out:
 
 int hvm_handle_xsetbv(u32 index, u64 new_bv)
 {
-    struct segment_register sreg;
-    struct vcpu *curr = current;
+    int rc;
 
-    hvm_get_segment_register(curr, x86_seg_ss, &sreg);
-    if ( sreg.attr.fields.dpl != 0 )
-        goto err;
+    hvm_event_crX(XCR0, new_bv, current->arch.xcr0);
 
-    hvm_event_crX(XCR0, new_bv, curr->arch.xcr0);
+    rc = handle_xsetbv(index, new_bv);
+    if ( rc )
+        hvm_inject_hw_exception(TRAP_gp_fault, 0);
 
-    if ( handle_xsetbv(index, new_bv) )
-        goto err;
-
-    return 0;
-err:
-    hvm_inject_hw_exception(TRAP_gp_fault, 0);
-    return -1;
+    return rc;
 }
 
 int hvm_set_efer(uint64_t value)
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2609,10 +2609,11 @@ void svm_vmexit_handler(struct cpu_user_
         break;
 
     case VMEXIT_XSETBV:
-        if ( (inst_len = __get_instruction_length(current, INSTR_XSETBV))==0 )
-            break;
-        if ( hvm_handle_xsetbv(regs->ecx,
-                               (regs->rdx << 32) | regs->_eax) == 0 )
+        if ( vmcb_get_cpl(vmcb) )
+            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+        else if ( (inst_len = __get_instruction_length(v, INSTR_XSETBV)) &&
+                  hvm_handle_xsetbv(regs->ecx,
+                                    (regs->rdx << 32) | regs->_eax) == 0 )
             __update_guest_eip(regs, inst_len);
         break;
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86/HVM: XSETBV intercept needs to check CPL on SVM only

[Andrew Cooper]

On 27/11/15 11:01, Jan Beulich wrote:
> VMX doesn't need a software CPL check on the XSETBV intercept, and
> SVM can do that check without resorting to hvm_get_segment_register().
>
> Clean up what is left of hvm_handle_xsetbv(), namely make it return a
> proper error code.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Your statement matches the statement in the manual.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH] x86/HVM: XSETBV intercept needs to check CPL on SVM only

[Boris Ostrovsky]

On 11/27/2015 06:01 AM, Jan Beulich wrote:
> VMX doesn't need a software CPL check on the XSETBV intercept, and
> SVM can do that check without resorting to hvm_get_segment_register().
>
> Clean up what is left of hvm_handle_xsetbv(), namely make it return a
> proper error code.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Re: [PATCH] x86/HVM: XSETBV intercept needs to check CPL on SVM only

[Tian, Kevin]

> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Friday, November 27, 2015 7:02 PM
> 
> VMX doesn't need a software CPL check on the XSETBV intercept, and
> SVM can do that check without resorting to hvm_get_segment_register().
> 
> Clean up what is left of hvm_handle_xsetbv(), namely make it return a
> proper error code.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>