matthieu/frr
1
0
Fork 0
forked from Mirror/frr
Code Pull requests Activity

bgpd: Pass the right reuse_list when handling it via bgp_reuse_timer thread

Browse source 
This fixes the crash:

```
==14759== Invalid read of size 8
==14759==    at 0x31032B: bgp_reuselist_del (bgp_damp.c:51)
==14759==    by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69)
==14759==    by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387)
==14759==    by 0x311016: bgp_reuse_timer (bgp_damp.c:230)
==14759==    by 0x4F227CC: thread_call (thread.c:2008)
==14759==    by 0x4EDB7D7: frr_run (libfrr.c:1216)
==14759==    by 0x1EF748: main (bgp_main.c:525)
==14759==  Address 0x48 is not stack'd, malloc'd or (recently) free'd
==14759==
==14759==
==14759== Process terminating with default action of signal 11 (SIGSEGV)
==14759==    at 0x59CC7F5: raise (raise.c:46)
==14759==    by 0x4F10CEB: core_handler (sigevent.c:261)
==14759==    by 0x59CC97F: ??? (in /lib/x86_64-linux-gnu/libpthread-2.27.so)
==14759==    by 0x31032A: bgp_reuselist_del (bgp_damp.c:51)
==14759==    by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69)
==14759==    by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387)
==14759==    by 0x311016: bgp_reuse_timer (bgp_damp.c:230)
==14759==    by 0x4F227CC: thread_call (thread.c:2008)
==14759==    by 0x4EDB7D7: frr_run (libfrr.c:1216)
==14759==    by 0x1EF748: main (bgp_main.c:525)
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
This commit is contained in:
Donatas Abraitis 2024-04-24 16:59:25 +03:00
parent f8e6b7ce45
commit 70ac630b39
3 changed files with 24 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
bgpd/bgp_damp.c
View file

@ -43,13 +43,16 @@ static void bgp_reuselist_switch(struct reuselist *source,
SLIST_INSERT_HEAD(target, info, entry); SLIST_INSERT_HEAD(target, info, entry);
} }
static void bgp_damp_info_unclaim(struct bgp_damp_info *bdi) static void bgp_damp_info_unclaim(struct bgp_damp_info *bdi,
struct reuselist *list)
{ {
assert(bdi && bdi->config); assert(bdi && bdi->config);
if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX) if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX)
bgp_reuselist_del(&bdi->config->no_reuse_list, bdi); bgp_reuselist_del(&bdi->config->no_reuse_list, bdi);
else else
bgp_reuselist_del(&bdi->config->reuse_list[bdi->index], bdi); bgp_reuselist_del(list ? list
: &bdi->config->reuse_list[bdi->index],
bdi);
bdi->config = NULL; bdi->config = NULL;
} }
@ -61,7 +64,7 @@ static void bgp_damp_info_claim(struct bgp_damp_info *bdi,
bdi->config = bdc; bdi->config = bdc;
return; return;
} }
bgp_damp_info_unclaim(bdi); bgp_damp_info_unclaim(bdi, NULL);
bdi->config = bdc; bdi->config = bdc;
bdi->afi = bdc->afi; bdi->afi = bdc->afi;
bdi->safi = bdc->safi; bdi->safi = bdc->safi;
@ -119,7 +122,7 @@ static void bgp_reuse_list_add(struct bgp_damp_info *bdi,
/* Delete BGP dampening information from reuse list. */ /* Delete BGP dampening information from reuse list. */
static void bgp_reuse_list_delete(struct bgp_damp_info *bdi) static void bgp_reuse_list_delete(struct bgp_damp_info *bdi)
{ {
bgp_damp_info_unclaim(bdi); bgp_damp_info_unclaim(bdi, NULL);
} }
static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi, static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi,
@ -132,7 +135,7 @@ static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi,
static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi) static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi)
{ {
bgp_damp_info_unclaim(bdi); bgp_damp_info_unclaim(bdi, NULL);
} }
/* Return decayed penalty value. */ /* Return decayed penalty value. */
@ -210,8 +213,7 @@ static void bgp_reuse_timer(struct event *t)
} }
if (bdi->penalty <= bdc->reuse_limit / 2.0) { if (bdi->penalty <= bdc->reuse_limit / 2.0) {
bgp_reuselist_del(&plist, bdi); bgp_damp_info_free(bdi, &plist, 1);
bgp_damp_info_free(bdi, 1);
} else { } else {
bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX; bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX;
bgp_reuselist_switch(&plist, bdi, bgp_reuselist_switch(&plist, bdi,
@ -357,22 +359,18 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest,
if (bdi->penalty > bdc->reuse_limit / 2.0) if (bdi->penalty > bdc->reuse_limit / 2.0)
bdi->t_updated = t_now; bdi->t_updated = t_now;
else { else
bgp_damp_info_unclaim(bdi); bgp_damp_info_free(bdi, NULL, 0);
bgp_damp_info_free(bdi, 0);
}
return status; return status;
} }
void bgp_damp_info_free(struct bgp_damp_info *bdi, int withdraw) void bgp_damp_info_free(struct bgp_damp_info *bdi, struct reuselist *list,
int withdraw)
{ {
assert(bdi); assert(bdi);
if (bdi->path == NULL) { bgp_damp_info_unclaim(bdi, list);
XFREE(MTYPE_BGP_DAMP_INFO, bdi);
return;
}
bdi->path->extra->damp_info = NULL; bdi->path->extra->damp_info = NULL;
bgp_path_info_unset_flag(bdi->dest, bdi->path, bgp_path_info_unset_flag(bdi->dest, bdi->path,
@ -495,15 +493,12 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc,
bgp_process(bgp, bdi->dest, bdi->path, bdi->afi, bgp_process(bgp, bdi->dest, bdi->path, bdi->afi,
bdi->safi); bdi->safi);
} }
bgp_reuselist_del(list, bdi); bgp_damp_info_free(bdi, list, 1);
bgp_damp_info_free(bdi, 1);
} }
} }
while ((bdi = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) { while ((bdi = SLIST_FIRST(&bdc->no_reuse_list)) != NULL)
bgp_reuselist_del(&bdc->no_reuse_list, bdi); bgp_damp_info_free(bdi, &bdc->no_reuse_list, 1);
bgp_damp_info_free(bdi, 1);
}
/* Free decay array */ /* Free decay array */
XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->decay_array); XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->decay_array);

3
bgpd/bgp_damp.h
View file

@ -130,7 +130,8 @@ extern int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest,
afi_t afi, safi_t safi, int attr_change); afi_t afi, safi_t safi, int attr_change);
extern int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, extern int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest,
afi_t afi, safi_t saff); afi_t afi, safi_t saff);
extern void bgp_damp_info_free(struct bgp_damp_info *bdi, int withdraw); extern void bgp_damp_info_free(struct bgp_damp_info *bdi,
struct reuselist *list, int withdraw);
extern void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, extern void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc,
afi_t afi, safi_t safi); afi_t afi, safi_t safi);
extern void bgp_damp_config_clean(struct bgp_damp_config *bdc); extern void bgp_damp_config_clean(struct bgp_damp_config *bdc);

12
bgpd/bgp_route.c
View file

@ -251,7 +251,7 @@ void bgp_path_info_extra_free(struct bgp_path_info_extra **extra)
e = *extra; e = *extra;
if (e->damp_info) if (e->damp_info)
bgp_damp_info_free(e->damp_info, 0); bgp_damp_info_free(e->damp_info, NULL, 0);
e->damp_info = NULL; e->damp_info = NULL;
if (e->vrfleak && e->vrfleak->parent) { if (e->vrfleak && e->vrfleak->parent) {
struct bgp_path_info *bpi = struct bgp_path_info *bpi =
@ -15827,9 +15827,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name,
while (pi) { while (pi) {
if (pi->extra && pi->extra->damp_info) { if (pi->extra && pi->extra->damp_info) {
pi_temp = pi->next; pi_temp = pi->next;
bgp_damp_info_free( bgp_damp_info_free(pi->extra->damp_info,
pi->extra->damp_info, NULL, 1);
1);
pi = pi_temp; pi = pi_temp;
} else } else
pi = pi->next; pi = pi->next;
@ -15866,9 +15865,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name,
bdi->afi, bdi->afi,
bdi->safi); bdi->safi);
} }
bgp_damp_info_free( bgp_damp_info_free(pi->extra->damp_info,
pi->extra->damp_info, NULL, 1);
1);
pi = pi_temp; pi = pi_temp;
} else } else
pi = pi->next; pi = pi->next;