[libvirt] [PATCH] qemu: Remove hostdev entry when freeing the depending network entry

[Laine Stump]

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?