From nobody Mon Sep 16 19:14:47 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 171993238948710.563786564897669; Tue, 2 Jul 2024 07:59:49 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 73D22A4B; Tue, 2 Jul 2024 10:59:48 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 47A461460; Tue, 2 Jul 2024 10:59:16 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 0624E1427; Tue, 2 Jul 2024 10:59:13 -0400 (EDT) Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) (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 CABA1C6B for ; Tue, 2 Jul 2024 10:59:11 -0400 (EDT) Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-1f6a837e9a3so18086615ad.1 for ; Tue, 02 Jul 2024 07:59:11 -0700 (PDT) Received: from localhost.localdomain ([49.47.192.165]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fb01e36dbdsm11535745ad.203.2024.07.02.07.59.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Jul 2024 07:59:10 -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_H3,RCVD_IN_MSPIKE_WL,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=1719932350; x=1720537150; 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=3YSqFyWSThAtxYmiZBUnKdiFV1K2wt2HS1U/J4FY8QA=; b=Jv065eEoILP0gLp0z3dGB2mC9HO2Q7bFwk8yCwBuSGJwQ3N+G/+mnd7TG7Rq3j12GJ Gv8K/4TSxU017eoB3lY48RIVFRQ4ysliVEaAAODSKpbvdAKy72js1sDZ2RRuA5Aivuab Bp6nNULQREhJHpANhGEpuSbTNMGb0pCE4xjlMAgqFB0wpzynS4DiLnjtTGrS+pPlsaV9 mxchH//wyBiK64I53MAwlI0Tr0bEQQOagDiXzjf/DEEn3G78zvEvBksWDoytCeIo2OBx x9/qjTkyJ9858Pus67lcuUdwGg+UmKcdd9GPt4dddPjJ9plUwjkHWeoUC5U7opMmk5GX PMPA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719932350; x=1720537150; 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=3YSqFyWSThAtxYmiZBUnKdiFV1K2wt2HS1U/J4FY8QA=; b=How5yrcEDRwUPcj2ZDa5U2AEJoBe4MI/1f8DgsQsiA0mQVJMEzGnQRzQxF9grK1lH1 +lTAwjrFWlpgZPzTVg6giEq6f7t1FTJ0trRijMghLT5wMA8ek99Cj2eCjxgTWP6kkDL8 wmlJALI5GsBrV/S6r63HH3+KhiaN5tjJZF/YfH8QXDc896z8TrQCTei+kOrVmvcV+2Qu Om4SqBrTuqYYBmggTZzcepxyOZASW/a7sUIUemmCgRLYwTODXxL2Ow53ZB/Kf+Xv3Uze piNYZtH8TIdfepVYLOeNA6IpUvcKC9Zg3G8iBSF1T4S8+Afv3DB7JJp8OOn5YmZGtcmF rryQ== X-Gm-Message-State: AOJu0YxochaTrFYw2Svt/XUPaZ5WWYWrU4Vx/vdZ0A+jLlmWgIYsx/1c a9pOKAtVvTdbeSWiVDv3esCBaPLaiBehJQ7VZzqVjyG9sl0Aj0A4q/Sdvw== X-Google-Smtp-Source: AGHT+IE1RK+axpAV5Lt3jYXNTKixFjSg9roTGjmPD2gzH4R2SohY0gvBvE+JRdcMAGsB0p27FKX9ag== X-Received: by 2002:a17:902:c944:b0:1f7:3a4:f669 with SMTP id d9443c01a7336-1fadbd09038mr56382515ad.69.1719932350283; Tue, 02 Jul 2024 07:59:10 -0700 (PDT) From: Rayhan Faizel To: devel@lists.libvirt.org Subject: [PATCH v2] conf: Fix rawio/sgio checks for non-scsi hostdev devices Date: Tue, 2 Jul 2024 20:27:13 +0530 Message-Id: <20240702145713.3682494-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: G47XBM3AIXRCE76EWUFSYX2VL2PM5PV5 X-Message-ID-Hash: G47XBM3AIXRCE76EWUFSYX2VL2PM5PV5 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: 1719932391604100001 Content-Type: text/plain; charset="utf-8" The current hostdev parsing logic sets rawio or sgio even if the hostdev ty= pe is not 'scsi'. The rawio field in virDomainHostdevSubsysSCSI overlaps with wwpn field in virDomainHostdevSubsysSCSIVHost, consequently setting a bogus pointer value such as 0x1 or 0x2 from virDomainHostdevSubsysSCSIVHost's point of view. This leads to a segmentation fault when it attempts to free wwpn. While setting sgio does not appear to crash, it shares the same flawed logic as setting rawio. Instead, we ensure these are set only after the hostdev type check succeeds. This patch also adds two test cases to exercise both scenarios. Fixes: bdb95b520c53f9bacc6504fc51381bac4813be38 Signed-off-by: Rayhan Faizel Reviewed-by: Michal Privoznik --- [Changes in v2] - Reworked fix to use temporary variables instead of xpath queries. - Added comments for future reference. - Mention fixed commit. src/conf/domain_conf.c | 26 +++++++++--- ...scsi-vhost-rawio-invalid.x86_64-latest.err | 1 + .../hostdev-scsi-vhost-rawio-invalid.xml | 41 +++++++++++++++++++ ...-scsi-vhost-sgio-invalid.x86_64-latest.err | 1 + .../hostdev-scsi-vhost-sgio-invalid.xml | 41 +++++++++++++++++++ tests/qemuxmlconftest.c | 2 + 6 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.= x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.= xml create mode 100644 tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.x= 86_64-latest.err create mode 100644 tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.x= ml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 334152c4ff..7033b4e9fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6220,6 +6220,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainHostdevSubsysMediatedDev *mdevsrc =3D &def->source.subsys.u.m= dev; virTristateBool managed; g_autofree char *model =3D NULL; + virDomainDeviceSGIO sgio; + virTristateBool rawio; int rv; =20 /* @managed can be read from the xml document - it is always an @@ -6259,7 +6261,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, if ((rv =3D virXMLPropEnum(node, "sgio", virDomainDeviceSGIOTypeFromString, VIR_XML_PROP_NONZERO, - &scsisrc->sgio)) < 0) { + &sgio)) < 0) { return -1; } =20 @@ -6269,18 +6271,30 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, _("sgio is only supported for scsi host device"= )); return -1; } + + /* Set sgio only after checking if hostdev type is 'scsi' to avoid + * clobbering other union structures. + */ + scsisrc->sgio =3D sgio; } =20 if ((rv =3D virXMLPropTristateBool(node, "rawio", VIR_XML_PROP_NONE, - &scsisrc->rawio)) < 0) { + &rawio)) < 0) { return -1; } =20 - if (rv > 0 && def->source.subsys.type !=3D VIR_DOMAIN_HOSTDEV_SUBSYS_T= YPE_SCSI) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("rawio is only supported for scsi host device")); - return -1; + if (rv > 0) { + if (def->source.subsys.type !=3D VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SC= SI) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("rawio is only supported for scsi host device")); + return -1; + } + + /* Set rawio only after checking if hostdev type is 'scsi' to avoid + * clobbering other union structures. + */ + scsisrc->rawio =3D rawio; } =20 if (def->source.subsys.type !=3D VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV && diff --git a/tests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.x86_64-= latest.err b/tests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.x86_64-= latest.err new file mode 100644 index 0000000000..ddaf449658 --- /dev/null +++ b/tests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.x86_64-latest.= err @@ -0,0 +1 @@ +XML error: rawio is only supported for scsi host device diff --git a/tests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.xml b/t= ests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.xml new file mode 100644 index 0000000000..80760ab941 --- /dev/null +++ b/tests/qemuxmlconfdata/hostdev-scsi-vhost-rawio-invalid.xml @@ -0,0 +1,41 @@ + + QEMUGuest2 + c7a5fdbd-edaf-9466-926a-d65c16db1809 + 219100 + 219100 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + + +
+ + +
+ + +
+ + +
+ + + + + + + + +
+ + + diff --git a/tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.x86_64-l= atest.err b/tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.x86_64-la= test.err new file mode 100644 index 0000000000..32256aad1c --- /dev/null +++ b/tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.x86_64-latest.e= rr @@ -0,0 +1 @@ +XML error: sgio is only supported for scsi host device diff --git a/tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.xml b/te= sts/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.xml new file mode 100644 index 0000000000..c549bddc1d --- /dev/null +++ b/tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.xml @@ -0,0 +1,41 @@ + + QEMUGuest2 + c7a5fdbd-edaf-9466-926a-d65c16db1809 + 219100 + 219100 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + + +
+ + +
+ + +
+ + +
+ + + + + + + + +
+ + + diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 8e0d47c6fd..1236ed5df1 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2993,6 +2993,8 @@ mymain(void) DO_TEST_CAPS_LATEST("sound-device-virtio") =20 DO_TEST_CAPS_LATEST_FAILURE("disk-network-iscsi-zero-hosts-invalid") + DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-scsi-vhost-rawio-invalid") + DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-scsi-vhost-sgio-invalid") =20 /* check that all input files were actually used here */ if (testConfXMLCheck(existingTestCases) < 0) --=20 2.34.1