From 44a9b872e82b26f26123dd6bf775f1ae8a697e43 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 4 Mar 2019 20:38:27 -0600 Subject: [PATCH] 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 Reviewed-by: John Ferlan --- src/conf/snapshot_conf.c | 7 +++++-- src/qemu/qemu_domain.c | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index ee51193067..29ad36a1ab 100644 --- 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 0cfb52971e..44453a5a7a 100644 --- 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; } -- 2.39.5