Re: [libvirt] [PATCH] qemuDomainAttachNetDevice: Avoid @originalError leak

On 11/11/2016 10:33 AM, Michal Privoznik wrote: > Coverity identified that this variable might be leaked. And it's > right. If an error occurred and we have to roll back the control > jumps to try_remove label where we save the current error (see > 0e82fa4c34 for more info). However, inside the code a jump onto > other label is possible thus leaking the error object. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > > Best viewed with 'git show --ignore-space-change' > > src/qemu/qemu_hotplug.c | 42 +++++++++++++++++++++--------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > Yuck ;-) All because qemuBuildHostNetStr adds the "id=" string into the middle somewhere... > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 1bc2706..0dcbee6 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -1326,32 +1326,32 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, > if (vlan < 0) { > if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) { > char *netdev_name; > - if (virAsprintf(&netdev_name, "host%s", net->info.alias) < 0) > - goto cleanup; > - qemuDomainObjEnterMonitor(driver, vm); > - if (charDevPlugged && > - qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) > - VIR_WARN("Failed to remove associated chardev %s", charDevAlias); > - if (netdevPlugged && > - qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) > - VIR_WARN("Failed to remove network backend for netdev %s", > - netdev_name); > - ignore_value(qemuDomainObjExitMonitor(driver, vm)); > - VIR_FREE(netdev_name); > + if (virAsprintf(&netdev_name, "host%s", net->info.alias) >= 0) { > + qemuDomainObjEnterMonitor(driver, vm); > + if (charDevPlugged && > + qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) > + VIR_WARN("Failed to remove associated chardev %s", charDevAlias); > + if (netdevPlugged && > + qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) > + VIR_WARN("Failed to remove network backend for netdev %s", > + netdev_name); > + ignore_value(qemuDomainObjExitMonitor(driver, vm)); > + VIR_FREE(netdev_name); > + } > } else { > VIR_WARN("Unable to remove network backend"); > } > } else { > char *hostnet_name; > - if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) > - goto cleanup; > - qemuDomainObjEnterMonitor(driver, vm); > - if (hostPlugged && > - qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) > - VIR_WARN("Failed to remove network backend for vlan %d, net %s", > - vlan, hostnet_name); > - ignore_value(qemuDomainObjExitMonitor(driver, vm)); > - VIR_FREE(hostnet_name); > + if (virAsprintf(&hostnet_name, "host%s", net->info.alias) >= 0) { In qemuBuildHostNetStr when vlan >= 0, the printing of the id is gated on net->info.alias - that doesn't happen here, but then again it's printing into "name=".. So looking at qemuDomainRemoveNetDevice - I see a slightly different removal algorithm... John

> + qemuDomainObjEnterMonitor(driver, vm); > + if (hostPlugged && > + qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) > + VIR_WARN("Failed to remove network backend for vlan %d, net %s", > + vlan, hostnet_name); > + ignore_value(qemuDomainObjExitMonitor(driver, vm)); > + VIR_FREE(hostnet_name); > + } > } > virSetError(originalError); > virFreeError(originalError); > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list