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