From 376a708d028731d620b2710e73a199b2704a377fb503aa808165a5c4ba4e375d Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Fri, 12 Apr 2019 15:52:39 +0000 Subject: [PATCH] Accepting request 693774 from home:jfehlig:branches:Virtualization Properly fix failing tests. - Fix and re-enable snapshot tests f66f70ac-snapshot-fix-use-after-free.patch 2a07c990-api-CVE-2019-3886.patch, ae076bb4-remote-CVE-2019-3886.patch - spec: BuildRequires rpcgen since ae076bb4-remote-CVE-2019-3886.patch ebe9c6ea-qemu-firmware-dirent.patch OBS-URL: https://build.opensuse.org/request/show/693774 OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=746 --- ....patch => 2a07c990-api-CVE-2019-3886.patch | 5 +- ...tch => ae076bb4-remote-CVE-2019-3886.patch | 5 +- ebe9c6ea-qemu-firmware-dirent.patch | 43 ++++++++++++++ f66f70ac-snapshot-fix-use-after-free.patch | 56 +++++++++++++++++++ libvirt.changes | 13 ++++- libvirt.spec | 31 ++++------ mprivozn-test-fix-proposal.patch | 33 ----------- 7 files changed, 125 insertions(+), 61 deletions(-) rename CVE-2019-3886-api.patch => 2a07c990-api-CVE-2019-3886.patch (87%) rename CVE-2019-3886-remote.patch => ae076bb4-remote-CVE-2019-3886.patch (88%) create mode 100644 ebe9c6ea-qemu-firmware-dirent.patch create mode 100644 f66f70ac-snapshot-fix-use-after-free.patch delete mode 100644 mprivozn-test-fix-proposal.patch diff --git a/CVE-2019-3886-api.patch b/2a07c990-api-CVE-2019-3886.patch similarity index 87% rename from CVE-2019-3886-api.patch rename to 2a07c990-api-CVE-2019-3886.patch index 69c71a4..a789212 100644 --- a/CVE-2019-3886-api.patch +++ b/2a07c990-api-CVE-2019-3886.patch @@ -1,6 +1,6 @@ -commit 69f94df6afe2ea8e2034903d6423c783e0c535e8 +commit 2a07c990bd9143d7a0fe8d1b6b7c763c52185240 Author: Daniel P. Berrangé -Date: Wed Apr 3 15:00:49 2019 +0100 +Date: Wed Mar 27 10:59:58 2019 +0000 api: disallow virDomainGetHostname for read-only connections @@ -9,6 +9,7 @@ Date: Wed Apr 3 15:00:49 2019 +0100 forbidden on a read-only connection to libvirt. Fixes CVE-2019-3886 + Reviewed-by: Jim Fehlig Signed-off-by: Daniel P. Berrangé Index: libvirt-5.2.0/src/libvirt-domain.c diff --git a/CVE-2019-3886-remote.patch b/ae076bb4-remote-CVE-2019-3886.patch similarity index 88% rename from CVE-2019-3886-remote.patch rename to ae076bb4-remote-CVE-2019-3886.patch index 9cfba0b..f60168d 100644 --- a/CVE-2019-3886-remote.patch +++ b/ae076bb4-remote-CVE-2019-3886.patch @@ -1,6 +1,6 @@ -commit 9737baf530d80eff19d46a5feb130d3064d47d64 +commit ae076bb40e0e150aef41361b64001138d04d6c60 Author: Daniel P. Berrangé -Date: Wed Apr 3 15:00:50 2019 +0100 +Date: Wed Mar 27 11:22:49 2019 +0000 remote: enforce ACL write permission for getting guest time & hostname @@ -9,6 +9,7 @@ Date: Wed Apr 3 15:00:50 2019 +0100 permissions check must validate "write" permission not "read". Fixes CVE-2019-3886 + Reviewed-by: Jim Fehlig Signed-off-by: Daniel P. Berrangé Index: libvirt-5.2.0/src/remote/remote_protocol.x diff --git a/ebe9c6ea-qemu-firmware-dirent.patch b/ebe9c6ea-qemu-firmware-dirent.patch new file mode 100644 index 0000000..a3cfbfd --- /dev/null +++ b/ebe9c6ea-qemu-firmware-dirent.patch @@ -0,0 +1,43 @@ +commit ebe9c6eab77e2da500c24430addfcd9f10b1676d +Author: Daniel P. Berrangé +Date: Tue Apr 2 13:27:44 2019 +0100 + + qemu: don't rely on the non-portable d_type field in dirent + + d_type is a non-portable extension to the struct dirent and even if it + exists, its value may be DT_UNKNOWN if the filesystem doesn't support + it. This is common with older versions of XFS which have ftype=0 + feature. + + Signed-off-by: Daniel P. Berrangé + +Index: libvirt-5.2.0/src/qemu/qemu_firmware.c +=================================================================== +--- libvirt-5.2.0.orig/src/qemu/qemu_firmware.c ++++ libvirt-5.2.0/src/qemu/qemu_firmware.c +@@ -924,9 +924,7 @@ qemuFirmwareBuildFileList(virHashTablePt + while ((rc = virDirRead(dirp, &ent, dir)) > 0) { + VIR_AUTOFREE(char *) filename = NULL; + VIR_AUTOFREE(char *) path = NULL; +- +- if (ent->d_type != DT_REG && ent->d_type != DT_LNK) +- continue; ++ struct stat sb; + + if (STRPREFIX(ent->d_name, ".")) + continue; +@@ -937,6 +935,14 @@ qemuFirmwareBuildFileList(virHashTablePt + if (virAsprintf(&path, "%s/%s", dir, filename) < 0) + goto cleanup; + ++ if (stat(path, &sb) < 0) { ++ virReportSystemError(errno, _("Unable to access %s"), path); ++ goto cleanup; ++ } ++ ++ if (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode)) ++ continue; ++ + if (virHashUpdateEntry(files, filename, path) < 0) + goto cleanup; + diff --git a/f66f70ac-snapshot-fix-use-after-free.patch b/f66f70ac-snapshot-fix-use-after-free.patch new file mode 100644 index 0000000..9b40870 --- /dev/null +++ b/f66f70ac-snapshot-fix-use-after-free.patch @@ -0,0 +1,56 @@ +commit f66f70acbe22527423b781cb6178859309843706 +Author: Eric Blake +Date: Mon Apr 8 11:45:47 2019 -0500 + + snapshot: Fix use-after-free during snapshot delete + + Commit b647d2195 introduced a use-after-free situation when the caller + is trying to delete a snapshot and its children: if the callback + function deletes the parent, it is no longer safe to query the parent + to learn which children also need to be deleted (where we previously + saved deleting the parent for last). To fix the problem, while still + maintaining support for topological visits of callback functions, we + have to stash off any information needed for later traversal prior to + using a callback function (virDomainMomentForEachChild already does + this, it is only virDomainMomentActOnDescendant that was running into + problems). + + Sadly, the testsuite did not cover the problem at the time. Worse, + even though I later added commit 280a2b41e to catch problems like + this, and even though that test is indeed sufficient to detect the + problem when run under valgrind or suitable MALLOC_PERTURB_ settings, + I'm guilty of not running the test in such an environment. Thus, + v5.2.0 has a regression that could have been prevented had we used the + testsuite to its full power. On the bright side, deleting snapshots + requires ACL domain:snapshot, which is arguably as powerful as + domain:write, so I don't think this use-after-free forms a security + hole. + + At some point, it would be nice to convert virDomainMomentObj into a + virObject, at which point, the solution is even simpler: add + virObjectRef/Unref around the callback. But as that will require + auditing even more places in the code, I went with the simplest patch + for the regression fix. + + Fixes: b647d2195 + Reported-by: Roman Bogorodskiy + Signed-off-by: Eric Blake + Tested-by: Roman Bogorodskiy + +Index: libvirt-5.2.0/src/conf/virdomainmomentobjlist.c +=================================================================== +--- libvirt-5.2.0.orig/src/conf/virdomainmomentobjlist.c ++++ libvirt-5.2.0/src/conf/virdomainmomentobjlist.c +@@ -80,9 +80,11 @@ virDomainMomentActOnDescendant(void *pay + { + virDomainMomentObjPtr obj = payload; + struct moment_act_on_descendant *curr = data; ++ virDomainMomentObj tmp = *obj; + ++ /* Careful: curr->iter can delete obj, hence the need for tmp */ + (curr->iter)(payload, name, curr->data); +- curr->number += 1 + virDomainMomentForEachDescendant(obj, ++ curr->number += 1 + virDomainMomentForEachDescendant(&tmp, + curr->iter, + curr->data); + return 0; diff --git a/libvirt.changes b/libvirt.changes index 0341f96..0a1132c 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,11 +1,18 @@ +------------------------------------------------------------------- +Thu Apr 11 23:00:48 UTC 2019 - James Fehlig + +- Fix and re-enable snapshot tests + f66f70ac-snapshot-fix-use-after-free.patch + ------------------------------------------------------------------- Fri Apr 5 19:58:10 UTC 2019 - James Fehlig - CVE-2019-3886: disallow virDomainGetHostname and virDomainGetTime for read-only connections and users - CVE-2019-3886-api.patch, CVE-2019-3886-remote.patch + 2a07c990-api-CVE-2019-3886.patch, + ae076bb4-remote-CVE-2019-3886.patch bsc#1131595 -- spec: BuildRequires rpcgen since CVE-2019-3886-remote.patch +- spec: BuildRequires rpcgen since ae076bb4-remote-CVE-2019-3886.patch touches remote_protocol.x ------------------------------------------------------------------- @@ -25,7 +32,7 @@ Wed Apr 3 18:08:00 UTC 2019 - Jim Fehlig 5a64c202-xenconfig-support-max-grant-frames.patch - Added patches: ff376c62-tests-fix-mocking-stat-lstat.patch, - mprivozn-test-fix-proposal.patch + ebe9c6ea-qemu-firmware-dirent.patch ------------------------------------------------------------------- Thu Mar 21 21:40:06 UTC 2019 - James Fehlig diff --git a/libvirt.spec b/libvirt.spec index 593ccd5..ce8460b 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -338,12 +338,13 @@ Source99: baselibs.conf Source100: %{name}-rpmlintrc # Upstream patches Patch0: ff376c62-tests-fix-mocking-stat-lstat.patch -Patch1: CVE-2019-3886-api.patch -Patch2: CVE-2019-3886-remote.patch +Patch1: ebe9c6ea-qemu-firmware-dirent.patch +Patch2: 2a07c990-api-CVE-2019-3886.patch +Patch3: ae076bb4-remote-CVE-2019-3886.patch +Patch4: f66f70ac-snapshot-fix-use-after-free.patch # Patches pending upstream review Patch100: libxl-dom-reset.patch Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch -Patch102: mprivozn-test-fix-proposal.patch # Need to go upstream Patch150: xen-pv-cdrom.patch Patch151: blockcopy-check-dst-identical-device.patch @@ -877,9 +878,10 @@ libvirt plugin for NSS for translating domain names into IP addresses. %patch0 -p1 %patch1 -p1 %patch2 -p1 +%patch3 -p1 +%patch4 -p1 %patch100 -p1 %patch101 -p1 -%patch102 -p1 %patch150 -p1 %patch151 -p1 %patch152 -p1 @@ -1227,32 +1229,19 @@ mv %{buildroot}/%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \ %check cd tests -SKIP_C_TESTS="" -SKIP_SCRIPT_TESTS="" +SKIP_TESTS="" # These tests don't current work in a mock build root # virnetsockettest: needs unsupported linux-user syscalls -SKIP_C_TESTS="$SKIP_TESTS virnetsockettest" +SKIP_TESTS="$SKIP_TESTS virnetsockettest" # virportallocatortest fails on aarch64 due to unsupported IPV6_V6ONLY flag %ifarch aarch64 -SKIP_C_TESTS="$SKIP_TESTS virportallocatortest" +SKIP_TESTS="$SKIP_TESTS virportallocatortest" %endif -# Temporarily remove the snapshot tests until they stabilize -SKIP_SCRIPT_TESTS="$SKIP_SCRIPT_TESTS virsh-snapshot" -# Remove tests that dont work on 32-bit arch -#%ifarch %{ix86} armv7l -#SKIP_C_TESTS="$SKIP_C_TESTS qemufirmwaretest" -#%endif -for i in $SKIP_C_TESTS +for i in $SKIP_TESTS do rm -f $i printf 'int main(void) { return 0; }' > $i.c done -for i in $SKIP_SCRIPT_TESTS -do - rm -f $i - printf '#!/bin/sh\n exit 0\n' > $i - chmod +x $i -done make %{?_smp_mflags} if ! make %{?_smp_mflags} check VIR_TEST_DEBUG=1 diff --git a/mprivozn-test-fix-proposal.patch b/mprivozn-test-fix-proposal.patch deleted file mode 100644 index 494c8b1..0000000 --- a/mprivozn-test-fix-proposal.patch +++ /dev/null @@ -1,33 +0,0 @@ -Patch proposed by Michal on libvirt list - -This patch fixes firmware test failures. - -https://www.redhat.com/archives/libvir-list/2019-April/msg00156.html - -Index: libvirt-5.2.0/src/qemu/qemu_firmware.c -=================================================================== ---- libvirt-5.2.0.orig/src/qemu/qemu_firmware.c -+++ libvirt-5.2.0/src/qemu/qemu_firmware.c -@@ -924,8 +924,9 @@ qemuFirmwareBuildFileList(virHashTablePt - while ((rc = virDirRead(dirp, &ent, dir)) > 0) { - VIR_AUTOFREE(char *) filename = NULL; - VIR_AUTOFREE(char *) path = NULL; -+ struct stat sb; - -- if (ent->d_type != DT_REG && ent->d_type != DT_LNK) -+ if (ent->d_type != DT_REG && ent->d_type != DT_LNK && ent->d_type != DT_UNKNOWN) - continue; - - if (STRPREFIX(ent->d_name, ".")) -@@ -937,6 +938,11 @@ qemuFirmwareBuildFileList(virHashTablePt - if (virAsprintf(&path, "%s/%s", dir, filename) < 0) - goto cleanup; - -+ if (ent->d_type == DT_UNKNOWN && -+ stat(path, &sb) >= 0 && -+ ((sb.st_mode & S_IFMT) != S_IFREG && (sb.st_mode & S_IFMT) != S_IFLNK)) -+ continue; -+ - if (virHashUpdateEntry(files, filename, path) < 0) - goto cleanup; -