Hi,

eigrpd(8) should only free the configured interfaces after the eigrp instances
on shutdown otherwise an use after free will happen and the name of the iface
will also not be displayed since that information was already disposed. The
use-after-free actually doesn't manifest everytime, but after a few attempts it
shows. Note: I'm running with /etc/malloc.conf -> S

...
eigrp_if_reset:  as 666 family ipv4
eigrpd(66548) in free(): use after free 0x1b0c8e656080
...

afterwards:
...
eigrp_if_reset: vio0 as 666 family ipv4
...

Also while here I shuffled the pledge(2) promises to the canonical form.

I don't have eigrpd(8) on production so I can't really say this is the best
solution, and although it's working for me with /etc/examples/eigrpd.conf it
really needs an extra review from the experts.

Index: eigrpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/eigrpd/eigrpd.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 eigrpd.c
--- eigrpd.c    2 Sep 2016 17:59:58 -0000       1.21
+++ eigrpd.c    1 Aug 2018 18:12:43 -0000
@@ -268,7 +268,7 @@ main(int argc, char *argv[])
            eigrpd_conf->rdomain) == -1)
                fatalx("kr_init failed");
 
-       if (pledge("inet rpath stdio sendfd", NULL) == -1)
+       if (pledge("stdio rpath inet sendfd", NULL) == -1)
                fatal("pledge");
 
        event_dispatch();
@@ -641,25 +641,6 @@ merge_config(struct eigrpd_conf *conf, s
        conf->fib_priority_external = xconf->fib_priority_external;
        conf->fib_priority_summary = xconf->fib_priority_summary;
 
-       /* merge interfaces */
-       TAILQ_FOREACH_SAFE(iface, &conf->iface_list, entry, itmp) {
-               /* find deleted ifaces */
-               if ((xi = if_lookup(xconf, iface->ifindex)) == NULL) {
-                       TAILQ_REMOVE(&conf->iface_list, iface, entry);
-                       free(iface);
-               }
-       }
-       TAILQ_FOREACH_SAFE(xi, &xconf->iface_list, entry, itmp) {
-               /* find new ifaces */
-               if ((iface = if_lookup(conf, xi->ifindex)) == NULL) {
-                       TAILQ_REMOVE(&xconf->iface_list, xi, entry);
-                       TAILQ_INSERT_TAIL(&conf->iface_list, xi, entry);
-                       continue;
-               }
-
-               /* TODO update existing ifaces */
-       }
-
        /* merge instances */
        TAILQ_FOREACH_SAFE(eigrp, &conf->instances, entry, etmp) {
                /* find deleted instances */
@@ -701,6 +682,25 @@ merge_config(struct eigrpd_conf *conf, s
                /* update existing instances */
                merge_instances(conf, eigrp, xe);
        }
+
+       /* merge interfaces */
+       TAILQ_FOREACH_SAFE(iface, &conf->iface_list, entry, itmp) {
+               /* find deleted ifaces */
+               if ((xi = if_lookup(xconf, iface->ifindex)) == NULL) {
+                       TAILQ_REMOVE(&conf->iface_list, iface, entry);
+                       free(iface);
+               }
+       }
+       TAILQ_FOREACH_SAFE(xi, &xconf->iface_list, entry, itmp) {
+               /* find new ifaces */
+               if ((iface = if_lookup(conf, xi->ifindex)) == NULL) {
+                       TAILQ_REMOVE(&xconf->iface_list, xi, entry);
+                       TAILQ_INSERT_TAIL(&conf->iface_list, xi, entry);
+                       continue;
+               }
+
+               /* TODO update existing ifaces */
+       }       
 
        /* resend addresses to activate new interfaces */
        if (eigrpd_process == PROC_MAIN)

Reply via email to