From c668e70ec68d918b95d49159eb70c2e364a55c68 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 --- bgpd/bgp_aspath.h | 9 +++++++++ bgpd/bgp_filter.c | 18 ++++++++++++++++++ bgpd/bgp_filter.h | 9 +++++++++ bgpd/bgp_routemap.c | 30 ++++++++++++++++++++++-------- 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h index ebfc7d087d66..4c4bee9ec8d1 100644 --- a/bgpd/bgp_aspath.h +++ b/bgpd/bgp_aspath.h @@ -65,6 +65,15 @@ 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..2140d60e45f6 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) { + struct bp_as_excl_list *cur_bp = aslist->bp_list; + struct bp_as_excl_list *next_bp = NULL; XFREE(MTYPE_AS_STR, aslist->name); XFREE(MTYPE_AS_LIST, aslist); + + while(cur_bp) { + next_bp = cur_bp->next; + XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_bp); + cur_bp = next_bp; + } + } /* 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 bp_as_excl_list *cur_bp; for (filter = aslist->head; filter; filter = next) { next = filter->next; @@ -308,6 +318,14 @@ static void as_list_delete(struct as_list *aslist) else list->head = aslist->next; + cur_bp = aslist->bp_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..bfc2712e9889 100644 --- a/bgpd/bgp_filter.h +++ b/bgpd/bgp_filter.h @@ -25,6 +25,13 @@ struct as_filter { int64_t seq; }; + +struct bp_as_excl_list { + struct bp_as_excl_list *next; + struct aspath_exclude *bp_as_excl; +}; + + /* AS path filter list. */ struct as_list { char *name; @@ -34,6 +41,8 @@ struct as_list { struct as_filter *head; struct as_filter *tail; + /* back pointer to the aspath_excludes */ + struct bp_as_excl_list *bp_list; }; diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 36e04c5e681e..7c13de7c0ecc 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 bp_as_excl_list *bp; const char *str = arg; static const char asp_acl[] = "as-path-access-list"; @@ -2348,16 +2341,37 @@ 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) { + bp = XCALLOC(MTYPE_ROUTE_MAP_COMPILED, + sizeof(struct bp_as_excl_list)); + bp->bp_as_excl = ase; + bp->next = ase->exclude_aspath_acl->bp_list; + ase->exclude_aspath_acl->bp_list = bp; + } + + return ase; } static void route_aspath_exclude_free(void *rule) { struct aspath_exclude *ase = rule; + struct bp_as_excl_list *cur_bp = NULL; + struct bp_as_excl_list *prev_bp = 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_bp = ase->exclude_aspath_acl->bp_list; + while (cur_bp) { + if (cur_bp->bp_as_excl == ase) { + cur_bp = prev_bp; + XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_bp); + break; + } + } XFREE(MTYPE_ROUTE_MAP_COMPILED, ase); }