Re: [libvirt] [PATCH 3/4] util: Fix for NULL dereference

On 11/10/2018 01:56 PM, Radoslaw Biernacki wrote: > The device xml parser code does not set "model" while parsing > <interface type='hostdev'> > <source> > <address type='pci' domain='0x0002' bus='0x01' slot='0x00' function='0x2'/> > </source> > </interface> > virDomainDefPtr def->nets[i]->model can be NULL while latter compares strings with > STREQ instead of STREQ_NULLABLE. > > Fixes: ac47e4a6225 (qemu: replace "def->nets[i]" with "net" and "def->sounds[i]" with "sound") > Fixes: c7fc151eec7 (qemu: assign virtio devices to PCIe slot when appropriate) > Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki(a)linaro.org&gt; > --- > src/qemu/qemu_domain_address.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c > index 27c9bfb946..15d25481d8 100644 > --- a/src/qemu/qemu_domain_address.c > +++ b/src/qemu/qemu_domain_address.c > @@ -232,8 +232,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) > for (i = 0; i < def->nnets; i++) { > virDomainNetDefPtr net = def->nets[i]; > > - if (net->model && > - STREQ(net->model, "spapr-vlan")) { > + if (STREQ_NULLABLE(net->model, "spapr-vlan")) { > net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; > } We don't require curly braces around single line bodies. Actually the opposite, our coding style says there should be none. This exception to the rule was discussed many times but without any result. Anyway, 'make syntax-check' would have caught this.

> > @@ -325,7 +324,7 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, > virDomainNetDefPtr net = def->nets[i]; > > if (net->model && > - STREQ(net->model, "virtio") && > + STREQ_NULLABLE(net->model, "virtio") && Looks like you've forgotten to remove net->model check ;-) > net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { > net->info.type = type; > } > @@ -634,14 +633,14 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, > * addresses for other hostdev devices. > */ > if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || > - STREQ(net->model, "usb-net")) { > + STREQ_NULLABLE(net->model, "usb-net")) { > return 0; > } > > - if (STREQ(net->model, "virtio")) > + if (STREQ_NULLABLE(net->model, "virtio")) > return virtioFlags; > > - if (STREQ(net->model, "e1000e")) > + if (STREQ_NULLABLE(net->model, "e1000e")) > return pcieFlags; > > return pciFlags; > The rest looks okay. Michal