From nobody Sat May 4 22:32:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1554743332443870.9438120313577; Mon, 8 Apr 2019 10:08:52 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E63C7308794B; Mon, 8 Apr 2019 17:08:45 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7E6DE5D71E; Mon, 8 Apr 2019 17:08:44 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 312A341F3D; Mon, 8 Apr 2019 17:08:36 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x38H8BLU008875 for ; Mon, 8 Apr 2019 13:08:11 -0400 Received: by smtp.corp.redhat.com (Postfix) id 63FB85DA35; Mon, 8 Apr 2019 17:08:11 +0000 (UTC) Received: from blue.redhat.com (ovpn-117-110.phx2.redhat.com [10.3.117.110]) by smtp.corp.redhat.com (Postfix) with ESMTP id F2EFA5D9C9; Mon, 8 Apr 2019 17:08:08 +0000 (UTC) From: Eric Blake To: libvir-list@redhat.com Date: Mon, 8 Apr 2019 12:08:06 -0500 Message-Id: <20190408170806.4301-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, bogorodskiy@gmail.com Subject: [libvirt] [PATCH] snapshot: Fix use-after-free during snapshot delete X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.45]); Mon, 08 Apr 2019 17:08:50 +0000 (UTC) Content-Type: text/plain; charset="utf-8" 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 Signed-off-by: Eric Blake --- src/conf/virdomainmomentobjlist.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentob= jlist.c index 65e82f632c..66eb66017b 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -80,9 +80,11 @@ virDomainMomentActOnDescendant(void *payload, { virDomainMomentObjPtr obj =3D payload; struct moment_act_on_descendant *curr =3D data; + virDomainMomentObj tmp =3D *obj; + /* Careful: curr->iter can delete obj, hence the need for tmp */ (curr->iter)(payload, name, curr->data); - curr->number +=3D 1 + virDomainMomentForEachDescendant(obj, + curr->number +=3D 1 + virDomainMomentForEachDescendant(&tmp, curr->iter, curr->data); return 0; --=20 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list