lib/bgpd: re-fix bgp_info_extra_free()

Make the wart slightly less bad... also there is still a possible write
after free here.  This needs to be fixed again, properly, by some
structure changes.

Signed-off-by: David Lamparter <equinox@diac24.net>

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 041049d05b3f52550db0e72e6cf124030e81d3ce..bf97e262304e5c4a0d12cb3dd2fc309efaeb3c05 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -188,8 +188,24 @@ static void bgp_info_extra_free(struct bgp_info_extra **extra)
        if (e->parent) {
                struct bgp_info *bi = (struct bgp_info *)e->parent;
 
-               if (bi->net)
-                       bi->net = bgp_unlock_node((struct bgp_node *)bi->net);
+               if (bi->net) {
+                       /* FIXME: since multiple e may have the same e->parent
+                        * and e->parent->net is holding a refcount for each
+                        * of them, we need to do some fudging here.
+                        *
+                        * WARNING: if bi->net->lock drops to 0, bi may be
+                        * freed as well (because bi->net was holding the
+                        * last reference to bi) => write after free!
+                        */
+                       unsigned refcount;
+
+                       bi = bgp_info_lock(bi);
+                       refcount = bi->net->lock - 1;
+                       bgp_unlock_node((struct bgp_node *)bi->net);
+                       if (!refcount)
+                               bi->net = NULL;
+                       bgp_info_unlock(bi);
+               }
                bgp_info_unlock(e->parent);
                e->parent = NULL;
        }

diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h
index f7eac095463ebb11ab819478d1cf96fafa8378e9..60c2cbd4a483c3e646595ebc4434b2f16badd6ff 100644 (file)
--- a/bgpd/bgp_table.h
+++ b/bgpd/bgp_table.h
@@ -128,9 +128,9 @@ static inline struct bgp_node *bgp_node_parent_nolock(struct bgp_node *node)
 /*
  * bgp_unlock_node
  */
-static inline struct bgp_node *bgp_unlock_node(struct bgp_node *node)
+static inline void bgp_unlock_node(struct bgp_node *node)
 {
-       return (struct bgp_node *)route_unlock_node(bgp_node_to_rnode(node));
+       route_unlock_node(bgp_node_to_rnode(node));
 }
 
 /*

diff --git a/lib/table.h b/lib/table.h
index ac7df3e6950b455a93c95354620e2e204baf2293..8304abe59b38ec5b3b349b1848a412ccf266a9a3 100644 (file)
--- a/lib/table.h
+++ b/lib/table.h
@@ -235,17 +235,13 @@ static inline struct route_node *route_lock_node(struct route_node *node)
 }
 
 /* Unlock node. */
-static inline struct route_node *route_unlock_node(struct route_node *node)
+static inline void route_unlock_node(struct route_node *node)
 {
        assert(node->lock > 0);
        (*(unsigned *)&node->lock)--;
 
-       if (node->lock == 0) {
+       if (node->lock == 0)
                route_node_delete(node);
-               return NULL;
-       }
-
-       return node;
 }
 
 /*