]> git.puffer.fish Git - mirror/frr.git/commitdiff
bgpd: fix static analyzer issues around bgp pointer 17983/head
authorPhilippe Guibert <philippe.guibert@6wind.com>
Thu, 9 Jan 2025 20:31:01 +0000 (21:31 +0100)
committerDonatas Abraitis <donatas@opensourcerouting.org>
Mon, 3 Feb 2025 08:54:14 +0000 (10:54 +0200)
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 <philippe.guibert@6wind.com>
bgpd/bgp_vty.c

index 2c02796dd3cfa37281298525570a151207783975..8d8471cf2e25ce5d77770714f461a3ceafb48181 100644 (file)
@@ -1585,27 +1585,29 @@ 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;
                }
 
+               if (!bgp) {
+                       vty_out(vty, "BGP instance not found\n");
+                       return CMD_WARNING_CONFIG_FAILED;
+               }
                /*
                 * If we just instantiated the default instance, complete
                 * any pending VRF-VPN leaking that was configured via
                 * earlier "router bgp X vrf FOO" blocks.
                 */
-               if (bgp && inst_type == BGP_INSTANCE_TYPE_DEFAULT)
+               if (inst_type == BGP_INSTANCE_TYPE_DEFAULT)
                        vpn_leak_postchange_all();
 
-               if (inst_type == BGP_INSTANCE_TYPE_VRF ||
-                   IS_BGP_INSTANCE_HIDDEN(bgp)) {
+               if (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);