From 1d224e5a4c9168dfe7fbb65f834693664209be3a Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Fri, 16 Feb 2024 15:31:14 +0100 Subject: [PATCH] bgpd: fix no bgp as-path access-list issue router bgp 65001 no bgp ebgp-requires-policy neighbor 192.168.1.2 remote-as external neighbor 192.168.1.2 timers 3 10 address-family ipv4 unicast neighbor 192.168.1.2 route-map r2 in exit-address-family ! ip prefix-list p1 seq 5 permit 172.16.255.31/32 ! route-map r2 permit 10 match ip address prefix-list p1 set as-path exclude 65003 route-map r2 permit 20 set as-path exclude all ! we make the following commands bgp as-path access-list FIRST permit ^65 bgp as-path access-list SECOND permit 2 route-map r2 permit 6 set as-path exclude as-path-access-list SECOND and then no bgp as-path access-list SECOND permit 2 clear bgp * we have the following crash in bgp Stack trace of thread 536083: #0 0x00007f87f8aacfe1 raise (libpthread.so.0 + 0x12fe1) #1 0x00007f87f8cf6870 core_handler (libfrr.so.0 + 0xf6870) #2 0x00007f87f8aad140 __restore_rt (libpthread.so.0 + 0x13140) #3 0x00007f87f89a5122 __GI___regexec (libc.so.6 + 0xdf122) #4 0x000055d7f198b4a7 aspath_filter_exclude_acl (bgpd + 0x2054a7) #5 0x000055d7f1902187 route_set_aspath_exclude (bgpd + 0x17c187) #6 0x00007f87f8ce54b0 route_map_apply_ext (libfrr.so.0 + 0xe54b0) #7 0x000055d7f18da925 bgp_input_modifier (bgpd + 0x154925) #8 0x000055d7f18e0647 bgp_update (bgpd + 0x15a647) #9 0x000055d7f18e4772 bgp_nlri_parse_ip (bgpd + 0x15e772) #10 0x000055d7f18c38ae bgp_nlri_parse (bgpd + 0x13d8ae) #11 0x000055d7f18c6b7a bgp_update_receive (bgpd + 0x140b7a) #12 0x000055d7f18c8ff3 bgp_process_packet (bgpd + 0x142ff3) #13 0x00007f87f8d0dce0 thread_call (libfrr.so.0 + 0x10dce0) #14 0x00007f87f8cacb28 frr_run (libfrr.so.0 + 0xacb28) #15 0x000055d7f18435da main (bgpd + 0xbd5da) #16 0x00007f87f88e9d0a __libc_start_main (libc.so.6 + 0x23d0a) #17 0x000055d7f18415fa _start (bgpd + 0xbb5fa) analysis crash is due to the fact that there were always a pointer from as-path exclude to deleted as-path access list. fix we add a backpointer mechanism to manage the dependency beetween as-path access-list and aspath exclude. Signed-off-by: Francois Dumontet (cherry picked from commit 100ef15262316b34ec46220806dc816db729870f) --- bgpd/bgp_aspath.h | 8 ++++++++ bgpd/bgp_filter.c | 20 ++++++++++++++++++-- bgpd/bgp_filter.h | 6 ++++++ bgpd/bgp_routemap.c | 34 ++++++++++++++++++++++++++-------- 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h index ebfc7d087d66..2a831c3a5510 100644 --- a/bgpd/bgp_aspath.h +++ b/bgpd/bgp_aspath.h @@ -65,6 +65,14 @@ struct aspath { #define ASPATH_STR_DEFAULT_LEN 32 +/* `set as-path exclude ASn' */ +struct aspath_exclude { + struct aspath *aspath; + bool exclude_all; + char *exclude_aspath_acl_name; + struct as_list *exclude_aspath_acl; +}; + /* Prototypes. */ extern void aspath_init(void); extern void aspath_finish(void); diff --git a/bgpd/bgp_filter.c b/bgpd/bgp_filter.c index ad541b67ad44..a85117965aaa 100644 --- a/bgpd/bgp_filter.c +++ b/bgpd/bgp_filter.c @@ -205,8 +205,17 @@ static struct as_list *as_list_new(void) static void as_list_free(struct as_list *aslist) { - XFREE(MTYPE_AS_STR, aslist->name); - XFREE(MTYPE_AS_LIST, aslist); + struct aspath_exclude_list *cur_bp = aslist->exclude_list; + struct aspath_exclude_list *next_bp = NULL; + + while (cur_bp) { + next_bp = cur_bp->next; + XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_bp); + cur_bp = next_bp; + } + + XFREE (MTYPE_AS_STR, aslist->name); + XFREE (MTYPE_AS_LIST, aslist); } /* Insert new AS list to list of as_list. Each as_list is sorted by @@ -290,6 +299,7 @@ static void as_list_delete(struct as_list *aslist) { struct as_list_list *list; struct as_filter *filter, *next; + struct aspath_exclude_list *cur_bp; for (filter = aslist->head; filter; filter = next) { next = filter->next; @@ -308,6 +318,12 @@ static void as_list_delete(struct as_list *aslist) else list->head = aslist->next; + cur_bp = aslist->exclude_list; + while (cur_bp) { + cur_bp->bp_as_excl->exclude_aspath_acl = NULL; + cur_bp = cur_bp->next; + } + as_list_free(aslist); } diff --git a/bgpd/bgp_filter.h b/bgpd/bgp_filter.h index 1890fd3d9664..2d9f07ce84a3 100644 --- a/bgpd/bgp_filter.h +++ b/bgpd/bgp_filter.h @@ -25,6 +25,11 @@ struct as_filter { int64_t seq; }; +struct aspath_exclude_list { + struct aspath_exclude_list *next; + struct aspath_exclude *bp_as_excl; +}; + /* AS path filter list. */ struct as_list { char *name; @@ -34,6 +39,7 @@ struct as_list { struct as_filter *head; struct as_filter *tail; + struct aspath_exclude_list *exclude_list; }; diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index e0db43f676ad..97447f3acd4d 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -2323,17 +2323,10 @@ static const struct route_map_rule_cmd route_set_aspath_prepend_cmd = { route_set_aspath_prepend_free, }; -/* `set as-path exclude ASn' */ -struct aspath_exclude { - struct aspath *aspath; - bool exclude_all; - char *exclude_aspath_acl_name; - struct as_list *exclude_aspath_acl; -}; - static void *route_aspath_exclude_compile(const char *arg) { struct aspath_exclude *ase; + struct aspath_exclude_list *ael; const char *str = arg; static const char asp_acl[] = "as-path-access-list"; @@ -2348,16 +2341,41 @@ static void *route_aspath_exclude_compile(const char *arg) ase->exclude_aspath_acl = as_list_lookup(str); } else ase->aspath = aspath_str2aspath(str, bgp_get_asnotation(NULL)); + + if (ase->exclude_aspath_acl) { + ael = XCALLOC(MTYPE_ROUTE_MAP_COMPILED, + sizeof(struct aspath_exclude_list)); + ael->bp_as_excl = ase; + ael->next = ase->exclude_aspath_acl->exclude_list; + ase->exclude_aspath_acl->exclude_list = ael; + } + return ase; } static void route_aspath_exclude_free(void *rule) { struct aspath_exclude *ase = rule; + struct aspath_exclude_list *cur_ael = NULL; + struct aspath_exclude_list *prev_ael = NULL; aspath_free(ase->aspath); if (ase->exclude_aspath_acl_name) XFREE(MTYPE_TMP, ase->exclude_aspath_acl_name); + if (ase->exclude_aspath_acl) + cur_ael = ase->exclude_aspath_acl->exclude_list; + while (cur_ael) { + if (cur_ael->bp_as_excl == ase) { + if (prev_ael) + prev_ael->next = cur_ael->next; + else + ase->exclude_aspath_acl->exclude_list = NULL; + XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_ael); + break; + } + prev_ael = cur_ael; + cur_ael = cur_ael->next; + } XFREE(MTYPE_ROUTE_MAP_COMPILED, ase); }