Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bgpd/bgp_snmp_bgp4v2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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("");
Expand Down
18 changes: 18 additions & 0 deletions tests/topotests/bgp_snmp_bgp4v2_notification/r2/bgpd.conf
Original file line number Diff line number Diff line change
@@ -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
!
17 changes: 17 additions & 0 deletions tests/topotests/bgp_snmp_bgp4v2_notification/r2/snmpd.conf
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions tests/topotests/bgp_snmp_bgp4v2_notification/r2/zebra.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
!
interface r2-eth0
ip address 192.168.12.2/24
!
19 changes: 19 additions & 0 deletions tests/topotests/bgp_snmp_bgp4v2_notification/rr/bgpd.conf
Original file line number Diff line number Diff line change
@@ -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
!
4 changes: 4 additions & 0 deletions tests/topotests/bgp_snmp_bgp4v2_notification/rr/zebra.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
!
interface rr-eth0
ip address 192.168.12.4/24
!
Original file line number Diff line number Diff line change
@@ -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))
Loading