Re: [PATCH] domctl: fix IRQ permission granting/revocation

[Julien Grall <julien.grall@linaro.org>]

Hi Jan,

On 10/12/2014 08:07, Jan Beulich wrote:
> Commit 545607eb3c ("x86: fix various issues with handling guest IRQs")
> wasn't really consistent in one respect: The granting of access to an
> IRQ shouldn't assume the pIRQ->IRQ translation to be the same in both
> domains. In fact it is wrong to assume that a translation is already/
> still in place at the time access is being granted/revoked.

With the change to the interface, some part of libxl may misuse 
xc_domain_irq_permission. For instance in tools/libxl/libxl_create.c:

1178         ret = irq >= 0 ? xc_physdev_map_pirq(CTX->xch, domid, irq, 
&irq)

We get the PIRQ of domain domid in irq.

1179                        : -EOVERFLOW;
1180         if (!ret)
1181             ret = xc_domain_irq_permission(CTX->xch, domid, irq,
1)

Here, the PIRQ of the current domain should be passed. Fortunately, for 
this specific case, the PIRQs are the same. But this is confusing.

>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -981,18 +981,18 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>
>       case XEN_DOMCTL_irq_permission:
>       {
> -        unsigned int pirq = op->u.irq_permission.pirq;
> +        unsigned int pirq = op->u.irq_permission.pirq, irq;
>           int allow = op->u.irq_permission.allow_access;
>
>           if ( pirq >= d->nr_pirqs )
>               ret = -EINVAL;
> -        else if ( !pirq_access_permitted(current->domain, pirq) ||
> +        else if ( !(irq = pirq_access_permitted(current->domain, pirq)) ||
>                     xsm_irq_permission(XSM_HOOK, d, pirq, allow) )

As the pirq may not be the same in domain d, the XSM permission is wrong 
here.

In anycase, it looks weird to use pirq here because the guy who defines 
the policy may not know the PIRQ value.

Regards,

-- 
Julien Grall