[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] domctl: fix IRQ permission granting/revocation
On Wed, 2014-12-10 at 08:07 +0000, 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. Specifically you need to do the translation using the mapping of the domain doing the granting, not the domain being granted too, correct? It takes a little bit of thought to figure out which domain to check here, it would be worth a sentence or two explaining why this is the right one. > 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)) || I find hiding an assignment inside the second condition in a chain of if's to be rather obfuscated. Doing an assignment in a standalone if statement is one thing, this is going to far IMHO. Also, you range check pirq against d->nr_pirqs but then translate it against current->domain, is that correct? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |