Re: [Xen-devel] [PATCH v2] x86/hvm/viridian: zero and check vcpu context __pad field

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

Sorry. Disregard this. I posted too early; it's missing an extern for zero_page.

  Paul

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@xxxxxxxxxx]
> Sent: 30 March 2016 12:25
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Paul Durrant; Keir (Xen.org); Jan Beulich; Andrew Cooper
> Subject: [PATCH v2] x86/hvm/viridian: zero and check vcpu context __pad
> field
> 
> Commit 57844631 "save APIC assist vector" added an extra field to the
> viridian vcpu context save record. This field was only a uint8_t and
> so an extra __pad field was also added to pad up to the next 64-bit
> boundary.
> 
> This patch makes sure that __pad field is zeroed on save and checked
> for zero on restore. This prevents a potential leak of information
> from the stack and a compatibility check against future use of the
> space occupied by the __pad field.
> 
> This patch also adds a memset to make sure that the viridian domain
> context is fully zeroed. This is not strictly necessary but helps
> make the code more robust if fields are added to that struct in
> future.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> 
> v2:
>  - drop is_zero() helper an use memcmp against zero_page instead.
>  - add memset to viridian_save_domain_ctxt() to reduce potential
>    for information leakage in future.
> ---
>  xen/arch/x86/hvm/viridian.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index 5c76c1a..165f58e 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -785,6 +785,8 @@ static int viridian_save_domain_ctxt(struct domain
> *d, hvm_domain_context_t *h)
>      if ( !is_viridian_domain(d) )
>          return 0;
> 
> +    memset(&ctxt, 0, sizeof(ctxt));
> +
>      ctxt.time_ref_count = d->arch.hvm_domain.viridian.time_ref_count.val;
>      ctxt.hypercall_gpa  = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
>      ctxt.guest_os_id    = d->arch.hvm_domain.viridian.guest_os_id.raw;
> @@ -824,6 +826,8 @@ static int viridian_save_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
>      for_each_vcpu( d, v ) {
>          struct hvm_viridian_vcpu_context ctxt;
> 
> +        memset(&ctxt, 0, sizeof(ctxt));
> +
>          ctxt.apic_assist_msr = v->arch.hvm_vcpu.viridian.apic_assist.msr.raw;
>          ctxt.apic_assist_vector = 
> v->arch.hvm_vcpu.viridian.apic_assist.vector;
> 
> @@ -851,6 +855,9 @@ static int viridian_load_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
>      if ( hvm_load_entry_zeroextend(VIRIDIAN_VCPU, h, &ctxt) != 0 )
>          return -EINVAL;
> 
> +    if ( memcmp(&ctxt._pad, zero_page, sizeof(ctxt._pad)) )
> +        return -EINVAL;
> +
>      v->arch.hvm_vcpu.viridian.apic_assist.msr.raw = ctxt.apic_assist_msr;
>      if ( v->arch.hvm_vcpu.viridian.apic_assist.msr.fields.enabled )
>          initialize_apic_assist(v);
> --
> 2.1.4


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