Re: [Xen-devel] [PATCH v5] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Petre Ovidiu PIRCALABU <ppircalabu@xxxxxxxxxxxxxxx>]

On Vi, 2017-10-13 at 03:51 -0600, Jan Beulich wrote:
> > 
> > 
> > +            BUILD_BUG_ON(sizeof(struct
> > xen_hvm_altp2m_set_mem_access_multi) <
> > +                         sizeof(struct
> > compat_hvm_altp2m_set_mem_access_multi));
> What good does this do?
> Sorry, I don't understand how these size checks should look (are we
> testing that the left hand side is at least as wide as the right hand
> side?). Could you please give an example? Thanks!

> > +     */
> > +    nat.altp2m_op->version  = a.version;
> > +    nat.altp2m_op->cmd      = a.cmd;
> > +    nat.altp2m_op->domain   = a.domain;
> > +    nat.altp2m_op->pad1     = a.pad1;
> > +    nat.altp2m_op->pad2     = a.pad2;
> There are _still_ no size checks here.
I'm not sure I understand what kind of size checks should be used here.
Are you expecting something similar with the CHECK_FIELD_ macro?
> > 
> > +            if ( nat.altp2m_op->u.set_mem_access_multi.opaque > 0
> > )
> Please also check rc here to avoid needlessly copying back. In
> fact _only_ checking rc ought to be fine.
Actually, in this case rc is overwritten in do_altp2m_op (-EFAULT if
the copy didn't work or the result of hypercall_create_continuation
otherwise).
I have used the check for nat.altp2m_op->u.set_mem_access_multi.opaque
as it usually gets set to a positive value only if the hypercall gets
preempted,

Many thanks for your support,
Petre
> 
> > 
> 
> Jan
> 
> 
> ________________________
> This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel