From a152692f5ac207b5f55104b18ddc3146089db474 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Mon, 26 Aug 2024 10:23:12 +0200 Subject: [PATCH] bgpd: fix labels static-analyser Fix static-analyser warnings with BGP labels: > $ scan-build make -j12 > bgpd/bgp_updgrp_packet.c:819:10: warning: Access to field 'extra' results in a dereference of a null pointer (loaded from variable 'path') [core.NullDereference] > ? &path->extra->labels->label[0] > ^~~~~~~~~ Signed-off-by: Louis Scalbert --- bgpd/bgp_bmp.c | 6 +++--- bgpd/bgp_evpn.c | 2 +- bgpd/bgp_label.c | 2 +- bgpd/bgp_mac.c | 2 +- bgpd/bgp_mplsvpn.c | 4 ++-- bgpd/bgp_nht.c | 4 ++-- bgpd/bgp_route.c | 24 +++++------------------- bgpd/bgp_route.h | 6 +++++- bgpd/bgp_routemap.c | 2 +- bgpd/bgp_rpki.c | 2 +- bgpd/bgp_updgrp_packet.c | 2 +- bgpd/bgp_zebra.c | 4 ++-- bgpd/rfapi/rfapi_import.c | 4 ++-- bgpd/rfapi/rfapi_rib.c | 2 +- bgpd/rfapi/rfapi_vty.c | 6 +++--- bgpd/rfapi/vnc_import_bgp.c | 18 +++++++++--------- 16 files changed, 40 insertions(+), 50 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 43f8006e2d4b..675e4765e406 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1221,7 +1221,7 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) (safi == SAFI_MPLS_VPN)) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); - bpi_num_labels = bgp_path_info_num_labels(bpi); + bpi_num_labels = BGP_PATH_INFO_NUM_LABELS(bpi); if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED) && CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { @@ -1356,7 +1356,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) break; } - bpi_num_labels = bgp_path_info_num_labels(bpi); + bpi_num_labels = BGP_PATH_INFO_NUM_LABELS(bpi); bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, @@ -1434,7 +1434,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) break; } - bpi_num_labels = bgp_path_info_num_labels(bpi); + bpi_num_labels = BGP_PATH_INFO_NUM_LABELS(bpi); bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd, diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index ef7b13f323d7..cb5c8983151b 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -3013,7 +3013,7 @@ bgp_create_evpn_bgp_path_info(struct bgp_path_info *parent_pi, if (parent_pi->extra) pi->extra->igpmetric = parent_pi->extra->igpmetric; - if (bgp_path_info_num_labels(parent_pi)) + if (BGP_PATH_INFO_NUM_LABELS(parent_pi)) pi->extra->labels = bgp_labels_intern(parent_pi->extra->labels); bgp_path_info_add(dest, pi); diff --git a/bgpd/bgp_label.c b/bgpd/bgp_label.c index ad40fef62c61..af10111600cc 100644 --- a/bgpd/bgp_label.c +++ b/bgpd/bgp_label.c @@ -208,7 +208,7 @@ mpls_label_t bgp_adv_label(struct bgp_dest *dest, struct bgp_path_info *pi, if (!dest || !pi || !to) return MPLS_INVALID_LABEL; - remote_label = bgp_path_info_num_labels(pi) + remote_label = BGP_PATH_INFO_NUM_LABELS(pi) ? pi->extra->labels->label[0] : MPLS_INVALID_LABEL; from = pi->peer; diff --git a/bgpd/bgp_mac.c b/bgpd/bgp_mac.c index bc44a212cdc9..86d6281ed157 100644 --- a/bgpd/bgp_mac.c +++ b/bgpd/bgp_mac.c @@ -169,7 +169,7 @@ static void bgp_process_mac_rescan_table(struct bgp *bgp, struct peer *peer, && !dest_affected) continue; - num_labels = bgp_path_info_num_labels(pi); + num_labels = BGP_PATH_INFO_NUM_LABELS(pi); label_pnt = num_labels ? &pi->extra->labels->label[0] : NULL; diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 9de1c5f4c259..f7ca51e14638 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -2322,7 +2322,7 @@ static void vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ } num_labels = origin_local ? 0 - : bgp_path_info_num_labels(path_vpn); + : BGP_PATH_INFO_NUM_LABELS(path_vpn); label_pnt = num_labels ? path_vpn->extra->labels->label : NULL; } @@ -4212,7 +4212,7 @@ void bgp_mplsvpn_nh_label_bind_register_local_label(struct bgp *bgp, struct bgp_mplsvpn_nh_label_bind_cache_head *tree; mpls_label_t label; - label = bgp_path_info_num_labels(pi) + label = BGP_PATH_INFO_NUM_LABELS(pi) ? decode_label(&pi->extra->labels->label[0]) : MPLS_INVALID_LABEL; diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index f75df1e12dc0..0259da06dd48 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -505,7 +505,7 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, return 1; else if (safi == SAFI_UNICAST && pi && pi->sub_type == BGP_ROUTE_IMPORTED && - bgp_path_info_num_labels(pi) && !bnc->is_evpn_gwip_nexthop) + BGP_PATH_INFO_NUM_LABELS(pi) && !bnc->is_evpn_gwip_nexthop) return bgp_isvalid_nexthop_for_l3vpn(bnc, pi); else if (safi == SAFI_MPLS_VPN && pi && pi->sub_type != BGP_ROUTE_IMPORTED) @@ -1310,7 +1310,7 @@ void evaluate_paths(struct bgp_nexthop_cache *bnc) if (safi == SAFI_UNICAST && path->sub_type == BGP_ROUTE_IMPORTED && - bgp_path_info_num_labels(path) && + BGP_PATH_INFO_NUM_LABELS(path) && !(bre && bre->type == OVERLAY_INDEX_GATEWAY_IP)) { bnc_is_valid_nexthop = bgp_isvalid_nexthop_for_l3vpn(bnc, path) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index e76329fca6fc..49f3c77ada5d 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -327,7 +327,7 @@ struct bgp_path_info_extra *bgp_path_info_extra_get(struct bgp_path_info *pi) bool bgp_path_info_has_valid_label(const struct bgp_path_info *path) { - if (!bgp_path_info_num_labels(path)) + if (!BGP_PATH_INFO_NUM_LABELS(path)) return false; return bgp_is_valid_label(&path->extra->labels->label[0]); @@ -339,27 +339,13 @@ bool bgp_path_info_labels_same(const struct bgp_path_info *bpi, uint8_t bpi_num_labels; const mpls_label_t *bpi_label; - bpi_num_labels = bgp_path_info_num_labels(bpi); + bpi_num_labels = BGP_PATH_INFO_NUM_LABELS(bpi); bpi_label = bpi_num_labels ? bpi->extra->labels->label : NULL; return bgp_labels_same(bpi_label, bpi_num_labels, (const mpls_label_t *)label, n); } -uint8_t bgp_path_info_num_labels(const struct bgp_path_info *pi) -{ - if (!pi) - return 0; - - if (!pi->extra) - return 0; - - if (!pi->extra->labels) - return 0; - - return pi->extra->labels->num_labels; -} - /* Free bgp route information. */ void bgp_path_info_free_with_caller(const char *name, struct bgp_path_info *path) @@ -2252,7 +2238,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, * off box as that the RT and RD created are localy * significant and globaly useless. */ - if (safi == SAFI_MPLS_VPN && bgp_path_info_num_labels(pi) && + if (safi == SAFI_MPLS_VPN && BGP_PATH_INFO_NUM_LABELS(pi) && pi->extra->labels->label[0] == BGP_PREVENT_VRF_2_VRF_LEAK) return false; @@ -6861,7 +6847,7 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p, bgp, p, pi); } } else { - if (bgp_path_info_num_labels(pi)) + if (BGP_PATH_INFO_NUM_LABELS(pi)) label = decode_label( &pi->extra->labels->label[0]); } @@ -10540,7 +10526,7 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn, json_nexthop_global = json_object_new_object(); } - if (bgp_path_info_num_labels(path)) { + if (BGP_PATH_INFO_NUM_LABELS(path)) { bgp_evpn_label2str(path->extra->labels->label, path->extra->labels->num_labels, vni_buf, sizeof(vni_buf)); diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 5b433b555813..b1c835630196 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -561,6 +561,11 @@ struct bgp_aggregate { /* path PREFIX (addpath rxid NUMBER) */ #define PATH_ADDPATH_STR_BUFFER PREFIX2STR_BUFFER + 32 +#define BGP_PATH_INFO_NUM_LABELS(pi) \ + ((pi) && (pi)->extra && (pi)->extra->labels \ + ? (pi)->extra->labels->num_labels \ + : 0) + enum bgp_path_type { BGP_PATH_SHOW_ALL, BGP_PATH_SHOW_BESTPATH, @@ -748,7 +753,6 @@ extern void bgp_path_info_delete(struct bgp_dest *dest, extern struct bgp_path_info_extra * bgp_path_info_extra_get(struct bgp_path_info *path); extern bool bgp_path_info_has_valid_label(const struct bgp_path_info *path); -extern uint8_t bgp_path_info_num_labels(const struct bgp_path_info *pi); extern void bgp_path_info_set_flag(struct bgp_dest *dest, struct bgp_path_info *path, uint32_t flag); extern void bgp_path_info_unset_flag(struct bgp_dest *dest, diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 79c61e2ee293..d0b65a7426d5 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -1081,7 +1081,7 @@ route_match_vni(void *rule, const struct prefix *prefix, void *object) return RMAP_NOOP; for (label_cnt = 0; label_cnt < BGP_MAX_LABELS && - label_cnt < bgp_path_info_num_labels(path); + label_cnt < BGP_PATH_INFO_NUM_LABELS(path); label_cnt++) { if (vni == label2vni(&path->extra->labels->label[label_cnt])) return RMAP_MATCH; diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index 9db7b1529614..f9cbf240312e 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -666,7 +666,7 @@ static void revalidate_bgp_node(struct bgp_dest *bgp_dest, afi_t afi, struct bgp_path_info *path = bgp_dest_get_bgp_path_info(bgp_dest); - num_labels = bgp_path_info_num_labels(path); + num_labels = BGP_PATH_INFO_NUM_LABELS(path); label = num_labels ? path->extra->labels->label : NULL; (void)bgp_update(ain->peer, bgp_dest_get_prefix(bgp_dest), diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c index 8cd851b9ac5c..bed00a66402f 100644 --- a/bgpd/bgp_updgrp_packet.c +++ b/bgpd/bgp_updgrp_packet.c @@ -813,7 +813,7 @@ struct bpacket *subgroup_update_packet(struct update_subgroup *subgrp) label_pnt = &label; num_labels = 1; } else { - num_labels = bgp_path_info_num_labels(path); + num_labels = BGP_PATH_INFO_NUM_LABELS(path); label_pnt = num_labels ? &path->extra->labels->label[0] diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 2d9100b89517..6e2efabf8f40 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1311,7 +1311,7 @@ static void bgp_zebra_announce_parse_nexthop( api_nh->srte_color = bgp_attr_get_color(info->attr); if (bgp_debug_zebra(&api->prefix)) { - if (bgp_path_info_num_labels(mpinfo)) { + if (BGP_PATH_INFO_NUM_LABELS(mpinfo)) { zlog_debug("%s: p=%pFX, bgp_is_valid_label: %d", __func__, p, bgp_is_valid_label( @@ -1385,7 +1385,7 @@ static void bgp_zebra_announce_parse_nexthop( mpinfo->peer->sort == BGP_PEER_CONFED)) *allow_recursion = true; - num_labels = bgp_path_info_num_labels(mpinfo); + num_labels = BGP_PATH_INFO_NUM_LABELS(mpinfo); labels = num_labels ? mpinfo->extra->labels->label : NULL; if (num_labels && (is_evpn || bgp_is_valid_label(&labels[0]))) { diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c index 2afcb2f45c8c..44dfc88cf765 100644 --- a/bgpd/rfapi/rfapi_import.c +++ b/bgpd/rfapi/rfapi_import.c @@ -1272,7 +1272,7 @@ rfapiRouteInfo2NextHopEntry(struct rfapi_ip_prefix *rprefix, /* label comes from MP_REACH_NLRI label */ vo->v.l2addr.label = - bgp_path_info_num_labels(bpi) + BGP_PATH_INFO_NUM_LABELS(bpi) ? decode_label(&bpi->extra->labels->label[0]) : MPLS_INVALID_LABEL; @@ -4167,7 +4167,7 @@ static void rfapiBgpTableFilteredImport(struct bgp *bgp, BGP_PATH_REMOVED)) continue; - if (bgp_path_info_num_labels(bpi)) + if (BGP_PATH_INFO_NUM_LABELS(bpi)) label = decode_label( &bpi->extra->labels ->label[0]); diff --git a/bgpd/rfapi/rfapi_rib.c b/bgpd/rfapi/rfapi_rib.c index a0bdf4961f23..53e416b2eef1 100644 --- a/bgpd/rfapi/rfapi_rib.c +++ b/bgpd/rfapi/rfapi_rib.c @@ -692,7 +692,7 @@ static void rfapiRibBi2Ri(struct bgp_path_info *bpi, struct rfapi_info *ri, /* label comes from MP_REACH_NLRI label */ vo->v.l2addr.label = - bgp_path_info_num_labels(bpi) + BGP_PATH_INFO_NUM_LABELS(bpi) ? decode_label(&bpi->extra->labels->label[0]) : MPLS_INVALID_LABEL; diff --git a/bgpd/rfapi/rfapi_vty.c b/bgpd/rfapi/rfapi_vty.c index 9bfb6c4b45e6..8f1f509bbbe6 100644 --- a/bgpd/rfapi/rfapi_vty.c +++ b/bgpd/rfapi/rfapi_vty.c @@ -413,7 +413,7 @@ void rfapi_vty_out_vncinfo(struct vty *vty, const struct prefix *p, XFREE(MTYPE_ECOMMUNITY_STR, s); } - if (bgp_path_info_num_labels(bpi)) { + if (BGP_PATH_INFO_NUM_LABELS(bpi)) { if (bpi->extra->labels->label[0] == BGP_PREVENT_VRF_2_VRF_LEAK) vty_out(vty, " label=VRF2VRF"); else @@ -1052,7 +1052,7 @@ static int rfapiPrintRemoteRegBi(struct bgp *bgp, void *stream, snprintf(buf_un, sizeof(buf_un), "%s", inet_ntop(pfx_vn.family, &pfx_vn.u.prefix, buf_ntop, sizeof(buf_ntop))); - if (bgp_path_info_num_labels(bpi)) { + if (BGP_PATH_INFO_NUM_LABELS(bpi)) { uint32_t l = decode_label(&bpi->extra->labels->label[0]); snprintf(buf_vn, sizeof(buf_vn), "Label: %d", l); } else /* should never happen */ @@ -1161,7 +1161,7 @@ static int rfapiPrintRemoteRegBi(struct bgp *bgp, void *stream, } } } - if (tun_type != BGP_ENCAP_TYPE_MPLS && bgp_path_info_num_labels(bpi)) { + if (tun_type != BGP_ENCAP_TYPE_MPLS && BGP_PATH_INFO_NUM_LABELS(bpi)) { uint32_t l = decode_label(&bpi->extra->labels->label[0]); if (!MPLS_LABEL_IS_NULL(l)) { diff --git a/bgpd/rfapi/vnc_import_bgp.c b/bgpd/rfapi/vnc_import_bgp.c index 2bb7c1b161f5..e688638ed70c 100644 --- a/bgpd/rfapi/vnc_import_bgp.c +++ b/bgpd/rfapi/vnc_import_bgp.c @@ -470,7 +470,7 @@ static void vnc_import_bgp_add_route_mode_resolve_nve_one_bi( if (bgp_attr_get_ecommunity(bpi->attr)) ecommunity_merge(new_ecom, bgp_attr_get_ecommunity(bpi->attr)); - if (bgp_path_info_num_labels(bpi)) + if (BGP_PATH_INFO_NUM_LABELS(bpi)) label = decode_label(&bpi->extra->labels->label[0]); else label = MPLS_INVALID_LABEL; @@ -1704,7 +1704,7 @@ static void vnc_import_bgp_exterior_add_route_it( else prd = NULL; - if (bgp_path_info_num_labels(bpi_interior)) + if (BGP_PATH_INFO_NUM_LABELS(bpi_interior)) label = decode_label( &bpi_interior->extra->labels ->label[0]); @@ -1878,7 +1878,7 @@ void vnc_import_bgp_exterior_del_route( else prd = NULL; - if (bgp_path_info_num_labels(bpi_interior)) + if (BGP_PATH_INFO_NUM_LABELS(bpi_interior)) label = decode_label( &bpi_interior->extra->labels ->label[0]); @@ -2030,7 +2030,7 @@ void vnc_import_bgp_exterior_add_route_interior( else prd = NULL; - if (bgp_path_info_num_labels(bpi_interior)) + if (BGP_PATH_INFO_NUM_LABELS(bpi_interior)) label = decode_label( &bpi_interior->extra->labels->label[0]); else @@ -2147,7 +2147,7 @@ void vnc_import_bgp_exterior_add_route_interior( else prd = NULL; - if (bgp_path_info_num_labels(bpi)) + if (BGP_PATH_INFO_NUM_LABELS(bpi)) label = decode_label( &bpi->extra->labels ->label[0]); @@ -2174,7 +2174,7 @@ void vnc_import_bgp_exterior_add_route_interior( else prd = NULL; - if (bgp_path_info_num_labels(bpi_interior)) + if (BGP_PATH_INFO_NUM_LABELS(bpi_interior)) label = decode_label( &bpi_interior->extra->labels ->label[0]); @@ -2297,7 +2297,7 @@ void vnc_import_bgp_exterior_add_route_interior( else prd = NULL; - if (bgp_path_info_num_labels(bpi_interior)) + if (BGP_PATH_INFO_NUM_LABELS(bpi_interior)) label = decode_label( &bpi_interior->extra->labels->label[0]); else @@ -2409,7 +2409,7 @@ void vnc_import_bgp_exterior_del_route_interior( else prd = NULL; - if (bgp_path_info_num_labels(bpi_interior)) + if (BGP_PATH_INFO_NUM_LABELS(bpi_interior)) label = decode_label( &bpi_interior->extra->labels->label[0]); else @@ -2491,7 +2491,7 @@ void vnc_import_bgp_exterior_del_route_interior( else prd = NULL; - if (bgp_path_info_num_labels(bpi)) + if (BGP_PATH_INFO_NUM_LABELS(bpi)) label = decode_label( &bpi->extra->labels->label[0]); else