Xen Security Advisory 178 (CVE-2016-4963) - Unsanitised driver domain input in libxl device handling

* Xen Security Advisory 178 (CVE-2016-4963) - Unsanitised driver domain input in libxl device handling
@ 2016-06-06 16:56 Xen.org security team
  0 siblings, 0 replies; 2+ messages in thread
From: Xen.org security team @ 2016-06-06 16:56 UTC (permalink / raw)
  To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team

[-- Attachment #1: Type: text/plain, Size: 6930 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

            Xen Security Advisory CVE-2016-4963 / XSA-178
                              version 4

       Unsanitised driver domain input in libxl device handling

UPDATES IN VERSION 4
====================

Clarify that issue goes back as far as driver domain suppoprt.

Mention backports.

ISSUE DESCRIPTION
=================

libxl's device-handling code freely uses and trusts information from
the backend directories in xenstore.

The backend domain (driver domain) can store bogus data in the
backend, causing libxl's enquiry functions to fail, confusing
management tools.

A driver domain can also remove its backend directory from xenstore
entirely, preventing the device from showing up in device listings and
preventing it from being removed and replaced.

A driver domain can cause libxl to generate disk eject events for
disks for which the driver domain is not responsible.

IMPACT
======

A malicious driver domain can deny service to management tools.

VULNERABLE SYSTEMS
==================

This vulnerability is only applicable to systems which are using
driver domains, and then only where the driver domain is not intended
to be fully trusted with respect to the host.

Such Xen systems using libxl based toolstacks (for example xl or
libvirt with the libxl driver) are vulnerable.

Note that even with this vulnerability a driver domain based system is
better from a security point of view, than a system where devices are
provided directly by dom0.  Users and vendors of systems using driver
domains should not change their configuration.

All versions of libxl which support the relevant driver domains are
vulnerable.

MITIGATION
==========

No mitigation is available.

CREDITS
=======

This issue was discovered by Wei Liu from Citrix.

RESOLUTION
==========

Applying the appropriate attached patch set from XSA-175, plus the
appropriate attached patch set below, resolves this issue.

xsa178-unstable/*.patch           xen-unstable

For earlier Xen releases, patches have been prepared and applied to
the staging-4.X branches, back to Xen 4.3.  The Xen Project CI system,
osstest, is currently doing basic functional testing of these
backports.  However, driver domains are not tested by osstest, nor
have they been tested by the Xen Project Security Team.  The patches
can be found in xen.git:
  xen.git commits 2805844bf20e..562ecb389444    Xen 4.6
  xen.git commits 6265a6fc7f58..d8ac67eff778    Xen 4.5
  xen.git commits 551948806424..a0b99aba04d9    Xen 4.4
  xen.git commits 5811d6bdf5bb..08ea21f2361d    Xen 4.3
See:
  http://xenbits.xen.org/gitweb/?p=xen.git;a=summary
  git://xenbits.xen.org/xen.git
  http://xenbits.xen.org/git-http/xen.git

$ sha256sum xsa178-*/*
fd6a1f858d44f618a4e792553598005871f63d12e718bc9b5477d14bf0113386  xsa178-unstable/0001-libxl-Make-copy-of-every-xs-backend-in-libxl-in-_gen.patch
ee6cf66ad385203c49d9b030959715fb885a250aa36b85080e6985a603bb1ddb  xsa178-unstable/0002-libxl-Do-not-trust-backend-in-libxl__device_exists.patch
ea29cf28609c2d467fb7a620601af7bf434b098a7554dada956f11ed50c1b895  xsa178-unstable/0003-libxl-Do-not-trust-backend-for-vtpm-in-getinfo-excep.patch
a2abc4308d9a18f49a02e6ca8ba913d4d9890867b7816dcc19b548836b65af6c  xsa178-unstable/0004-libxl-Do-not-trust-backend-for-vtpm-in-getinfo-uuid.patch
2884e6566c59ae95792d4282e174c6b3d201c1e006b9e0ab57fbaad2b62ecfb9  xsa178-unstable/0005-libxl-cdrom-eject-and-insert-write-to-libxl.patch
d6ac82211d056a386d18b8296a6a1f2e8a65e8156594595b9c34a3a377f1cf98  xsa178-unstable/0006-libxl-Do-not-trust-backend-for-disk-eject-vdev.patch
4c8bb7bee3b624b02796afdfa0157ea1dc49a7f54f34912f992bae201b6bfe40  xsa178-unstable/0007-libxl-Do-not-trust-backend-for-disk-fix-driver-domai.patch
556b14e8783ddd7ad0cb9a561ca43a40b37ccb27cd56337e7714ac0f796ce21b  xsa178-unstable/0008-libxl-Do-not-trust-backend-for-disk-in-getinfo.patch
b51aaa8cca1f367ae51ffb65240831617d4cab4a3fa6d0a2d42728e99ee8cee8  xsa178-unstable/0009-libxl-Do-not-trust-backend-for-cdrom-insert.patch
3ef493e6bda2d2b96a89cf18b55d43fbdb84a2cd5c10c88f04299434c629ba2b  xsa178-unstable/0010-libxl-Do-not-trust-backend-for-channel-in-getinfo.patch
da4db890c9e73fca006bc381f2208f9bff0fc35990c4dd51d59999db27072d33  xsa178-unstable/0011-libxl-Rename-libxl__device_-nic-channel-_from_xs_be-.patch
ae8b043a83cc35beee2205ab621b6f5bc6543f6d4dcdc06c97e07b1a17ca94bf  xsa178-unstable/0012-libxl-Rename-READ_BACKEND-to-READ_LIBXLDEV.patch
936c44de9a344b0634b7bff4f5b3cf9c034a0080e87d267e7a84683a967d1bff  xsa178-unstable/0013-libxl-Have-READ_LIBXLDEV-use-libxl_path-rather-than-.patch
3b65a3140387651cf2ed1bcf8668efecd58fbd274a62a03d785c269b55bea8fe  xsa178-unstable/0014-libxl-Do-not-trust-backend-in-nic-getinfo.patch
6d009153b98fd58f316efa4f39c821cf609b54184726e15f887947321610ed14  xsa178-unstable/0015-libxl-Do-not-trust-backend-for-nic-in-devid_to_devic.patch
3105c062bb2017681f47499e2dd2f6cd2996539068f216a5af7d6143bc726eda  xsa178-unstable/0016-libxl-Do-not-trust-backend-for-nic-in-list.patch
97961ce38d8d77e9d91ee85052fd33e04d19f45e5ddfec61f82dc9c8a78158ea  xsa178-unstable/0017-libxl-Do-not-trust-backend-in-channel-list.patch
6ebb611501b66dca66259d3a790e30ae6d892eb27c6d06577d8f399d619c286b  xsa178-unstable/0018-libxl-Do-not-trust-backend-for-vusb.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

HOWEVER note that deployment of the patches for XSA-175 (which are a
prerequisite for the patches for XSA-178) is restricted.  See
XSA-175's `Deployment During Embargo' section for details.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.


(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iQEcBAEBAgAGBQJXVasRAAoJEIP+FMlX6CvZW9EIAJZjfuCUa2pbO8oOGsjqvlSe
J9scuF2hHiQB4ii04ORwqtgaE0TmttKQMTL8XAZBvp2f5F1MJsXIlteD9kBFApEA
5lzxvVie+sP8qUj5LvZUkby1tlohWiJfylx8dOQTvU6vkzYIAS+z2dt30+KpNLL5
P7bfJEcHBMJw33PApTQpVe9NRfQox6mVxDEG+1O6W7DsX0uSH8wLh21o355kMYxO
3+ym9GXKuHVI3FFHjEtTTEbpM2s7KlSApreAEvQIe68KKjkjFIadRe+4JCrAfqg3
F8pE+VI2eH3ZMGrf52Mhko9PR9DjUc8oQiTSp/H5a10mhTaMfePDU1zADcsZmmw=
=ArWH
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa178-unstable/0001-libxl-Make-copy-of-every-xs-backend-in-libxl-in-_gen.patch --]
[-- Type: application/octet-stream, Size: 3937 bytes --]

From d0712483981daf5a748c1cd083fe61d8d9ea8102 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 16:19:28 +0100
Subject: [PATCH 01/21] libxl: Make copy of every xs backend in /libxl in
 _generic_add

We want to stop libxl trustingly reading information from the backend
directory (since this is, of course, writeable by the backend, which
might be a semi-trusted driver domain).

In principle it is wrong in current libxl for anything to try to
divine virtual device configuration from xenstore: the JSON domain
config ought to supply that, and xenstore should only tell us which
devices actually exist.

However:

Firstly, there are several existing places where configuration
information is retrieved from xenstore rather than JSON.  We do not
want to reen gineer this in a security patch.

Secondly, we want to make a security patch which can be backported to
versions of libxl without the JSON configuration machinery.

So we take the expedient approach of keeping a copy of the
configuration somewhere we trust, namely /libxl.  This is obviously
fairly low-risk, although it does write significantly more keys in
xenstore.

In this patch we make this change in libxl__device_generic_add.  This
is responsible for actually writing the vast majority of device
information to xenstore.  There are a few loose ends which will be
dealt with in a moment.

Likewise, changes to readers to use the new location will appear in
further patches.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 docs/misc/xenstore-paths.markdown |  4 ++++
 tools/libxl/libxl_device.c        | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index 2f545c1..261ee42 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -549,6 +549,10 @@ Path in xenstore to the frontend, normally
 Path in xenstore to the backend, normally
 /local/domain/$BACKEND_DOMID/backend/$KIND/$DOMID/$DEVID
 
+#### /libxl/$DOMID/device/$KIND/$DEVID/$NODE
+
+Trustworthy copy of /local/domain/$DOMID/backend/$KIND/$DEVID/$NODE.
+
 #### /libxl/$DOMID/dm-version ("qemu\_xen"|"qemu\_xen\_traditional") = [n,INTERNAL]
 
 The device model version for a domain.
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 16384f8..4b61b4c 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -185,6 +185,29 @@ retry_transaction:
         xs_write(ctx->xsh, t, GCSPRINTF("%s/frontend", backend_path),
                  frontend_path, strlen(frontend_path));
         libxl__xs_writev(gc, t, backend_path, bents);
+
+        /*
+         * We make a copy of everything for the backend in the libxl
+         * path as well.  This means we don't need to trust the
+         * backend.  Ideally this information would not be used and we
+         * would use the information from the json configuration
+         * instead.  But there are still places in libxl that try to
+         * reconstruct a config from xenstore.
+         *
+         * This duplication will typically produces duplicate keys
+         * which will go out of date, but that's OK because nothing
+         * reads those.  For example, there is usually
+         *   /libxl/$guest/device/$kind/$devid/state
+         * which starts out containing XenbusStateInitialising ("1")
+         * just like the copy in
+         *  /local/domain/$driverdom/backend/$guest/$kind/$devid/state
+         * but which won't ever be updated.
+         *
+         * This duplication is superfluous and messy but as discussed
+         * the proper fix is more intrusive than we want to do now.
+         */
+        rc = libxl__xs_writev(gc, t, libxl_path, bents);
+        if (rc) goto out;
     }
 
     if (!create_transaction)
-- 
2.1.4


[-- Attachment #3: xsa178-unstable/0002-libxl-Do-not-trust-backend-in-libxl__device_exists.patch --]
[-- Type: application/octet-stream, Size: 1054 bytes --]

From 24a66d54e50f93059c37a62c40cb957324196cd2 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 4 May 2016 15:04:35 +0100
Subject: [PATCH 02/21] libxl: Do not trust backend in libxl__device_exists

To determine whether a device is supposed to exist, look in /libxl,
rather than the backend.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 4b61b4c..4717027 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -54,7 +54,7 @@ int libxl__device_exists(libxl__gc *gc, xs_transaction_t t,
                          libxl__device *device)
 {
     int rc;
-    char *be_path = libxl__device_backend_path(gc, device);
+    char *be_path = libxl__device_libxl_path(gc, device);
     const char *dir;
 
     rc = libxl__xs_read_checked(gc, t, be_path, &dir);
-- 
2.1.4


[-- Attachment #4: xsa178-unstable/0003-libxl-Do-not-trust-backend-for-vtpm-in-getinfo-excep.patch --]
[-- Type: application/octet-stream, Size: 1980 bytes --]

From 985fbad8be66189670570dbb7cac2bb606d82ed0 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 17:18:44 +0100
Subject: [PATCH 03/21] libxl: Do not trust backend for vtpm in getinfo (except
 uuid)

* Do not check the backend for existence.  We have already read the
  /libxl path so know that the vtpm exists (or is supposed to); if the
  backend doesn't exist then that must be the backend's doing.
* Get the frontend path from the /libxl directory.
* The frontend domid is the guest domid, and does not need to be read
  from xenstore (!)

We still attempt to read the uuid from the backend.  This will be
fixed in the next patch.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 75539e9..3dd47ec 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2241,9 +2241,6 @@ int libxl_device_vtpm_getinfo(libxl_ctx *ctx,
     if (!vtpminfo->backend) {
         goto err;
     }
-    if(!libxl__xs_read(gc, XBT_NULL, vtpminfo->backend)) {
-       goto err;
-    }
 
     rc = libxl__backendpath_parse_domid(gc, vtpminfo->backend,
                                         &vtpminfo->backend_id);
@@ -2262,11 +2259,8 @@ int libxl_device_vtpm_getinfo(libxl_ctx *ctx,
     vtpminfo->rref = val ? strtoul(val, NULL, 10) : -1;
 
     vtpminfo->frontend = xs_read(ctx->xsh, XBT_NULL,
-          GCSPRINTF("%s/frontend", vtpminfo->backend), NULL);
-
-    val = libxl__xs_read(gc, XBT_NULL,
-          GCSPRINTF("%s/frontend-id", vtpminfo->backend));
-    vtpminfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+          GCSPRINTF("%s/frontend", libxl_path), NULL);
+    vtpminfo->frontend_id = domid;
 
     val = libxl__xs_read(gc, XBT_NULL,
           GCSPRINTF("%s/uuid", vtpminfo->backend));
-- 
2.1.4


[-- Attachment #5: xsa178-unstable/0004-libxl-Do-not-trust-backend-for-vtpm-in-getinfo-uuid.patch --]
[-- Type: application/octet-stream, Size: 1835 bytes --]

From 4831002680a39e327a105402f0162e3d57c9d86f Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 16:57:14 +0100
Subject: [PATCH 04/21] libxl: Do not trust backend for vtpm in getinfo (uuid)

Use uuid from /libxl, rather than from backend.  I think the backend
is not supposed to change the uuid, since it seems to be set by libxl
during setup.

If in fact the backend is supposed to be able to change the uuid, this
patch needs to be dropped and replaced by a patch which makes the vtpm
uuid lookup tolerate bad or missing data.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3dd47ec..a1cc220 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2203,7 +2203,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *n
                                               &vtpm->backend_domid);
           if (rc) return NULL;
 
-          tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/uuid", be_path));
+          tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/uuid", libxl_path));
           if (tmp) {
               if(libxl_uuid_from_string(&(vtpm->uuid), tmp)) {
                   LOG(ERROR, "%s/uuid is a malformed uuid?? (%s) Probably a bug!!\n", be_path, tmp);
@@ -2263,7 +2263,7 @@ int libxl_device_vtpm_getinfo(libxl_ctx *ctx,
     vtpminfo->frontend_id = domid;
 
     val = libxl__xs_read(gc, XBT_NULL,
-          GCSPRINTF("%s/uuid", vtpminfo->backend));
+          GCSPRINTF("%s/uuid", libxl_path));
     if(val == NULL) {
        LOG(ERROR, "%s/uuid does not exist!", vtpminfo->backend);
        goto err;
-- 
2.1.4


[-- Attachment #6: xsa178-unstable/0005-libxl-cdrom-eject-and-insert-write-to-libxl.patch --]
[-- Type: application/octet-stream, Size: 2645 bytes --]

From d637ba5a50f3c654f5caa7c251e623cace1640f8 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 19:15:13 +0100
Subject: [PATCH 05/21] libxl: cdrom eject and insert: write to /libxl

Copy the new type and params values to /libxl, so that the information
in /libxl is kept up to date.

This is needed so that we can return this trustworthy information,
rather than trusting the backend-writeable parts of xenstore.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a1cc220..39a2e03 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2896,7 +2896,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     libxl_domain_config d_config;
     int rc, dm_ver;
     libxl__device device;
-    const char * path;
+    const char *path, *libxl_path;
     char * tmp;
     libxl__domain_userdata_lock *lock = NULL;
     xs_transaction_t t = XBT_NULL;
@@ -2970,6 +2970,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     if (rc) goto out;
 
     path = libxl__device_backend_path(gc, &device);
+    libxl_path = libxl__device_libxl_path(gc, &device);
 
     insert = flexarray_make(gc, 4, 1);
 
@@ -3018,8 +3019,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
             goto out;
         }
 
-        rc = libxl__xs_writev(gc, t, path,
-                              libxl__xs_kvs_of_flexarray(gc, empty, empty->count));
+        char **kvs = libxl__xs_kvs_of_flexarray(gc, empty, empty->count);
+
+        rc = libxl__xs_writev(gc, t, path, kvs);
+        if (rc) goto out;
+
+        rc = libxl__xs_writev(gc, t, libxl_path, kvs);
         if (rc) goto out;
 
         rc = libxl__xs_transaction_commit(gc, &t);
@@ -3056,8 +3061,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         rc = libxl__set_domain_configuration(gc, domid, &d_config);
         if (rc) goto out;
 
-        rc = libxl__xs_writev(gc, t, path,
-                              libxl__xs_kvs_of_flexarray(gc, insert, insert->count));
+        char **kvs = libxl__xs_kvs_of_flexarray(gc, insert, insert->count);
+
+        rc = libxl__xs_writev(gc, t, path, kvs);
+        if (rc) goto out;
+
+        rc = libxl__xs_writev(gc, t, libxl_path, kvs);
         if (rc) goto out;
 
         rc = libxl__xs_transaction_commit(gc, &t);
-- 
2.1.4


[-- Attachment #7: xsa178-unstable/0006-libxl-Do-not-trust-backend-for-disk-eject-vdev.patch --]
[-- Type: application/octet-stream, Size: 2593 bytes --]

From a38b1590b8d05f18f37755e981edd4cb51f1098d Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 16:23:35 +0100
Subject: [PATCH 06/21] libxl: Do not trust backend for disk eject vdev

For disk eject, use configured vdev from /libxl, not backend.

The backend directory is writeable by driver domains.  This means that
a malicious driver domain could cause libxl to see a wrong vdev,
confusing the user or the toolstack.

Use the vdev from the /libxl space, rather than the backend.

For convenience, we read the vdev from the /libxl space into the evg
during setup and copy it on each event, rather than reading it afresh
each time (which would in any case involve generating or saving a copy
of the relevant /libxl path).

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 39a2e03..57ddc35 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1357,8 +1357,7 @@ static void disk_eject_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
     disk->pdev_path = strdup(""); /* xxx fixme malloc failure */
     disk->format = LIBXL_DISK_FORMAT_EMPTY;
     /* this value is returned to the user: do not free right away */
-    disk->vdev = xs_read(CTX->xsh, XBT_NULL,
-                         GCSPRINTF("%s/dev", backend), NULL);
+    disk->vdev = libxl__strdup(NOGC, evg->vdev);
     disk->removable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;
@@ -1381,9 +1380,6 @@ int libxl_evenable_disk_eject(libxl_ctx *ctx, uint32_t guest_domid,
     evg->domid = guest_domid;
     LIBXL_LIST_INSERT_HEAD(&CTX->disk_eject_evgens, evg, entry);
 
-    evg->vdev = strdup(vdev);
-    if (!evg->vdev) { rc = ERROR_NOMEM; goto out; }
-
     uint32_t domid = libxl_get_stubdom_id(ctx, guest_domid);
 
     if (!domid)
@@ -1401,6 +1397,13 @@ int libxl_evenable_disk_eject(libxl_ctx *ctx, uint32_t guest_domid,
                                  devid);
     evg->be_ptr_path = libxl__sprintf(NOGC, "%s/backend", libxl_path);
 
+    const char *configured_vdev;
+    rc = libxl__xs_read_checked(gc, XBT_NULL,
+            GCSPRINTF("%s/vdev", libxl_path), &configured_vdev);
+    if (rc) goto out;
+
+    evg->vdev = libxl__strdup(NOGC, configured_vdev);
+
     rc = libxl__ev_xswatch_register(gc, &evg->watch,
                                     disk_eject_xswatch_callback, path);
     if (rc) goto out;
-- 
2.1.4


[-- Attachment #8: xsa178-unstable/0007-libxl-Do-not-trust-backend-for-disk-fix-driver-domai.patch --]
[-- Type: application/octet-stream, Size: 10993 bytes --]

From 7f08021df653dbc44898c70eacff143363d8cc5d Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 18:29:45 +0100
Subject: [PATCH 07/21] libxl: Do not trust backend for disk; fix driver domain
 disks list

Rework libxl__device_disk_from_xs_be (which takes a backend path) into
to libxl__device_disk_from_xenstore (which takes a libxl path).

libxl__device_disk_from_xenstore now finds the backend path itself,
although it doesn't use it any more for most of its functions.  We
rename the variable from be_path to backend_path to make sure we
didn't miss any cases.

All the data collection is now done by reading from the copy in
/libxl.

libxl_device_disk_list and its helper libxl__append_disk_list (which
used to be libxl__append_disk_list_of_type) need extensive rework,
because they now need to specify the /libxl path rather than the
backend path.

To do that they enumerate disks by looking in the appropriate area in
/libxl.  Previously they scanned various of the backend directories in
dom0 (which was broken for driver domains).  It is no longer necessary
to enumerate the various disk backends, because they all use the same
paths in /devices.  libxl__device_disk_from_xenstore will parse the
type out of the backend path, for itself.  (Indeed, it did so before -
the now-gone type parameter to libxl__append_disk_list_of_type wasn't
used other than to construct the directory to list.)

Finally, remove a redundant store to pdisk->backend_domid in
libxl__append_disk_list[_of_type].  Even before this commit, that
store was not needed because libxl_device_disk_init (called by
libxl__device_disk_from_xenstore) would zero it.  Now it overwrites
the correct backend domid with zero; so remove it.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v2: Also fix up COLO reads, following rebase
---
 tools/libxl/libxl.c | 94 +++++++++++++++++++++++++++--------------------------
 1 file changed, 48 insertions(+), 46 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 57ddc35..5aeacc4 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2635,8 +2635,8 @@ void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
     device_disk_add(egc, domid, disk, aodev, NULL, NULL);
 }
 
-static int libxl__device_disk_from_xs_be(libxl__gc *gc,
-                                         const char *be_path,
+static int libxl__device_disk_from_xenstore(libxl__gc *gc,
+                                         const char *libxl_path,
                                          libxl_device_disk *disk)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -2646,9 +2646,21 @@ static int libxl__device_disk_from_xs_be(libxl__gc *gc,
 
     libxl_device_disk_init(disk);
 
-    rc = sscanf(be_path, "/local/domain/%d/", &disk->backend_domid);
+    const char *backend_path;
+    rc = libxl__xs_read_checked(gc, XBT_NULL,
+                                GCSPRINTF("%s/backend", libxl_path),
+                                &backend_path);
+    if (rc) goto out;
+
+    if (!backend_path) {
+        LOG(ERROR, "disk %s does not exist (no backend path", libxl_path);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = sscanf(backend_path, "/local/domain/%d/", &disk->backend_domid);
     if (rc != 1) {
-        LOG(ERROR, "Unable to fetch device backend domid from %s", be_path);
+        LOG(ERROR, "Unable to fetch device backend domid from %s", backend_path);
         goto cleanup;
     }
 
@@ -2659,7 +2671,7 @@ static int libxl__device_disk_from_xs_be(libxl__gc *gc,
      * enabled.
      */
     tmp = xs_read(ctx->xsh, XBT_NULL,
-                  GCSPRINTF("%s/params", be_path), &len);
+                  GCSPRINTF("%s/params", libxl_path), &len);
     if (tmp && strchr(tmp, ':')) {
         disk->pdev_path = strdup(strchr(tmp, ':') + 1);
         free(tmp);
@@ -2668,24 +2680,24 @@ static int libxl__device_disk_from_xs_be(libxl__gc *gc,
     }
 
     tmp = xs_read(ctx->xsh, XBT_NULL,
-                  GCSPRINTF("%s/colo-host", be_path), &len);
+                  GCSPRINTF("%s/colo-host", libxl_path), &len);
     if (tmp) {
         libxl_defbool_set(&disk->colo_enable, true);
         disk->colo_host = tmp;
 
         tmp = xs_read(ctx->xsh, XBT_NULL,
-                      GCSPRINTF("%s/colo-port", be_path), &len);
+                      GCSPRINTF("%s/colo-port", libxl_path), &len);
         if (!tmp) {
-            LOG(ERROR, "Missing xenstore node %s/colo-port", be_path);
+            LOG(ERROR, "Missing xenstore node %s/colo-port", libxl_path);
             goto cleanup;
         }
         disk->colo_port = atoi(tmp);
 
 #define XS_READ_COLO(param, item) do {                                  \
         tmp = xs_read(ctx->xsh, XBT_NULL,                               \
-                      GCSPRINTF("%s/"#param"", be_path), &len);         \
+                      GCSPRINTF("%s/"#param"", libxl_path), &len);         \
         if (!tmp) {                                                     \
-            LOG(ERROR, "Missing xenstore node %s/"#param"", be_path);   \
+            LOG(ERROR, "Missing xenstore node %s/"#param"", libxl_path);   \
             goto cleanup;                                               \
         }                                                               \
         disk->item = tmp;                                               \
@@ -2699,31 +2711,31 @@ static int libxl__device_disk_from_xs_be(libxl__gc *gc,
     }
 
     tmp = libxl__xs_read(gc, XBT_NULL,
-                         GCSPRINTF("%s/type", be_path));
+                         GCSPRINTF("%s/type", libxl_path));
     if (!tmp) {
-        LOG(ERROR, "Missing xenstore node %s/type", be_path);
+        LOG(ERROR, "Missing xenstore node %s/type", libxl_path);
         goto cleanup;
     }
     libxl_string_to_backend(ctx, tmp, &(disk->backend));
 
     disk->vdev = xs_read(ctx->xsh, XBT_NULL,
-                         GCSPRINTF("%s/dev", be_path), &len);
+                         GCSPRINTF("%s/dev", libxl_path), &len);
     if (!disk->vdev) {
-        LOG(ERROR, "Missing xenstore node %s/dev", be_path);
+        LOG(ERROR, "Missing xenstore node %s/dev", libxl_path);
         goto cleanup;
     }
 
     tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf
-                         (gc, "%s/removable", be_path));
+                         (gc, "%s/removable", libxl_path));
     if (!tmp) {
-        LOG(ERROR, "Missing xenstore node %s/removable", be_path);
+        LOG(ERROR, "Missing xenstore node %s/removable", libxl_path);
         goto cleanup;
     }
     disk->removable = atoi(tmp);
 
-    tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/mode", be_path));
+    tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/mode", libxl_path));
     if (!tmp) {
-        LOG(ERROR, "Missing xenstore node %s/mode", be_path);
+        LOG(ERROR, "Missing xenstore node %s/mode", libxl_path);
         goto cleanup;
     }
     if (!strcmp(tmp, "w"))
@@ -2732,9 +2744,9 @@ static int libxl__device_disk_from_xs_be(libxl__gc *gc,
         disk->readwrite = 0;
 
     tmp = libxl__xs_read(gc, XBT_NULL,
-                         GCSPRINTF("%s/device-type", be_path));
+                         GCSPRINTF("%s/device-type", libxl_path));
     if (!tmp) {
-        LOG(ERROR, "Missing xenstore node %s/device-type", be_path);
+        LOG(ERROR, "Missing xenstore node %s/device-type", libxl_path);
         goto cleanup;
     }
     disk->is_cdrom = !strcmp(tmp, "cdrom");
@@ -2743,15 +2755,17 @@ static int libxl__device_disk_from_xs_be(libxl__gc *gc,
 
     return 0;
 cleanup:
+    rc = ERROR_FAIL;
+ out:
     libxl_device_disk_dispose(disk);
-    return ERROR_FAIL;
+    return rc;
 }
 
 int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid,
                               const char *vdev, libxl_device_disk *disk)
 {
     GC_INIT(ctx);
-    char *dompath, *path;
+    char *dom_xl_path, *libxl_path;
     int devid = libxl__device_disk_dev_number(vdev, NULL, NULL);
     int rc = ERROR_FAIL;
 
@@ -2760,39 +2774,34 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid,
 
     libxl_device_disk_init(disk);
 
-    dompath = libxl__xs_get_dompath(gc, domid);
-    if (!dompath) {
+    dom_xl_path = libxl__xs_libxl_path(gc, domid);
+    if (!dom_xl_path) {
         goto out;
     }
-    path = libxl__xs_read(gc, XBT_NULL,
-                          GCSPRINTF("%s/device/vbd/%d/backend", dompath,
-                                    devid));
-    if (!path)
-        goto out;
+    libxl_path = GCSPRINTF("%s/device/vbd/%d", dom_xl_path, devid);
 
-    rc = libxl__device_disk_from_xs_be(gc, path, disk);
+    rc = libxl__device_disk_from_xenstore(gc, libxl_path, disk);
 out:
     GC_FREE;
     return rc;
 }
 
 
-static int libxl__append_disk_list_of_type(libxl__gc *gc,
+static int libxl__append_disk_list(libxl__gc *gc,
                                            uint32_t domid,
-                                           const char *type,
                                            libxl_device_disk **disks,
                                            int *ndisks)
 {
-    char *be_path = NULL;
+    char *libxl_dir_path = NULL;
     char **dir = NULL;
     unsigned int n = 0;
     libxl_device_disk *pdisk = NULL, *pdisk_end = NULL;
     int rc=0;
     int initial_disks = *ndisks;
 
-    be_path = GCSPRINTF("%s/backend/%s/%d",
-                        libxl__xs_get_dompath(gc, 0), type, domid);
-    dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
+    libxl_dir_path = GCSPRINTF("%s/device/vbd",
+                        libxl__xs_libxl_path(gc, domid));
+    dir = libxl__xs_directory(gc, XBT_NULL, libxl_dir_path, &n);
     if (dir && n) {
         libxl_device_disk *tmp;
         tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
@@ -2803,10 +2812,9 @@ static int libxl__append_disk_list_of_type(libxl__gc *gc,
         pdisk_end = *disks + initial_disks + n;
         for (; pdisk < pdisk_end; pdisk++, dir++) {
             const char *p;
-            p = GCSPRINTF("%s/%s", be_path, *dir);
-            if ((rc=libxl__device_disk_from_xs_be(gc, p, pdisk)))
+            p = GCSPRINTF("%s/%s", libxl_dir_path, *dir);
+            if ((rc=libxl__device_disk_from_xenstore(gc, p, pdisk)))
                 goto out;
-            pdisk->backend_domid = 0;
             *ndisks += 1;
         }
     }
@@ -2822,13 +2830,7 @@ libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *n
 
     *num = 0;
 
-    rc = libxl__append_disk_list_of_type(gc, domid, "vbd", &disks, num);
-    if (rc) goto out_err;
-
-    rc = libxl__append_disk_list_of_type(gc, domid, "tap", &disks, num);
-    if (rc) goto out_err;
-
-    rc = libxl__append_disk_list_of_type(gc, domid, "qdisk", &disks, num);
+    rc = libxl__append_disk_list(gc, domid, &disks, num);
     if (rc) goto out_err;
 
     GC_FREE;
-- 
2.1.4


[-- Attachment #9: xsa178-unstable/0008-libxl-Do-not-trust-backend-for-disk-in-getinfo.patch --]
[-- Type: application/octet-stream, Size: 1383 bytes --]

From 4e47263f25956f015ce8054c37473511c066a0b1 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 19:10:45 +0100
Subject: [PATCH 08/21] libxl: Do not trust backend for disk in getinfo

Do not read the frontend path out of the backend.  We have it in our
hand.  Likewise the guest (frontend) domid was one of our parameters (!)

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 5aeacc4..2d2b077 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2880,9 +2880,8 @@ int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
     val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/ring-ref", fe_path));
     diskinfo->rref = val ? strtoul(val, NULL, 10) : -1;
     diskinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
-                                 GCSPRINTF("%s/frontend", diskinfo->backend), NULL);
-    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/frontend-id", diskinfo->backend));
-    diskinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+                                 GCSPRINTF("%s/frontend", libxl_path), NULL);
+    diskinfo->frontend_id = domid;
 
     GC_FREE;
     return 0;
-- 
2.1.4


[-- Attachment #10: xsa178-unstable/0009-libxl-Do-not-trust-backend-for-cdrom-insert.patch --]
[-- Type: application/octet-stream, Size: 3778 bytes --]

From 5399ab94a9224b4a826fd5c6a1b8b258292d1efd Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 19:13:17 +0100
Subject: [PATCH 09/21] libxl: Do not trust backend for cdrom insert

Use the /libxl path where appropriate.  Rename `path' variable to
`be_path' to make sure we caught all the occurrences.

Specifically, when checking that the device still exists, check the
`frontend' value in /libxl, rather than anything in the backend
directory.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2d2b077..ded2040 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2900,7 +2900,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     libxl_domain_config d_config;
     int rc, dm_ver;
     libxl__device device;
-    const char *path, *libxl_path;
+    const char *be_path, *libxl_path;
     char * tmp;
     libxl__domain_userdata_lock *lock = NULL;
     xs_transaction_t t = XBT_NULL;
@@ -2973,7 +2973,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     rc = libxl__device_from_disk(gc, domid, disk, &device);
     if (rc) goto out;
 
-    path = libxl__device_backend_path(gc, &device);
+    be_path = libxl__device_backend_path(gc, &device);
     libxl_path = libxl__device_libxl_path(gc, &device);
 
     insert = flexarray_make(gc, 4, 1);
@@ -3013,19 +3013,19 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     for (;;) {
         rc = libxl__xs_transaction_start(gc, &t);
         if (rc) goto out;
-        /* Sanity check: make sure the backend exists before writing here */
-        tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/frontend", path));
+        /* Sanity check: make sure the device exists before writing here */
+        tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/frontend", libxl_path));
         if (!tmp)
         {
             LOG(ERROR, "Internal error: %s does not exist",
-                GCSPRINTF("%s/frontend", path));
+                GCSPRINTF("%s/frontend", libxl_path));
             rc = ERROR_FAIL;
             goto out;
         }
 
         char **kvs = libxl__xs_kvs_of_flexarray(gc, empty, empty->count);
 
-        rc = libxl__xs_writev(gc, t, path, kvs);
+        rc = libxl__xs_writev(gc, t, be_path, kvs);
         if (rc) goto out;
 
         rc = libxl__xs_writev(gc, t, libxl_path, kvs);
@@ -3052,12 +3052,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     for (;;) {
         rc = libxl__xs_transaction_start(gc, &t);
         if (rc) goto out;
-        /* Sanity check: make sure the backend exists before writing here */
-        tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/frontend", path));
+        /* Sanity check: make sure the device exists before writing here */
+        tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/frontend", libxl_path));
         if (!tmp)
         {
             LOG(ERROR, "Internal error: %s does not exist",
-                GCSPRINTF("%s/frontend", path));
+                GCSPRINTF("%s/frontend", libxl_path));
             rc = ERROR_FAIL;
             goto out;
         }
@@ -3067,7 +3067,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
 
         char **kvs = libxl__xs_kvs_of_flexarray(gc, insert, insert->count);
 
-        rc = libxl__xs_writev(gc, t, path, kvs);
+        rc = libxl__xs_writev(gc, t, be_path, kvs);
         if (rc) goto out;
 
         rc = libxl__xs_writev(gc, t, libxl_path, kvs);
-- 
2.1.4


[-- Attachment #11: xsa178-unstable/0010-libxl-Do-not-trust-backend-for-channel-in-getinfo.patch --]
[-- Type: application/octet-stream, Size: 1633 bytes --]

From b2362b04e2d5fbd1a39019adf9e7e5f85cbdf2e1 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 4 May 2016 15:57:10 +0100
Subject: [PATCH 10/21] libxl: Do not trust backend for channel in getinfo

Do not read the frontend path out of the backend.  We have it in our
hand.  Likewise the guest (frontend) domid was one of our parameters (!)

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ded2040..05f3ba1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4065,12 +4065,8 @@ int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
 
     val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", fe_path));
     channelinfo->state = val ? strtoul(val, NULL, 10) : -1;
-    channelinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
-                                    GCSPRINTF("%s/frontend",
-                                    channelinfo->backend), NULL);
-    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/frontend-id",
-                         channelinfo->backend));
-    channelinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+    channelinfo->frontend = libxl__strdup(NOGC, fe_path);
+    channelinfo->frontend_id = domid;
     val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/ring-ref", fe_path));
     channelinfo->rref = val ? strtoul(val, NULL, 10) : -1;
     val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/port", fe_path));
-- 
2.1.4


[-- Attachment #12: xsa178-unstable/0011-libxl-Rename-libxl__device_-nic-channel-_from_xs_be-.patch --]
[-- Type: application/octet-stream, Size: 3137 bytes --]

From 47ac7a8558135553ebb190fbfef3438ce16c3581 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 4 May 2016 16:18:36 +0100
Subject: [PATCH 11/21] libxl: Rename libxl__device_{nic,channel}_from_xs_be to
 _from_xenstore

We are going to change these functions to expect, and be passed, a
/libxl path.  So it is wrong that they are called _from_xs_be.

Neither function reads anything which isn't found in both places, so
we can and will change the call sites later.

The only remaining function in libxl called *_from_xs_be relates to
PCI devices, for which the backend domain is hardcoded to 0 throughout
the libxl_pci.c.

No functional change.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 05f3ba1..e43713e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3585,7 +3585,7 @@ out:
     return;
 }
 
-static int libxl__device_nic_from_xs_be(libxl__gc *gc,
+static int libxl__device_nic_from_xenstore(libxl__gc *gc,
                                         const char *be_path,
                                         libxl_device_nic *nic)
 {
@@ -3649,7 +3649,7 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
     if (!path)
         goto out;
 
-    rc = libxl__device_nic_from_xs_be(gc, path, nic);
+    rc = libxl__device_nic_from_xenstore(gc, path, nic);
     if (rc) goto out;
 
     rc = 0;
@@ -3684,7 +3684,7 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
         for (; pnic < pnic_end; pnic++, dir++) {
             const char *p;
             p = GCSPRINTF("%s/%s", be_path, *dir);
-            rc = libxl__device_nic_from_xs_be(gc, p, pnic);
+            rc = libxl__device_nic_from_xenstore(gc, p, pnic);
             if (rc) goto out;
             pnic->backend_domid = 0;
         }
@@ -3934,7 +3934,7 @@ int libxl__init_console_from_channel(libxl__gc *gc,
     return 0;
 }
 
-static int libxl__device_channel_from_xs_be(libxl__gc *gc,
+static int libxl__device_channel_from_xenstore(libxl__gc *gc,
                                             const char *be_path,
                                             libxl_device_channel *channel)
 {
@@ -3943,7 +3943,7 @@ static int libxl__device_channel_from_xs_be(libxl__gc *gc,
 
     libxl_device_channel_init(channel);
 
-    /* READ_BACKEND is from libxl__device_nic_from_xs_be above */
+    /* READ_BACKEND is from libxl__device_nic_from_xenstore above */
     channel->name = READ_BACKEND(NOGC, "name");
     tmp = READ_BACKEND(gc, "connection");
     if (!strcmp(tmp, "pty")) {
@@ -3998,7 +3998,7 @@ static int libxl__append_channel_list(libxl__gc *gc,
         }
         *channels = tmp;
         next = *channels + *nchannels + devid;
-        rc = libxl__device_channel_from_xs_be(gc, be_path, next);
+        rc = libxl__device_channel_from_xenstore(gc, be_path, next);
         if (rc) goto out;
         next->devid = devid;
         devid++;
-- 
2.1.4


[-- Attachment #13: xsa178-unstable/0012-libxl-Rename-READ_BACKEND-to-READ_LIBXLDEV.patch --]
[-- Type: application/octet-stream, Size: 4021 bytes --]

From 729ba26c1180288fd93585af4328482e60babf2a Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 4 May 2016 16:07:02 +0100
Subject: [PATCH 12/21] libxl: Rename READ_BACKEND to READ_LIBXLDEV

We are going to want to change all the functions that use READ_BACKEND
to get untrustworthy information from the backend, to use trustworthy
information from /libxl.

This will involve replacing READ_BACKEND, which reads from be_path,
with a similar macro READ_LIBXLDEV, which reads from libxl_path.

The macro name change generates a lot of clutter in the diff.  So we
break it out into this separate patch.  Here, we rename the macro, but
the implementation does not really match the new name.

So, another way to look at this, is that we have transformed the bug:
 * All of the backends use READ_BACKEND, which is unsafe
into the new bug:
 * READ_LIBXLDEV actually reads be_path, which is unsafe.

There is no functional change as yet.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e43713e..b5578cb 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -21,8 +21,8 @@
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
 #define BACKEND_STRING_SIZE 5
 
-/* Utility to read backend xenstore keys */
-#define READ_BACKEND(tgc, subpath) ({                                   \
+/* Utility to read /libxl or backend xenstore keys, from be_path */
+#define READ_LIBXLDEV(tgc, subpath) ({                                  \
         rc = libxl__xs_read_checked(tgc, XBT_NULL,                      \
                                     GCSPRINTF("%s/" subpath, be_path),  \
                                     &tmp);                              \
@@ -3594,7 +3594,7 @@ static int libxl__device_nic_from_xenstore(libxl__gc *gc,
 
     libxl_device_nic_init(nic);
 
-    tmp = READ_BACKEND(gc, "handle");
+    tmp = READ_LIBXLDEV(gc, "handle");
     if (tmp)
         nic->devid = atoi(tmp);
     else
@@ -3602,7 +3602,7 @@ static int libxl__device_nic_from_xenstore(libxl__gc *gc,
 
     /* nic->mtu = */
 
-    tmp = READ_BACKEND(gc, "mac");
+    tmp = READ_LIBXLDEV(gc, "mac");
     if (tmp) {
         rc = libxl__parse_mac(tmp, nic->mac);
         if (rc) goto out;
@@ -3610,13 +3610,13 @@ static int libxl__device_nic_from_xenstore(libxl__gc *gc,
         memset(nic->mac, 0, sizeof(nic->mac));
     }
 
-    nic->ip = READ_BACKEND(NOGC, "ip");
-    nic->bridge = READ_BACKEND(NOGC, "bridge");
-    nic->script = READ_BACKEND(NOGC, "script");
-    nic->coloft_forwarddev = READ_BACKEND(NOGC, "forwarddev");
+    nic->ip = READ_LIBXLDEV(NOGC, "ip");
+    nic->bridge = READ_LIBXLDEV(NOGC, "bridge");
+    nic->script = READ_LIBXLDEV(NOGC, "script");
+    nic->coloft_forwarddev = READ_LIBXLDEV(NOGC, "forwarddev");
 
     /* vif_ioemu nics use the same xenstore entries as vif interfaces */
-    tmp = READ_BACKEND(gc, "type");
+    tmp = READ_LIBXLDEV(gc, "type");
     if (tmp) {
         rc = libxl_nic_type_from_string(tmp, &nic->nictype);
         if (rc) goto out;
@@ -3944,13 +3944,13 @@ static int libxl__device_channel_from_xenstore(libxl__gc *gc,
     libxl_device_channel_init(channel);
 
     /* READ_BACKEND is from libxl__device_nic_from_xenstore above */
-    channel->name = READ_BACKEND(NOGC, "name");
-    tmp = READ_BACKEND(gc, "connection");
+    channel->name = READ_LIBXLDEV(NOGC, "name");
+    tmp = READ_LIBXLDEV(gc, "connection");
     if (!strcmp(tmp, "pty")) {
         channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
     } else if (!strcmp(tmp, "socket")) {
         channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
-        channel->u.socket.path = READ_BACKEND(NOGC, "path");
+        channel->u.socket.path = READ_LIBXLDEV(NOGC, "path");
     } else {
         rc = ERROR_INVAL;
         goto out;
-- 
2.1.4


[-- Attachment #14: xsa178-unstable/0013-libxl-Have-READ_LIBXLDEV-use-libxl_path-rather-than-.patch --]
[-- Type: application/octet-stream, Size: 2377 bytes --]

From 8e37e743331110b4fec5928689d74dceda5eb608 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Tue, 3 May 2016 15:40:18 +0100
Subject: [PATCH 13/21] libxl: Have READ_LIBXLDEV use libxl_path rather than
 be_path

Fix the just-introduced bug in this macro: now it reads the
trustworthy libxl_path.  Change the variable name in the two functions
(nic and channel) which use it.

Shuffling the bump in the carpet along, we now introduce three new
bugs: the three call sites pass a backend path where a frontend path
is expected.

No functional change.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b5578cb..d7d7775 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -21,10 +21,10 @@
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
 #define BACKEND_STRING_SIZE 5
 
-/* Utility to read /libxl or backend xenstore keys, from be_path */
+/* Utility to read /libxl xenstore keys, from libxl_path */
 #define READ_LIBXLDEV(tgc, subpath) ({                                  \
         rc = libxl__xs_read_checked(tgc, XBT_NULL,                      \
-                                    GCSPRINTF("%s/" subpath, be_path),  \
+                                    GCSPRINTF("%s/" subpath, libxl_path),  \
                                     &tmp);                              \
         if (rc) goto out;                                               \
         (char*)tmp;                                                     \
@@ -3586,7 +3586,7 @@ out:
 }
 
 static int libxl__device_nic_from_xenstore(libxl__gc *gc,
-                                        const char *be_path,
+                                        const char *libxl_path,
                                         libxl_device_nic *nic)
 {
     const char *tmp;
@@ -3935,7 +3935,7 @@ int libxl__init_console_from_channel(libxl__gc *gc,
 }
 
 static int libxl__device_channel_from_xenstore(libxl__gc *gc,
-                                            const char *be_path,
+                                            const char *libxl_path,
                                             libxl_device_channel *channel)
 {
     const char *tmp;
-- 
2.1.4


[-- Attachment #15: xsa178-unstable/0014-libxl-Do-not-trust-backend-in-nic-getinfo.patch --]
[-- Type: application/octet-stream, Size: 1261 bytes --]

From 9eb1f76bc67f7cf5a9fb86f3aaf01fe2932de1fa Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Tue, 3 May 2016 16:35:21 +0100
Subject: [PATCH 14/21] libxl: Do not trust backend in nic getinfo

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index d7d7775..0f6648a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3752,10 +3752,8 @@ int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
     nicinfo->rref_tx = val ? strtoul(val, NULL, 10) : -1;
     val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/rx-ring-ref", nicpath));
     nicinfo->rref_rx = val ? strtoul(val, NULL, 10) : -1;
-    nicinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
-                                 GCSPRINTF("%s/frontend", nicinfo->backend), NULL);
-    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/frontend-id", nicinfo->backend));
-    nicinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+    nicinfo->frontend = libxl__strdup(NOGC, nicpath);
+    nicinfo->frontend_id = domid;
 
     rc = 0;
  out:
-- 
2.1.4


[-- Attachment #16: xsa178-unstable/0015-libxl-Do-not-trust-backend-for-nic-in-devid_to_devic.patch --]
[-- Type: application/octet-stream, Size: 1581 bytes --]

From df1c2b3e2b3412c851a7ecaa056d1653d2f9f650 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 4 May 2016 16:20:05 +0100
Subject: [PATCH 15/21] libxl: Do not trust backend for nic in devid_to_device

libxl_devid_to_device_nic should read the information it needs from
the /libxl/device path, not the backend.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0f6648a..8cc9114 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3635,7 +3635,7 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
                               int devid, libxl_device_nic *nic)
 {
     GC_INIT(ctx);
-    char *libxl_dom_path, *path;
+    char *libxl_dom_path, *libxl_path;
     int rc = ERROR_FAIL;
 
     libxl_device_nic_init(nic);
@@ -3643,13 +3643,9 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
     if (!libxl_dom_path)
         goto out;
 
-    path = libxl__xs_read(gc, XBT_NULL,
-                          GCSPRINTF("%s/device/vif/%d/backend", libxl_dom_path,
-                                    devid));
-    if (!path)
-        goto out;
+    libxl_path = GCSPRINTF("%s/device/vif/%d", libxl_dom_path, devid);
 
-    rc = libxl__device_nic_from_xenstore(gc, path, nic);
+    rc = libxl__device_nic_from_xenstore(gc, libxl_path, nic);
     if (rc) goto out;
 
     rc = 0;
-- 
2.1.4


[-- Attachment #17: xsa178-unstable/0016-libxl-Do-not-trust-backend-for-nic-in-list.patch --]
[-- Type: application/octet-stream, Size: 3000 bytes --]

From ee0b02e920847b5ff198f0d43968cda9c544c983 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 4 May 2016 16:23:57 +0100
Subject: [PATCH 16/21] libxl: Do not trust backend for nic in list

libxl_device_nic_list should use the /libxl path to search for
devices, and for obtaining the device information.

The "type" parameter was always "vif".  Abolish it.  (In any case,
paths in /libxl/device are named after the frontend type which is
constant, not the backend type which might in future vary.)

Abolish a redundant store to pnic->backend_domid.  Before this commit,
that store was not needed because libxl_device_nic_init (called by
libxl__device_nic_from_xenstore) would zero it.  Now it overwrites the
correct backend domid with zero; so remove it.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8cc9114..daa3417 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3654,21 +3654,20 @@ out:
     return rc;
 }
 
-static int libxl__append_nic_list_of_type(libxl__gc *gc,
+static int libxl__append_nic_list(libxl__gc *gc,
                                            uint32_t domid,
-                                           const char *type,
                                            libxl_device_nic **nics,
                                            int *nnics)
 {
-    char *be_path = NULL;
+    char *libxl_dir_path = NULL;
     char **dir = NULL;
     unsigned int n = 0;
     libxl_device_nic *pnic = NULL, *pnic_end = NULL;
     int rc;
 
-    be_path = GCSPRINTF("%s/backend/%s/%d", libxl__xs_get_dompath(gc, 0),
-                        type, domid);
-    dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
+    libxl_dir_path = GCSPRINTF("%s/device/vif",
+                               libxl__xs_libxl_path(gc, domid));
+    dir = libxl__xs_directory(gc, XBT_NULL, libxl_dir_path, &n);
     if (dir && n) {
         libxl_device_nic *tmp;
         tmp = realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n));
@@ -3679,10 +3678,9 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
         pnic_end = *nics + *nnics + n;
         for (; pnic < pnic_end; pnic++, dir++) {
             const char *p;
-            p = GCSPRINTF("%s/%s", be_path, *dir);
+            p = GCSPRINTF("%s/%s", libxl_dir_path, *dir);
             rc = libxl__device_nic_from_xenstore(gc, p, pnic);
             if (rc) goto out;
-            pnic->backend_domid = 0;
         }
         *nnics += n;
     }
@@ -3700,7 +3698,7 @@ libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num
 
     *num = 0;
 
-    rc = libxl__append_nic_list_of_type(gc, domid, "vif", &nics, num);
+    rc = libxl__append_nic_list(gc, domid, &nics, num);
     if (rc) goto out_err;
 
     GC_FREE;
-- 
2.1.4


[-- Attachment #18: xsa178-unstable/0017-libxl-Do-not-trust-backend-in-channel-list.patch --]
[-- Type: application/octet-stream, Size: 2228 bytes --]

From 69f618db0f3c4b31e2da51793a60e53a7e6706e1 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 4 May 2016 16:59:38 +0100
Subject: [PATCH 17/21] libxl: Do not trust backend in channel list

Read the name from /libxl/device.  Pass the /libxl path to
libxl__device_channel_from_xenstore.

This removes the final route by which READ_LIBXLDEV might receive a
backend path.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v2: Remove be_path variable which is now no longer used.
---
 tools/libxl/libxl.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index daa3417..bfd1ff7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3958,7 +3958,7 @@ static int libxl__append_channel_list(libxl__gc *gc,
                                               libxl_device_channel **channels,
                                               int *nchannels)
 {
-    char *libxl_dir_path = NULL, *be_path = NULL;
+    char *libxl_dir_path = NULL;
     char **dir = NULL;
     unsigned int n = 0, devid = 0;
     libxl_device_channel *next = NULL;
@@ -3975,10 +3975,7 @@ static int libxl__append_channel_list(libxl__gc *gc,
         libxl_device_channel *tmp;
 
         libxl_path = GCSPRINTF("%s/%s", libxl_dir_path, dir[i]);
-        be_path = libxl__xs_read(gc, XBT_NULL,
-                                 GCSPRINTF("%s/backend", libxl_path));
-        if (!be_path) continue;
-        name = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/name", be_path));
+        name = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/name", libxl_path));
         /* 'channels' are consoles with names, so ignore all consoles
            without names */
         if (!name) continue;
@@ -3990,7 +3987,7 @@ static int libxl__append_channel_list(libxl__gc *gc,
         }
         *channels = tmp;
         next = *channels + *nchannels + devid;
-        rc = libxl__device_channel_from_xenstore(gc, be_path, next);
+        rc = libxl__device_channel_from_xenstore(gc, libxl_path, next);
         if (rc) goto out;
         next->devid = devid;
         devid++;
-- 
2.1.4


[-- Attachment #19: xsa178-unstable/0018-libxl-Do-not-trust-backend-for-vusb.patch --]
[-- Type: application/octet-stream, Size: 2312 bytes --]

From f41515d293213ba9e1e017e9d680efd4bb5d8b87 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Thu, 5 May 2016 16:17:26 +0100
Subject: [PATCH 18/21] libxl: Do not trust backend for vusb

Read the type from /libxl, rather than the backend.  (We still trust
the backend for details such as the number of ports, etc.; these are
not a security problem.)

In getinfo, use the computed frontend path, and the incoming domid,
rather than needlessly reading these values from the backend.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v2: New patch following rebase.
---
 tools/libxl/libxl_pvusb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index 7af7e4d..58cf21c 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -401,7 +401,7 @@ libxl_device_usbctrl_list(libxl_ctx *ctx, uint32_t domid, int *num)
             if (ret) goto out;
             usbctrl->version = READ_SUBPATH_INT(be_path, "usb-ver");
             usbctrl->ports = READ_SUBPATH_INT(be_path, "num-ports");
-            libxl_usbctrl_type_from_string(READ_SUBPATH(be_path, "type"),
+            libxl_usbctrl_type_from_string(READ_SUBPATH(libxl_path, "type"),
                                            &usbctrl->type);
 
 #undef READ_SUBPATH
@@ -459,12 +459,11 @@ int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
     usbctrlinfo->evtch = READ_SUBPATH_INT(fe_path, "event-channel");
     usbctrlinfo->ref_urb = READ_SUBPATH_INT(fe_path, "urb-ring-ref");
     usbctrlinfo->ref_conn = READ_SUBPATH_INT(fe_path, "urb-ring-ref");
-    tmp = READ_SUBPATH(be_path, "frontend");
-    usbctrlinfo->frontend = libxl__strdup(NOGC, tmp);
-    usbctrlinfo->frontend_id = READ_SUBPATH_INT(be_path, "frontend-id");
+    usbctrlinfo->frontend = libxl__strdup(NOGC, fe_path);
+    usbctrlinfo->frontend_id = domid;
     usbctrlinfo->ports = READ_SUBPATH_INT(be_path, "num-ports");
     usbctrlinfo->version = READ_SUBPATH_INT(be_path, "usb-ver");;
-    tmp = READ_SUBPATH(be_path, "type");
+    tmp = READ_SUBPATH(libxl_path, "type");
     libxl_usbctrl_type_from_string(tmp, &usbctrlinfo->type);
 
 #undef READ_SUBPATH
-- 
2.1.4


[-- Attachment #20: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread