diff mbox

[2/3] x86/HVM: limit writes to outgoing TSS during task switch

Message ID 58345C7F0200007800120CD5@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich Nov. 22, 2016, 1:55 p.m. UTC 
The only fields modified are EIP, EFLAGS, GPRs, and segment selectors.
CR3 in particular is not supposed to be updated.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
x86/HVM: limit writes to outgoing TSS during task switch

The only fields modified are EIP, EFLAGS, GPRs, and segment selectors.
CR3 in particular is not supposed to be updated.

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

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2952,7 +2952,6 @@ void hvm_task_switch(
     if ( taskswitch_reason == TSW_iret )
         eflags &= ~X86_EFLAGS_NT;
 
-    tss.cr3    = v->arch.hvm_vcpu.guest_cr[3];
     tss.eip    = regs->eip;
     tss.eflags = eflags;
     tss.eax    = regs->eax;
@@ -2979,8 +2978,10 @@ void hvm_task_switch(
     hvm_get_segment_register(v, x86_seg_ldtr, &segr);
     tss.ldt = segr.sel;
 
-    rc = hvm_copy_to_guest_virt(
-        prev_tr.base, &tss, sizeof(tss), PFEC_page_present);
+    rc = hvm_copy_to_guest_virt(prev_tr.base + offsetof(typeof(tss), eip),
+                                &tss.eip,
+                                (void *)&tss.trace - (void *)&tss.eip,
+                                PFEC_page_present);
     if ( rc != HVMCOPY_okay )
         goto out;

Comments

Andrew Cooper Nov. 22, 2016, 4:46 p.m. UTC | #1 
On 22/11/16 13:55, Jan Beulich wrote:
> The only fields modified are EIP, EFLAGS, GPRs, and segment selectors.
> CR3 in particular is not supposed to be updated.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2952,7 +2952,6 @@ void hvm_task_switch(
>      if ( taskswitch_reason == TSW_iret )
>          eflags &= ~X86_EFLAGS_NT;
>  
> -    tss.cr3    = v->arch.hvm_vcpu.guest_cr[3];
>      tss.eip    = regs->eip;
>      tss.eflags = eflags;
>      tss.eax    = regs->eax;
> @@ -2979,8 +2978,10 @@ void hvm_task_switch(
>      hvm_get_segment_register(v, x86_seg_ldtr, &segr);
>      tss.ldt = segr.sel;
>  
> -    rc = hvm_copy_to_guest_virt(
> -        prev_tr.base, &tss, sizeof(tss), PFEC_page_present);
> +    rc = hvm_copy_to_guest_virt(prev_tr.base + offsetof(typeof(tss), eip),
> +                                &tss.eip,
> +                                (void *)&tss.trace - (void *)&tss.eip,

How about offsetof(typeof(tss), trace) - offsetof(typeof(tss), eip)?

It avoids some casts and void pointer arithmetic.

Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich Nov. 23, 2016, 8:30 a.m. UTC | #2 
>>> On 22.11.16 at 17:46, <andrew.cooper3@citrix.com> wrote:
> On 22/11/16 13:55, Jan Beulich wrote:
>> The only fields modified are EIP, EFLAGS, GPRs, and segment selectors.
>> CR3 in particular is not supposed to be updated.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -2952,7 +2952,6 @@ void hvm_task_switch(
>>      if ( taskswitch_reason == TSW_iret )
>>          eflags &= ~X86_EFLAGS_NT;
>>  
>> -    tss.cr3    = v->arch.hvm_vcpu.guest_cr[3];
>>      tss.eip    = regs->eip;
>>      tss.eflags = eflags;
>>      tss.eax    = regs->eax;
>> @@ -2979,8 +2978,10 @@ void hvm_task_switch(
>>      hvm_get_segment_register(v, x86_seg_ldtr, &segr);
>>      tss.ldt = segr.sel;
>>  
>> -    rc = hvm_copy_to_guest_virt(
>> -        prev_tr.base, &tss, sizeof(tss), PFEC_page_present);
>> +    rc = hvm_copy_to_guest_virt(prev_tr.base + offsetof(typeof(tss), eip),
>> +                                &tss.eip,
>> +                                (void *)&tss.trace - (void *)&tss.eip,
> 
> How about offsetof(typeof(tss), trace) - offsetof(typeof(tss), eip)?
> 
> It avoids some casts and void pointer arithmetic.

Oh, yes, that's better.

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

Thanks, Jan
diff mbox

Patch

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2952,7 +2952,6 @@  void hvm_task_switch(
     if ( taskswitch_reason == TSW_iret )
         eflags &= ~X86_EFLAGS_NT;
 
-    tss.cr3    = v->arch.hvm_vcpu.guest_cr[3];
     tss.eip    = regs->eip;
     tss.eflags = eflags;
     tss.eax    = regs->eax;
@@ -2979,8 +2978,10 @@  void hvm_task_switch(
     hvm_get_segment_register(v, x86_seg_ldtr, &segr);
     tss.ldt = segr.sel;
 
-    rc = hvm_copy_to_guest_virt(
-        prev_tr.base, &tss, sizeof(tss), PFEC_page_present);
+    rc = hvm_copy_to_guest_virt(prev_tr.base + offsetof(typeof(tss), eip),
+                                &tss.eip,
+                                (void *)&tss.trace - (void *)&tss.eip,
+                                PFEC_page_present);
     if ( rc != HVMCOPY_okay )
         goto out;