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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 30 March 2016 12:23
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org)
> Subject: Re: [PATCH] x86/hvm/viridian: zero and check vcpu context __pad
> field
> 
> >>> On 30.03.16 at 12:32, <paul.durrant@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/hvm/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian.c
> > @@ -824,6 +824,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));
> 
> How about just adding an empty initializer to the declaration?
> 

I think having a 'zero the entire struct' call at the start is better as it 
will cover any additions made to the struct in future. It's what I had 
mistakenly assumed was already there. In fact I think adding a similar call 
into the domain context save function would probably be worthwhile.

> > @@ -834,6 +836,15 @@ static int viridian_save_vcpu_ctxt(struct domain
> *d, hvm_domain_context_t *h)
> >      return 0;
> >  }
> >
> > +static bool_t is_zero(void *p, size_t size)
> 
> At the very least this wants to be a pointer to const.
> 
> > +{
> > +    while ( size-- )
> > +        if ( *(uint8_t *)p++ != 0 )
> > +            return 0;
> > +
> > +    return 1;
> > +}
> > +
> >  static int viridian_load_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> >  {
> >      int vcpuid;
> > @@ -851,6 +862,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 ( !is_zero(&ctxt._pad, sizeof(ctxt._pad)) )
> > +        return -EINVAL;
> 
> "memcmp(&ctx._pad, zero_page, sizeof(ctxt._pad))" would be an
> alternative not requiring any new helper function.
> 

Ah, I didn't know about the zero_page definition. I'll use that and drop the 
helper.

  Paul

> Jan


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