Merge pull request #9321 from donaldsharp/no_leak_re

zebra: Prevent memory leak if route is rejected early
This commit is contained in:
Igor Ryzhov 2021-08-10 11:39:30 +03:00 committed by GitHub
commit bdb7b7c5d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 25 additions and 6 deletions

View File

@ -403,6 +403,11 @@ extern int rib_add(afi_t afi, safi_t safi, vrf_id_t vrf_id, int type,
extern int rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p, extern int rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p,
struct prefix_ipv6 *src_p, struct route_entry *re, struct prefix_ipv6 *src_p, struct route_entry *re,
struct nexthop_group *ng); struct nexthop_group *ng);
/*
* -1 -> some sort of error
* 0 -> an add
* 1 -> an update
*/
extern int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p, extern int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
struct prefix_ipv6 *src_p, struct prefix_ipv6 *src_p,
struct route_entry *re, struct route_entry *re,

View File

@ -2110,6 +2110,15 @@ static void zread_route_add(ZAPI_HANDLER_ARGS)
ret = rib_add_multipath_nhe(afi, api.safi, &api.prefix, src_p, ret = rib_add_multipath_nhe(afi, api.safi, &api.prefix, src_p,
re, &nhe); re, &nhe);
/*
* rib_add_multipath_nhe only fails in a couple spots
* and in those spots we have not freed memory
*/
if (ret == -1) {
client->error_cnt++;
XFREE(MTYPE_RE, re);
}
/* At this point, these allocations are not needed: 're' has been /* At this point, these allocations are not needed: 're' has been
* retained or freed, and if 're' still exists, it is using * retained or freed, and if 're' still exists, it is using
* a reference to a shared group object. * a reference to a shared group object.
@ -2121,15 +2130,15 @@ static void zread_route_add(ZAPI_HANDLER_ARGS)
/* Stats */ /* Stats */
switch (api.prefix.family) { switch (api.prefix.family) {
case AF_INET: case AF_INET:
if (ret > 0) if (ret == 0)
client->v4_route_add_cnt++; client->v4_route_add_cnt++;
else if (ret < 0) else if (ret == 1)
client->v4_route_upd8_cnt++; client->v4_route_upd8_cnt++;
break; break;
case AF_INET6: case AF_INET6:
if (ret > 0) if (ret == 0)
client->v6_route_add_cnt++; client->v6_route_add_cnt++;
else if (ret < 0) else if (ret == 1)
client->v6_route_upd8_cnt++; client->v6_route_upd8_cnt++;
break; break;
} }

View File

@ -3453,6 +3453,10 @@ void rib_lookup_and_pushup(struct prefix_ipv4 *p, vrf_id_t vrf_id)
* allocate: if they allocate a nexthop_group or backup nexthop info, they * allocate: if they allocate a nexthop_group or backup nexthop info, they
* must free those objects. If this returns < 0, an error has occurred and the * must free those objects. If this returns < 0, an error has occurred and the
* route_entry 're' has not been captured; the caller should free that also. * route_entry 're' has not been captured; the caller should free that also.
*
* -1 -> error
* 0 -> Add
* 1 -> update
*/ */
int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p, int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
struct prefix_ipv6 *src_p, struct route_entry *re, struct prefix_ipv6 *src_p, struct route_entry *re,
@ -3567,11 +3571,12 @@ int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
SET_FLAG(re->status, ROUTE_ENTRY_CHANGED); SET_FLAG(re->status, ROUTE_ENTRY_CHANGED);
rib_addnode(rn, re, 1); rib_addnode(rn, re, 1);
ret = 1;
/* Free implicit route.*/ /* Free implicit route.*/
if (same) if (same) {
ret = 1;
rib_delnode(rn, same); rib_delnode(rn, same);
}
/* See if we can remove some RE entries that are queued for /* See if we can remove some RE entries that are queued for
* removal, but won't be considered in rib processing. * removal, but won't be considered in rib processing.