Re: [PATCH 2/2] network: Don't crash on domain destroy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 22, 2017 at 03:43:17PM +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1434882

Imagine the following scenario:

1) virsh net-start default
2) virsh start myFavouriteDomain
3) virsh net-destroy default
4) virsh destroy myFavouriteDomain

(assuming myFavouriteDomain has an interface from default
network)

Regardless of how unlikely this scenario looks like, we should
not crash. The problem is, on net-destroy in
networkShutdownNetworkVirtual() the virMacMap module is unrefed,
but the stale pointer is kept around. Thus when the domain
destroy procedure comes in, networkReleaseActualDevice() and
subsequently networkMacMgrDel() is called. This function sees the
stale pointer and starts calling the virMacMap module APIs which
work over freed memory.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/network/bridge_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0ea8e2348..3fcdd702e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2488,7 +2488,8 @@ networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver,
    if (network->def->bandwidth)
        virNetDevBandwidthClear(network->def->bridge);

-    virObjectUnref(network->macmap);
+    if (!virObjectUnref(network->macmap))
+        network->macmap = NULL;


You should set the macmap to NULL unconditionally.  If someone is
holding a reference to it, they can decide when to use it.  And they can
unref it just before you're going to use it.  And you should never use
anything you don't have reference for.  And moreover, the network has
always the only reference, nobody else takes this pointer anywhere.

ACK with that changed.

The macmap could also be part of the network, created when the network
was defined, but it would have a "enabled" boolean which would be
toggled on network start-up.  That way you wouldn't have to deal with
references at all.  They are useless in this case anyway, as mentioned
above.

    if (network->radvdPid > 0) {
        char *radvdpidbase;
--
2.11.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux