File xsa178-0006-libxl-Do-not-trust-backend-for-disk-eject-vdev.patch of Package xen.6121
References: bsc#979670 CVE-2016-4963 XSA-178
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>
From 62b4d4769ca39fd5263da20d786a7b9a80a22d9a Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 8 Jun 2016 15:42:19 +0100
Subject: [PATCH] libxl: Fix NULL pointer due to XSA-178 fix wrong XS nodename
In "libxl: Do not trust backend for disk eject vdev" (c69871a2fb26 on
xen.git#staging) we changed libxl_evenable_disk_eject to read the
device vdev out of xenstore from the /libxl path, rather than the
backend path, and to read it during setup rather than on each event.
However, the patch has a mistake:
- GCSPRINTF("%s/dev", backend), NULL);
+ GCSPRINTF("%s/vdev", libxl_path), &configured_vdev);
^
Spot the extra "v". This causes configured_vdev always to be NULL.
configured_vdev is passed to [libxl__]strdup.
In Xen 4.6 and later libxl__strdup is used and tolerates NULL.
evg->vdev is set to NULL. This propagates to the `vdev' field in the
generated event. This may or may not cause further trouble, depending
on the calling application. In our osstest test cases it does not
cause any trouble, so the bug goes undetected.
In Xen 4.5 and earlier, the strdup does not tolerate NULL, and libxl
crashes immediately. This has been detected by osstest as a
regression in Xen 4.5.
IMO this patch should be applied immediately to
xen.git#staging-4.5 (to check that it fixes the osstest regression)
xen.git#staging (to check that it does not break master
Subject to passes, it should then be propagated to all supported
stable trees and also be mentioned in an update to XSA-178.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
--- xen-4.4.4-testing.orig/tools/libxl/libxl.c
+++ xen-4.4.4-testing/tools/libxl/libxl.c
@@ -1193,8 +1193,7 @@ static void disk_eject_xswatch_callback(
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,
- libxl__sprintf(gc, "%s/dev", backend), NULL);
+ disk->vdev = libxl__strdup(NOGC, evg->vdev);
disk->removable = 1;
disk->readwrite = 0;
disk->is_cdrom = 1;
@@ -1217,9 +1216,6 @@ int libxl_evenable_disk_eject(libxl_ctx
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)
@@ -1237,6 +1233,13 @@ int libxl_evenable_disk_eject(libxl_ctx
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/dev", 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;
