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 in XENPF_enter_acpi_sleep
 * map_domain_pirq has IS_PRIV_FOR checked in its callers:
   * physdev_map_pirq checks when acquiring the RCU lock
   * ioapic_guest_write is checked in PHYSDEVOP_apic_write
 * PHYSDEVOP_{manage_pci_add,manage_pci_add_ext,pci_device_add} are
   checked by xsm_resource_plug_pci in pci_add_device
 * PHYSDEVOP_manage_pci_remove is checked by xsm_resource_unplug_pci
   in pci_remove_device
 * PHYSDEVOP_{restore_msi,restore_msi_ext} are checked by
   xsm_resource_setup_pci in pci_restore_msi_state
 * do_console_io has changed to IS_PRIV from an explicit domid==0

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Jan Beulich <jbeulich@suse.com>
Committed-by: Keir Fraser <keir@xen.org>

diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index e60173f9524852cb2e995f8ce96256b1d688dbb2..c693bd9dd415c16bba4cd4bde3cf7cc0a82a2944 100644 (file)
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -239,7 +239,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 8b542405ca5b7da6404cf404f870898ddc4e163f..658774ad406e7cfcdcf315ad598dc20ba1fb4656 100644 (file)
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1293,9 +1293,6 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(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/irq.c b/xen/arch/x86/irq.c
index 05cede5b5359e19fa8492a8acaac9414d570cd7f..238600af7ed9f10ade25d031a33b4b1fa0bf37c4 100644 (file)
--- 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 81489ec2d5121b72396f82eaddf84c754a26d710..af2eafa6f553d6d02fad069c8fb3e1faa15cbd58 100644 (file)
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4493,9 +4493,6 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
         XEN_GUEST_HANDLE_PARAM(e820entry_t) buffer_param;
         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 7800473894b3aa1bd28d9e820deb1e535824a3ae..32a861a140fff9464465e5e196bbfb95a81f8f6e 100644 (file)
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -109,12 +109,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 )
     {
@@ -238,10 +232,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;
@@ -433,9 +423,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -450,9 +437,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -467,8 +451,10 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&irq_op, arg, 1) != 0 )
             break;
 
-        ret = -EPERM;
-        if ( !IS_PRIV(v->domain) )
+        /* Use the APIC check since this dummy hypercall should still only
+         * be called by the domain with access to program the ioapic */
+        ret = xsm_apic(v->domain, cmd);
+        if ( ret )
             break;
 
         /* Vector is only used by hypervisor, and dom0 shouldn't
@@ -517,9 +503,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -530,9 +513,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -545,10 +525,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -571,10 +547,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -595,10 +567,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -610,10 +578,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -631,10 +595,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -650,10 +610,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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;
@@ -668,10 +624,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(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/common/kexec.c b/xen/common/kexec.c
index 25ebd6ae978702147140f995962fa35e5189f7f0..d4f633268ad4ed7a723ccd23ad93270ac3406a9f 100644 (file)
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -852,9 +852,6 @@ static int do_kexec_op_internal(unsigned long op,
     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 ae798c95dc7f4b44338113a932ae293c7dc97bbe..d40508144bc713a726be68627be5c2cb4d73701e 100644 (file)
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -921,12 +921,6 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(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/drivers/char/console.c b/xen/drivers/char/console.c
index ff360fe4df0a4dde9295fa42ce2ddf3e60c819c0..b2c3ee3419ec2b4c4aa67a3ef95cb15128bb88d1 100644 (file)
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -406,12 +406,6 @@ long do_console_io(int cmd, int count, XEN_GUEST_HANDLE_PARAM(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;

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 4384552f53e4ecff61f34d1179a58ad4c0a0f39b..fb00a01febc8d89dcc82e53ba1d30110b69a6fa3 100644 (file)
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -161,6 +161,8 @@ static XSM_INLINE int xsm_pm_op(void)
 
 static XSM_INLINE int xsm_do_mca(void)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
@@ -223,6 +225,10 @@ static XSM_INLINE int xsm_memory_stat_reservation(struct domain *d1, struct doma
 
 static XSM_INLINE int xsm_console_io(struct domain *d, int cmd)
 {
+#ifndef VERBOSE
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
+#endif
     return 0;
 }
 
@@ -233,11 +239,15 @@ static XSM_INLINE int xsm_profile(struct domain *d, int op)
 
 static XSM_INLINE int xsm_kexec(void)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_schedop_shutdown(struct domain *d1, struct domain *d2)
 {
+    if ( !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }
 
@@ -336,26 +346,36 @@ static XSM_INLINE int xsm_resource_unplug_core(void)
 
 static XSM_INLINE int xsm_resource_plug_pci(uint32_t machine_bdf)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_resource_unplug_pci(uint32_t machine_bdf)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_resource_setup_pci(uint32_t machine_bdf)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_resource_setup_gsi(int gsi)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_resource_setup_misc(void)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
@@ -396,6 +416,8 @@ static XSM_INLINE int xsm_map_domain_pirq(struct domain *d, int irq, void *data)
 
 static XSM_INLINE int xsm_unmap_domain_pirq(struct domain *d, int irq)
 {
+    if ( !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
@@ -494,6 +516,8 @@ static XSM_INLINE int xsm_mem_sharing(struct domain *d)
 
 static XSM_INLINE int xsm_apic(struct domain *d, int cmd)
 {
+    if ( !IS_PRIV(d) )
+        return -EPERM;
     return 0;
 }
 
@@ -534,6 +558,8 @@ static XSM_INLINE int xsm_efi_call(void)
 
 static XSM_INLINE int xsm_acpi_sleep(void)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
@@ -549,6 +575,8 @@ static XSM_INLINE int xsm_getidletime(void)
 
 static XSM_INLINE int xsm_machine_memory_map(void)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 

diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index e60c6f44d2cf7b20e0e63488c1f8f2df106be806..c8a799914d368308fb17d808064c9fdd07256f58 100644 (file)
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1141,10 +1141,11 @@ static int flask_apic(struct domain *d, int cmd)
 
     switch ( cmd )
     {
-    case PHYSDEVOP_APIC_READ:
+    case PHYSDEVOP_apic_read:
+    case PHYSDEVOP_alloc_irq_vector:
         perm = XEN__READAPIC;
         break;
-    case PHYSDEVOP_APIC_WRITE:
+    case PHYSDEVOP_apic_write:
         perm = XEN__WRITEAPIC;
         break;
     default: