[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] domctl: fix IRQ permission granting/revocation
On 12/11/2014 06:44 AM, Jan Beulich wrote: On 10.12.14 at 10:53, <Ian.Campbell@xxxxxxxxxxxxx> wrote:On Wed, 2014-12-10 at 08:07 +0000, Jan Beulich wrote:--- 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.Changed. My main intention was to avoid having to add another "break;".Also, you range check pirq against d->nr_pirqs but then translate it against current->domain, is that correct?No, it's not. As much as xsm_irq_permission(XSM_HOOK, d, pirq, allow) is bogus, yet it's not clear to me whether switching that to use the Xen IRQ number is okay without any other changes. Daniel? Jan At the moment, this XSM hook does not inspect the PIRQ argument, so it is safe to change it to use the Xen IRQ number. The XSM hooks currently only inspect the IRQ at mapping time; with this change (and the prior changes that fixed up the IRQ permissions rangesets), it may make sense to either add a check here or move the check in order to catch the error earlier. -- Daniel De Graaf National Security Agency _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |