diff options
| author | Stephen Worley <sworley@cumulusnetworks.com> | 2019-11-01 15:52:47 -0400 | 
|---|---|---|
| committer | Stephen Worley <sworley@cumulusnetworks.com> | 2019-11-01 16:06:19 -0400 | 
| commit | c7c0b007a4b086ea1bc953de0a46e07bd80e66ce (patch) | |
| tree | 0d6197d6c57c314249a512f6a3083d4c5eacbdf3 /zebra/zebra_vrf.c | |
| parent | 7a6fc8774ddf74df312c7d4ce0bbdbb5ac01c5fa (diff) | |
zebra: separate zebra_vrf_lookup_table_with_id()
We were creating `other` tables in rib_del(), vty commands, and
dataplane return callback via the zebra_vrf_table_with_table_id()
API.
Seperate the API into only a lookup, never create
and added another with `get` in the name (following the standard
we use in other table APIs).
Then changed the rib_del(), rib_find_rn_from_ctx(), and show route
summary vty command to use the lookup API instead.
This was found via a crash where two different vrfs though they owned
the table. On delete, one free'd all the nodes, and then the other tried
to use them. It required specific timing of a VRF existing, going away,
and coming back again to cause the crash.
=23464== Invalid read of size 8
==23464==    at 0x179EA4: rib_dest_from_rnode (rib.h:433)
==23464==    by 0x17ACB1: zebra_vrf_delete (zebra_vrf.c:253)
==23464==    by 0x48F3D45: vrf_delete (vrf.c:243)
==23464==    by 0x48F4468: vrf_terminate (vrf.c:532)
==23464==    by 0x13D8C5: sigint (main.c:172)
==23464==    by 0x48DD25C: quagga_sigevent_process (sigevent.c:105)
==23464==    by 0x48F0502: thread_fetch (thread.c:1417)
==23464==    by 0x48AC82B: frr_run (libfrr.c:1023)
==23464==    by 0x13DD02: main (main.c:483)
==23464==  Address 0x5152788 is 104 bytes inside a block of size 112 free'd
==23464==    at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==23464==    by 0x48B25B8: qfree (memory.c:129)
==23464==    by 0x48EA335: route_node_destroy (table.c:500)
==23464==    by 0x48E967F: route_node_free (table.c:90)
==23464==    by 0x48E9742: route_table_free (table.c:124)
==23464==    by 0x48E9599: route_table_finish (table.c:60)
==23464==    by 0x170CEA: zebra_router_free_table (zebra_router.c:165)
==23464==    by 0x170DB4: zebra_router_release_table (zebra_router.c:188)
==23464==    by 0x17AAD2: zebra_vrf_disable (zebra_vrf.c:222)
==23464==    by 0x48F3F0C: vrf_disable (vrf.c:313)
==23464==    by 0x48F3CCF: vrf_delete (vrf.c:223)
==23464==    by 0x48F4468: vrf_terminate (vrf.c:532)
==23464==  Block was alloc'd at
==23464==    at 0x4837B65: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==23464==    by 0x48B24A2: qcalloc (memory.c:110)
==23464==    by 0x48EA2FE: route_node_create (table.c:488)
==23464==    by 0x48E95C7: route_node_new (table.c:66)
==23464==    by 0x48E95E5: route_node_set (table.c:75)
==23464==    by 0x48E9EA9: route_node_get (table.c:326)
==23464==    by 0x48E1EDB: srcdest_rnode_get (srcdest_table.c:244)
==23464==    by 0x16EA4B: rib_add_multipath (zebra_rib.c:2730)
==23464==    by 0x1A5310: zread_route_add (zapi_msg.c:1592)
==23464==    by 0x1A7B8E: zserv_handle_commands (zapi_msg.c:2579)
==23464==    by 0x19D689: zserv_process_messages (zserv.c:523)
==23464==    by 0x48F09F8: thread_call (thread.c:1599)
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Diffstat (limited to 'zebra/zebra_vrf.c')
| -rw-r--r-- | zebra/zebra_vrf.c | 27 | 
1 files changed, 23 insertions, 4 deletions
diff --git a/zebra/zebra_vrf.c b/zebra/zebra_vrf.c index f425c0e49e..c392303760 100644 --- a/zebra/zebra_vrf.c +++ b/zebra/zebra_vrf.c @@ -343,13 +343,12 @@ int zebra_vrf_has_config(struct zebra_vrf *zvrf)   * - case VRF backend is default : on default VRF only   * - case VRF backend is netns : on all VRFs   */ -struct route_table *zebra_vrf_table_with_table_id(afi_t afi, safi_t safi, -						  vrf_id_t vrf_id, -						  uint32_t table_id) +struct route_table *zebra_vrf_lookup_table_with_table_id(afi_t afi, safi_t safi, +							 vrf_id_t vrf_id, +							 uint32_t table_id)  {  	struct zebra_vrf *zvrf = vrf_info_lookup(vrf_id);  	struct other_route_table ort, *otable; -	struct route_table *table;  	if (!zvrf)  		return NULL; @@ -364,9 +363,28 @@ struct route_table *zebra_vrf_table_with_table_id(afi_t afi, safi_t safi,  	ort.safi = safi;  	ort.table_id = table_id;  	otable = otable_find(&zvrf->other_tables, &ort); +  	if (otable)  		return otable->table; +	return NULL; +} + +struct route_table *zebra_vrf_get_table_with_table_id(afi_t afi, safi_t safi, +						      vrf_id_t vrf_id, +						      uint32_t table_id) +{ +	struct zebra_vrf *zvrf = vrf_info_lookup(vrf_id); +	struct other_route_table *otable; +	struct route_table *table; + +	table = zebra_vrf_lookup_table_with_table_id(afi, safi, vrf_id, +						     table_id); + +	if (table) +		goto done; + +	/* Create it as an `other` table */  	table = zebra_router_get_table(zvrf, table_id, afi, safi);  	otable = XCALLOC(MTYPE_OTHER_TABLE, sizeof(*otable)); @@ -376,6 +394,7 @@ struct route_table *zebra_vrf_table_with_table_id(afi_t afi, safi_t safi,  	otable->table = table;  	otable_add(&zvrf->other_tables, otable); +done:  	return table;  }  | 
