Re: [PATCH] snapshot: Don't hose list on deletion failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/17/18 7:30 PM, Eric Blake wrote:
If qemuDomainSnapshotDiscard() fails for any reason (rare,
but possible with an ill-timed ENOMEM or if
qemuDomainSnapshotForEachQcow2() has problems talking to the
qemu guest monitor), then an attempt to retry the snapshot
deletion API will crash because we didn't undo the effects
of virDomainSnapshotDropParent() temporarily rearranging the
internal list structures, and the second attempt to drop
parents will dereference NULL.  Fix it by instead only
rearranging the internal list on success.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---

Found by accident, since I copied this code to my checkpoint
code and hit the segfault there.  This one has been latent
for years, but is not a security bug since there is no escalation
if you already have sufficient privilege to delete snapshots.

  src/qemu/qemu_driver.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3f143e91c1..9a3f2a090d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16699,8 +16699,9 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
          snap->first_child = NULL;
          ret = 0;
      } else {
-        virDomainSnapshotDropParent(snap);
          ret = qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata_only);
+        if (ret == 0)
+            virDomainSnapshotDropParent(snap);

NACK; this creates a use-after-free scenario, since snap is freed by qemuDomainSnapshotDiscard on success. I'll have to come up with something different.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux