]> git.puffer.fish Git - mirror/frr.git/commit
zebra: Fix use after free in rib_process_result 3856/head
authorDonald Sharp <sharpd@cumulusnetworks.com>
Sun, 24 Feb 2019 00:58:20 +0000 (19:58 -0500)
committerDonald Sharp <sharpd@cumulusnetworks.com>
Sun, 24 Feb 2019 01:03:48 +0000 (20:03 -0500)
commit5f27bcba2ab29e9661c070f466c73a3cad359eef
treefd43f817bff0954b6f62cd25d96b682468dffd3f
parent5fa343088527419802314442d8fad5176ba8ac16
zebra: Fix use after free in rib_process_result

Running zebra after commit 888756b208edc7935705d95b83f9513acc21e78a
in valgrind produces this item:

==17102== Invalid read of size 8
==17102==    at 0x44D84C: rib_dest_from_rnode (rib.h:375)
==17102==    by 0x4546ED: rib_process_result (zebra_rib.c:1904)
==17102==    by 0x45436D: rib_process_dplane_results (zebra_rib.c:3295)
==17102==    by 0x4D0902B: thread_call (thread.c:1607)
==17102==    by 0x4CC3983: frr_run (libfrr.c:1011)
==17102==    by 0x4266F6: main (main.c:473)
==17102==  Address 0x83bd468 is 88 bytes inside a block of size 96 free'd
==17102==    at 0x4A35F54: free (vg_replace_malloc.c:530)
==17102==    by 0x4CCAC00: qfree (memory.c:129)
==17102==    by 0x4D03DC6: route_node_destroy (table.c:501)
==17102==    by 0x4D039EE: route_node_free (table.c:90)
==17102==    by 0x4D03971: route_node_delete (table.c:382)
==17102==    by 0x44D82A: route_unlock_node (table.h:256)
==17102==    by 0x454617: rib_process_result (zebra_rib.c:1882)
==17102==    by 0x45436D: rib_process_dplane_results (zebra_rib.c:3295)
==17102==    by 0x4D0902B: thread_call (thread.c:1607)
==17102==    by 0x4CC3983: frr_run (libfrr.c:1011)
==17102==    by 0x4266F6: main (main.c:473)
==17102==  Block was alloc'd at
==17102==    at 0x4A36FF6: calloc (vg_replace_malloc.c:752)
==17102==    by 0x4CCAA2D: qcalloc (memory.c:110)
==17102==    by 0x4D03D88: route_node_create (table.c:489)
==17102==    by 0x4D0360F: route_node_new (table.c:65)
==17102==    by 0x4D034F8: route_node_set (table.c:74)
==17102==    by 0x4D03486: route_node_get (table.c:327)
==17102==    by 0x4CFB700: srcdest_rnode_get (srcdest_table.c:243)
==17102==    by 0x4545C1: rib_process_result (zebra_rib.c:1872)
==17102==    by 0x45436D: rib_process_dplane_results (zebra_rib.c:3295)
==17102==    by 0x4D0902B: thread_call (thread.c:1607)
==17102==    by 0x4CC3983: frr_run (libfrr.c:1011)
==17102==    by 0x4266F6: main (main.c:473)
==17102==

This is happening because of this order of events:

1) Route is deleted in the main thread and scheduled for rib processing.
2) Rib garbage collection is run and we remove the route node since it
is no longer needed.
3) Data plane returns from the deletion in the kernel and we call
the srcdest_rnode_get function to get the prefix that was deleted.
This recreates a new route node.  This creates a route_node with
a lock count of 1, which we freed via the route_unlock_node call.
Then we continued to use the rn pointer.  Which leaves us with use
after frees.

The solution is, of course, to just move the unlock the node at the
end of the function if we have a route_node.

Fixes: #3854
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
zebra/zebra_rib.c