From 5e7939a561d3250e100d8e9a16daf141bac1eeeb Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 14 Jan 2020 21:03:11 -0500 Subject: [PATCH] zebra: Do not accept illegal safi's for route installation The only two safi's that are usable for zebra for installation of routes into the rib are SAFI_UNICAST and SAFI_MULTICAST. The acceptance of other safi's is causing a memory leak: Direct leak of 56 byte(s) in 1 object(s) allocated from: #0 0x5332f2 in calloc (/usr/lib/frr/zebra+0x5332f2) #1 0x7f594adc29db in qcalloc /opt/build/frr/lib/memory.c:110:27 #2 0x686849 in zebra_vrf_get_table_with_table_id /opt/build/frr/zebra/zebra_vrf.c:390:11 #3 0x65a245 in rib_add_multipath /opt/build/frr/zebra/zebra_rib.c:2591:10 #4 0x7211bc in zread_route_add /opt/build/frr/zebra/zapi_msg.c:1616:8 #5 0x73063c in zserv_handle_commands /opt/build/frr/zebra/zapi_msg.c:2682:2 Collapse Sequence of events: Upon vrf creation there is a zvrf->table[afi][safi] data structure that tables are auto created for. These tables only create SAFI_UNICAST and SAFI_MULTICAST tables. Since these are the only safi types that are zebra can actually work on. zvrf data structures also have a zvrf->otable data structure that tracks in a RB tree other tables that are created ( say you have routes stuck in any random table in the 32bit route table space in linux ). This data structure is only used if the lookup in zvrf->table[afi][safi] fails. After creation if we pass a route down from an upper level protocol that has non unicast or multicast safi *but* has the actual tableid of the vrf we are in, the initial lookup will always return NULL leaving us to look in the otable. This will create a data structure to track this data. If after this event you pass in a second route with the same afi/safi/table_id, the otable will be created and attempted to be stored, but the RB_TREE_UNIQ data structure when it sees this will return the original otable returned and the lookup function zebra_vrf_get_table_with_table_id will just drop the second otable. Signed-off-by: Quentin Young Signed-off-by: Donald Sharp --- zebra/zapi_msg.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 651babdebac5..5614ec54230c 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -41,6 +41,7 @@ #include "lib/vrf.h" #include "lib/libfrr.h" #include "lib/sockopt.h" +#include "lib/lib_errors.h" #include "zebra/zebra_router.h" #include "zebra/rib.h" @@ -1612,6 +1613,14 @@ static void zread_route_add(ZAPI_HANDLER_ARGS) if (CHECK_FLAG(api.message, ZAPI_MESSAGE_SRCPFX)) src_p = &api.src_prefix; + if (api.safi != SAFI_UNICAST && api.safi != SAFI_MULTICAST) { + flog_warn(EC_LIB_ZAPI_MISSMATCH, + "%s: Received safi: %d but we can only accept UNICAST or MULTICAST", + __func__, api.safi); + nexthop_group_delete(&ng); + XFREE(MTYPE_RE, re); + return; + } ret = rib_add_multipath(afi, api.safi, &api.prefix, src_p, re, ng); /* Stats */