Xen Security Advisory 266 (CVE-2018-12892) - libxl fails to honour readonly flag on HVM emulated SCSI disks
Xen.org security team <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.announce |
|---|---|
| Message-ID | <E1fYHbf-0005MU-57__37430.750570684$1530133781$gmane$org@xenbits.xenproject.org> |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2018-12892 / XSA-266
version 3
libxl fails to honour readonly flag on HVM emulated SCSI disks
UPDATES IN VERSION 3
====================
Public release.
ISSUE DESCRIPTION
=================
libxl fails to pass the readonly flag to qemu when setting up a SCSI
disk, due to what was probably an erroneous merge conflict resolution.
IMPACT
======
Malicious guest administrators or (in some situations) users may be
able to write to supposedly read-only disk images.
VULNERABLE SYSTEMS
==================
Only emulated SCSI disks (specified as "sd" in the libxl disk
configuration, or an equivalent) are affected. IDE disks ("hd") are
not affected (because attempts to make them readonly are rejected).
Additionally, CDROM devices (that is, devices specified to be
presented to the guest as CDROMs, regardless of the nature of the
backing storage on the host) are not affected; they are always
readonly.
Only systems using qemu-xen (rather than qemu-xen-traditional) as the
device model version are vulnerable.
Only systems using libxl or libxl-based toolstacks are vulnerable.
(This includes xl, and libvirt with the libxl driver.)
The vulnerability is present in Xen versions 4.7 and later.
(In earlier versions, provided that the patch for XSA-142 has been
applied, attempts to create readonly disks are rejected.)
If the host and guest together usually support PVHVM, the issue is
exploitable only if the malicious guest administrator has control of
the guest kernel or guest kernel command line.
MITIGATION
==========
Switching to qemu-xen-traditional will avoid this vulnerability.
This can be done with
device_model_version="qemu-xen-traditional"
in the xl configuration file.
Using stub domain device models (which necessarily involves switching
to qemu-xen-traditional) will also avoid this vulnerability.
This can be done with
device_model_stubdomain_override=true
in the xl configuration file.
All of these mitigations are liable to have other guest-visible
effects or even regressions.
It may be possible, depending on the configuration, to make the
underlying storage object readonly, or to make it reject writes.
CREDITS
=======
This issue was discovered by Andrew Reimers of OrionVM.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
xsa266/*.patch xen-unstable
xsa266-4.10/*.patch Xen 4.10.x
xsa266-4.9/*.patch Xen 4.9.x
xsa266-4.8/*.patch Xen 4.8.x
xsa266-4.7/*.patch Xen 4.7.x
xsa266-4.6/*.patch Xen 4.6.x
$ sha256sum xsa266* xsa266*/*
d0d998bb3c2f36b0795cdf86d52aa2da3eee72218f9073f398fc6fd2cf5719cd xsa266.meta
0e5634c9b730e2e022bfef9ded2bb81b7740d05911dae6499671db5cb90663c0 xsa266-4.7/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
e6dcef1bdd890a245cb9181266fc1378d77b08cf06c063f35a0835ab3b99cf91 xsa266-4.7/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
19ce6f236702219eb4831ed597f82dc81122fd517131e826643cee95b53d9f1c xsa266-4.8/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
e0a4c616218bc42abada75aa5fa0c3e35da6b6334fe50d6104a5892ffebcdb04 xsa266-4.8/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
9fd48f20da140731bb71dde07035b938cf0966339449a0b6833787767c588c0a xsa266-4.9/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
f23d0e76f15b1f6af487adc36a84cf2591197548ca7cab8ee84be72a87424cf7 xsa266-4.9/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
3d857f38d11b5531a651a45c2f151ac1493260524d4f49ead6833b5f1d599e64 xsa266-4.10/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
e380976abd77b5b46d69c9564aca3acf9bf467b36645ac34e035aba89d081591 xsa266-4.10/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
160dc8c8a918cae7259c252af098206f9eff357e52bdfc0b15553e9c31c587e6 xsa266/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
2b44fd6baac094c82145667a16d9b1530b97fa342d0e635c831425b53a336266 xsa266/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
$
DEPLOYMENT DURING EMBARGO
=========================
Deployment of patches or mitigations is NOT permitted (except where
all the affected systems and VMs are administered and used only by
organisations which are members of the Xen Project Security Issues
Predisclosure List). Specifically, deployment on public cloud systems
is NOT permitted.
This is because all of the patches and mitigations make significant
guest-visible changes. In particular, applying the patch will cause
the emulated SCSI disk object to be reported to the guest as readonly,
when previously it was reported as writeable.
Deployment is permitted only AFTER the embargo ends.
(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
iQEcBAEBCAAGBQJbM+5LAAoJEIP+FMlX6CvZ60YH/i11vnbKl2aKf8e+xchv3Ouf
9egSbsy9T8DfvQLZuXQJ4pXoIR8aRrpbZBK5G6HrK3N+eyVnOoRGR+c5nR4k6QFi
kG+huw1xogN1TJyf1ln1zpy4sTJt7jmw5ZQEVqoHgsiwgifJiLKVMClQAsvNRkgq
su+k4zii863l+2KJdrnsQUlSiO0rHxIgJOs6txSNKHuyJmasHata7O20fcbZ2eNY
g+SMK3QinOTSGTK8gDJQcsBGm3XdmC3OOoXt3DjLvl2/NwAB51oSFr+wdDHl0k8s
jVzRvBwauOelMyteH80lENJLVej52NVMhWDufWu7iGhoh9fZvD3xubO9zFeCtOY=
=UpOb
-----END PGP SIGNATURE-----
_______________________________________________
Xen-announce mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-announce
xsa266.meta
(application/octet-stream, 1.5 KB) - not displayed
xsa266-4.7/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
(application/octet-stream, 3 KB)
From ebdf7b35e0478d7a7af3374f2b32ad9b8cb02a73 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:51:36 +0100 Subject: [PATCH 1/2] libxl: qemu_disk_scsi_drive_string: Break out common parts of disk config The generated configurations are identical apart from, in some cases, reordering of the id=%s element. So, overall, no functional change. This is part of XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 6cbbc3c..660c01e 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -771,6 +771,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; + char *common = GCSPRINTF("cache=writeback"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -778,8 +779,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, switch (colo_mode) { case LIBXL__COLO_NONE: drive = libxl__sprintf - (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", - target_path, unit, format); + (gc, "%s,file=%s,if=scsi,bus=0,unit=%d,format=%s", + common, target_path, unit, format); break; case LIBXL__COLO_PRIMARY: /* @@ -792,13 +793,13 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * vote-threshold=1 */ drive = GCSPRINTF( - "if=scsi,bus=0,unit=%d,cache=writeback,driver=quorum," + "%s,if=scsi,bus=0,unit=%d,,driver=quorum," "id=%s," "children.0.file.filename=%s," "children.0.driver=%s," "read-pattern=fifo," "vote-threshold=1", - unit, exportname, target_path, format); + common, unit, exportname, target_path, format); break; case LIBXL__COLO_SECONDARY: /* @@ -812,14 +813,14 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * file.backing.backing=exportname, */ drive = GCSPRINTF( - "if=scsi,bus=0,unit=%d,cache=writeback,driver=replication," + "%s,if=scsi,bus=0,unit=%d,driver=replication," "mode=secondary," "file.driver=qcow2," "file.file.filename=%s," "file.backing.driver=qcow2," "file.backing.file.filename=%s," "file.backing.backing=%s", - unit, active_disk, hidden_disk, exportname); + common, unit, active_disk, hidden_disk, exportname); break; default: abort(); -- 2.1.4
xsa266-4.7/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
(application/octet-stream, 2.8 KB)
From c5c26de8e9498a0887be249cac4218fe4698de23 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:54:53 +0100 Subject: [PATCH 2/2] libxl: restore passing "readonly=" to qemu for SCSI disks A read-only check was introduced for XSA-142, commit ef6cb76026 ("libxl: relax readonly check introduced by XSA-142 fix") added the passing of the extra setting, but commit dab0539568 ("Introduce COLO mode and refactor relevant function") dropped the passing of the setting again, quite likely due to improper re-basing. Restore the readonly= parameter to SCSI disks. For IDE disks this is supposed to be rejected; add an assert. And there is a bare ad-hoc disk drive string in libxl__build_device_model_args_new, which we also update. This is XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 660c01e..72d5e64 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -771,7 +771,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; - char *common = GCSPRINTF("cache=writeback"); + char *common = GCSPRINTF("cache=writeback,readonly=%s", + disk->readwrite ? "off" : "on"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -838,6 +839,8 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path, const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; + + assert(disk->readwrite); /* should have been checked earlier */ switch (colo_mode) { case LIBXL__COLO_NONE: @@ -1401,8 +1404,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, if (strncmp(disks[i].vdev, "sd", 2) == 0) { if (colo_mode == LIBXL__COLO_SECONDARY) { drive = libxl__sprintf - (gc, "if=none,driver=%s,file=%s,id=%s", - format, target_path, disks[i].colo_export); + (gc, "if=none,driver=%s,file=%s,id=%s,readonly=%s", + format, target_path, disks[i].colo_export, + disks[i].readwrite ? "off" : "on"); flexarray_append(dm_args, "-drive"); flexarray_append(dm_args, drive); -- 2.1.4
xsa266-4.8/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
(application/octet-stream, 3 KB)
From 3e0c724354f93fc85ffafac72c68cd28e468146f Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:51:36 +0100 Subject: [PATCH 1/2] libxl: qemu_disk_scsi_drive_string: Break out common parts of disk config The generated configurations are identical apart from, in some cases, reordering of the id=%s element. So, overall, no functional change. This is part of XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index b6bc407..e9d4cc6 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -773,6 +773,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; + char *common = GCSPRINTF("cache=writeback"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -780,8 +781,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, switch (colo_mode) { case LIBXL__COLO_NONE: drive = libxl__sprintf - (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", - target_path, unit, format); + (gc, "%s,file=%s,if=scsi,bus=0,unit=%d,format=%s", + common, target_path, unit, format); break; case LIBXL__COLO_PRIMARY: /* @@ -794,13 +795,13 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * vote-threshold=1 */ drive = GCSPRINTF( - "if=scsi,bus=0,unit=%d,cache=writeback,driver=quorum," + "%s,if=scsi,bus=0,unit=%d,,driver=quorum," "id=%s," "children.0.file.filename=%s," "children.0.driver=%s," "read-pattern=fifo," "vote-threshold=1", - unit, exportname, target_path, format); + common, unit, exportname, target_path, format); break; case LIBXL__COLO_SECONDARY: /* @@ -814,14 +815,14 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * file.backing.backing=exportname, */ drive = GCSPRINTF( - "if=scsi,bus=0,unit=%d,cache=writeback,driver=replication," + "%s,if=scsi,bus=0,unit=%d,driver=replication," "mode=secondary," "file.driver=qcow2," "file.file.filename=%s," "file.backing.driver=qcow2," "file.backing.file.filename=%s," "file.backing.backing=%s", - unit, active_disk, hidden_disk, exportname); + common, unit, active_disk, hidden_disk, exportname); break; default: abort(); -- 2.1.4
xsa266-4.8/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
(application/octet-stream, 2.8 KB)
From aedff52aa9064632ac37cbe2b8e02d7f881ac338 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:54:53 +0100 Subject: [PATCH 2/2] libxl: restore passing "readonly=" to qemu for SCSI disks A read-only check was introduced for XSA-142, commit ef6cb76026 ("libxl: relax readonly check introduced by XSA-142 fix") added the passing of the extra setting, but commit dab0539568 ("Introduce COLO mode and refactor relevant function") dropped the passing of the setting again, quite likely due to improper re-basing. Restore the readonly= parameter to SCSI disks. For IDE disks this is supposed to be rejected; add an assert. And there is a bare ad-hoc disk drive string in libxl__build_device_model_args_new, which we also update. This is XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index e9d4cc6..fe32923 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -773,7 +773,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; - char *common = GCSPRINTF("cache=writeback"); + char *common = GCSPRINTF("cache=writeback,readonly=%s", + disk->readwrite ? "off" : "on"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -840,6 +841,8 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path, const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; + + assert(disk->readwrite); /* should have been checked earlier */ switch (colo_mode) { case LIBXL__COLO_NONE: @@ -1403,8 +1406,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, if (strncmp(disks[i].vdev, "sd", 2) == 0) { if (colo_mode == LIBXL__COLO_SECONDARY) { drive = libxl__sprintf - (gc, "if=none,driver=%s,file=%s,id=%s", - format, target_path, disks[i].colo_export); + (gc, "if=none,driver=%s,file=%s,id=%s,readonly=%s", + format, target_path, disks[i].colo_export, + disks[i].readwrite ? "off" : "on"); flexarray_append(dm_args, "-drive"); flexarray_append(dm_args, drive); -- 2.1.4
xsa266-4.9/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
(application/octet-stream, 3.1 KB)
From 3abf0feed50bd0c3450b6b168424693baf0f7039 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:51:36 +0100 Subject: [PATCH 1/2] libxl: qemu_disk_scsi_drive_string: Break out common parts of disk config The generated configurations are identical apart from, in some cases, reordering of the id=%s element. So, overall, no functional change. This is part of XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 40f9dc7..96d5702 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -773,6 +773,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; + char *common = GCSPRINTF("cache=writeback"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -780,8 +781,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, switch (colo_mode) { case LIBXL__COLO_NONE: drive = libxl__sprintf - (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", - target_path, unit, format); + (gc, "%s,file=%s,if=scsi,bus=0,unit=%d,format=%s", + common, target_path, unit, format); break; case LIBXL__COLO_PRIMARY: /* @@ -794,13 +795,13 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * vote-threshold=1 */ drive = GCSPRINTF( - "if=scsi,bus=0,unit=%d,cache=writeback,driver=quorum," + "%s,if=scsi,bus=0,unit=%d,,driver=quorum," "id=%s," "children.0.file.filename=%s," "children.0.driver=%s," "read-pattern=fifo," "vote-threshold=1", - unit, exportname, target_path, format); + common, unit, exportname, target_path, format); break; case LIBXL__COLO_SECONDARY: /* @@ -814,7 +815,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * file.backing.backing=exportname, */ drive = GCSPRINTF( - "if=scsi,id=top-colo,bus=0,unit=%d,cache=writeback," + "%s,if=scsi,id=top-colo,bus=0,unit=%d," "driver=replication," "mode=secondary," "top-id=top-colo," @@ -823,7 +824,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, "file.backing.driver=qcow2," "file.backing.file.filename=%s," "file.backing.backing=%s", - unit, active_disk, hidden_disk, exportname); + common, unit, active_disk, hidden_disk, exportname); break; default: abort(); -- 2.1.4
xsa266-4.9/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
(application/octet-stream, 2.8 KB)
From 44110b046be937fa75f8822a99c21418a299abb3 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:54:53 +0100 Subject: [PATCH 2/2] libxl: restore passing "readonly=" to qemu for SCSI disks A read-only check was introduced for XSA-142, commit ef6cb76026 ("libxl: relax readonly check introduced by XSA-142 fix") added the passing of the extra setting, but commit dab0539568 ("Introduce COLO mode and refactor relevant function") dropped the passing of the setting again, quite likely due to improper re-basing. Restore the readonly= parameter to SCSI disks. For IDE disks this is supposed to be rejected; add an assert. And there is a bare ad-hoc disk drive string in libxl__build_device_model_args_new, which we also update. This is XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 96d5702..b049b44 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -773,7 +773,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; - char *common = GCSPRINTF("cache=writeback"); + char *common = GCSPRINTF("cache=writeback,readonly=%s", + disk->readwrite ? "off" : "on"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -842,6 +843,8 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path, const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; + + assert(disk->readwrite); /* should have been checked earlier */ switch (colo_mode) { case LIBXL__COLO_NONE: @@ -1546,8 +1549,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, if (strncmp(disks[i].vdev, "sd", 2) == 0) { if (colo_mode == LIBXL__COLO_SECONDARY) { drive = libxl__sprintf - (gc, "if=none,driver=%s,file=%s,id=%s", - format, target_path, disks[i].colo_export); + (gc, "if=none,driver=%s,file=%s,id=%s,readonly=%s", + format, target_path, disks[i].colo_export, + disks[i].readwrite ? "off" : "on"); flexarray_append(dm_args, "-drive"); flexarray_append(dm_args, drive); -- 2.1.4
xsa266-4.10/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
(application/octet-stream, 3.1 KB)
From 82f98f8484f47163a06e4d5610dba6e5fc459e78 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:51:36 +0100 Subject: [PATCH 1/2] libxl: qemu_disk_scsi_drive_string: Break out common parts of disk config The generated configurations are identical apart from, in some cases, reordering of the id=%s element. So, overall, no functional change. This is part of XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index b51178b..28bbeb6 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -798,6 +798,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; + char *common = GCSPRINTF("cache=writeback"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -805,8 +806,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, switch (colo_mode) { case LIBXL__COLO_NONE: drive = libxl__sprintf - (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", - target_path, unit, format); + (gc, "%s,file=%s,if=scsi,bus=0,unit=%d,format=%s", + common, target_path, unit, format); break; case LIBXL__COLO_PRIMARY: /* @@ -819,13 +820,13 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * vote-threshold=1 */ drive = GCSPRINTF( - "if=scsi,bus=0,unit=%d,cache=writeback,driver=quorum," + "%s,if=scsi,bus=0,unit=%d,,driver=quorum," "id=%s," "children.0.file.filename=%s," "children.0.driver=%s," "read-pattern=fifo," "vote-threshold=1", - unit, exportname, target_path, format); + common, unit, exportname, target_path, format); break; case LIBXL__COLO_SECONDARY: /* @@ -839,7 +840,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, * file.backing.backing=exportname, */ drive = GCSPRINTF( - "if=scsi,id=top-colo,bus=0,unit=%d,cache=writeback," + "%s,if=scsi,id=top-colo,bus=0,unit=%d," "driver=replication," "mode=secondary," "top-id=top-colo," @@ -848,7 +849,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, "file.backing.driver=qcow2," "file.backing.file.filename=%s," "file.backing.backing=%s", - unit, active_disk, hidden_disk, exportname); + common, unit, active_disk, hidden_disk, exportname); break; default: abort(); -- 2.1.4
xsa266-4.10/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
(application/octet-stream, 2.8 KB)
From ba3f1fa1ab6e83745682cac784e680a1abf7da7d Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:54:53 +0100 Subject: [PATCH 2/2] libxl: restore passing "readonly=" to qemu for SCSI disks A read-only check was introduced for XSA-142, commit ef6cb76026 ("libxl: relax readonly check introduced by XSA-142 fix") added the passing of the extra setting, but commit dab0539568 ("Introduce COLO mode and refactor relevant function") dropped the passing of the setting again, quite likely due to improper re-basing. Restore the readonly= parameter to SCSI disks. For IDE disks this is supposed to be rejected; add an assert. And there is a bare ad-hoc disk drive string in libxl__build_device_model_args_new, which we also update. This is XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 28bbeb6..3dc317a 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -798,7 +798,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode) { char *drive = NULL; - char *common = GCSPRINTF("cache=writeback"); + char *common = GCSPRINTF("cache=writeback,readonly=%s", + disk->readwrite ? "off" : "on"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -867,6 +868,8 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path, const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; + + assert(disk->readwrite); /* should have been checked earlier */ switch (colo_mode) { case LIBXL__COLO_NONE: @@ -1576,8 +1579,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, if (strncmp(disks[i].vdev, "sd", 2) == 0) { if (colo_mode == LIBXL__COLO_SECONDARY) { drive = libxl__sprintf - (gc, "if=none,driver=%s,file=%s,id=%s", - format, target_path, disks[i].colo_export); + (gc, "if=none,driver=%s,file=%s,id=%s,readonly=%s", + format, target_path, disks[i].colo_export, + disks[i].readwrite ? "off" : "on"); flexarray_append(dm_args, "-drive"); flexarray_append(dm_args, drive); -- 2.1.4
xsa266/0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
(application/octet-stream, 2.8 KB)
From d79be88da7f7b2ce8784f7c7f3d4cee61fe3c270 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:51:36 +0100 Subject: [PATCH 1/2] libxl: qemu_disk_scsi_drive_string: Break out common parts of disk config The generated configurations are identical apart from, in some cases, reordering of the id=%s element. So, overall, no functional change. This is part of XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 18ada69..deab371 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -798,6 +798,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode, const char **id_ptr) { char *drive = NULL; + char *common = GCSPRINTF("if=none,cache=writeback"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -806,25 +807,23 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, switch (colo_mode) { case LIBXL__COLO_NONE: id = GCSPRINTF("scsi0-hd%d", unit); - drive = GCSPRINTF("file=%s,if=none,id=%s,format=%s,cache=writeback", - target_path, id, format); + drive = GCSPRINTF("file=%s,id=%s,format=%s,%s", + target_path, id, format, common); break; case LIBXL__COLO_PRIMARY: id = exportname; drive = GCSPRINTF( - "if=none,cache=writeback,driver=quorum," - "id=%s," + "%s,id=%s,driver=quorum," "children.0.file.filename=%s," "children.0.driver=%s," "read-pattern=fifo," "vote-threshold=1", - id, target_path, format); + common, id, target_path, format); break; case LIBXL__COLO_SECONDARY: id = "top-colo"; drive = GCSPRINTF( - "if=none,id=%s,cache=writeback," - "driver=replication," + "%s,id=%s,driver=replication," "mode=secondary," "top-id=top-colo," "file.driver=qcow2," @@ -832,7 +831,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, "file.backing.driver=qcow2," "file.backing.file.filename=%s," "file.backing.backing=%s", - id, active_disk, hidden_disk, exportname); + common, id, active_disk, hidden_disk, exportname); break; default: abort(); -- 2.1.4
xsa266/0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
(application/octet-stream, 2.8 KB)
From bcc586cbc05ffaaf5ef8faae74dc3d4743bf97f8 Mon Sep 17 00:00:00 2001 From: Ian Jackson <[email protected]> Date: Wed, 13 Jun 2018 15:54:53 +0100 Subject: [PATCH 2/2] libxl: restore passing "readonly=" to qemu for SCSI disks A read-only check was introduced for XSA-142, commit ef6cb76026 ("libxl: relax readonly check introduced by XSA-142 fix") added the passing of the extra setting, but commit dab0539568 ("Introduce COLO mode and refactor relevant function") dropped the passing of the setting again, quite likely due to improper re-basing. Restore the readonly= parameter to SCSI disks. For IDE disks this is supposed to be rejected; add an assert. And there is a bare ad-hoc disk drive string in libxl__build_device_model_args_new, which we also update. This is XSA-266. Reported-by: Andrew Reimers <[email protected]> Signed-off-by: Jan Beulich <[email protected]> Signed-off-by: Ian Jackson <[email protected]> --- tools/libxl/libxl_dm.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index deab371..bad3ef5 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -798,7 +798,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int colo_mode, const char **id_ptr) { char *drive = NULL; - char *common = GCSPRINTF("if=none,cache=writeback"); + char *common = GCSPRINTF("if=none,readonly=%s,cache=writeback", + disk->readwrite ? "off" : "on"); const char *exportname = disk->colo_export; const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; @@ -852,6 +853,8 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path, const char *active_disk = disk->active_disk; const char *hidden_disk = disk->hidden_disk; + assert(disk->readwrite); /* should have been checked earlier */ + switch (colo_mode) { case LIBXL__COLO_NONE: drive = GCSPRINTF @@ -1574,8 +1577,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, const char *drive_id; if (colo_mode == LIBXL__COLO_SECONDARY) { drive = libxl__sprintf - (gc, "if=none,driver=%s,file=%s,id=%s", - format, target_path, disks[i].colo_export); + (gc, "if=none,driver=%s,file=%s,id=%s,readonly=%s", + format, target_path, disks[i].colo_export, + disks[i].readwrite ? "off" : "on"); flexarray_append(dm_args, "-drive"); flexarray_append(dm_args, drive); -- 2.1.4