[PATCH for 8.5.0] qemu_hotplug: Don't skip cleanup in qemuDomainAttachNetDevice()
Introduced in v8.4.0-rc1~183 but the first real problem introduced in v8.4.0-rc1~170, there's a qemuBuildInterfaceConnect() call inside of qemuDomainAttachNetDevice(). If the former fails, then the function is immediately returned from instead of jumping onto the cleanup label. This is crucial, because at this point the domain definition contains 'borrowed' net definition, which is then freed, since an error was met. The domain definition is then left with a dangling pointer which leads to all sorts of different crashes. Fixes: 29d022b1eb7b2330ed224a08509e6d3a5eeecc53 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2102009 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ee44649d48..27e68370cf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1265,7 +1265,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net); if (qemuBuildInterfaceConnect(vm, net, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) - return -1; + goto cleanup; iface_connected = true; -- 2.35.1
On Fri, Jul 01, 2022 at 09:43:16 +0200, Michal Privoznik wrote:
Introduced in v8.4.0-rc1~183 but the first real problem introduced in v8.4.0-rc1~170, there's a qemuBuildInterfaceConnect() call inside of qemuDomainAttachNetDevice(). If the former fails, then the function is immediately returned from instead of jumping onto the cleanup label. This is crucial, because at this point the domain definition contains 'borrowed' net definition, which is then freed, since an error was met. The domain definition is then left with a dangling pointer which leads to all sorts of different crashes.
Fixes: 29d022b1eb7b2330ed224a08509e6d3a5eeecc53 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2102009 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ee44649d48..27e68370cf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1265,7 +1265,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net);
if (qemuBuildInterfaceConnect(vm, net, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) - return -1; + goto cleanup;
iface_connected = true;
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On 7/1/22 10:43, Jiri Denemark wrote:
On Fri, Jul 01, 2022 at 09:43:16 +0200, Michal Privoznik wrote:
Introduced in v8.4.0-rc1~183 but the first real problem introduced in v8.4.0-rc1~170, there's a qemuBuildInterfaceConnect() call inside of qemuDomainAttachNetDevice(). If the former fails, then the function is immediately returned from instead of jumping onto the cleanup label. This is crucial, because at this point the domain definition contains 'borrowed' net definition, which is then freed, since an error was met. The domain definition is then left with a dangling pointer which leads to all sorts of different crashes.
Fixes: 29d022b1eb7b2330ed224a08509e6d3a5eeecc53 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2102009 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ee44649d48..27e68370cf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1265,7 +1265,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net);
if (qemuBuildInterfaceConnect(vm, net, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) - return -1; + goto cleanup;
iface_connected = true;
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Merged, thanks. Michal
participants (3)
-
Jiri Denemark -
Michal Privoznik -
Michal Prívozník