376a708d02
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
57 lines
2.8 KiB
Diff
57 lines
2.8 KiB
Diff
commit f66f70acbe22527423b781cb6178859309843706
|
|
Author: Eric Blake <eblake@redhat.com>
|
|
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 <bogorodskiy@gmail.com>
|
|
Signed-off-by: Eric Blake <eblake@redhat.com>
|
|
Tested-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
|
|
|
|
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;
|