From 793dd156c82c9771be24175ff2a76c164a8fc54f Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 9 Jan 2025 21:31:01 +0100 Subject: [PATCH] bgpd: fix static analyzer issues around bgp pointer Some static analyzer issues can be observed in BGP code: > In file included from ./lib/zebra.h:13, > from lib/event.c:8: > ./lib/compiler.h:222:26: note: '#pragma message: Remove `clear thread cpu` command' > 222 | #define CPP_NOTICE(text) _Pragma(CPP_STR(message text)) > | ^~~~~~~ > lib/event.c:433:1: note: in expansion of macro 'CPP_NOTICE' > 433 | CPP_NOTICE("Remove `clear thread cpu` command") > | ^~~~~~~~~~ > bgpd/bgp_vty.c:1592:5: warning: Access to field 'as_pretty' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1592 | bgp->as_pretty); > | ^~~~~~~~~~~~~~ > bgpd/bgp_vty.c:1599:5: warning: Access to field 'as_pretty' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1599 | bgp->as_pretty); > | ^~~~~~~~~~~~~~ > bgpd/bgp_vty.c:1612:7: warning: Access to field 'flags' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1612 | IS_BGP_INSTANCE_HIDDEN(bgp)) { > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./bgpd/bgpd.h:2906:3: note: expanded from macro 'IS_BGP_INSTANCE_HIDDEN' > 2906 | (CHECK_FLAG(_bgp->flags, BGP_FLAG_INSTANCE_HIDDEN) && \ > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./lib/zebra.h:274:31: note: expanded from macro 'CHECK_FLAG' > 274 | #define CHECK_FLAG(V,F) ((V) & (F)) > | ^~~ > bgpd/bgp_vty.c:1614:4: warning: Access to field 'flags' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1614 | UNSET_FLAG(bgp->flags, BGP_FLAG_INSTANCE_HIDDEN); > | ^ ~~~ > ./lib/zebra.h:276:34: note: expanded from macro 'UNSET_FLAG' > 276 | #define UNSET_FLAG(V,F) (V) &= ~(F) > | ~ ^ > 4 warnings generated. > Static Analysis warning summary compared to base: Fix those issues by protecting the bgp pointer. Signed-off-by: Philippe Guibert --- bgpd/bgp_vty.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index f246e3e4be47..f071359d058f 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -1589,14 +1589,13 @@ DEFUN_NOSH (router_bgp, switch (ret) { case BGP_ERR_AS_MISMATCH: vty_out(vty, "BGP is already running; AS is %s\n", - bgp->as_pretty); + bgp ? bgp->as_pretty : "unknown."); return CMD_WARNING_CONFIG_FAILED; case BGP_ERR_INSTANCE_MISMATCH: vty_out(vty, "BGP instance name and AS number mismatch\n"); - vty_out(vty, - "BGP instance is already running; AS is %s\n", - bgp->as_pretty); + vty_out(vty, "BGP instance is already running; AS is %s\n", + bgp ? bgp->as_pretty : "unknown."); return CMD_WARNING_CONFIG_FAILED; } @@ -1608,8 +1607,7 @@ DEFUN_NOSH (router_bgp, if (bgp && inst_type == BGP_INSTANCE_TYPE_DEFAULT) vpn_leak_postchange_all(); - if (inst_type == BGP_INSTANCE_TYPE_VRF || - IS_BGP_INSTANCE_HIDDEN(bgp)) { + if (bgp && (inst_type == BGP_INSTANCE_TYPE_VRF || IS_BGP_INSTANCE_HIDDEN(bgp))) { bgp_vpn_leak_export(bgp); UNSET_FLAG(bgp->flags, BGP_FLAG_INSTANCE_HIDDEN); UNSET_FLAG(bgp->flags, BGP_FLAG_DELETE_IN_PROGRESS);