[Qemu-devel] [PATCH v3 0/2] restrict the privilege of the xenstore connection

[Qemu-devel] [PATCH v3 0/2] restrict the privilege of the xenstore connection

[Stefano Stabellini]

Hi all,

this patch series introduces a new command line option to restrict the
privilege of the xenstore connection. Used together with -runas, can
help secure the execution of QEMU in Dom0.


Changes in v3:
- introduce emulator_id and use in the xenstore path
- move qemu_xen_opts to xen-common.c


Stefano Stabellini (2):
      xen: separate the xenstore_record_dm_state calls for pv and hvm machines
      xen: introduce xsrestrict and emulator_id

 hw/xenpv/xen_machine_pv.c |   15 +++++++++++++++
 include/hw/xen/xen.h      |    2 ++
 qemu-options.hx           |   19 +++++++++++++++++++
 vl.c                      |    8 ++++++++
 xen-common-stub.c         |    2 ++
 xen-common.c              |   46 +++++++++++++++++----------------------------
 xen-hvm.c                 |   37 ++++++++++++++++++++++++++++--------
 7 files changed, 92 insertions(+), 37 deletions(-)

[Qemu-devel] [PATCH v3 1/2] xen: separate the xenstore_record_dm_state calls for pv and hvm machines

[Stefano Stabellini]

The following patch will introduce a new option to restrict the
privilege of the xenstore connection. In that case, we do not want to
use multiple xenstore connections, but just the one, with lower
privileges.

For this reason, split the xenstore_record_dm_state calls for pv and hvm
machines, so that in the hvm case QEMU will reuse the same xenstore
connection. (At the moment it opens two and uses the second one for the
xenstore_record_dm_state call.)

As the xenstore_record_dm_state is not very useful anymore, just remove
it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---
Changes in v2:

- remove xenstore_record_dm_state and open code the xenstore write
instead
---
 hw/xenpv/xen_machine_pv.c |   10 ++++++++++
 xen-common.c              |   29 -----------------------------
 xen-hvm.c                 |    7 +++++++
 3 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 2e545d2..68758a0 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -45,6 +45,16 @@ static void xen_init_pv(MachineState *machine)
     switch (xen_mode) {
     case XEN_ATTACH:
         /* nothing to do, xend handles everything */
+        {
+            char path[50];
+            /* record state running */
+            snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
+            if (!xs_write(xenstore, XBT_NULL, path, "running", strlen("running"))) {
+                fprintf(stderr, "error recording state\n");
+                exit(1);
+            }
+        }
+
         break;
     case XEN_CREATE:
         if (xen_domain_build_pv(kernel_filename, initrd_filename,
diff --git a/xen-common.c b/xen-common.c
index 56359ca..5573c9e 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -83,33 +83,6 @@ void xenstore_store_pv_console_info(int i, CharDriverState *chr)
     }
 }
 
-
-static void xenstore_record_dm_state(struct xs_handle *xs, const char *state)
-{
-    char path[50];
-
-    if (xs == NULL) {
-        fprintf(stderr, "xenstore connection not initialized\n");
-        exit(1);
-    }
-
-    snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
-    if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) {
-        fprintf(stderr, "error recording dm state\n");
-        exit(1);
-    }
-}
-
-
-static void xen_change_state_handler(void *opaque, int running,
-                                     RunState state)
-{
-    if (running) {
-        /* record state running */
-        xenstore_record_dm_state(xenstore, "running");
-    }
-}
-
 static int xen_init(MachineState *ms)
 {
     xen_xc = xen_xc_interface_open(0, 0, 0);
@@ -117,8 +90,6 @@ static int xen_init(MachineState *ms)
         xen_be_printf(NULL, 0, "can't open xen interface\n");
         return -1;
     }
-    qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
-
     return 0;
 }
 
diff --git a/xen-hvm.c b/xen-hvm.c
index 315864c..8079b8e 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1107,7 +1107,14 @@ static void xen_hvm_change_state_handler(void *opaque, int running,
     XenIOState *state = opaque;
 
     if (running) {
+        char path[50];
         xen_main_loop_prepare(state);
+
+        snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
+        if (!xs_write(state->xenstore, XBT_NULL, path, "running", strlen("running"))) {
+            fprintf(stderr, "error recording state\n");
+            exit(1);
+        }
     }
 
     xen_set_ioreq_server_state(xen_xc, xen_domid,
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 2/2] xen: introduce xsrestrict and emulator_id

[Stefano Stabellini]

Introduce a new command line option "xenopts" with two suboptions:
xsrestrict (boolean) and emulator_id (numeric). When xsrestrict=on is
passed, QEMU will restrict the xenstore connection calling xs_restrict.
Also it won't initialize the pv backends as they require higher
privileges. The emulator_id is used to pass an id that identifies a
specific QEMU instance running.

Change the xenstore path for startup notification to
/local/domain/0/device-model/$DOMID/$emulator_id/state, so that
multiple QEMUs can start in parallel.

It requires a toolstack change to allow it to read/write to
/local/domain/0/device-model/$DOMID, and listen to
/local/domain/0/device-model/$DOMID/$emulator_id/state.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---
Changes in v3:
- introduce emulator_id and use in the xenstore path
- move qemu_xen_opts to xen-common.c

Changes in v2:

- change the xenpv machine xenstore path for startup notification to
device-model/$DOMID/pv/state.
---
 hw/xenpv/xen_machine_pv.c |    9 +++++++--
 include/hw/xen/xen.h      |    2 ++
 qemu-options.hx           |   19 +++++++++++++++++++
 vl.c                      |    8 ++++++++
 xen-common-stub.c         |    2 ++
 xen-common.c              |   17 +++++++++++++++++
 xen-hvm.c                 |   34 ++++++++++++++++++++++++----------
 7 files changed, 79 insertions(+), 12 deletions(-)

diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 68758a0..0b4af97 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -46,9 +46,14 @@ static void xen_init_pv(MachineState *machine)
     case XEN_ATTACH:
         /* nothing to do, xend handles everything */
         {
-            char path[50];
+            char path[50], id[40] = "";
+            QemuOpts *opts = QTAILQ_FIRST(&qemu_xen_opts.head);
+            if (qemu_opt_find(opts, "emulator_id")) {
+                uint32_t emulator_id = qemu_opt_get_number(opts, "emulator_id", 0);
+                snprintf(id, sizeof (id), "/%u", emulator_id);
+            }
             /* record state running */
-            snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
+            snprintf(path, sizeof (path), "device-model/%u%s/state", xen_domid, id);
             if (!xs_write(xenstore, XBT_NULL, path, "running", strlen("running"))) {
                 fprintf(stderr, "error recording state\n");
                 exit(1);
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 4356af4..1114f2f 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -51,4 +51,6 @@ void xen_register_framebuffer(struct MemoryRegion *mr);
 #  define HVM_MAX_VCPUS 32
 #endif
 
+extern QemuOptsList qemu_xen_opts;
+
 #endif /* QEMU_HW_XEN_H */
diff --git a/qemu-options.hx b/qemu-options.hx
index ec356f6..551e492 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3114,6 +3114,25 @@ the guest clock runs ahead of the host clock. Typically this happens
 when the shift value is high (how high depends on the host machine).
 ETEXI
 
+DEF("xenopts", HAS_ARG, QEMU_OPTION_xenopts, \
+    "-xenopts [xsrestrict=on|off][,emulator_id=id]\n" \
+    "                Xen Specific Options\n", QEMU_ARCH_ALL)
+STEXI
+@item -xenopts [xsrestrict=on|off][,emulator_id=id]
+@findex -xenopts
+Options for the Xen hypervisor:
+
+@option{xsrestrict=on} will cause QEMU to restrict its xenstore
+connection to the privilege level of the guest it is serving. This will
+cause QEMU not to initialize the Xen PV backends, as they require an higher
+privilege level.
+
+@option{emulator_id=id} specifies the numeric ID of the specific QEMU
+instance, when multiple QEMU instances are running for the same guest
+domain.
+ETEXI
+
+
 DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
     "-watchdog i6300esb|ib700\n" \
     "                enable virtual hardware watchdog [default=none]\n",
diff --git a/vl.c b/vl.c
index 15bccc4..b8a209f 100644
--- a/vl.c
+++ b/vl.c
@@ -2816,6 +2816,7 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_numa_opts);
     qemu_add_opts(&qemu_icount_opts);
     qemu_add_opts(&qemu_semihosting_config_opts);
+    qemu_add_opts(&qemu_xen_opts);
 
     runstate_init();
 
@@ -3627,6 +3628,13 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+            case QEMU_OPTION_xenopts:
+                opts = qemu_opts_parse(qemu_find_opts("xenopts"),
+                                              optarg, 0);
+                if (!opts) {
+                    exit(1);
+                }
+                break;
             case QEMU_OPTION_incoming:
                 if (!incoming) {
                     runstate_set(RUN_STATE_INMIGRATE);
diff --git a/xen-common-stub.c b/xen-common-stub.c
index 906f991..6792c2c 100644
--- a/xen-common-stub.c
+++ b/xen-common-stub.c
@@ -8,6 +8,8 @@
 #include "qemu-common.h"
 #include "hw/xen/xen.h"
 
+QemuOptsList qemu_xen_opts = { };
+
 void xenstore_store_pv_console_info(int i, CharDriverState *chr)
 {
 }
diff --git a/xen-common.c b/xen-common.c
index 5573c9e..a87cf87 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -23,6 +23,23 @@
     do { } while (0)
 #endif
 
+QemuOptsList qemu_xen_opts = {
+    .name = "xenopts",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_xen_opts.head),
+    .merge_lists = true,
+    .desc = {
+        {
+            .name = "xsrestrict",
+            .type = QEMU_OPT_BOOL,
+        },
+        {
+            .name = "emulator_id",
+            .type = QEMU_OPT_NUMBER,
+        },
+        { /* end of list */ }
+    },
+};
+
 static int store_dev_info(int domid, CharDriverState *cs, const char *string)
 {
     struct xs_handle *xs = NULL;
diff --git a/xen-hvm.c b/xen-hvm.c
index 8079b8e..2646d5b 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1107,10 +1107,16 @@ static void xen_hvm_change_state_handler(void *opaque, int running,
     XenIOState *state = opaque;
 
     if (running) {
-        char path[50];
+        char path[50], id[40] = "";
+        QemuOpts *opts = QTAILQ_FIRST(&qemu_xen_opts.head);
+
         xen_main_loop_prepare(state);
 
-        snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
+        if (qemu_opt_find(opts, "emulator_id")) {
+            uint32_t emulator_id = qemu_opt_get_number(opts, "emulator_id", 0);
+            snprintf(id, sizeof (id), "/%u", emulator_id);
+        }
+        snprintf(path, sizeof (path), "device-model/%u%s/state", xen_domid, id);
         if (!xs_write(state->xenstore, XBT_NULL, path, "running", strlen("running"))) {
             fprintf(stderr, "error recording state\n");
             exit(1);
@@ -1192,6 +1198,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
     xen_pfn_t bufioreq_pfn;
     evtchn_port_t bufioreq_evtchn;
     XenIOState *state;
+    QemuOpts *opts;
 
     state = g_malloc0(sizeof (XenIOState));
 
@@ -1310,16 +1317,23 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
     state->device_listener = xen_device_listener;
     device_listener_register(&state->device_listener);
 
-    /* Initialize backend core & drivers */
-    if (xen_be_init() != 0) {
-        fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
-        return -1;
-    }
-    xen_be_register("console", &xen_console_ops);
-    xen_be_register("vkbd", &xen_kbdmouse_ops);
-    xen_be_register("qdisk", &xen_blkdev_ops);
     xen_read_physmap(state);
 
+    opts = QTAILQ_FIRST(&qemu_xen_opts.head);
+    if (qemu_opt_get_bool(opts, "xsrestrict", false)) {
+        xs_restrict(state->xenstore, xen_domid);
+    } else {
+        /* Initialize backend core & drivers */
+        if (xen_be_init() != 0) {
+            fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
+            return -1;
+        }
+
+        xen_be_register("console", &xen_console_ops);
+        xen_be_register("vkbd", &xen_kbdmouse_ops);
+        xen_be_register("qdisk", &xen_blkdev_ops);
+    }
+
     return 0;
 }
 
-- 
1.7.10.4