From nobody Tue Sep 9 03:19:40 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=pass(p=reject dis=none) header.from=lists.libvirt.org ARC-Seal: i=1; a=rsa-sha256; t=1748508431; cv=none; d=zohomail.com; s=zohoarc; b=CY9fJJPeKiO9GmFCUJHSNljHgEGq+ADlWZmRS9oWypEcM9OnAssq22XtU6QPVV7pecRHIoKKSDg0MdCdGuB/94NAth/Tfshc2/SGGsfmUwUdgilTwLVKkmzhTqr2bGScBt5GVYK4FHwCZvopH5/gFatY71wcBonlrZ19aMplzRs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1748508431; h=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Reply-To:References:Subject:Subject:To:To:Message-Id:Cc; bh=k853ewUit1tXtqdh9I+el4UysJSMYSjYfPBourIRcBg=; b=iQ9vDUJhi4kaFp60ZhWBek1JqE7vGY0pRvWggVXweCskPOKxs6P8/plsBDIkRnjpePbVL8Q4M4QjKohuDUrJy4spmh4mPm8wu0JmSfGQFk7RdMfD5Egr385OM08a732PhR6TmjjiwSwnk30Gpd1AK9mVGFHHpvIwli7/DAmBvGU= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=pass header.from= (p=reject dis=none) Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 17485084315471005.1723038533078; Thu, 29 May 2025 01:47:11 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 9B4671385; Thu, 29 May 2025 04:47:10 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 551C914B8; Thu, 29 May 2025 04:43:42 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id E68C01396; Thu, 29 May 2025 04:43:35 -0400 (EDT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id BB17913C4 for ; Thu, 29 May 2025 04:43:18 -0400 (EDT) Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-281-1-c0VQCFM2Cp2gFyYHW-YQ-1; Thu, 29 May 2025 04:43:16 -0400 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id EC10E19560A2 for ; Thu, 29 May 2025 08:43:15 +0000 (UTC) Received: from speedmetal.redhat.com (unknown [10.44.22.3]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 1EE3D19560AF for ; Thu, 29 May 2025 08:43:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL,RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1748508198; h=from:from: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; bh=eq8ZgyI3gyrY8aqoZ5BtVEdq6pjyom6rHDVZOoPnMMs=; b=fjXglEdYGkW9Q78JsNYO8ApBhuoaWivEQAwTcQqn7DrA7lW7hdPgxRlYghwDug0KlWsVRt vzEPdMsTd8krOsf6Q+vpK658fQeCVpxDcOthDjDiLy1QBBwrjaZU4YWeBW79XsAaaCVxAh 1pwV3m+ckG9h7rCib773w7jZPI47rRc= X-MC-Unique: 1-c0VQCFM2Cp2gFyYHW-YQ-1 X-Mimecast-MFC-AGG-ID: 1-c0VQCFM2Cp2gFyYHW-YQ_1748508196 To: devel@lists.libvirt.org Subject: [PATCH 03/15] storage_file_probe: qcow2GetExtensions: Fix qcow2 header extension parsing Date: Thu, 29 May 2025 10:42:57 +0200 Message-ID: <6ad5342848029b15f4334059e5b8e35fe56bc9fb.1748507931.git.pkrempa@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: X6Zm7F7ug4btLkqJMGV1izjV9452DlrVVUPrjDeiqX4_1748508196 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Message-ID-Hash: KVYK4UKVMKBNRNX2MTJV74GIPV33WQEG X-Message-ID-Hash: KVYK4UKVMKBNRNX2MTJV74GIPV33WQEG X-MailFrom: pkrempa@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: From: Peter Krempa via Devel Reply-To: Peter Krempa X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1748508433515116600 Content-Type: text/plain; charset="utf-8" From: Peter Krempa There are 3 bugs in the qcow2 header extension parser: 1) Header extension padding not taken into account Per qcow2 documentation (qemu.git/docs/interop/qcow2.txt, also now mirrored in the comment explaining the parser) each header extension entry is padded to a multiple of 8 bytes. Our parser didn't take the padding into account and advanced the current offset only by the 'length', which corresponds to the length of the data. This meant that in vast majority of cases only the first extension would be parsed correctly. For any other one we'd try to fetch the magic and length from wrong place. Luckily this wasn't problem for most of the almost 15 years this bug existed as we only cared about the backing file format string header which was always stored first by qemu. It is now a problem in the very specific case when a qcow2 image has a 'data-file' and also a backing store with format. In such case we'd parse the backing store format properly as it's the first header and 'data-file' being the second would be skipped. The buffer bounds checks were correct so we didn't violate any memory boundaries. 2) Integer underflow in calculation of end of header extension block If the image doesn't have a backing image, the 'backing_file_offset' qcow2 header field is 0. We use that value as 'extensions_end' which is used in the while loop to parse the extension entries. The check was done as "offset < (extensions_end - 8)", thus it unreflows when there's no filename. The design of the loop prevented anything bad from happening though. 3) Off-by-one when determining end of header extension block The aforementioned end of extension check above also has an off-by-one error as it allowed another loop if more than 8 bytes were available. Now the termination entry has data length of 0 bytes so we'd not be able to properly process that one. This wasn't a problem either as for now there's just the terminator having 0 byte lenght. This patch improves documentation by quoting the qcow2 interoperability document and adjusts the loop condition and lenght handling to comply with the specs. Interestingly we also had a test case for this specific scenario but the expected test output was wrong. Fixes: a93402d48b2996e5300754d299ef0d3f646aa098 Resolves: https://issues.redhat.com/browse/RHEL-93775 Signed-off-by: Peter Krempa Reviewed-by: J=C3=A1n Tomko --- src/storage_file/storage_file_probe.c | 62 +++++++++---------- .../out/qcow2datafile-qcow2_qcow2-datafile | 10 ++- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/storage_file/storage_file_probe.c b/src/storage_file/stora= ge_file_probe.c index 04a2dcd9aa..f8857fa934 100644 --- a/src/storage_file/storage_file_probe.c +++ b/src/storage_file/storage_file_probe.c @@ -415,31 +415,21 @@ qcow2GetExtensions(const char *buf, extension_start =3D virReadBufInt32BE(buf + QCOW2v3_HDR_SIZE); /* - * Traditionally QCow2 files had a layout of - * - * [header] - * [backingStoreName] - * - * Although the backingStoreName typically followed - * the header immediately, this was not required by - * the format. By specifying a higher byte offset for - * the backing file offset in the header, it was - * possible to leave space between the header and - * start of backingStore. - * - * This hack is now used to store extensions to the - * qcow2 format: + * QCow2 header extensions are stored directly after the header before + * the (optional) backing store filename: * * [header] * [extensions] * [backingStoreName] * - * Thus the file region to search for extensions is - * between the end of the header (QCOW2_HDR_TOTAL_SIZE) - * and the start of the backingStoreName (offset) + * For qcow2(v2) the [header] portion has a fixed size (QCOW2_HDR_TOTA= L_SIZE), + * whereas for qcow2v3 the size of the header itself is recorded inside + * the header (at offset QCOW2v3_HDR_SIZE). * - * for qcow2 v3 images, the length of the header - * is stored at QCOW2v3_HDR_SIZE + * Thus the file region to search for header extensions is + * between the end of the header and the start of the backingStoreName + * (QCOWX_HDR_BACKING_FILE_OFFSET) if a backing file is present (as + * otherwise the value at QCOWX_HDR_BACKING_FILE_OFFSET is 0) */ extension_end =3D virReadBufInt64BE(buf + QCOWX_HDR_BACKING_FILE_OFFSE= T); @@ -449,19 +439,28 @@ qcow2GetExtensions(const char *buf, if (extension_end > buf_size) return -1; - /* - * The extensions take format of - * - * int32: magic - * int32: length - * byte[length]: payload - * - * Unknown extensions can be ignored by skipping - * over "length" bytes in the data stream. - */ offset =3D extension_start; while (offset < (buf_size-8) && - offset < (extension_end-8)) { + (extension_end =3D=3D 0 || offset <=3D (extension_end - 8))) { + /** + * Directly after the image header, optional sections called header + * extensions can + * be stored. Each extension has a structure like the following: + * + * Byte 0 - 3: Header extension type: + * 0x00000000 - End of the header extension area + * 0xe2792aca - Backing file format name string + * 0x6803f857 - Feature name table + * 0x23852875 - Bitmaps extension + * 0x0537be77 - Full disk encryption header pointer + * 0x44415441 - External data file name string + * other - Unknown header extension, can be safely ignor= ed + * + * 4 - 7: Length of the header extension data + * 8 - n: Header extension data + * n - m: Padding to round up the header extension size to= the + * next multiple of 8. + */ unsigned int magic =3D virReadBufInt32BE(buf + offset); unsigned int len =3D virReadBufInt32BE(buf + offset + 4); @@ -510,7 +509,8 @@ qcow2GetExtensions(const char *buf, return 0; } - offset +=3D len; + /* take padding into account; see above */ + offset +=3D VIR_ROUND_UP(len, 8); } return 0; diff --git a/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafil= e b/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile index c5a5d99e9e..a200ba98fa 100644 --- a/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile +++ b/tests/virstoragetestdata/out/qcow2datafile-qcow2_qcow2-datafile @@ -1,7 +1,7 @@ path:ABS_SRCDIR/virstoragetestdata/images/qcow2datafile-datafile.qcow2 backingStoreRaw: datafile.qcow2 backingStoreRawFormat: qcow2(14) -dataFileRaw: +dataFileRaw: raw capacity: 1024 encryption: 0 relPath: @@ -10,6 +10,14 @@ format:qcow2 protocol:none hostname: + dataFileStoreSource for 'ABS_SRCDIR/virstoragetestdata/images/qcow2datafi= le-datafile.qcow2': + path: ABS_SRCDIR/virstoragetestdata/images/raw + capacity: 0 + encryption: 0 + type:file + format:raw + + path:ABS_SRCDIR/virstoragetestdata/images/datafile.qcow2 backingStoreRaw: backingStoreRawFormat: none(0) --=20 2.49.0