diff --git a/bgpd/bgp_flowspec.c b/bgpd/bgp_flowspec.c index 6695596c6f..e29508bf36 100644 --- a/bgpd/bgp_flowspec.c +++ b/bgpd/bgp_flowspec.c @@ -148,7 +148,7 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr, if (BGP_DEBUG(flowspec, FLOWSPEC)) { char return_string[BGP_FLOWSPEC_NLRI_STRING_MAX]; - char local_string[BGP_FLOWSPEC_NLRI_STRING_MAX * 2]; + char local_string[BGP_FLOWSPEC_NLRI_STRING_MAX*2+16]; char ec_string[BGP_FLOWSPEC_NLRI_STRING_MAX]; char *s = NULL; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index eedfe7503e..343f094b20 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -188,8 +188,24 @@ static void bgp_info_extra_free(struct bgp_info_extra **extra) if (e->parent) { struct bgp_info *bi = (struct bgp_info *)e->parent; - if (bi->net) - bi->net = bgp_unlock_node((struct bgp_node *)bi->net); + if (bi->net) { + /* FIXME: since multiple e may have the same e->parent + * and e->parent->net is holding a refcount for each + * of them, we need to do some fudging here. + * + * WARNING: if bi->net->lock drops to 0, bi may be + * freed as well (because bi->net was holding the + * last reference to bi) => write after free! + */ + unsigned refcount; + + bi = bgp_info_lock(bi); + refcount = bi->net->lock - 1; + bgp_unlock_node((struct bgp_node *)bi->net); + if (!refcount) + bi->net = NULL; + bgp_info_unlock(bi); + } bgp_info_unlock(e->parent); e->parent = NULL; } diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h index f7eac09546..60c2cbd4a4 100644 --- a/bgpd/bgp_table.h +++ b/bgpd/bgp_table.h @@ -128,9 +128,9 @@ static inline struct bgp_node *bgp_node_parent_nolock(struct bgp_node *node) /* * bgp_unlock_node */ -static inline struct bgp_node *bgp_unlock_node(struct bgp_node *node) +static inline void bgp_unlock_node(struct bgp_node *node) { - return (struct bgp_node *)route_unlock_node(bgp_node_to_rnode(node)); + route_unlock_node(bgp_node_to_rnode(node)); } /* diff --git a/lib/compiler.h b/lib/compiler.h index b19c33f65e..24b8fafd10 100644 --- a/lib/compiler.h +++ b/lib/compiler.h @@ -24,6 +24,9 @@ #if __clang_major__ > 3 || (__clang_major__ == 3 && __clang_minor__ >= 5) # define _RET_NONNULL , returns_nonnull #endif +#if __has_attribute(fallthrough) +# define _FALLTHROUGH __attribute__((fallthrough)); +#endif # define _CONSTRUCTOR(x) constructor(x) #elif defined(__GNUC__) #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9) @@ -34,6 +37,9 @@ # define _DESTRUCTOR(x) destructor(x) # define _ALLOC_SIZE(x) alloc_size(x) #endif +#if __GNUC__ >= 7 +# define _FALLTHROUGH __attribute__((fallthrough)); +#endif #endif #ifdef __sun @@ -55,6 +61,9 @@ #ifndef _ALLOC_SIZE # define _ALLOC_SIZE(x) #endif +#ifndef _FALLTHROUGH +#define _FALLTHROUGH +#endif /* * for warnings on macros, put in the macro content like this: diff --git a/lib/table.h b/lib/table.h index ac7df3e695..8304abe59b 100644 --- a/lib/table.h +++ b/lib/table.h @@ -235,17 +235,13 @@ static inline struct route_node *route_lock_node(struct route_node *node) } /* Unlock node. */ -static inline struct route_node *route_unlock_node(struct route_node *node) +static inline void route_unlock_node(struct route_node *node) { assert(node->lock > 0); (*(unsigned *)&node->lock)--; - if (node->lock == 0) { + if (node->lock == 0) route_node_delete(node); - return NULL; - } - - return node; } /* diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index 7c45ce261a..611d8d3681 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -323,8 +323,8 @@ static void pim_show_assert_winner_metric_helper(struct vty *vty, char addr_str[INET_ADDRSTRLEN]; struct pim_assert_metric *am; struct in_addr ifaddr; - char pref_str[5]; - char metr_str[7]; + char pref_str[16]; + char metr_str[16]; ifaddr = pim_ifp->primary_address; diff --git a/pimd/pim_oil.c b/pimd/pim_oil.c index f0f336fb73..a0debc0c78 100644 --- a/pimd/pim_oil.c +++ b/pimd/pim_oil.c @@ -37,19 +37,20 @@ char *pim_channel_oil_dump(struct channel_oil *c_oil, char *buf, size_t size) { + char *out; struct prefix_sg sg; int i; sg.src = c_oil->oil.mfcc_origin; sg.grp = c_oil->oil.mfcc_mcastgrp; - sprintf(buf, "%s IIF: %d, OIFS: ", pim_str_sg_dump(&sg), - c_oil->oil.mfcc_parent); + snprintf(buf, size, "%s IIF: %d, OIFS: ", pim_str_sg_dump(&sg), + c_oil->oil.mfcc_parent); + out = buf + strlen(buf); for (i = 0; i < MAXVIFS; i++) { if (c_oil->oil.mfcc_ttls[i] != 0) { - char buf1[10]; - sprintf(buf1, "%d ", i); - strcat(buf, buf1); + snprintf(out, buf + size - out, "%d ", i); + out += strlen(out); } } diff --git a/tests/bgpd/test_capability.c b/tests/bgpd/test_capability.c index 4612bdc26b..fef7d39ff5 100644 --- a/tests/bgpd/test_capability.c +++ b/tests/bgpd/test_capability.c @@ -821,7 +821,7 @@ static void parse_test(struct peer *peer, struct test_segment *t, int type) switch (type) { case CAPABILITY: len += 2; /* to cover the OPT-Param header */ - __attribute__ ((fallthrough)); + _FALLTHROUGH case OPT_PARAM: printf("len: %u\n", len); /* peek_for_as4 wants getp at capibility*/