From: Ian Campbell <ian.campbell@citrix.com>
Date: Wed, 27 Jan 2016 15:54:06 +0000 (+0000)
Subject: xen: hvm: restrict handles to only be able to operate on target domain
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=7e22e6ac48b19a58e5d0fbcca81bda6ea1bdc3d1;p=people%2Fliuw%2Flibxenctrl-split%2Fqemu-xen.git

xen: hvm: restrict handles to only be able to operate on target domain

Combined with running as non-root after initialisation this should
limit the damage which a compromised qemu process can do.

Several things TBD:
 - When to do this (only with runas?)
 - Behaviour if cannot lock down (warn vs exit, configurable)
 - How best to structure the above without massive code duplication.
 - Foreign memory handle is opened in common code but locked down in
   xen-hvm, which seems like a dangerous separation of duties (i.e.
   something might get moved and/or forgotten).

Only the handles opened/used by QEMU running as an HVM DM are handled.
In particular handles used as a backend are not locked down, in any
case they need to have the privilege of the backend domain (i.e. the
current one containing the QEMU process), not the frontend, so there
is not much to restrict.

NB gnttab is not used by HVM DM (only by PV backends), so it is not
handled here at all.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---

diff --git a/configure b/configure
index 9fb4bf493..1e861c185 100755
--- a/configure
+++ b/configure
@@ -252,6 +252,7 @@ xen=""
 xen_ctrl_version=""
 xen_pv_domain_build="no"
 xen_pci_passthrough=""
+xen_restrict_target="no"
 linux_aio=""
 cap_ng=""
 attr=""
@@ -2145,6 +2146,36 @@ if test "$xen_pv_domain_build" = "yes" &&
 	       "which requires Xen support."
 fi
 
+if test $xen_ctrl_version -ge 471 ; then
+    if
+      cat > $TMPC <<EOF &&
+#include <xenstore.h>
+#include <xenevtchn.h>
+#include <xenforeignmemory.h>
+int main(void) {
+  struct xs_handle *xsh;
+  xenforeignmemory_handle *xfmem;
+  xenevtchn_handle *xe;
+
+  xsh = xs_daemon_open();
+  xs_restrict(xsh, 0);
+
+  xfmem = xenforeignmemory_open(0, 0);
+  xenforeignmemory_restrict_target(xfmem, 0);
+
+  xe = xenevtchn_open(0, 0);
+  xenevtchn_restrict_target(xe, 0);
+
+  return 0;
+}
+EOF
+      compile_prog "" "$xen_stable_libs -lxenstore"
+    then
+    xen_restrict_target="yes"
+  fi
+fi
+
+
 ##########################################
 # libtool probe
 
@@ -4788,6 +4819,7 @@ fi
 echo "xen support       $xen"
 if test "$xen" = "yes" ; then
   echo "xen ctrl version  $xen_ctrl_version"
+  echo "xen restricted    $xen_restrict_target"
   echo "pv dom build      $xen_pv_domain_build"
 fi
 echo "brlapi support    $brlapi"
@@ -5163,6 +5195,9 @@ if test "$xen" = "yes" ; then
   if test "$xen_pv_domain_build" = "yes" ; then
     echo "CONFIG_XEN_PV_DOMAIN_BUILD=y" >> $config_host_mak
   fi
+  if test "$xen_restrict_target" = "yes" ; then
+    echo "CONFIG_XEN_RESTRICT_TARGET=y" >> $config_host_mak
+  fi
 fi
 if test "$linux_aio" = "yes" ; then
   echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 7a3cce0b8..ab7046ccc 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -372,4 +372,22 @@ static inline int xen_domain_create(xc_interface *xc, uint32_t ssidref,
 #endif
 #endif
 
+#ifndef CONFIG_XEN_RESTRICT_TARGET
+
+/* xs_restrict has been available forever, no need to stub */
+
+static inline int xenevtchn_restrict_target(xenevtchn_handle *h, int d)
+{
+    errno = ENOSYS;
+    return -1;
+}
+
+static inline int xenforeignmemory_restrict_target(xenforeignmemory_handle *h, int d)
+{
+    errno = ENOSYS;
+    return -1;
+}
+
+#endif
+
 #endif /* QEMU_HW_XEN_COMMON_H */
diff --git a/xen-hvm.c b/xen-hvm.c
index 039680a6d..73eef0cb3 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1168,6 +1168,27 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
     xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
 }
 
+static bool xen_restrict_targets = true; /* XXX configurable? */
+
+/*
+ * Always logs.
+ *
+ * Returns true if the domain construction should be aborted.
+ */
+static bool xen_restrict_target_failure_is_fatal(const char *which, int errnoval)
+{
+    if (errnoval == ENOSYS) {
+        error_report("restricting %s handle not supported on this system",
+                     which);
+        /* XXX should be fatal? Could be an option? */
+        return false;
+    }
+
+    error_report("restricting %s handle failed: %d: %s",
+                 which, errnoval, strerror(errnoval));
+    return true;
+}
+
 void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
 {
     int i, rc;
@@ -1183,12 +1204,28 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
         perror("xen: event channel open");
         goto err;
     }
+    rc = (xen_restrict_targets
+          ? xenevtchn_restrict_target(state->xce_handle, xen_domid)
+          : 0);
+    if (rc < 0 && xen_restrict_target_failure_is_fatal("evtchn", errno))
+        goto err;
 
     state->xenstore = xs_daemon_open();
     if (state->xenstore == NULL) {
         perror("xen: xenstore open");
         goto err;
     }
+    rc = (xen_restrict_targets
+          ? xs_restrict(state->xenstore, xen_domid)
+          : 0);
+    if (rc < 0 && xen_restrict_target_failure_is_fatal("xenstore", errno))
+        goto err;
+
+    rc = (xen_restrict_targets
+          ? xenforeignmemory_restrict_target(xen_fmem, xen_domid)
+          : 0);
+    if (rc < 0 && xen_restrict_target_failure_is_fatal("xenforeignmemory", errno))
+        goto err;
 
     rc = xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
     if (rc < 0) {