Skip to content

Commit

Permalink
zebra: Do not accept illegal safi's for route installation
Browse files Browse the repository at this point in the history
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 <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
  • Loading branch information
donaldsharp committed Jan 15, 2020
1 parent 4112bfe commit 67fcd50
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions zebra/zapi_msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 */
Expand Down

0 comments on commit 67fcd50

Please sign in to comment.