[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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,

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@xxxxxxxx>

--- 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.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.