From ed0336b77579505c09a40b099277d3b62b936c14 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 22 Jun 2021 23:14:47 +0300 Subject: [PATCH] bgpd: Make sure we don't miss to unlock for bgp_dest before returning bgp_node_lookup() increases `lock` which is not decreased on return. Signed-off-by: Donatas Abraitis --- bgpd/bgp_evpn.c | 12 +++++++++--- bgpd/bgp_evpn_mh.c | 4 +++- bgpd/bgp_evpn_vty.c | 26 +++++++++++++++++++++++++- bgpd/bgp_label.c | 2 +- bgpd/bgp_snmp.c | 4 ++-- bgpd/bgp_updgrp_adv.c | 6 ++++-- bgpd/bgp_zebra.c | 6 ++++-- 7 files changed, 48 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index d8e57419ee..8c5829905e 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -2628,8 +2628,10 @@ static int uninstall_evpn_route_entry_in_vrf(struct bgp *bgp_vrf, && (struct bgp_path_info *)pi->extra->parent == parent_pi) break; - if (!pi) + if (!pi) { + bgp_dest_unlock_node(dest); return 0; + } if (bgp_debug_zebra(NULL)) zlog_debug("... delete dest %p (l %d) pi %p (l %d, f 0x%x)", @@ -2690,8 +2692,10 @@ static int uninstall_evpn_route_entry(struct bgp *bgp, struct bgpevpn *vpn, && (struct bgp_path_info *)pi->extra->parent == parent_pi) break; - if (!pi) + if (!pi) { + bgp_dest_unlock_node(dest); return 0; + } /* Mark entry for deletion */ bgp_path_info_delete(dest, pi); @@ -5308,8 +5312,10 @@ int bgp_evpn_local_macip_del(struct bgp *bgp, vni_t vni, struct ethaddr *mac, } else { /* Re-instate the current remote best path if any */ dest = bgp_node_lookup(vpn->route_table, (struct prefix *)&p); - if (dest) + if (dest) { evpn_zebra_reinstall_best_route(bgp, vpn, dest); + bgp_dest_unlock_node(dest); + } } return 0; diff --git a/bgpd/bgp_evpn_mh.c b/bgpd/bgp_evpn_mh.c index 59bced6f93..7a84e5e738 100644 --- a/bgpd/bgp_evpn_mh.c +++ b/bgpd/bgp_evpn_mh.c @@ -269,8 +269,10 @@ static int bgp_evpn_es_route_uninstall(struct bgp *bgp, struct bgp_evpn_es *es, parent_pi) break; - if (!pi) + if (!pi) { + bgp_dest_unlock_node(dest); return 0; + } /* Mark entry for deletion */ bgp_path_info_delete(dest, pi); diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c index ed8a6a9506..3fc1dc1285 100644 --- a/bgpd/bgp_evpn_vty.c +++ b/bgpd/bgp_evpn_vty.c @@ -2418,6 +2418,10 @@ static void evpn_show_route_vni_multicast(struct vty *vty, struct bgp *bgp, if (!dest || !bgp_dest_has_bgp_path_info_data(dest)) { if (!json) vty_out(vty, "%% Network not in table\n"); + + if (dest) + bgp_dest_unlock_node(dest); + return; } @@ -2452,6 +2456,8 @@ static void evpn_show_route_vni_multicast(struct vty *vty, struct bgp *bgp, vty_out(vty, "\nDisplayed %u paths for requested prefix\n", path_cnt); } + + bgp_dest_unlock_node(dest); } /* @@ -2488,6 +2494,10 @@ static void evpn_show_route_vni_macip(struct vty *vty, struct bgp *bgp, if (!dest || !bgp_dest_has_bgp_path_info_data(dest)) { if (!json) vty_out(vty, "%% Network not in table\n"); + + if (dest) + bgp_dest_unlock_node(dest); + return; } @@ -2522,6 +2532,8 @@ static void evpn_show_route_vni_macip(struct vty *vty, struct bgp *bgp, vty_out(vty, "\nDisplayed %u paths for requested prefix\n", path_cnt); } + + bgp_dest_unlock_node(dest); } /* Disaplay EVPN routes for a ESI - VTY handler */ @@ -2592,6 +2604,10 @@ static void evpn_show_route_rd_macip(struct vty *vty, struct bgp *bgp, if (!dest || !bgp_dest_has_bgp_path_info_data(dest)) { if (!json) vty_out(vty, "%% Network not in table\n"); + + if (dest) + bgp_dest_unlock_node(dest); + return; } @@ -2627,6 +2643,8 @@ static void evpn_show_route_rd_macip(struct vty *vty, struct bgp *bgp, vty_out(vty, "\nDisplayed %u paths for requested prefix\n", path_cnt); } + + bgp_dest_unlock_node(dest); } /* @@ -2660,14 +2678,18 @@ static void evpn_show_route_rd(struct vty *vty, struct bgp *bgp, return; table = bgp_dest_get_bgp_table_info(rd_dest); - if (table == NULL) + if (table == NULL) { + bgp_dest_unlock_node(rd_dest); return; + } if (json) { json_rd = json_object_new_object(); json_object_string_add(json_rd, "rd", rd_str); } + bgp_dest_unlock_node(rd_dest); + /* Display all prefixes with this RD. */ for (dest = bgp_table_top(table); dest; dest = bgp_route_next(dest)) { const struct prefix_evpn *evp = @@ -2878,6 +2900,8 @@ static void evpn_show_route_rd_all_macip(struct vty *vty, struct bgp *bgp, json_rd = NULL; } } + + bgp_dest_unlock_node(dest); } if (json) { diff --git a/bgpd/bgp_label.c b/bgpd/bgp_label.c index bdab2ec36a..73ca9f07e0 100644 --- a/bgpd/bgp_label.c +++ b/bgpd/bgp_label.c @@ -89,8 +89,8 @@ int bgp_parse_fec_update(void) bgp_set_valid_label(&dest->local_label); } SET_FLAG(dest->flags, BGP_NODE_LABEL_CHANGED); - bgp_dest_unlock_node(dest); bgp_process(bgp, dest, afi, safi); + bgp_dest_unlock_node(dest); return 1; } diff --git a/bgpd/bgp_snmp.c b/bgpd/bgp_snmp.c index 61a6467ab6..4baa730c8d 100644 --- a/bgpd/bgp_snmp.c +++ b/bgpd/bgp_snmp.c @@ -689,12 +689,12 @@ static struct bgp_path_info *bgp4PathAttrLookup(struct variable *v, oid name[], dest = bgp_node_lookup(bgp->rib[AFI_IP][SAFI_UNICAST], (struct prefix *)addr); if (dest) { - bgp_dest_unlock_node(dest); - for (path = bgp_dest_get_bgp_path_info(dest); path; path = path->next) if (sockunion_same(&path->peer->su, &su)) return path; + + bgp_dest_unlock_node(dest); } } else { offset = name + v->namelen; diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index bb0c95e32f..af37130b72 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -883,7 +883,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) /* If default route is present in the local RIB, advertise the * route */ - if (dest != NULL) { + if (dest) { for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) { if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) @@ -895,6 +895,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) dest, subgrp, &attr, pi); } + bgp_dest_unlock_node(dest); } } else { if (!CHECK_FLAG(subgrp->sflags, @@ -907,7 +908,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) * clear adj_out for the 0.0.0.0/0 prefix in the BGP * table. */ - if (dest != NULL) { + if (dest) { /* Remove the adjacency for the previously * advertised default route */ @@ -923,6 +924,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) /* Free allocated information. */ adj_free(adj); } + bgp_dest_unlock_node(dest); } /* Advertise the default route */ diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 2bf57130be..49f57b5c03 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -2409,8 +2409,6 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient, if (!dest) return -1; - bgp_dest_unlock_node(dest); - switch (note) { case ZAPI_ROUTE_INSTALLED: new_select = NULL; @@ -2439,6 +2437,8 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient, flog_err(EC_BGP_INVALID_ROUTE, "selected route %pRN not found", dest); + + bgp_dest_unlock_node(dest); return -1; } } @@ -2469,6 +2469,8 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient, __func__, dest); break; } + + bgp_dest_unlock_node(dest); return 0; } -- 2.39.5