[Qemu-devel] [PATCH] snapshot: Reset err to NULL to avoid double free

[Qemu-devel] [PATCH] snapshot: Reset err to NULL to avoid double free

[Chris Spiegel]

If an error occurs in bdrv_snapshot_delete_by_id_or_name(), "err" is
freed.  If "err" is not set to NULL before calling
bdrv_snapshot_delete_by_id_or_name() again, it will not be updated on
error, and will be freed again.

This can be triggered by starting a VM with at least two drives and then
attempting to delete a non-existent snapshot.

Signed-off-by: Chris Spiegel <chris.spiegel@cypherpath.com>
---
 savevm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/savevm.c b/savevm.c
index 2d8eb96..08ec678 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1246,7 +1246,7 @@ int load_vmstate(const char *name)
 void do_delvm(Monitor *mon, const QDict *qdict)
 {
     BlockDriverState *bs;
-    Error *err = NULL;
+    Error *err;
     const char *name = qdict_get_str(qdict, "name");
 
     if (!find_vmstate_bs()) {
@@ -1257,6 +1257,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
     bs = NULL;
     while ((bs = bdrv_next(bs))) {
         if (bdrv_can_snapshot(bs)) {
+            err = NULL;
             bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
             if (err) {
                 monitor_printf(mon,
-- 
2.1.2

Re: [Qemu-devel] [PATCH] snapshot: Reset err to NULL to avoid double free

[Markus Armbruster]

Copying stable and maintainers.

Chris Spiegel <chris.spiegel@cypherpath.com> writes:

> If an error occurs in bdrv_snapshot_delete_by_id_or_name(), "err" is
> freed.  If "err" is not set to NULL before calling
> bdrv_snapshot_delete_by_id_or_name() again, it will not be updated on
> error, and will be freed again.
>
> This can be triggered by starting a VM with at least two drives and then
> attempting to delete a non-existent snapshot.
>
> Signed-off-by: Chris Spiegel <chris.spiegel@cypherpath.com>

Broken in commit a89d89d.

With the above sentence added to the commit message:

Reviewed-by: Markus Armbruster <armbru@redhat.com>

I checked the commit and the file for similar bugs, didn't find any.

Re: [Qemu-devel] [PATCH] snapshot: Reset err to NULL to avoid double free

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On Mon, Oct 06, 2014 at 09:33:45AM -0700, Chris Spiegel wrote:
> If an error occurs in bdrv_snapshot_delete_by_id_or_name(), "err" is
> freed.  If "err" is not set to NULL before calling
> bdrv_snapshot_delete_by_id_or_name() again, it will not be updated on
> error, and will be freed again.
> 
> This can be triggered by starting a VM with at least two drives and then
> attempting to delete a non-existent snapshot.
> 
> Signed-off-by: Chris Spiegel <chris.spiegel@cypherpath.com>
> ---
>  savevm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]