From nobody Mon Feb 9 04:46:00 2026 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 ARC-Seal: i=1; a=rsa-sha256; t=1569514931; cv=none; d=zoho.com; s=zohoarc; b=Zp60ntc1sxdlrQUg9ycWpzPZJFJI+XanjpclFheUxwE6RNkbSN2/G//krM/heCvKAgc9aVoszjBSySuUTYNJpyyA0/Vv73PZ+lwahabR2FwmV99qSRSwury7ZsCl9pCMCGqTTyW9JR8ndUOJ7uEy7oiBiLIeSXnzXWSZ+mA6R+A= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1569514931; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=MVSrQuAmaPmrWhoMlFUzvcncK+QpHql5g252ARipLp0=; b=ZyfixadhRUgytfp6pWlbOHMSBBMsQVWNtsL1UXFt5iaq1g1IdQDc/8/s5Obk1wFd+iTpdJcKiS+OBc7W6/2suhQY4tFnU+ccrYNglH8UI/Dr3QiS3QSH9QE8lBf88fZqXGBBSkLxkX+b94BKQ2dwfF9POeH/xeCfKU+Ziz/Yx+w= ARC-Authentication-Results: i=1; mx.zoho.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 header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1569514931152148.40717216689143; Thu, 26 Sep 2019 09:22:11 -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 A8CE210DCCA6; Thu, 26 Sep 2019 16:22:08 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 82F9E61F24; Thu, 26 Sep 2019 16:22:08 +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 434F9180B536; Thu, 26 Sep 2019 16:22:08 +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 x8QGEvaH003914 for ; Thu, 26 Sep 2019 12:14:57 -0400 Received: by smtp.corp.redhat.com (Postfix) id B8DE05D9E2; Thu, 26 Sep 2019 16:14:57 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 42E085D9D5 for ; Thu, 26 Sep 2019 16:14:55 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Thu, 26 Sep 2019 18:12:32 +0200 Message-Id: <43b7e78912698121b15fb430663ea7d654eedcce.1569514291.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 36/39] qemu: Don't leak storage perms on failure in qemuDomainAttachDiskGeneric 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.6.2 (mx1.redhat.com [10.5.110.64]); Thu, 26 Sep 2019 16:22:10 +0000 (UTC) Content-Type: text/plain; charset="utf-8" At the very beginning of the attach function the qemuDomainStorageSourceChainAccessAllow() is called which modifies CGroups, locks and seclabels for new disk and its backing chain. This must be followed by a counterpart which reverts back all the changes if something goes wrong. This boils down to calling qemuDomainStorageSourceChainAccessRevoke() which is done under 'error' label. But not all failure branches jump there. They just jump onto 'cleanup' label where no revoke is done. Such mistake is easy to do because 'cleanup' label does exist. Therefore, dissolve 'error' block in 'cleanup' and have everything jump onto 'cleanup' label. Signed-off-by: Michal Privoznik --- src/qemu/qemu_hotplug.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5f92c61aa9..75ffc099e1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -682,13 +682,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, bool blockdev =3D virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); =20 if (qemuDomainStorageSourceChainAccessAllow(driver, vm, disk->src) < 0) - goto cleanup; + return -1; =20 if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) - goto error; + goto cleanup; =20 if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) - goto error; + goto cleanup; =20 if (blockdev) { if (disk->copy_on_read =3D=3D VIR_TRISTATE_SWITCH_ON && @@ -705,13 +705,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, } =20 if (!(devstr =3D qemuBuildDiskDeviceStr(vm->def, disk, 0, priv->qemuCa= ps))) - goto error; + goto cleanup; =20 if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks + 1) < 0) - goto error; + goto cleanup; =20 if (qemuHotplugAttachManagedPR(driver, vm, disk->src, QEMU_ASYNC_JOB_N= ONE) < 0) - goto error; + goto cleanup; =20 qemuDomainObjEnterMonitor(driver, vm); =20 @@ -747,7 +747,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, =20 if (qemuDomainObjExitMonitor(driver, vm) < 0) { ret =3D -2; - goto error; + goto cleanup; } =20 virDomainAuditDisk(vm, NULL, disk->src, "attach", true); @@ -756,6 +756,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ret =3D 0; =20 cleanup: + if (ret < 0) + ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, = disk->src)); qemuDomainSecretDiskDestroy(disk); return ret; =20 @@ -772,9 +774,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ret =3D -2; =20 virDomainAuditDisk(vm, NULL, disk->src, "attach", false); - - error: - ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk= ->src)); goto cleanup; } =20 --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list