From nobody Fri Apr 19 13:59:02 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 ARC-Seal: i=1; a=rsa-sha256; t=1567611269; cv=none; d=zoho.com; s=zohoarc; b=hQjqY31dusOneU/c9CXMpfShPeqWyHbrllHKhj/37+0KvO8MyEaGxKNGqUDPu//w6+QBPxh4NAq3XMj8ouFieZ61hGWpevNanBXcPaMEZrKrZUwum0+x1QUEoAqUdGYBRiFdywgHajiBCxkngz5LvCFbbrlRA1ZaTI2cJuZSwsU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1567611269; 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=YrLY98sCmcHmELFIGI51Mcx+pcfthqeeC6gJYJjDkqI=; b=AalhK1KQkJAbS9qTwDAhf5L0Xp5BzFxUqPzBEbZO2kxp0XO/o0E3pkM6MbtUTSSHF2tfT5F933xyLG7AJBqbiTYiBhjNgtzWc2ykhyoJjWpgFwR2cidRg/AmQbGfpnCXdGEcVX7KoD49LqTxzLDuXg7WU3KZPBq+jGlJp7PjUWA= 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 1567611269855711.0599897807218; Wed, 4 Sep 2019 08:34:29 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ECA2B11A03; Wed, 4 Sep 2019 15:34:27 +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 9654560C57; Wed, 4 Sep 2019 15:34:27 +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 20AD5EEFE; Wed, 4 Sep 2019 15:34:26 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x84FYO3g028577 for ; Wed, 4 Sep 2019 11:34:24 -0400 Received: by smtp.corp.redhat.com (Postfix) id 07B0C60127; Wed, 4 Sep 2019 15:34:24 +0000 (UTC) Received: from angien.brq.redhat.com (unknown [10.43.2.229]) by smtp.corp.redhat.com (Postfix) with ESMTP id 857ED600CD for ; Wed, 4 Sep 2019 15:34:23 +0000 (UTC) From: Peter Krempa To: libvir-list@redhat.com Date: Wed, 4 Sep 2019 17:34:18 +0200 Message-Id: <202e67910d79b3f9cd6c9fc33f4f414071d90e74.1567611165.git.pkrempa@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/2] qemu: domain: Refactor cleanup in qemuDomainDetermineDiskChain 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.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 04 Sep 2019 15:34:29 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Use VIR_AUTOUNREF and get rid of the cleanup label. Signed-off-by: Peter Krempa Reviewed-by: J=C3=A1n Tomko --- src/qemu/qemu_domain.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7eb0b5e9a..8cb5e14ea4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10006,21 +10006,18 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr dri= ver, virStorageSourcePtr disksrc, bool report_broken) { - virQEMUDriverConfigPtr cfg =3D virQEMUDriverGetConfig(driver); + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg =3D virQEMUDriverGetConfig(d= river); virStorageSourcePtr src; /* iterator for the backing chain declared in= XML */ virStorageSourcePtr n; /* iterator for the backing chain detected from= disk */ qemuDomainObjPrivatePtr priv =3D vm->privateData; - int ret =3D -1; uid_t uid; gid_t gid; if (!disksrc) disksrc =3D disk->src; - if (virStorageSourceIsEmpty(disksrc)) { - ret =3D 0; - goto cleanup; - } + if (virStorageSourceIsEmpty(disksrc)) + return 0; /* There is no need to check the backing chain for disks without backi= ng * support */ @@ -10032,13 +10029,13 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr dri= ver, if (report_broken) virStorageFileReportBrokenChain(errno, disksrc, disksrc); - goto cleanup; + return -1; } /* terminate the chain for such images as the code below would do = */ if (!disksrc->backingStore && !(disksrc->backingStore =3D virStorageSourceNew())) - goto cleanup; + return -1; /* host cdrom requires special treatment in qemu, so we need to ch= eck * whether a block device is a cdrom */ @@ -10048,8 +10045,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr drive= r, virFileIsCDROM(disksrc->path) =3D=3D 1) disksrc->hostcdrom =3D true; - ret =3D 0; - goto cleanup; + return 0; } src =3D disksrc; @@ -10059,16 +10055,16 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr dri= ver, int rv =3D virStorageFileSupportsAccess(src); if (rv < 0) - goto cleanup; + return -1; if (rv > 0) { if (qemuDomainStorageFileInit(driver, vm, src, disksrc) < = 0) - goto cleanup; + return -1; if (virStorageFileAccess(src, F_OK) < 0) { virStorageFileReportBrokenChain(errno, src, disksrc); virStorageFileDeinit(src); - goto cleanup; + return -1; } virStorageFileDeinit(src); @@ -10079,33 +10075,27 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr dri= ver, /* We skipped to the end of the chain. Skip detection if there's the * terminator. (An allocated but empty backingStore) */ - if (src->backingStore) { - ret =3D 0; - goto cleanup; - } + if (src->backingStore) + return 0; qemuDomainGetImageIds(cfg, vm, src, disksrc, &uid, &gid); if (virStorageFileGetMetadata(src, uid, gid, report_broken) < 0) - goto cleanup; + return -1; for (n =3D src->backingStore; virStorageSourceIsBacking(n); n =3D n->b= ackingStore) { if (qemuDomainValidateStorageSource(n, priv->qemuCaps) < 0) - goto cleanup; + return -1; if (qemuDomainPrepareDiskSourceData(disk, n, cfg, priv->qemuCaps) = < 0) - goto cleanup; + return -1; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0) - goto cleanup; + return -1; } - ret =3D 0; - - cleanup: - virObjectUnref(cfg); - return ret; + return 0; } --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 19 13:59:02 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 ARC-Seal: i=1; a=rsa-sha256; t=1567611286; cv=none; d=zoho.com; s=zohoarc; b=GtOf6D/iITBxybuqXRmQES7MBSlWTvB7v4T/n+y1JETJ5xNmF1hh+x++etoNZXlwdQbh/1tEhEvq/x8xsJ6b/uhdcOLOvU98NyXFDByxhcXiv5DYYw8L/kyu/EYxU+BtK6Y7qzlaU+EyK//qR+0hmnp281ktKv9IGAbXJPJYc/Q= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1567611286; 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=gOr2Hm1TsQYkmU6unEjg/Pu1iHecn7zF4bdfp1aGUDY=; b=n+FCDjEGZomWeoXa2S1HHNHV35BG/2nRV4TgFe8+K+wMVhBkU7PQcz/VInmZ1oK9BLYcHs3pyrRfeHlQL6Kv7fsa37T74jgWBvrxDwdmt4XP+RbxkwGxRHOe3IZPlg0cccYmHQ00Igdy3QcHRdxqgkvOH+w+gkAjs7rWLroRuvc= 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 156761128651771.43579962902686; Wed, 4 Sep 2019 08:34:46 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 28AD2307D915; Wed, 4 Sep 2019 15:34:45 +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 F3CFA5D9E1; Wed, 4 Sep 2019 15:34: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 B568C1800B74; Wed, 4 Sep 2019 15:34:44 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x84FYQrp028592 for ; Wed, 4 Sep 2019 11:34:26 -0400 Received: by smtp.corp.redhat.com (Postfix) id B8459600CD; Wed, 4 Sep 2019 15:34:26 +0000 (UTC) Received: from angien.brq.redhat.com (unknown [10.43.2.229]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4251860127 for ; Wed, 4 Sep 2019 15:34:24 +0000 (UTC) From: Peter Krempa To: libvir-list@redhat.com Date: Wed, 4 Sep 2019 17:34:19 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 2/2] qemu: Prevent storage causing too much nested XML 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.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.48]); Wed, 04 Sep 2019 15:34:45 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Since libvirt stores the backing chain into the XML in a nested way it is the prime possibility to hit libxml2's parsing limit of 256 layers. Introduce code which will crawl the backing chain and verify that it's not too deep. The maximum nesting is set to 200 layers so that there's still some space left for attitional properties or nesting into snapshot XMLs. The check is applied to all disk use cases (starting, hotplug, media change) as well as block copy which changes image and snapshots. We simply report an error and refuse the operation. Without this check a restart of libvirtd would result in the status XML failing to be parsed and thus losing the VM. https://bugzilla.redhat.com/show_bug.cgi?id=3D1524278 Signed-off-by: Peter Krempa Reviewed-by: J=C3=A1n Tomko --- src/qemu/qemu_domain.c | 57 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_driver.c | 6 +++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8cb5e14ea4..af46aece76 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9986,6 +9986,54 @@ qemuDomainStorageAlias(const char *device, int depth) } +/** + * qemuDomainStorageSourceValidateDepth: + * @src: storage source chain to validate + * @add: offsets the calculated number of images + * @diskdst: optional disk target to use in error message + * + * The XML parser limits the maximum element nesting to 256 layers. As lib= virt + * reports the chain into the status and in some cases the config XML we m= ust + * validate that any user-provided chains will not exceed the XML nesting = limit + * when formatted to the XML. + * + * This function validates that the storage source chain starting @src is = at + * most 200 layers deep. @add allows to modify the calculated value to off= set + * the number to allow checking cases when new layers are going to be added + * to the chain. + * + * Returns 0 on success and -1 if the chain is too deep. Error is reported. + */ +int +qemuDomainStorageSourceValidateDepth(virStorageSourcePtr src, + int add, + const char *diskdst) +{ + virStorageSourcePtr n; + size_t nlayers =3D 0; + + for (n =3D src; virStorageSourceIsBacking(n); n =3D n->backingStore) + nlayers++; + + nlayers +=3D add; + + if (nlayers > 200) { + if (diskdst) + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("backing chains more than 200 layers deep are= not " + "supported for disk '%s'"), diskdst); + else + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("backing chains more than 200 layers deep are= not " + "supported")); + + return -1; + } + + return 0; +} + + /** * qemuDomainDetermineDiskChain: * @driver: qemu driver object @@ -10075,8 +10123,12 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driv= er, /* We skipped to the end of the chain. Skip detection if there's the * terminator. (An allocated but empty backingStore) */ - if (src->backingStore) + if (src->backingStore) { + if (qemuDomainStorageSourceValidateDepth(disksrc, 0, disk->dst) < = 0) + return -1; + return 0; + } qemuDomainGetImageIds(cfg, vm, src, disksrc, &uid, &gid); @@ -10095,6 +10147,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr drive= r, return -1; } + if (qemuDomainStorageSourceValidateDepth(disksrc, 0, disk->dst) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d097f23342..33eb501e2a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -802,6 +802,10 @@ int qemuDomainCheckDiskPresence(virQEMUDriverPtr drive= r, virDomainObjPtr vm, unsigned int flags); +int qemuDomainStorageSourceValidateDepth(virStorageSourcePtr src, + int add, + const char *diskdst); + int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78f5471b79..aa9efc684f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15315,6 +15315,9 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverP= tr driver, dd->disk =3D disk; + if (qemuDomainStorageSourceValidateDepth(disk->src, 1, disk->dst) < 0) + return -1; + if (!(dd->src =3D virStorageSourceCopy(snapdisk->src, false))) return -1; @@ -18330,6 +18333,9 @@ qemuDomainBlockCopyCommonValidateUserMirrorBackingS= tore(virStorageSourcePtr mirr _("backingStore of mirror without VIR_DOMAIN_BL= OCK_COPY_SHALLOW doesn't make sense")); return -1; } + + if (qemuDomainStorageSourceValidateDepth(mirror, 0, NULL) < 0) + return -1; } return 0; --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list