snapshot: Avoid latent use-after-free when cleaning snapshots

Right now, the only callers of qemuDomainSnapshotDiscardAllMetadata()
are right before freeing the virDomainSnapshotObjList, so it did not
matter if the list's metaroot (which points to all the defined root
snapshots) is left inconsistent. But an upcoming patch will want to
clear all snapshots if a bulk redefine fails partway through, in
which case things must be reset.  Make this work by teaching the
existing virDomainSnapshotUpdateRelations() to be safe regardless of
the incoming state of the metaroot (since we don't want to leak that
internal detail into qemu code), then fixing the qemu code to use
it after deleting all snapshots. Additionally, the qemu code must
reset vm->current_snapshot if the current snapshot was removed,
regardless of whether the overall removal succeeded or failed later.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ee511930672127c017d5561135d7e43b4674ad8d..29ad36a1ab3f73b473ce4c28070bd06681f29ea3 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -1206,13 +1206,16 @@ virDomainSnapshotSetRelations(void *payload,
 }
 
 /* Populate parent link and child count of all snapshots, with all
- * relations starting as 0/NULL.  Return 0 on success, -1 if a parent
- * is missing or if a circular relationship was requested.  */
+ * assigned defs having relations starting as 0/NULL. Return 0 on
+ * success, -1 if a parent is missing or if a circular relationship
+ * was requested. */
 int
 virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots)
 {
     struct snapshot_set_relation act = { snapshots, 0 };
 
+    snapshots->metaroot.nchildren = 0;
+    snapshots->metaroot.first_child = NULL;
     virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act);
     return act.err;
 }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0cfb52971e8ce96fb767dbab353e4f9912718903..44453a5a7a90a9c741c801e118e6fc448c2da673 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8625,6 +8625,10 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
     rem.err = 0;
     virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll,
                              &rem);
+    if (rem.current)
+        vm->current_snapshot = NULL;
+    if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0 && !rem.err)
+        rem.err = -1;
 
     return rem.err;
 }