Re: [Xen-devel] [PATCH v5 7/8] libxl/libxc: Move libxl_get_numainfo()'s hypercall buffer management to libxc

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2015-03-20 at 10:31 -0400, Boris Ostrovsky wrote:
> On 03/20/2015 09:56 AM, Ian Campbell wrote:
> > On Thu, 2015-03-19 at 17:54 -0400, Boris Ostrovsky wrote:
> >> diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
> >> index 411128e..607ae61 100644
> >> --- a/tools/libxc/xc_misc.c
> >> +++ b/tools/libxc/xc_misc.c
> >> @@ -209,22 +209,49 @@ out:
> >>       return ret;
> >>   }
> >>   
> >> -int xc_numainfo(xc_interface *xch,
> >> -                xc_numainfo_t *put_info)
> >> +int xc_numainfo(xc_interface *xch, unsigned *max_nodes,
> >> +                xc_meminfo_t *meminfo, uint32_t *distance)
> >>   {
> >>       int ret;
> >>       DECLARE_SYSCTL;
> >> +    DECLARE_HYPERCALL_BOUNCE(meminfo, *max_nodes * sizeof(*meminfo),
> >> +                             XC_HYPERCALL_BUFFER_BOUNCE_OUT);
> >> +    DECLARE_HYPERCALL_BOUNCE(distance,
> >> +                             *max_nodes * *max_nodes * sizeof(*distance),
> >> +                             XC_HYPERCALL_BUFFER_BOUNCE_OUT);
> >>   
> >> -    sysctl.cmd = XEN_SYSCTL_numainfo;
> >> +    if (meminfo && distance) {
> >> +        if ((ret = xc_hypercall_bounce_pre(xch, meminfo)))
> >> +            goto out;
> >> +        if ((ret = xc_hypercall_bounce_pre(xch, distance)))
> >> +            goto out;
> > Same comment about handling NULL as before.
> >
> > In addition what if only one of meminfo and distance is NULL? Is that
> > valid or do you need a !!meminfo ^ !!distance check?
> 
> I want to treat this as as an error here, which is why I have
>          } else if (meminfo || distance) {
>              errno = EINVAL;
>              return -1;
>          }
> 
> because the hypervisor will only attempt to copy numainfo things when 
> both are valid. Otherwise (i.e. even if only one is a NULL) it will 
> assume that this is a request for size. The alternative would be to add 
> another error there, which I decided not to do.

Sorry, I'd trimmed that bit before I thought of the issue. Yes, what you
had was right (although in general it is nicer to test for parameter
validity first).

Ian.


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