From: "Peter Krempa" <pkrempa(a)redhat.com&gt; To: libvir-list(a)redhat.com Cc: "Peter Krempa" <pkrempa(a)redhat.com&gt; Sent: Tuesday, August 27, 2013 7:21:24 PM Subject: [libvirt] [PATCH] qemu: Remove hostdev entry when freeing the depending network entry When using a <interface type="network"> that points to a network with hostdev forwarding mode a hostdev alias is created for the network. This allias is inserted into the hostdev list, but is backed with a part of the network object that it is connected to. When a VM is being stopped qemuProcessStop() calls networkReleaseActualDevice() which eventually frees the memory for the hostdev object. Afterwards when the domain definition is being freed by virDomainDefFree() an invalid pointer is accessed by virDomainHostdevDefFree() and may cause a crash of the daemon. This patch removes the entry in the hostdev list before freeing the depending memory to avoid this issue. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1000973 --- src/qemu/qemu_process.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 128618b..2a69c8d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4241,6 +4241,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, def = vm->def; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); + int hostdev_index; + if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { ignore_value(virNetDevMacVLanDeleteWithVPortProfile( net->ifname, &net->mac, @@ -4259,6 +4262,11 @@ void qemuProcessStop(virQEMUDriverPtr driver, virDomainNetGetActualBridgeName(net), net->ifname)); + if (hostdev) { + if ((hostdev_index = virDomainHostdevFind(def, hostdev, NULL)) > 0)

+ virDomainHostdevRemove(def, hostdev_index); + } + networkReleaseActualDevice(net); } -- 1.8.3.2 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 08/27/2013 01:21 PM, Peter Krempa wrote: > When using a <interface type="network"> that points to a network with > hostdev forwarding mode a hostdev alias is created for the network. This > allias is inserted into the hostdev list, but is backed with a part of > the network object that it is connected to. > > When a VM is being stopped qemuProcessStop() calls > networkReleaseActualDevice() which eventually frees the memory for the > hostdev object. Afterwards when the domain definition is being freed by > virDomainDefFree() an invalid pointer is accessed by > virDomainHostdevDefFree() and may cause a crash of the daemon. > > This patch removes the entry in the hostdev list before freeing the > depending memory to avoid this issue. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1000973 > --- > src/qemu/qemu_process.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 128618b..2a69c8d 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -4241,6 +4241,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, > def = vm->def; > for (i = 0; i < def->nnets; i++) { > virDomainNetDefPtr net = def->nets[i]; > + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); > + int hostdev_index; > + > if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { > ignore_value(virNetDevMacVLanDeleteWithVPortProfile( > net->ifname, &net->mac, > @@ -4259,6 +4262,11 @@ void qemuProcessStop(virQEMUDriverPtr driver, > virDomainNetGetActualBridgeName(net), > net->ifname)); > > + if (hostdev) { > + if ((hostdev_index = virDomainHostdevFind(def, hostdev, NULL)) > 0) > + virDomainHostdevRemove(def, hostdev_index); > + } > + > networkReleaseActualDevice(net); > } > I would like to have as much of this type of teardown as possible directly in networkReleaseActualDevice rather than requiring the callers of that function to take care of it, but in this case networkReleaseActualDevice doesn't have (and shouldn't have) access to the full domain definition which it would need in order to get to the hostdevs array, so this is the way we have to do it.

ACK. Two questions, though: 1) Is this a problem that would exist on the older branches that are in RHEL6 and Fedora 18/19? Or did it only come about when the hostdev teardown was split into two parts so that we could take advantage of the "device is detached" event from qemu?

2) Are the other places that networkReleaseActualDevice is called already getting their hostdev entry removed in some other way? Or do we need a similar patch in any other place?