]> git.puffer.fish Git - mirror/frr.git/commitdiff
ospfd: Prevent use after free on shutdown 10038/head
authorDonald Sharp <sharpd@nvidia.com>
Thu, 11 Nov 2021 18:25:35 +0000 (13:25 -0500)
committermergify-bot <noreply@mergify.io>
Thu, 11 Nov 2021 21:34:50 +0000 (21:34 +0000)
Running ospf_topo_vrf1 leads us to this valgrind issue:

==2386518== Invalid read of size 8
==2386518==    at 0x4971520: route_top (table.c:401)
==2386518==    by 0x181F08: ospf_interface_bfd_apply (ospf_bfd.c:126)
==2386518==    by 0x182069: ospf_interface_disable_bfd (ospf_bfd.c:158)
==2386518==    by 0x18BF51: ospf_del_if_params (ospf_interface.c:557)
==2386518==    by 0x18C584: ospf_if_delete_hook (ospf_interface.c:712)
==2386518==    by 0x490CA0B: hook_call_if_del (if.c:61)
==2386518==    by 0x490D1F3: if_delete_retain (if.c:286)
==2386518==    by 0x490D337: if_delete (if.c:309)
==2386518==    by 0x490CDED: if_destroy_via_zapi (if.c:200)
==2386518==    by 0x49940A9: zclient_interface_delete (zclient.c:2237)
==2386518==    by 0x4998062: zclient_read (zclient.c:3969)
==2386518==    by 0x4979529: thread_call (thread.c:1908)
==2386518==    by 0x4919918: frr_run (libfrr.c:1164)
==2386518==    by 0x181AC7: main (ospf_main.c:235)
==2386518==  Address 0x5df39a0 is 0 bytes inside a block of size 56 free'd
==2386518==    at 0x48399AB: free (vg_replace_malloc.c:538)
==2386518==    by 0x492A03E: qfree (memory.c:141)
==2386518==    by 0x4970C6F: route_table_free (table.c:141)
==2386518==    by 0x4970A36: route_table_finish (table.c:61)
==2386518==    by 0x18C543: ospf_if_delete_hook (ospf_interface.c:708)
==2386518==    by 0x490CA0B: hook_call_if_del (if.c:61)
==2386518==    by 0x490D1F3: if_delete_retain (if.c:286)
==2386518==    by 0x490D337: if_delete (if.c:309)
==2386518==    by 0x490CDED: if_destroy_via_zapi (if.c:200)
==2386518==    by 0x49940A9: zclient_interface_delete (zclient.c:2237)
==2386518==    by 0x4998062: zclient_read (zclient.c:3969)
==2386518==    by 0x4979529: thread_call (thread.c:1908)
==2386518==    by 0x4919918: frr_run (libfrr.c:1164)
==2386518==    by 0x181AC7: main (ospf_main.c:235)
==2386518==  Block was alloc'd at
==2386518==    at 0x483AB65: calloc (vg_replace_malloc.c:760)
==2386518==    by 0x4929EFC: qcalloc (memory.c:116)
==2386518==    by 0x49709F8: route_table_init_with_delegate (table.c:53)
==2386518==    by 0x49717F4: route_table_init (table.c:528)
==2386518==    by 0x18C328: ospf_if_new_hook (ospf_interface.c:659)
==2386518==    by 0x490C97D: hook_call_if_add (if.c:60)
==2386518==    by 0x490CE85: if_create_name (if.c:223)
==2386518==    by 0x490DF32: if_get_by_name (if.c:622)
==2386518==    by 0x4993F73: zclient_interface_add (zclient.c:2186)
==2386518==    by 0x4998062: zclient_read (zclient.c:3969)
==2386518==    by 0x4979529: thread_call (thread.c:1908)
==2386518==    by 0x4919918: frr_run (libfrr.c:1164)
==2386518==    by 0x181AC7: main (ospf_main.c:235)
==2386518==

Fix the ordering to do the individual node tree cleanup after we delete
the data we care about.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 9ffde6e1b037ff4d7c87aa2e22bc6d5823d9329c)

ospfd/ospf_interface.c

index 2fea385594bae1db6087d4816ebcfd397e12e4db..6014df88bb4cc14b43a7034b519645497f386d85 100644 (file)
@@ -705,11 +705,11 @@ static int ospf_if_delete_hook(struct interface *ifp)
         */
        ospf_del_if_params(ifp, IF_DEF_PARAMS(ifp));
 
-       route_table_finish(IF_OIFS(ifp));
-
        for (rn = route_top(IF_OIFS_PARAMS(ifp)); rn; rn = route_next(rn))
                if (rn->info)
                        ospf_del_if_params(ifp, rn->info);
+
+       route_table_finish(IF_OIFS(ifp));
        route_table_finish(IF_OIFS_PARAMS(ifp));
 
        XFREE(MTYPE_OSPF_IF_INFO, ifp->info);