Skip to content

Commit

Permalink
lib: Free Temporary Memory for AF_FLOWSPEC Prefix
Browse files Browse the repository at this point in the history
This commit addresses a leak where temporary memory allocated
earlier by the `prefix_copy` function for AF_FLOWSPEC prefixes
was not being freed. To ensure proper memory management, we now
release this temporary memory by calling `prefix_flowspec_ptr_free`.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in bgp_flowspec.test_bgp_flowspec_topo/r1.asan.bgpd.11539

=================================================================
==11539==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 56 byte(s) in 2 object(s) allocated from:
    #0 0x7feaa956ad28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    FRRouting#1 0x7feaa8f670da in qcalloc lib/memory.c:105
    FRRouting#2 0x7feaa8fac1d4 in prefix_copy lib/prefix.c:346
    FRRouting#3 0x7feaa8ff43e8 in route_node_get lib/table.c:274
    FRRouting#4 0x56247cc798c0 in bgp_node_get bgpd/bgp_table.h:236
    FRRouting#5 0x56247cc798c0 in bgp_afi_node_get bgpd/bgp_route.c:145
    FRRouting#6 0x56247cc92622 in bgp_update bgpd/bgp_route.c:4188
    FRRouting#7 0x56247ce55b21 in bgp_nlri_parse_flowspec bgpd/bgp_flowspec.c:193
    FRRouting#8 0x56247cc4cdd8 in bgp_nlri_parse bgpd/bgp_packet.c:350
    FRRouting#9 0x56247cc4f37c in bgp_update_receive bgpd/bgp_packet.c:2153
    FRRouting#10 0x56247cc591e2 in bgp_process_packet bgpd/bgp_packet.c:3214
    FRRouting#11 0x7feaa9005b99 in event_call lib/event.c:1979
    FRRouting#12 0x7feaa8f4a379 in frr_run lib/libfrr.c:1213
    FRRouting#13 0x56247cb51b21 in main bgpd/bgp_main.c:510
    FRRouting#14 0x7feaa7f8dc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 56 byte(s) leaked in 2 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <[email protected]>
  • Loading branch information
Keelan10 committed Oct 5, 2023
1 parent 373d46d commit a7fe30e
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 4 deletions.
2 changes: 2 additions & 0 deletions bgpd/bgp_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ static void bgp_node_destroy(route_table_delegate_t *delegate,

if (family2afi(node->p.family) == AFI_LINKSTATE)
prefix_linkstate_ptr_free(&node->p);
else if (node->p.family == AF_FLOWSPEC)
prefix_flowspec_ptr_free(&node->p);

XFREE(MTYPE_ROUTE_NODE, node);
}
Expand Down
16 changes: 12 additions & 4 deletions lib/prefix.c
Original file line number Diff line number Diff line change
Expand Up @@ -1304,6 +1304,17 @@ void prefix_linkstate_ptr_free(struct prefix *p)
p->u.prefix_linkstate.ptr = (uintptr_t)NULL;
}

void prefix_flowspec_ptr_free(struct prefix *p)
{
void *temp;

if (!p || p->family != AF_FLOWSPEC || !p->u.prefix_flowspec.ptr)
return;

temp = (void *)p->u.prefix_flowspec.ptr;
XFREE(MTYPE_PREFIX_FLOWSPEC, temp);
p->u.prefix_flowspec.ptr = (uintptr_t)NULL;
}

struct prefix *prefix_new(void)
{
Expand Down Expand Up @@ -1455,7 +1466,6 @@ unsigned prefix_hash_key(const void *pp)
{
struct prefix copy;
uint32_t len;
void *temp;

/* make sure *all* unused bits are zero, particularly including
* alignment /
Expand All @@ -1467,9 +1477,7 @@ unsigned prefix_hash_key(const void *pp)
len = jhash((void *)copy.u.prefix_flowspec.ptr,
copy.u.prefix_flowspec.prefixlen,
0x55aa5a5a);
temp = (void *)copy.u.prefix_flowspec.ptr;
XFREE(MTYPE_PREFIX_FLOWSPEC, temp);
copy.u.prefix_flowspec.ptr = (uintptr_t)NULL;
prefix_flowspec_ptr_free(&copy);
return len;
} else if (((struct prefix *)pp)->family == AF_LINKSTATE) {
len = jhash((void *)copy.u.prefix_linkstate.ptr, copy.prefixlen,
Expand Down
1 change: 1 addition & 0 deletions lib/prefix.h
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ static inline afi_t prefix_afi(union prefixconstptr pu)
extern unsigned int prefix_bit(const uint8_t *prefix, const uint16_t bit_index);

extern void prefix_linkstate_ptr_free(struct prefix *p);
extern void prefix_flowspec_ptr_free(struct prefix *p);

extern struct prefix *prefix_new(void);
extern void prefix_free(struct prefix **p);
Expand Down
7 changes: 7 additions & 0 deletions lib/table.c
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,8 @@ struct route_node *route_node_get(struct route_table *table,
if (node && node->info) {
if (family2afi(p->family) == AFI_LINKSTATE)
prefix_linkstate_ptr_free(p);
else if (p->family == AF_FLOWSPEC)
prefix_flowspec_ptr_free(p);

return route_lock_node(node);
}
Expand All @@ -295,6 +297,9 @@ struct route_node *route_node_get(struct route_table *table,
if (node->p.prefixlen == prefixlen) {
if (family2afi(p->family) == AFI_LINKSTATE)
prefix_linkstate_ptr_free(p);
else if (p->family == AF_FLOWSPEC)
prefix_flowspec_ptr_free(p);

return route_lock_node(node);
}

Expand Down Expand Up @@ -333,6 +338,8 @@ struct route_node *route_node_get(struct route_table *table,

if (family2afi(p->family) == AFI_LINKSTATE)
prefix_linkstate_ptr_free(p);
else if (p->family == AF_FLOWSPEC)
prefix_flowspec_ptr_free(p);

return new;
}
Expand Down

0 comments on commit a7fe30e

Please sign in to comment.