Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zebra: fix table heap-after-free crash #16614

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions zebra/zebra_rib.c
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,11 @@ void zebra_rtable_node_cleanup(struct route_table *table,
rib_unlink(node, re);
}

zebra_node_info_cleanup(node);
}

void zebra_node_info_cleanup(struct route_node *node)
{
if (node->info) {
rib_dest_t *dest = node->info;

Expand Down
90 changes: 88 additions & 2 deletions zebra/zebra_vrf.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,12 @@ static int zebra_vrf_enable(struct vrf *vrf)
static int zebra_vrf_disable(struct vrf *vrf)
{
struct zebra_vrf *zvrf = vrf->info;
struct zebra_vrf *zvrf_default = vrf_info_lookup(VRF_DEFAULT);
struct rib_table_info *info;
struct route_entry *re, *nre;
struct route_node *rn, *nrn;
struct interface *ifp;
bool rn_delete, table_delete;
afi_t afi;
safi_t safi;

Expand Down Expand Up @@ -216,9 +221,57 @@ static int zebra_vrf_disable(struct vrf *vrf)
* we no-longer need this pointer.
*/
for (safi = SAFI_UNICAST; safi <= SAFI_MULTICAST; safi++) {
zebra_router_release_table(zvrf, zvrf->table_id, afi,
safi);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear about removing this: if there's an empty table, wouldn't it be ok to call 'release' on it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now only remove it it is empty

if (!zvrf->table[afi][safi]) {
zebra_router_release_table(zvrf, zvrf->table_id,
afi, safi);
continue;
}

table_delete = true;
/* Assign the table to the default VRF.
* Although the table is not technically owned by the default VRF,
* the code assumes that unassigned routing tables are
* associated with the default VRF.
*/
info = route_table_get_info(zvrf->table[afi][safi]);
info->zvrf = zvrf_default;

rn = route_top(zvrf->table[afi][safi]);
while (rn) {
if (!rn->info) {
rn = route_next(rn);
continue;
}

/* Assign the kernel route entries to the default VRF,
* even though they are not actually owned by it.
*
* Remove route nodes that have been created by FRR daemons.
* They are not needed if the VRF is disabled.
*/
RNODE_FOREACH_RE_SAFE (rn, re, nre) {
if (re->type == ZEBRA_ROUTE_KERNEL) {
re->vrf_id = VRF_DEFAULT;
rn_delete = false;
continue;
}
rib_unlink(rn, re);
}
if (rn_delete) {
nrn = route_next(rn);
zebra_node_info_cleanup(rn);
rn = nrn;
} else {
table_delete = false;
rn = route_next(rn);
}
}
zvrf->table[afi][safi] = NULL;

if (table_delete)
/* Only delete the table if it is empty */
zebra_router_release_table(zvrf, zvrf->table_id,
afi, safi);
}
}

Expand Down Expand Up @@ -349,17 +402,50 @@ static void zebra_rnhtable_node_cleanup(struct route_table *table,
static void zebra_vrf_table_create(struct zebra_vrf *zvrf, afi_t afi,
safi_t safi)
{
vrf_id_t vrf_id = zvrf->vrf->vrf_id;
struct rib_table_info *info;
struct route_entry *re;
struct route_node *rn;
struct prefix p;

assert(!zvrf->table[afi][safi]);

/* Attempt to retrieve the Linux routing table using zvrf->table_id.
* If the table was created before the VRF, it will already exist.
* Otherwise, create a new table.
*/
zvrf->table[afi][safi] =
zebra_router_get_table(zvrf, zvrf->table_id, afi, safi);

/* If the table existed before the VRF was created, info->zvrf was
* referring to the default VRF.
* Assign the table to the new VRF.
* Note: FRR does not allow multiple VRF interfaces to be created with the
* same table ID.
*/
info = route_table_get_info(zvrf->table[afi][safi]);
info->zvrf = zvrf;

/* If the table existed before the VRF was created, their routing entries
* was owned by the default VRF.
* Re-assign all the routing entries to the new VRF.
*/
for (rn = route_top(zvrf->table[afi][safi]); rn; rn = route_next(rn)) {
if (!rn->info)
continue;

RNODE_FOREACH_RE (rn, re)
re->vrf_id = vrf_id;
}

memset(&p, 0, sizeof(p));
p.family = afi2family(afi);

if (route_node_lookup_maynull(zvrf->table[afi][safi], &p))
/* do not override the existing default route */
return;

/* create a fake default route */
rn = srcdest_rnode_get(zvrf->table[afi][safi], &p, NULL);
zebra_rib_create_dest(rn);
}
Expand Down
2 changes: 2 additions & 0 deletions zebra/zebra_vrf.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ extern void zebra_vrf_init(void);

extern void zebra_rtable_node_cleanup(struct route_table *table,
struct route_node *node);
extern void zebra_node_info_cleanup(struct route_node *node);


#ifdef __cplusplus
}
Expand Down
11 changes: 7 additions & 4 deletions zebra/zebra_vty.c
Original file line number Diff line number Diff line change
Expand Up @@ -939,14 +939,17 @@ static void do_show_route_helper(struct vty *vty, struct zebra_vrf *zvrf,
vty_out(vty, "\n");
if (ctx->multi || zvrf_id(zvrf) != VRF_DEFAULT
|| tableid) {
if (!tableid)
vty_out(vty, "VRF %s:\n",
zvrf_name(zvrf));
else
if (vrf_is_backend_netns() && tableid)
vty_out(vty,
"VRF %s table %u:\n",
zvrf_name(zvrf),
tableid);
else if (tableid)
vty_out(vty, "Table %u:\n",
tableid);
else
vty_out(vty, "VRF %s:\n",
zvrf_name(zvrf));
}
ctx->header_done = true;
first = 0;
Expand Down
Loading