From nobody Mon Sep 16 19:10:29 2024 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=fail(p=none dis=none) header.from=gmail.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1720086775574714.4265697223807; Thu, 4 Jul 2024 02:52:55 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 3DAF1E96; Thu, 4 Jul 2024 05:52:54 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 54F0C11B3; Thu, 4 Jul 2024 05:52:30 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 9FF30E96; Thu, 4 Jul 2024 05:52:28 -0400 (EDT) Received: from mail-pg1-f175.google.com (mail-pg1-f175.google.com [209.85.215.175]) (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 E153E110A for ; Thu, 4 Jul 2024 05:52:26 -0400 (EDT) Received: by mail-pg1-f175.google.com with SMTP id 41be03b00d2f7-71884eda768so267246a12.1 for ; Thu, 04 Jul 2024 02:52:26 -0700 (PDT) Received: from localhost.localdomain ([49.47.192.165]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fb15217780sm30530445ad.109.2024.07.04.02.52.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Jul 2024 02:52:25 -0700 (PDT) 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_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720086745; x=1720691545; darn=lists.libvirt.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=V1/koujlt13q9c8iijc51D/aLjwxPlswtsEmSOMVLSs=; b=A9BMjDKnHjDAeyJ3Oygl+nPWYzXPd21D8D9jDJo4jBLwHh5khDsaHtc0gOYMStpeC6 /kg+eivU4u97Y+q69A1NzKZwMtXm3bdXmwjIANlzKhT4nfgw7Saa41oqIZzpA41ZrxrM GdtHTwIgQE2WTDqZQ1fbIvRUg30o6FO/OI5Fd+QtXVyhl9GvcpocnYXVg+o3Py78gmZw dwf2js/5YCaQLjOX4Nc48NUNLKyrrW59xcIYN/tq6LGVPwN5+45QgPC8zYEJ6omFHNzL 4MCKtxG7ScKwqv2l4Ib0kBQlShmOc13VTUW7bLeAmLbzbGMvt1f2KPtHzS151T/onlwb C0ww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720086745; x=1720691545; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=V1/koujlt13q9c8iijc51D/aLjwxPlswtsEmSOMVLSs=; b=nttF1HZMopgA/feAwwc+Sh8oe+YALgu7hIXc3H7aBCxWAaTxLMWgynLSH6j4HFnUvR oZPe6fhgYPxxE5DlPPOj8e30Gpyza0D9w/8f2mThsUrhWoq5OKyp9ALpYxc3sTfPevbb BvHzhMknFQCEoPcFiY3v+DKAJEizEE09sMpL0tzJ+b8pvsBmep3ycjdmjhCrYF7zyRhG MfzKE1PUb0spqHd6wfW39G53vcu1Z7129+PRIwKvjfwdHttIPreHzFMKBYffd4DccA/7 OwaxSmEDTiFcy4LQH2I68Z7yRzRxfF5+vi0stRFwwlwAZZdxPv+aPpaCdvWVCA58pl3g MCnw== X-Gm-Message-State: AOJu0YwydqUidGgOGPHVTO5lWxo5zB1AbFl5QJNhJI1PzCB8QtLFJuLW nxx/p2FLoXmvC1CVxkdmTUcIsN7EdOXux7wxpJcyu6D2BykL2h/5t2cKqw== X-Google-Smtp-Source: AGHT+IGMFh45jbbOSMu01Jo2Kig0cyUYtW8vm5OuRtHItzXvlNWMizWQJEXz0Zucak+LkLzDixMs9A== X-Received: by 2002:a05:6a20:d491:b0:1c0:c95d:af44 with SMTP id adf61e73a8af0-1c0cc8b3ae2mr974895637.49.1720086745303; Thu, 04 Jul 2024 02:52:25 -0700 (PDT) From: Rayhan Faizel To: devel@lists.libvirt.org Subject: [PATCH] conf: Fix out-of-bounds write during cleanup of virDomainNumaDefNodeDistanceParseXML Date: Thu, 4 Jul 2024 15:22:07 +0530 Message-Id: <20240704095207.653185-1-rayhan.faizel@gmail.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: FOJDMILQ2AR2YGTLGZPVTCR5E6CTCMYS X-Message-ID-Hash: FOJDMILQ2AR2YGTLGZPVTCR5E6CTCMYS X-MailFrom: rayhan.faizel@gmail.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 CC: Rayhan Faizel 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: X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1720086778243100001 Content-Type: text/plain; charset="utf-8" mem_nodes[i].ndistances is written outside the loop causing an out-of-bounds write leading to heap corruption. While we are at it, the entire cleanup portion can be removed as it can be handled in virDomainNumaFree. One instance of VIR_FREE is also removed and replaced with g_autofree. This patch also adds a testcase which would be picked up by ASAN, if this portion regresses. Fixes: 742494eed8dbdde8b1d05a306032334e6226beea Signed-off-by: Rayhan Faizel Reviewed-by: Michal Privoznik --- src/conf/numa_conf.c | 30 ++++++------------- ...ance-nonexistent-sibling.x86_64-latest.err | 1 + .../cpu-numa-distance-nonexistent-sibling.xml | 29 ++++++++++++++++++ tests/qemuxmlconftest.c | 1 + 4 files changed, 40 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sib= ling.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sib= ling.xml diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index d8120de6d2..0a0e2911f7 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -343,8 +343,7 @@ virDomainNumaFree(virDomainNuma *numa) virBitmapFree(numa->mem_nodes[i].cpumask); virBitmapFree(numa->mem_nodes[i].nodeset); =20 - if (numa->mem_nodes[i].ndistances > 0) - g_free(numa->mem_nodes[i].distances); + g_free(numa->mem_nodes[i].distances); =20 g_free(numa->mem_nodes[i].caches); } @@ -685,9 +684,8 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, xmlXPathContextPtr ctxt, unsigned int cur_cell) { - int ret =3D -1; int sibling; - xmlNodePtr *nodes =3D NULL; + g_autofree xmlNodePtr *nodes =3D NULL; size_t i, ndistances =3D def->nmem_nodes; =20 if (ndistances =3D=3D 0) @@ -698,12 +696,12 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *d= ef, return 0; =20 if ((sibling =3D virXPathNodeSet("./distances[1]/sibling", ctxt, &node= s)) < 0) - goto cleanup; + return -1; =20 if (sibling =3D=3D 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("NUMA distances defined without siblings")); - goto cleanup; + return -1; } =20 for (i =3D 0; i < sibling; i++) { @@ -713,19 +711,19 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *d= ef, =20 if (virXMLPropUInt(nodes[i], "id", 10, VIR_XML_PROP_REQUIRED, &sibling_id) < 0) - goto cleanup; + return -1; =20 /* The "id" needs to be within numa/cell range */ if (sibling_id >=3D ndistances) { virReportError(VIR_ERR_XML_ERROR, _("'sibling_id %1$d' does not refer to a valid = cell within NUMA 'cell id %2$d'"), sibling_id, cur_cell); - goto cleanup; + return -1; } =20 if (virXMLPropUInt(nodes[i], "value", 10, VIR_XML_PROP_REQUIRED, &sibling_value) < 0) - goto cleanup; + return -1; =20 /* Assure LOCAL_DISTANCE <=3D "value" <=3D UNREACHABLE * and correct LOCAL_DISTANCE setting if such applies. @@ -739,7 +737,7 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, virReportError(VIR_ERR_XML_ERROR, _("'value %1$d' is invalid for 'sibling id %2$d= ' under NUMA 'cell id %3$d'"), sibling_value, sibling_id, cur_cell); - goto cleanup; + return -1; } =20 /* Apply the local / remote distance */ @@ -770,17 +768,7 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *de= f, rdist[cur_cell].value =3D sibling_value; } =20 - ret =3D 0; - - cleanup: - if (ret < 0) { - for (i =3D 0; i < ndistances; i++) - VIR_FREE(def->mem_nodes[i].distances); - def->mem_nodes[i].ndistances =3D 0; - } - VIR_FREE(nodes); - - return ret; + return 0; } =20 =20 diff --git a/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x8= 6_64-latest.err b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibli= ng.x86_64-latest.err new file mode 100644 index 0000000000..4866ff5e80 --- /dev/null +++ b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-la= test.err @@ -0,0 +1 @@ +XML error: 'sibling_id 2' does not refer to a valid cell within NUMA 'cell= id 1' diff --git a/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xm= l b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml new file mode 100644 index 0000000000..62a6c32fbe --- /dev/null +++ b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml @@ -0,0 +1,29 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219100 + 219100 + 16 + + hvm + + + + + + + + + + + + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 4a711fceeb..af49934c33 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2160,6 +2160,7 @@ mymain(void) DO_TEST_CAPS_LATEST_PARSE_ERROR("cpu-numa3"); DO_TEST_CAPS_LATEST("cpu-numa-disjoint"); DO_TEST_CAPS_LATEST("cpu-numa-memshared"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("cpu-numa-distance-nonexistent-sibling= "); =20 /* host-model cpu expansion depends on the cpu reported by qemu and th= us * we invoke it for all real capability dumps we have */ --=20 2.34.1