ospf6d: Prevent use after free

ospf6_lsa_unlock may free the lsa data structure as such
we cannot use the passed in data structure after freeing it.

Provide a mechanism to know if the data has been freed
using the same usage patterns of other _unlock functions
in FRR.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

diff --git a/ospf6d/ospf6_flood.c b/ospf6d/ospf6_flood.c
index 85d02c186b70bf4ebe1ddc4227dc2ce4132e9742..b144c6804e9e41bdffc5c3a9e85ba46af9547bcd 100644 (file)
--- a/ospf6d/ospf6_flood.c
+++ b/ospf6d/ospf6_flood.c
@@ -332,11 +332,12 @@ void ospf6_flood_interface(struct ospf6_neighbor *from, struct ospf6_lsa *lsa,
                                        if (req == on->last_ls_req) {
                                                /* sanity check refcount */
                                                assert(req->lock >= 2);
-                                               ospf6_lsa_unlock(req);
+                                               req = ospf6_lsa_unlock(req);
                                                on->last_ls_req = NULL;
                                        }
-                                       ospf6_lsdb_remove(req,
-                                                         on->request_list);
+                                       if (req)
+                                               ospf6_lsdb_remove(
+                                                       req, on->request_list);
                                        ospf6_check_nbr_loading(on);
                                        continue;
                                }
@@ -348,7 +349,7 @@ void ospf6_flood_interface(struct ospf6_neighbor *from, struct ospf6_lsa *lsa,
                                                zlog_debug(
                                                        "Received is newer, remove requesting");
                                        if (req == on->last_ls_req) {
-                                               ospf6_lsa_unlock(req);
+                                               req = ospf6_lsa_unlock(req);
                                                on->last_ls_req = NULL;
                                        }
                                        if (req)

diff --git a/ospf6d/ospf6_lsa.c b/ospf6d/ospf6_lsa.c
index bcfd975879c12e74d5cde303aadc51da8f7e6051..aa32fae6adba64d964277c82790a1604edc040c2 100644 (file)
--- a/ospf6d/ospf6_lsa.c
+++ b/ospf6d/ospf6_lsa.c
@@ -608,16 +608,17 @@ void ospf6_lsa_lock(struct ospf6_lsa *lsa)
 }
 
 /* decrement reference counter of struct ospf6_lsa */
-void ospf6_lsa_unlock(struct ospf6_lsa *lsa)
+struct ospf6_lsa *ospf6_lsa_unlock(struct ospf6_lsa *lsa)
 {
        /* decrement reference counter */
        assert(lsa->lock > 0);
        lsa->lock--;
 
        if (lsa->lock != 0)
-               return;
+               return lsa;
 
        ospf6_lsa_delete(lsa);
+       return NULL;
 }
 
 

diff --git a/ospf6d/ospf6_lsa.h b/ospf6d/ospf6_lsa.h
index 02f9f9d26cd337210829b2243d752a3f23a9effb..5519dd1b80d39a1d741019dc2640699001df5176 100644 (file)
--- a/ospf6d/ospf6_lsa.h
+++ b/ospf6d/ospf6_lsa.h
@@ -227,7 +227,7 @@ extern void ospf6_lsa_delete(struct ospf6_lsa *lsa);
 extern struct ospf6_lsa *ospf6_lsa_copy(struct ospf6_lsa *);
 
 extern void ospf6_lsa_lock(struct ospf6_lsa *);
-extern void ospf6_lsa_unlock(struct ospf6_lsa *);
+extern struct ospf6_lsa *ospf6_lsa_unlock(struct ospf6_lsa *);
 
 extern int ospf6_lsa_expire(struct thread *);
 extern int ospf6_lsa_refresh(struct thread *);

diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c
index 0a9f1c6f7ced9bfa0f7c4add1a35754aa33078d9..18fcec82c118f7858904ac8262b78a0c98aeaf85 100644 (file)
--- a/ospf6d/ospf6_lsdb.c
+++ b/ospf6d/ospf6_lsdb.c
@@ -136,7 +136,7 @@ void ospf6_lsdb_add(struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb)
                }
                /* to free the lookup lock in node get*/
                route_unlock_node(current);
-               ospf6_lsa_unlock(old);
+               old = ospf6_lsa_unlock(old);
        }
 
        ospf6_lsdb_count_assert(lsdb);
@@ -164,7 +164,7 @@ void ospf6_lsdb_remove(struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb)
 
        route_unlock_node(node); /* to free the lookup lock */
        route_unlock_node(node); /* to free the original lock */
-       ospf6_lsa_unlock(lsa);
+       lsa = ospf6_lsa_unlock(lsa);
 
        ospf6_lsdb_count_assert(lsdb);
 }
@@ -279,7 +279,7 @@ struct ospf6_lsa *ospf6_lsdb_next(const struct route_node *iterend,
 {
        struct route_node *node = lsa->rn;
 
-       ospf6_lsa_unlock(lsa);
+       lsa = ospf6_lsa_unlock(lsa);
 
        do
                node = route_next_until(node, iterend);
@@ -316,7 +316,7 @@ void ospf6_lsdb_lsa_unlock(struct ospf6_lsa *lsa)
        if (lsa != NULL) {
                if (lsa->rn != NULL)
                        route_unlock_node(lsa->rn);
-               ospf6_lsa_unlock(lsa);
+               lsa = ospf6_lsa_unlock(lsa);
        }
 }
 

diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c
index 21f9b0722cd8749a2993471187aec60caa9eb59a..31862a2298f4e7feeb3cac3a8d0d74bd3bf81ca5 100644 (file)
--- a/ospf6d/ospf6_message.c
+++ b/ospf6d/ospf6_message.c
@@ -1948,9 +1948,9 @@ int ospf6_lsreq_send(struct thread *thread)
        }
 
        if (last_req != NULL) {
-               if (on->last_ls_req != NULL) {
-                       ospf6_lsa_unlock(on->last_ls_req);
-               }
+               if (on->last_ls_req != NULL)
+                       on->last_ls_req = ospf6_lsa_unlock(on->last_ls_req);
+
                ospf6_lsa_lock(last_req);
                on->last_ls_req = last_req;
        }