Merge pull request #16623 from FRRouting/mergify/bp/stable/10.0/pr-16546

bgpd: fix, do not access peer->notify.data when it is null (backport #16546)

7 files changed, 182 insertions, 1 deletions

diff --git a/bgpd/bgp_snmp_bgp4v2.c b/bgpd/bgp_snmp_bgp4v2.c
index 0c8ed33d43..5b8da5aeba 100644
--- a/bgpd/bgp_snmp_bgp4v2.c
+++ b/bgpd/bgp_snmp_bgp4v2.c
@@ -340,7 +340,8 @@ static uint8_t *bgpv2PeerErrorsTable(struct variable *v, oid name[],
 		}
 		return SNMP_STRING("");
 	case BGP4V2_PEER_LAST_ERROR_RECEIVED_DATA:
-		if (peer->last_reset == PEER_DOWN_NOTIFY_RECEIVED)
+		if (peer->last_reset == PEER_DOWN_NOTIFY_RECEIVED &&
+		    peer->notify.data)
 			return SNMP_STRING(peer->notify.data);
 		else
 			return SNMP_STRING("");
diff --git a/tests/topotests/bgp_snmp_bgp4v2_notification/r2/bgpd.conf b/tests/topotests/bgp_snmp_bgp4v2_notification/r2/bgpd.conf
new file mode 100644
index 0000000000..a550b4c143
--- /dev/null
+++ b/tests/topotests/bgp_snmp_bgp4v2_notification/r2/bgpd.conf
@@ -0,0 +1,18 @@
+!
+!debug bgp updates
+!
+router bgp 65002
+ no bgp ebgp-requires-policy
+ no bgp network import-check
+ no bgp default ipv4-unicast
+ neighbor 192.168.12.4 remote-as external
+ neighbor 192.168.12.4 timers 1 3
+ neighbor 192.168.12.4 timers connect 1
+ !
+ address-family ipv4 unicast
+  neighbor 192.168.12.4 activate
+  neighbor 192.168.12.4 addpath-tx-all-paths
+ exit-address-family
+!
+agentx
+!
diff --git a/tests/topotests/bgp_snmp_bgp4v2_notification/r2/snmpd.conf b/tests/topotests/bgp_snmp_bgp4v2_notification/r2/snmpd.conf
new file mode 100644
index 0000000000..032b93b676
--- /dev/null
+++ b/tests/topotests/bgp_snmp_bgp4v2_notification/r2/snmpd.conf
@@ -0,0 +1,17 @@
+agentAddress 127.0.0.1,[::1]
+
+group public_group v1 public
+group public_group v2c public
+access public_group "" any noauth prefix all all none
+
+rocommunity public default
+
+view all included .1
+
+iquerySecName frr
+rouser frr
+
+master agentx
+
+agentXSocket /etc/frr/agentx
+agentXPerms 777 755 root frr
diff --git a/tests/topotests/bgp_snmp_bgp4v2_notification/r2/zebra.conf b/tests/topotests/bgp_snmp_bgp4v2_notification/r2/zebra.conf
new file mode 100644
index 0000000000..d7dfd899cc
--- /dev/null
+++ b/tests/topotests/bgp_snmp_bgp4v2_notification/r2/zebra.conf
@@ -0,0 +1,4 @@
+!
+interface r2-eth0
+ ip address 192.168.12.2/24
+!
diff --git a/tests/topotests/bgp_snmp_bgp4v2_notification/rr/bgpd.conf b/tests/topotests/bgp_snmp_bgp4v2_notification/rr/bgpd.conf
new file mode 100644
index 0000000000..145a8f008e
--- /dev/null
+++ b/tests/topotests/bgp_snmp_bgp4v2_notification/rr/bgpd.conf
@@ -0,0 +1,19 @@
+!
+!debug bgp updates
+!
+router bgp 65004
+ no bgp ebgp-requires-policy
+ no bgp network import-check
+ no bgp default ipv4-unicast
+ neighbor 192.168.12.2 remote-as external
+ neighbor 192.168.12.2 timers 1 3
+ neighbor 192.168.12.2 timers connect 1
+ !
+ address-family ipv4 unicast
+  neighbor 192.168.12.2 activate
+  neighbor 192.168.12.2 addpath-tx-all-paths
+  neighbor 192.168.12.2 route-server-client
+ exit-address-family
+!
+agentx
+!
diff --git a/tests/topotests/bgp_snmp_bgp4v2_notification/rr/zebra.conf b/tests/topotests/bgp_snmp_bgp4v2_notification/rr/zebra.conf
new file mode 100644
index 0000000000..eb8a486438
--- /dev/null
+++ b/tests/topotests/bgp_snmp_bgp4v2_notification/rr/zebra.conf
@@ -0,0 +1,4 @@
+!
+interface rr-eth0
+ ip address 192.168.12.4/24
+!
diff --git a/tests/topotests/bgp_snmp_bgp4v2_notification/test_bgp_snmp_bgp4v2_notification.py b/tests/topotests/bgp_snmp_bgp4v2_notification/test_bgp_snmp_bgp4v2_notification.py
new file mode 100755
index 0000000000..c41ef30bb5
--- /dev/null
+++ b/tests/topotests/bgp_snmp_bgp4v2_notification/test_bgp_snmp_bgp4v2_notification.py
@@ -0,0 +1,118 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Copyright 2024 6WIND S.A.
+#
+
+
+"""
+Test BGP OPEN NOTIFY (Configuration mismatch) followed by snmpwalk.
+"""
+
+import os
+import sys
+import json
+import pytest
+import functools
+
+CWD = os.path.dirname(os.path.realpath(__file__))
+sys.path.append(os.path.join(CWD, "../"))
+
+# pylint: disable=C0413
+# Import topogen and topotest helpers
+from lib.topogen import Topogen, TopoRouter, get_topogen
+from lib import topotest
+
+pytestmark = [pytest.mark.bgpd, pytest.mark.snmp]
+
+
+def build_topo(tgen):
+    """Build function"""
+
+    tgen.add_router("r2")
+    tgen.add_router("rr")
+
+    switch = tgen.add_switch("s1")
+    switch.add_link(tgen.gears["r2"])
+    switch.add_link(tgen.gears["rr"])
+
+
+def setup_module(mod):
+    snmpd = os.system("which snmpd")
+    if snmpd:
+        error_msg = "SNMP not installed - skipping"
+        pytest.skip(error_msg)
+
+    tgen = Topogen(build_topo, mod.__name__)
+    tgen.start_topology()
+
+    for rname, router in tgen.routers().items():
+        router.load_config(
+            TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))
+        )
+        router.load_config(
+            TopoRouter.RD_BGP,
+            os.path.join(CWD, "{}/bgpd.conf".format(rname)),
+            "-M snmp",
+        )
+        router.load_config(
+            TopoRouter.RD_SNMP,
+            os.path.join(CWD, "{}/snmpd.conf".format(rname)),
+            "-Le -Ivacm_conf,usmConf,iquery -V -DAgentX",
+        )
+
+    tgen.start_router()
+
+
+def teardown_module(mod):
+    tgen = get_topogen()
+    tgen.stop_topology()
+
+
+def test_bgp_open_notification_change_configuration():
+    tgen = get_topogen()
+
+    tgen.gears["rr"].vtysh_multicmd(
+        """
+configure terminal
+router bgp 65004
+neighbor 192.168.12.2  password 8888"
+"""
+    )
+    tgen.net["r2"].cmd("snmpwalk -v 2c -c public 127.0.0.1 .1.3.6.1.4.1.7336.4.2.1")
+    tgen.gears["rr"].vtysh_multicmd(
+        """
+configure terminal
+router bgp 65004
+no neighbor 192.168.12.2  password 8888"
+"""
+    )
+
+    def _check_bgp_session():
+        r2 = tgen.gears["r2"]
+
+        output = json.loads(r2.vtysh_cmd("show bgp summary json"))
+        expected = {
+            "ipv4Unicast": {"peers": {"192.168.12.4": {"state": "Established"}}}
+        }
+
+        return topotest.json_cmp(output, expected)
+
+    test_func1 = functools.partial(_check_bgp_session)
+    _, result1 = topotest.run_and_expect(test_func1, None, count=120, wait=0.5)
+
+    assert result1 is None, "Failed to verify the bgp session"
+
+
+def test_memory_leak():
+    "Run the memory leak test and report results."
+    tgen = get_topogen()
+    if not tgen.is_memleak_enabled():
+        pytest.skip("Memory leak test/report is disabled")
+
+    tgen.report_memory_leaks()
+
+
+if __name__ == "__main__":
+    args = ["-s"] + sys.argv[1:]
+    sys.exit(pytest.main(args))