[Xen-devel] [PATCH v4 3/6] libxl: allow /local/domain/$LIBXL_TOOLSTACK_DOMID/device-model/$DOMID to be written by $DOMID

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

The device model is going to restrict its xenstore connection to $DOMID
level, using XS_RESTRICT, only implemented by oxenstored at present.

Let qemu-xen access
/local/domain/$LIBXL_TOOLSTACK_DOMID/device-model/$DOMID, as it is
required by QEMU to read/write the physmap. It doesn't contain any
information the guest is not already fully aware of.

Add a maximum limit of physmap entries to save, so that the guest cannot
DOS the toolstack.

Information under
/local/domain/$LIBXL_TOOLSTACK_DOMID/device-model/$DOMID include:

- the device model status, which is updated by the device model before
the guest is unpaused, then ignored by both QEMU and libxl

- the guest physmap, which contains the memory layout of the guest. The
guest is already in possession of this information. A malicious guest
could modify the physmap entries causing QEMU to read wrong information
at restore time. QEMU would populate its XenPhysmap list with wrong
entries that wouldn't match its own device emulation addresses, leading
to a failure in restoring the domain. But it could not compromise the
security of the system because the addresses are only used for checks
against QEMU's addresses.


In the case of qemu-traditional, the state node is used to issue
commands to the device model, so avoid to change permissions in that
case.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


---
Changes in v4:
- add error message in case count > MAX_PHYSMAP_ENTRIES
- add a note to xenstore-paths.markdown about the possible change in
privilege level
- only change permissions if xsrestrict is supported

Changes in v3:
- use LIBXL_TOOLSTACK_DOMID instead of 0 in the commit message
- update commit message with more info on why it is safe
- add a limit on the number of physmap entries to save and restore

Changes in v2:
- fix permissions to actually do what intended
- use LIBXL_TOOLSTACK_DOMID instead of 0
---
 docs/misc/xenstore-paths.markdown |    7 +++++--
 tools/libxl/libxl_dm.c            |   12 +++++++++++-
 tools/libxl/libxl_dom.c           |    7 +++++++
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/docs/misc/xenstore-paths.markdown 
b/docs/misc/xenstore-paths.markdown
index 780f601..99e038b 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -306,10 +306,13 @@ A virtual network device backend. Described by
 
 A PV console backend. Described in [console.txt](console.txt)
 
-#### ~/device-model/$DOMID/* [INTERNAL]
+#### ~/device-model/$DOMID/* [w]
 
 Information relating to device models running in the domain. $DOMID is
-the target domain of the device model.
+the target domain of the device model. When the device model is running
+at the same privilege level of the guest domain, this path does not
+contain any sensitive information and becomes guest writeable. Otherwise
+it is INTERNAL.
 
 #### ~/libxl/disable_udev = ("1"|"0") []
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index d5f230a..ecae536 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1531,6 +1531,7 @@ void libxl__spawn_local_dm(libxl__egc *egc, 
libxl__dm_spawn_state *dmss)
     char **pass_stuff;
     const char *dm;
     int dm_state_fd = -1;
+    struct xs_permissions rwperm[2];
 
     if (libxl_defbool_val(b_info->device_model_stubdomain)) {
         abort();
@@ -1573,7 +1574,16 @@ void libxl__spawn_local_dm(libxl__egc *egc, 
libxl__dm_spawn_state *dmss)
     }
 
     path = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID, domid, "");
-    xs_mkdir(ctx->xsh, XBT_NULL, path);
+    if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN &&
+        libxl__check_qemu_supported(gc, dm, "xsrestrict")) {
+        rwperm[0].id = LIBXL_TOOLSTACK_DOMID;
+        rwperm[0].perms = XS_PERM_NONE;
+        rwperm[1].id = domid;
+        rwperm[1].perms = XS_PERM_WRITE;
+        libxl__xs_mkdir(gc, XBT_NULL, path, rwperm, 2);
+    } else {
+        xs_mkdir(ctx->xsh, XBT_NULL, path);
+    }
 
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
         b_info->device_model_version
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index f408646..c8523da 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1677,6 +1677,8 @@ static inline char *physmap_path(libxl__gc *gc, uint32_t 
dm_domid,
                                        phys_offset, node);
 }
 
+#define MAX_PHYSMAP_ENTRIES 12
+
 int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
         uint32_t *len, void *dss_void)
 {
@@ -1698,6 +1700,11 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
                 &num);
     count = num;
 
+    if (count > MAX_PHYSMAP_ENTRIES) {
+        LOG(ERROR, "Max physmap entries reached");
+        return -1;
+    }
+
     *len = sizeof(version) + sizeof(count);
     *buf = calloc(1, *len);
     ptr = *buf;
-- 
1.7.10.4


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