Re: [PATCH] conf: Fix rawio/sgio checks for non-scsi hostdev devices

The current hostdev parsing logic sets rawio or sgio even if the hostdev type 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 test the presence of their xpaths along with the hostdev type before setting either attributes. This patch also adds two test cases to exercise both scenarios.

Signed-off-by: Rayhan Faizel <rayhan.faizel(a)gmail.com&gt; --- src/conf/domain_conf.c | 33 ++++++++------- ...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, 102 insertions(+), 17 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.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/hostdev-scsi-vhost-sgio-invalid.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 334152c4ff..5f8c6f2672 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6220,7 +6220,6 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainHostdevSubsysMediatedDev *mdevsrc = &def->source.subsys.u.mdev; virTristateBool managed; g_autofree char *model = NULL; - int rv; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -6256,33 +6255,33 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, return -1; } - if ((rv = virXMLPropEnum(node, "sgio", - virDomainDeviceSGIOTypeFromString, - VIR_XML_PROP_NONZERO, - &scsisrc->sgio)) < 0) { + if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + virXPathBoolean("boolean(./@sgio)", ctxt)) {