From 30281f4d0ded3fcbcd5bdf233c5af1623962ecd8 Mon Sep 17 00:00:00 2001
From: Mark McLoughlin <markmc@redhat.com>
Date: Fri, 15 May 2009 12:32:52 +0100
Subject: [PATCH 10/16] Fix segfault after tap host_net_remove

https://bugzilla.redhat.com/491150

If you remove a tap host device, then the associated VLANClientState
is freed, but we never run the down script, close the file descriptor
or free the TAPState memory.

The segfault here:

  772         for (vc = s->vc->vlan->first_client; vc; vc = vc->next) {

is due to that we haven't removed the tap_can_send() fd handler and
the VLANClientState has already freed.

Another obvious side effect of this bug is that the tap device still
exists (e.g. look at ifconfig -a) after host_net_remove.

This has been fixed upstream by:

  http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commitdiff;h=a34b6eb77

I've extracted the parts needed to cleanup the tap device and sort out
the net_cleanup() function. I've also not added cleanup as an argument
to qemu_new_vlan_client() so as to avoid changing each of the twenty
callers. The intention is to make the patch easier to review, while
statying as close as possible to upstream.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Message-Id: <1242387172.25932.5.camel@blaa>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Bugzilla: 501729
RH-Upstream-status: not-applicable
Acked-by: Glauber Costa <glommer@redhat.com>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Juan Quintela <quintela@redhat.com>
---
 qemu/net.c |   48 +++++++++++++++++++++++++++++++++---------------
 qemu/net.h |    2 ++
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/qemu/net.c b/qemu/net.c
index a077e1c..76628be 100644
--- a/qemu/net.c
+++ b/qemu/net.c
@@ -362,6 +362,9 @@ void qemu_del_vlan_client(VLANClientState *vc)
     while (*pvc != NULL)
         if (*pvc == vc) {
             *pvc = vc->next;
+            if (vc->cleanup) {
+                vc->cleanup(vc);
+            }
             free(vc->name);
             free(vc->model);
             free(vc);
@@ -905,6 +908,20 @@ static void tap_set_offload(VLANClientState *vc, int csum, int tso4, int tso6,
 }
 #endif /* TUNSETOFFLOAD */
 
+static int launch_script(const char *setup_script, const char *ifname, int fd);
+
+static void tap_cleanup(VLANClientState *vc)
+{
+    TAPState *s = vc->opaque;
+
+    if (s->down_script[0])
+        launch_script(s->down_script, s->down_script_arg, s->fd);
+
+    qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
+    close(s->fd);
+    qemu_free(s);
+}
+
 /* fd support */
 
 static TAPState *net_tap_fd_init(VLANState *vlan,
@@ -921,6 +938,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan,
     s->fd = fd;
     s->has_vnet_hdr = vnet_hdr != 0;
     s->vc = qemu_new_vlan_client(vlan, model, name, tap_receive, NULL, s);
+    s->vc->cleanup = tap_cleanup;
 #ifdef HAVE_IOVEC
     s->vc->fd_readv = tap_receive_iov;
 #endif
@@ -1242,6 +1260,14 @@ static void vde_from_qemu(void *opaque, const uint8_t *buf, int size)
     }
 }
 
+static void vde_cleanup(VLANClientState *vc)
+{
+    VDEState *s = vc->opaque;
+    qemu_set_fd_handler(vde_datafd(s->vde), NULL, NULL, NULL);
+    vde_close(s->vde);
+    qemu_free(s);
+}
+
 static int net_vde_init(VLANState *vlan, const char *model,
                         const char *name, const char *sock,
                         int port, const char *group, int mode)
@@ -1265,6 +1291,7 @@ static int net_vde_init(VLANState *vlan, const char *model,
         return -1;
     }
     s->vc = qemu_new_vlan_client(vlan, model, name, vde_from_qemu, NULL, s);
+    s->vc->cleanup = vde_cleanup;
     qemu_set_fd_handler(vde_datafd(s->vde), vde_to_qemu, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str), "sock=%s,fd=%d",
              sock, vde_datafd(s->vde));
@@ -2051,27 +2078,18 @@ void net_cleanup(void)
 {
     VLANState *vlan;
 
-#if !defined(_WIN32)
     /* close network clients */
     for(vlan = first_vlan; vlan != NULL; vlan = vlan->next) {
-        VLANClientState *vc;
+        VLANClientState *vc = vlan->first_client;
 
-        for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
-            if (vc->fd_read == tap_receive) {
-                TAPState *s = vc->opaque;
+        while (vc) {
+            VLANClientState *next = vc->next;
 
-                if (s->down_script[0])
-                    launch_script(s->down_script, s->down_script_arg, s->fd);
-            }
-#if defined(CONFIG_VDE)
-            if (vc->fd_read == vde_from_qemu) {
-                VDEState *s = vc->opaque;
-                vde_close(s->vde);
-            }
-#endif
+            qemu_del_vlan_client(vc);
+
+            vc = next;
         }
     }
-#endif
 }
 
 void net_client_check(void)
diff --git a/qemu/net.h b/qemu/net.h
index 88f33b7..028878b 100644
--- a/qemu/net.h
+++ b/qemu/net.h
@@ -9,6 +9,7 @@ typedef ssize_t (IOReadvHandler)(void *, const struct iovec *, int);
 
 typedef struct VLANClientState VLANClientState;
 
+typedef void (NetCleanup) (VLANClientState *);
 typedef void (LinkStatusChanged)(VLANClientState *);
 typedef void (SetOffload)(VLANClientState *, int, int, int, int);
 
@@ -18,6 +19,7 @@ struct VLANClientState {
     /* Packets may still be sent if this returns zero.  It's used to
        rate-limit the slirp code.  */
     IOCanRWHandler *fd_can_read;
+    NetCleanup *cleanup;
     LinkStatusChanged *link_status_changed;
     int link_down;
     SetOffload *set_offload;
-- 
1.6.3.rc4.29.g8146