From nobody Mon Feb 9 10:34:36 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 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=1617285444; cv=none; d=zohomail.com; s=zohoarc; b=c+JkDDiXWobvH/W+TvnIxQA+UgTPc1GqZN54RAS0g+wdBuUumQnxY5G4Z0qD5veT96NarBzIXwsZH9v53ZKFVqxQsdE1Ah7O6saWiahYIHV6+GBstirNL9fMt5ZOGMMleV/pVNyZxpvLlc3s9hqNpCuTeVmfDtRx0jpMknQMvDo= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1617285444; 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; bh=pJd92Exf2vDaJYx0KIUZZIRh/quxxZ8OtGayTVTf1Po=; b=A8nlPDd+ANn405bdrQ4zsJHSIAdPQWPQXfXrugLXuTxKCasCRcZ3UhZ+oWsAVbKFB+ed6RYbgLzTN8fk/R79+s50X+PHOBme2VnrFPhlmEIaLdX24zl3sSwPX4e3lEZO682ItcwJmVCYYuLDQ1dpQ16zn2/rOId51URL7lhZnEo= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1617285444851879.0077508338705; Thu, 1 Apr 2021 06:57:24 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-167-tUHXApvNNmyNJt64arHsOg-1; Thu, 01 Apr 2021 09:55:21 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8786B81CBF4; Thu, 1 Apr 2021 13:54: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 3E81B1037E82; Thu, 1 Apr 2021 13:54: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 EB9354A703; Thu, 1 Apr 2021 13:54:26 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 131DqmHT023028 for ; Thu, 1 Apr 2021 09:52:48 -0400 Received: by smtp.corp.redhat.com (Postfix) id 2F6805C237; Thu, 1 Apr 2021 13:52:48 +0000 (UTC) Received: from speedmetal.redhat.com (unknown [10.40.208.33]) by smtp.corp.redhat.com (Postfix) with ESMTP id 852F45C1A1 for ; Thu, 1 Apr 2021 13:52:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1617285443; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=pJd92Exf2vDaJYx0KIUZZIRh/quxxZ8OtGayTVTf1Po=; b=e4GhaT20CVVgcjYeGB7leJ8RY0qUUhSp5FwQjxQfTGqdBB1+YrBOWUE8+6OMbrj7QtVFYM q95yEKF0tgmHqwNEnuWXGQlsKkZ9sH7VjvdlLR9IKqPHXgyMyuF8IeRsvXSPZyjwGcp4gN oBiwC/NNXluh96SxL0x6mGYBHz7TsiI= X-MC-Unique: tUHXApvNNmyNJt64arHsOg-1 From: Peter Krempa To: libvir-list@redhat.com Subject: [PATCH 02/39] virStorageSourceGetMetadata: Use depth limit instead of unique path checking Date: Thu, 1 Apr 2021 15:52:01 +0200 Message-Id: <098085cf6812348ec0c09c504598dbc105b220c1.1617285119.git.pkrempa@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com 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: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" Prevent unbounded chains by limiting the recursion depth of virStorageSourceGetMetadataRecurse to the maximum number of image layers we limit anyways. This removes the last use of virStorageSourceGetUniqueIdentifier which will allow us to delete some crusty old infrastructure which isn't really needed. Signed-off-by: Peter Krempa Reviewed-by: J=C3=A1n Tomko --- src/qemu/qemu_domain.c | 4 ++- src/security/virt-aa-helper.c | 5 +++- src/storage_file/storage_source.c | 44 +++++++++++++------------------ src/storage_file/storage_source.h | 1 + tests/virstoragetest.c | 3 ++- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f818fce271..cf6d41dcad 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7627,7 +7627,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, qemuDomainGetImageIds(cfg, vm, src, disksrc, &uid, &gid); - if (virStorageSourceGetMetadata(src, uid, gid, report_broken) < 0) + if (virStorageSourceGetMetadata(src, uid, gid, + QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_D= EPTH, + report_broken) < 0) return -1; for (n =3D src->backingStore; virStorageSourceIsBacking(n); n =3D n->b= ackingStore) { diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index c615305320..7e29e43c2e 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -937,9 +937,12 @@ get_files(vahControl * ctl) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. + * + * The maximum depth is limited to 200 layers similarly to the qemu + * implementation. */ if (!disk->src->backingStore) - virStorageSourceGetMetadata(disk->src, -1, -1, false); + virStorageSourceGetMetadata(disk->src, -1, -1, 200, false); /* XXX should handle open errors more careful than just ignoring = them. */ diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_s= ource.c index ffe150a9b0..19b06b02b8 100644 --- a/src/storage_file/storage_source.c +++ b/src/storage_file/storage_source.c @@ -1297,11 +1297,9 @@ virStorageSourceGetMetadataRecurseReadHeader(virStor= ageSourcePtr src, uid_t uid, gid_t gid, char **buf, - size_t *headerLen, - GHashTable *cycle) + size_t *headerLen) { int ret =3D -1; - const char *uniqueName; ssize_t len; if (virStorageSourceInitAs(src, uid, gid) < 0) @@ -1312,19 +1310,6 @@ virStorageSourceGetMetadataRecurseReadHeader(virStor= ageSourcePtr src, goto cleanup; } - if (!(uniqueName =3D virStorageSourceGetUniqueIdentifier(src))) - goto cleanup; - - if (virHashHasEntry(cycle, uniqueName)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("backing store for %s (%s) is self-referential"), - NULLSTR(src->path), uniqueName); - goto cleanup; - } - - if (virHashAddEntry(cycle, uniqueName, NULL) < 0) - goto cleanup; - if ((len =3D virStorageSourceRead(src, 0, VIR_STORAGE_MAX_HEADER, buf)= ) < 0) goto cleanup; @@ -1343,7 +1328,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePt= r src, virStorageSourcePtr parent, uid_t uid, gid_t gid, bool report_broken, - GHashTable *cycle, + size_t max_depth, unsigned int depth) { virStorageFileFormat orig_format =3D src->format; @@ -1352,9 +1337,16 @@ virStorageSourceGetMetadataRecurse(virStorageSourceP= tr src, g_autofree char *buf =3D NULL; g_autoptr(virStorageSource) backingStore =3D NULL; - VIR_DEBUG("path=3D%s format=3D%d uid=3D%u gid=3D%u", + VIR_DEBUG("path=3D%s format=3D%d uid=3D%u gid=3D%u depth=3D%u", NULLSTR(src->path), src->format, - (unsigned int)uid, (unsigned int)gid); + (unsigned int)uid, (unsigned int)gid, depth); + + if (depth > max_depth) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("backing store for %s is self-referential or too = deeply nested"), + NULLSTR(src->path)); + return -1; + } if (src->format =3D=3D VIR_STORAGE_FILE_AUTO_SAFE) src->format =3D VIR_STORAGE_FILE_AUTO; @@ -1369,7 +1361,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePt= r src, } if (virStorageSourceGetMetadataRecurseReadHeader(src, parent, uid, gid, - &buf, &headerLen, cyc= le) < 0) + &buf, &headerLen) < 0) return -1; if (virStorageFileProbeGetMetadata(src, buf, headerLen) < 0) @@ -1396,7 +1388,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePt= r src, if ((rv =3D virStorageSourceGetMetadataRecurse(backingStore, paren= t, uid, gid, report_broken, - cycle, depth + 1)) < = 0) { + max_depth, depth + 1)= ) < 0) { if (!report_broken) return 0; @@ -1427,7 +1419,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePt= r src, * Extract metadata about the storage volume with the specified * image format. If image format is VIR_STORAGE_FILE_AUTO, it * will probe to automatically identify the format. Recurses through - * the entire chain. + * the chain up to @max_depth layers. * * Open files using UID and GID (or pass -1 for the current user/group). * Treat any backing files without explicit type as raw, unless ALLOW_PROB= E. @@ -1445,14 +1437,14 @@ virStorageSourceGetMetadataRecurse(virStorageSource= Ptr src, int virStorageSourceGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, + size_t max_depth, bool report_broken) { - g_autoptr(GHashTable) cycle =3D virHashNew(NULL); virStorageType actualType =3D virStorageSourceGetActualType(src); - VIR_DEBUG("path=3D%s format=3D%d uid=3D%u gid=3D%u report_broken=3D%d", + VIR_DEBUG("path=3D%s format=3D%d uid=3D%u gid=3D%u max_depth=3D%zu rep= ort_broken=3D%d", src->path, src->format, (unsigned int)uid, (unsigned int)gid, - report_broken); + max_depth, report_broken); if (src->format <=3D VIR_STORAGE_FILE_NONE) { if (actualType =3D=3D VIR_STORAGE_TYPE_DIR) @@ -1462,5 +1454,5 @@ virStorageSourceGetMetadata(virStorageSourcePtr src, } return virStorageSourceGetMetadataRecurse(src, src, uid, gid, - report_broken, cycle, 1); + report_broken, max_depth, 1); } diff --git a/src/storage_file/storage_source.h b/src/storage_file/storage_s= ource.h index 6eb795b301..5e05bde7b1 100644 --- a/src/storage_file/storage_source.h +++ b/src/storage_file/storage_source.h @@ -134,6 +134,7 @@ virStorageSourceSupportsBackingChainTraversal(const vir= StorageSource *src); int virStorageSourceGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, + size_t max_depth, bool report_broken) ATTRIBUTE_NONNULL(1); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 84dd813b80..157d577c7d 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -101,7 +101,8 @@ testStorageFileGetMetadata(const char *path, def->path =3D g_strdup(path); - if (virStorageSourceGetMetadata(def, uid, gid, true) < 0) + /* 20 is picked as an arbitrary depth, since the chains used here don'= t exceed it */ + if (virStorageSourceGetMetadata(def, uid, gid, 20, true) < 0) return NULL; return g_steal_pointer(&def); --=20 2.29.2