Re: [Xen-devel] [PATCH v6 5/5] xen: introduce XENMEM_pin

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.09.13 at 18:16, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 
>>> wrote:
> +static long pin(XEN_GUEST_HANDLE_PARAM(xen_pin_t) arg)
> +{
> +    int rc;
> +    unsigned long i = 0, j;
> +    struct xen_pin pin;
> +    xen_pfn_t gpfn, mfn;
> +    struct domain *d = NULL;
> +    unsigned int memflags = 0;
> +
> +    if ( copy_from_guest(&pin, arg, 1) )
> +        return -EFAULT;
> +
> +    /* Various sanity checks. */
> +    if ( /* Extent orders are sensible? */
> +         (pin.in.extent_order > MAX_ORDER) ||
> +         /* Sizes of input list do not overflow a long? */

Again, please use singular above.

> +         ((~0UL >> pin.in.extent_order) < pin.in.nr_extents) )
> +    {
> +        rc = -EFAULT;
> +        goto fail;
> +    }
> +
> +    if ( !guest_handle_okay(pin.in.extent_start, pin.in.nr_extents) )
> +    {
> +        rc = -EFAULT;
> +        goto fail;
> +    }
> +
> +    d = rcu_lock_domain_by_any_id(pin.in.domid);
> +    if ( d == NULL )
> +    {
> +        rc = -ESRCH;
> +        goto fail;
> +    }

And again all the above yields rcu_unlock_domain() being called
without the domain having got locked.

> +
> +    if ( !is_hardware_domain(d) )
> +    {
> +        rc = -EPERM;
> +        goto fail;
> +    }
> +
> +    memflags |= MEMF_bits(domain_clamp_alloc_bitsize(
> +        d,
> +        XENMEMF_get_address_bits(pin.in.mem_flags) ? :
> +        (BITS_PER_LONG+PAGE_SHIFT)));
> +
> +    for ( ; i < pin.in.nr_extents; i++ )
> +    {
> +        if ( unlikely(__copy_from_guest_offset(
> +                      &gpfn, pin.in.extent_start, i, 1)) )
> +        {
> +            rc = -EFAULT;
> +            goto fail;
> +        }
> +
> +        if ( generic_fls64(gpfn << PAGE_SHIFT) > memflags )

Didn't you mean MEMF_bits(memflags) here?

> +        {
> +            rc = -EINVAL;
> +            goto fail;
> +        }
> +
> +        rc = guest_physmap_pin_range(d, gpfn, pin.in.extent_order);
> +        if ( rc )
> +            goto fail;
> +
> +        for ( j = 0; j < (1UL << pin.in.extent_order); j++ )
> +        {
> +            mfn = gmfn_to_mfn(d, gpfn);

I'm afraid you didn't even build test this on x86: There's no
gmfn_to_mfn() there.

Jan


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