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

[Xen-devel] [PATCH 11/20] xen: use XSM instead of IS_PRIV where duplicated



The Xen hypervisor has two basic access control function calls: IS_PRIV
and the xsm_* functions. Most privileged operations currently require
that both checks succeed, and many times the checks are at different
locations in the code. This patch eliminates the explicit and implicit
IS_PRIV checks that are duplicated in XSM hooks.

When XSM_ENABLE is not defined or when the dummy XSM module is used,
this patch should not change any functionality. Because the locations of
privilege checks have sometimes moved below argument validation, error
returns of some functions may change from EPERM to EINVAL or ESRCH if
called with invalid arguments and from a domain without permission to
perform the operation.

Some checks are removed due to non-obvious duplicates in their callers:

 * acpi_enter_sleep is checked by its only caller
 * map_domain_pirq has IS_PRIV_FOR checked in physdev_map_pirq
 * PHYSDEVOP_alloc_irq_vector is a noop, does not need IS_PRIV
 * Many PHYSDEVOP access checks are within the implementation functions
 * do_platform_op, do_domctl, and do_sysctl all have per-operation
   XSM hooks
 * do_console_io has changed to IS_PRIV from an explicit domid==0

Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Cc: Keir Fraser <keir@xxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/acpi/power.c         |  2 +-
 xen/arch/x86/cpu/mcheck/mce.c     |  3 ---
 xen/arch/x86/domctl.c             | 25 ++++++++++++++----
 xen/arch/x86/irq.c                |  3 +--
 xen/arch/x86/mm.c                 |  3 ---
 xen/arch/x86/physdev.c            | 54 ---------------------------------------
 xen/arch/x86/platform_hypercall.c |  3 ---
 xen/common/domctl.c               | 33 +++---------------------
 xen/common/kexec.c                |  3 ---
 xen/common/schedule.c             |  6 -----
 xen/common/sysctl.c               |  3 ---
 xen/drivers/char/console.c        |  6 -----
 12 files changed, 25 insertions(+), 119 deletions(-)

diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index 9e1f989..c7b37ef 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -238,7 +238,7 @@ static long enter_state_helper(void *data)
  */
 int acpi_enter_sleep(struct xenpf_enter_acpi_sleep *sleep)
 {
-    if ( !IS_PRIV(current->domain) || !acpi_sinfo.pm1a_cnt_blk.address )
+    if ( !acpi_sinfo.pm1a_cnt_blk.address )
         return -EPERM;
 
     /* Sanity check */
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index a89df6d..f399054 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1379,9 +1379,6 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
     struct xen_mc_msrinject *mc_msrinject;
     struct xen_mc_mceinject *mc_mceinject;
 
-    if (!IS_PRIV(v->domain) )
-        return x86_mcerr(NULL, -EPERM);
-
     ret = xsm_do_mca();
     if ( ret )
         return x86_mcerr(NULL, ret);
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 3ccf712..9bc2452 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -54,6 +54,26 @@ long arch_do_domctl(
 
     switch ( domctl->cmd )
     {
+    /* TODO: the following do not have XSM hooks yet */
+    case XEN_DOMCTL_set_cpuid:
+    case XEN_DOMCTL_suppress_spurious_page_faults:
+    case XEN_DOMCTL_debug_op:
+    case XEN_DOMCTL_gettscinfo:
+    case XEN_DOMCTL_settscinfo:
+    case XEN_DOMCTL_audit_p2m:
+    case XEN_DOMCTL_gdbsx_guestmemio:
+    case XEN_DOMCTL_gdbsx_pausevcpu:
+    case XEN_DOMCTL_gdbsx_unpausevcpu:
+    case XEN_DOMCTL_gdbsx_domstatus:
+    /* getpageframeinfo[23] needs an extra check to avoid allowing queries of 
some remote GFNs */
+    case XEN_DOMCTL_getpageframeinfo2:
+    case XEN_DOMCTL_getpageframeinfo3:
+        if ( !IS_PRIV(current->domain) )
+            return -EPERM;
+    }
+
+    switch ( domctl->cmd )
+    {
 
     case XEN_DOMCTL_shadow_op:
     {
@@ -802,11 +822,6 @@ long arch_do_domctl(
             break;
         bind = &(domctl->u.bind_pt_irq);
 
-        ret = -EPERM;
-        if ( !IS_PRIV(current->domain) &&
-             !irq_access_permitted(current->domain, bind->machine_irq) )
-            goto unbind_out;
-
         ret = xsm_unbind_pt_irq(d, bind);
         if ( ret )
             goto unbind_out;
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index c87027b..70b0f03 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1853,8 +1853,7 @@ int map_domain_pirq(
     ASSERT(spin_is_locked(&d->event_lock));
 
     if ( !IS_PRIV(current->domain) &&
-         !(IS_PRIV_FOR(current->domain, d) &&
-           irq_access_permitted(current->domain, pirq)))
+         !irq_access_permitted(current->domain, pirq))
         return -EPERM;
 
     if ( pirq < 0 || pirq >= d->nr_pirqs || irq < 0 || irq >= nr_irqs )
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 4af4130..4732c81 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4790,9 +4790,6 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
         XEN_GUEST_HANDLE(e820entry_t) buffer;
         unsigned int i;
 
-        if ( !IS_PRIV(current->domain) )
-            return -EINVAL;
-
         rc = xsm_machine_memory_map();
         if ( rc )
             return rc;
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index bf133fa..d6ea4f0 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -110,12 +110,6 @@ int physdev_map_pirq(domid_t domid, int type, int *index, 
int *pirq_p,
     if ( ret )
         return ret;
 
-    if ( !IS_PRIV_FOR(current->domain, d) )
-    {
-        ret = -EPERM;
-        goto free_domain;
-    }
-
     /* Verify or get irq. */
     switch ( type )
     {
@@ -239,10 +233,6 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
             goto free_domain;
     }
 
-    ret = -EPERM;
-    if ( !IS_PRIV_FOR(current->domain, d) )
-        goto free_domain;
-
     ret = xsm_unmap_domain_pirq(d, domain_pirq_to_irq(d, pirq));
     if ( ret )
         goto free_domain;
@@ -434,9 +424,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         ret = -EFAULT;
         if ( copy_from_guest(&apic, arg, 1) != 0 )
             break;
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
         ret = xsm_apic(v->domain, cmd);
         if ( ret )
             break;
@@ -451,9 +438,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         ret = -EFAULT;
         if ( copy_from_guest(&apic, arg, 1) != 0 )
             break;
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
         ret = xsm_apic(v->domain, cmd);
         if ( ret )
             break;
@@ -468,10 +452,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         if ( copy_from_guest(&irq_op, arg, 1) != 0 )
             break;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
-
         /* Vector is only used by hypervisor, and dom0 shouldn't
            touch it in its world, return irq_op.irq as the vecotr,
            and make this hypercall dummy, and also defer the vector 
@@ -518,9 +498,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
 
     case PHYSDEVOP_manage_pci_add: {
         struct physdev_manage_pci manage_pci;
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
         ret = -EFAULT;
         if ( copy_from_guest(&manage_pci, arg, 1) != 0 )
             break;
@@ -531,9 +508,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
 
     case PHYSDEVOP_manage_pci_remove: {
         struct physdev_manage_pci manage_pci;
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
         ret = -EFAULT;
         if ( copy_from_guest(&manage_pci, arg, 1) != 0 )
             break;
@@ -546,10 +520,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         struct physdev_manage_pci_ext manage_pci_ext;
         struct pci_dev_info pdev_info;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(current->domain) )
-            break;
-
         ret = -EFAULT;
         if ( copy_from_guest(&manage_pci_ext, arg, 1) != 0 )
             break;
@@ -572,10 +542,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         struct physdev_pci_device_add add;
         struct pci_dev_info pdev_info;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(current->domain) )
-            break;
-
         ret = -EFAULT;
         if ( copy_from_guest(&add, arg, 1) != 0 )
             break;
@@ -596,10 +562,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
     case PHYSDEVOP_pci_device_remove: {
         struct physdev_pci_device dev;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
-
         ret = -EFAULT;
         if ( copy_from_guest(&dev, arg, 1) != 0 )
             break;
@@ -612,10 +574,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
     case PHYSDEVOP_pci_mmcfg_reserved: {
         struct physdev_pci_mmcfg_reserved info;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(current->domain) )
-            break;
-
         ret = xsm_resource_setup_misc();
         if ( ret )
             break;
@@ -634,10 +592,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         struct physdev_restore_msi restore_msi;
         struct pci_dev *pdev;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
-
         ret = -EFAULT;
         if ( copy_from_guest(&restore_msi, arg, 1) != 0 )
             break;
@@ -653,10 +607,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         struct physdev_pci_device dev;
         struct pci_dev *pdev;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
-
         ret = -EFAULT;
         if ( copy_from_guest(&dev, arg, 1) != 0 )
             break;
@@ -671,10 +621,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
     case PHYSDEVOP_setup_gsi: {
         struct physdev_setup_gsi setup_gsi;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
-            break;
-
         ret = -EFAULT;
         if ( copy_from_guest(&setup_gsi, arg, 1) != 0 )
             break;
diff --git a/xen/arch/x86/platform_hypercall.c 
b/xen/arch/x86/platform_hypercall.c
index c049db7..f3304a2 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -65,9 +65,6 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xen_platform_op_t) 
u_xenpf_op)
     ret_t ret = 0;
     struct xen_platform_op curop, *op = &curop;
 
-    if ( !IS_PRIV(current->domain) )
-        return -EPERM;
-
     if ( copy_from_guest(op, u_xenpf_op, 1) )
         return -EFAULT;
 
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 2b1f182..ec8fa58 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -250,33 +250,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
     if ( op->interface_version != XEN_DOMCTL_INTERFACE_VERSION )
         return -EACCES;
 
-    switch ( op->cmd )
-    {
-    case XEN_DOMCTL_ioport_mapping:
-    case XEN_DOMCTL_memory_mapping:
-    case XEN_DOMCTL_bind_pt_irq:
-    case XEN_DOMCTL_unbind_pt_irq: {
-        struct domain *d;
-        bool_t is_priv = IS_PRIV(current->domain);
-        if ( !is_priv && ((d = rcu_lock_domain_by_id(op->domain)) != NULL) )
-        {
-            is_priv = IS_PRIV_FOR(current->domain, d);
-            rcu_unlock_domain(d);
-        }
-        if ( !is_priv )
-            return -EPERM;
-        break;
-    }
-#ifdef XSM_ENABLE
-    case XEN_DOMCTL_getdomaininfo:
-        break;
-#endif
-    default:
-        if ( !IS_PRIV(current->domain) )
-            return -EPERM;
-        break;
-    }
-
     if ( !domctl_lock_acquire() )
         return hypercall_create_continuation(
             __HYPERVISOR_domctl, "h", u_domctl);
@@ -892,10 +865,10 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
         if ( d == NULL )
             break;
 
-        if ( pirq >= d->nr_pirqs )
-            ret = -EINVAL;
-        else if ( xsm_irq_permission(d, pirq, allow) )
+        if ( xsm_irq_permission(d, pirq, allow) )
             ret = -EPERM;
+        else if ( pirq >= d->nr_pirqs )
+            ret = -EINVAL;
         else if ( allow )
             ret = irq_permit_access(d, pirq);
         else
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 2bc3e33..7dc6f24 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -851,9 +851,6 @@ static int do_kexec_op_internal(unsigned long op, 
XEN_GUEST_HANDLE(void) uarg,
     unsigned long flags;
     int ret = -EINVAL;
 
-    if ( !IS_PRIV(current->domain) )
-        return -EPERM;
-
     ret = xsm_kexec();
     if ( ret )
         return ret;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index eee74be..95142c0 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -919,12 +919,6 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE(void) arg)
         if ( d == NULL )
             break;
 
-        if ( !IS_PRIV_FOR(current->domain, d) )
-        {
-            rcu_unlock_domain(d);
-            return -EPERM;
-        }
-
         ret = xsm_schedop_shutdown(current->domain, d);
         if ( ret )
         {
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index ea68278..2cea0c3 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -33,9 +33,6 @@ long do_sysctl(XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl)
     struct xen_sysctl curop, *op = &curop;
     static DEFINE_SPINLOCK(sysctl_lock);
 
-    if ( !IS_PRIV(current->domain) )
-        return -EPERM;
-
     if ( copy_from_guest(op, u_sysctl, 1) )
         return -EFAULT;
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index e10bed5..3ff36b9 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -363,12 +363,6 @@ long do_console_io(int cmd, int count, 
XEN_GUEST_HANDLE(char) buffer)
     long rc;
     unsigned int idx, len;
 
-#ifndef VERBOSE
-    /* Only domain 0 may access the emergency console. */
-    if ( current->domain->domain_id != 0 )
-        return -EPERM;
-#endif
-
     rc = xsm_console_io(current->domain, cmd);
     if ( rc )
         return rc;
-- 
1.7.11.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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