Hi,

While looking at the code of ripd:

I think there are (also) 2 small memleaks in a debug/error path
(IMSG_REQUEST_ADD and IMSG_RESPONSE_ADD). It breaks out before adding the
struct rip_route as an entry by the add_entry function (which adds it and adds
a reference count) in the log_debug block.

clang-analyzer also pointed at a double-free and use of free'd data: the
function kroute_insert frees kr and returns -1 when struct kroute is duplicate.

Patch below (untested):


diff --git a/usr.sbin/ripd/kroute.c b/usr.sbin/ripd/kroute.c
index 6e7449e0909..71758a75e44 100644
--- a/usr.sbin/ripd/kroute.c
+++ b/usr.sbin/ripd/kroute.c
@@ -183,8 +183,7 @@ kr_change_fib(struct kroute_node *kr, struct kroute 
*kroute, int action)
 
                if (kroute_insert(kr) == -1) {
                        log_debug("kr_update_fib: cannot insert %s",
-                           inet_ntoa(kr->r.nexthop));
-                       free(kr);
+                           inet_ntoa(kroute->nexthop));
                }
        } else
                kr->r.nexthop.s_addr = kroute->nexthop.s_addr;
diff --git a/usr.sbin/ripd/ripe.c b/usr.sbin/ripd/ripe.c
index d83901e245f..1f6f9b6583f 100644
--- a/usr.sbin/ripd/ripe.c
+++ b/usr.sbin/ripd/ripe.c
@@ -351,6 +351,7 @@ ripe_dispatch_rde(int fd, short event, void *bula)
                                    NULL) {
                                        log_debug("unknown neighbor id %u",
                                            imsg.hdr.peerid);
+                                       free(rr);
                                        break;
                                }
                                add_entry(&nbr->rq_list, rr);
@@ -396,6 +397,7 @@ ripe_dispatch_rde(int fd, short event, void *bula)
                        if ((nbr = nbr_find_peerid(imsg.hdr.peerid)) == NULL) {
                                log_debug("unknown neighbor id %u",
                                    imsg.hdr.peerid);
+                               free(rr);
                                break;
                        }
                        iface = nbr->iface;

-- 
Kind regards,
Hiltjo

Reply via email to