File f66f70ac-snapshot-fix-use-after-free.patch of Package libvirt

Overview Repositories Revisions Requests Users Attributes Meta

File f66f70ac-snapshot-fix-use-after-free.patch of Package libvirt (Revision 9bddb6030a8ea26807beba9c6dc9769e)

Currently displaying revision 9bddb6030a8ea26807beba9c6dc9769e , Show latest

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;

Places

File f66f70ac-snapshot-fix-use-after-free.patch of Package libvirt (Revision 9bddb6030a8ea26807beba9c6dc9769e)

Places