[ovs-dev] [PATCH] ovn-controller: fix use-after-free in physical_run()

Lance Richardson lrichard at redhat.com
Sat Jul 8 21:31:15 UTC 2017


The hmap "tunnels" is persistent across IDL loop iterations, but
stores pointers to strings in the local db replica which can be
freed as database updates are processed. Fix by storing a copy
of the string in the hmap instead of a pointer to the string in
the replica.

Found via valgrind.

Fixes: 40128e371ec3 ("physical: Refactor port binding processing.")
Signed-off-by: Lance Richardson <lrichard at redhat.com>
---
 ovn/controller/physical.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index f2d9676..4f0011f 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -64,7 +64,7 @@ static struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
  * used to reach that chassis. */
 struct chassis_tunnel {
     struct hmap_node hmap_node;
-    const char *chassis_id;
+    char *chassis_id;
     ofp_port_t ofport;
     enum chassis_tunnel_type type;
 };
@@ -849,7 +849,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
                     tun = xmalloc(sizeof *tun);
                     hmap_insert(&tunnels, &tun->hmap_node,
                                 hash_string(chassis_id, 0));
-                    tun->chassis_id = chassis_id;
+                    tun->chassis_id = xstrdup(chassis_id);
                     tun->ofport = u16_to_ofp(ofport);
                     tun->type = tunnel_type;
                     physical_map_changed = true;
@@ -871,6 +871,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
         if (!simap_find(&new_tunnel_to_ofport, tun->chassis_id)) {
             hmap_remove(&tunnels, &tun->hmap_node);
             physical_map_changed = true;
+            free(tun->chassis_id);
             free(tun);
         }
     }
-- 
2.9.4



More information about the dev mailing list