ospfd: Fix heap corruption vulnerability when parsing SR-Algorithm TLV
When parsing the SR-Algorithm TLV in the OSPF Router Information Opaque
LSA, assure that not more than the maximum number of supported
algorithms are copied from the TLV.
Louis Scalbert [Thu, 12 Sep 2024 07:31:49 +0000 (09:31 +0200)]
isisd: fix rcap tlv double-free crash
A double-free crash happens when a subTLV of the "Router Capability"
TLV is not readable and a previous "Router Capability" TLV was read.
rcap was supposed to be freed later by isis_free_tlvs() ->
free_tlv_router_cap(). In 78774bbcd5 ("isisd: add isis flex-algo lsp
advertisement"), this was not the case because rcap was not saved to
tlvs->router_cap when the function returned early because of a subTLV
length issue.
Always set tlvs->router_cap to free the memory.
Note that this patch has the consequence that in case of subTLV error,
the previously read "Router Capability" subTLVs are kept in memory.
Fixes: 49efc80d34 ("isisd: Ensure rcap is freed in error case") Fixes: 78774bbcd5 ("isisd: add isis flex-algo lsp advertisement") Reported-by: Iggy Frankovic <iggyfran@amazon.com> Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit d61758140d33972c10ecbb72d0a3e528049dd8d6)
When an NHRP peer was forwarding a message, it was copying all
extensions from the originally received packet. The authentication
extension must be regenerated hop by hop per RFC2332.
This fix checks for the auth extension when copying extensions
and omits the original packet auth and instead regenerates a new auth extension.
- bgpd
- Fix as-path exclude modify crash
- Fix, do not access peer->notify.data when it is null
- Fix crash at no rpki
- Ignore RFC8212 for BGP Confederations
- Fix for CVE-2024-44070
- Relax OAD (One-Administration-Domain) for RFC8212
- Fix "bgp as-pah access-list" with "set aspath exclude" set/unset issues
- Check if we have really enough data before doing memcpy for FQDN capability
- Check if we have really enough data before doing memcpy for software version
- Set last reset reason to admin shutdown if it was manually
- Fix do not use api.backup_nexthop in ZAPI message
- isisd
- Fix crash when reading asla
- Add missing `exit` statement
- Fix update link params after circuit is up
- Fix crash at flex-algo without mpls-te
- Fix memory handling in isis_adj_process_threeway()
- Fix crash when calculating the neighbor spanning tree based on the fragmented LSP
- Fix crash when obtaining the next hop to calculate LFA on LAN links
- Fix memory leaks when the transition of neighbor state from non-UP to DOWN
- fix crash when displaying asla in json
- pimd
- Fix crash in pimd
- Fix msdp setting of sa->rp
- Fix crash on non-existent interface
- nhrpd
- Fix sending /32 shortcut
- mgmtd
- Don't add implicit state data when reading config from file
- Fix too early daemon detach of mgmtd
- ripd
- Fix show run output for distribute-list
- lib
- Fix distribute-list deletion
- Fix crash on distribute-list delete
- Fix incorrect use of error checking macro
- yang
- Added missed prefix to the yang file
- ospfd
- Fix internal ldp-sync state flags when feature is disabled
- ldpd
- Fix wrong gtsm count
- ripd
- Change the start value of sequence 1 to 0
- zebra
- Fix evpn mh bond member proto reinstall
- Fix to avoid two Vrfs with same table ids
- Fix missing static routes
- Ensure non-equal id's are not same nhg's
Donatas Abraitis [Fri, 14 Jun 2024 13:33:32 +0000 (16:33 +0300)]
docker: Set ABUILD_APK_INDEX_OPTS for frr build
In build() stage of abuild, it does `apk index ...` where frr* packages
are unsigned. We don't sign them here, and thus we need to specify `--allow-untrusted`.
Donatas Abraitis [Fri, 14 Jun 2024 08:37:23 +0000 (11:37 +0300)]
docker: Set ABUILD_APK_INDEX_OPTS for libyang
In build() stage of abuild, it does `apk index ...` where libyang* packages
are unsigned. We don't sign them here, and thus we need to specify `--allow-untrusted`.
Louis Scalbert [Tue, 27 Aug 2024 16:22:27 +0000 (18:22 +0200)]
isisd: fix update link params after circuit is up
If the link-params are set when the circuit not yet up, the link-params
are never updated.
isis_link_params_update() is called from isis_circuit_up() but returns
immediately because circuit->state != C_STATE_UP. circuit->state is
updated in isis_csm_state_change after isis_circuit_up().
Do not return isis_link_params_update() if circuit->state != C_STATE_UP.
Fixes: 0fdd8b2b11 ("isisd: update link params after circuit is up") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit 6ce6b7a8564f661495fec17f3ea33eeaf9e2f48c)
Donald Sharp [Tue, 27 Aug 2024 21:08:38 +0000 (17:08 -0400)]
tests: Fix bgp_default_originate_topo1_3
This test was killing bgp on r1 and r2
and then immediately testing that the
default route transitioned. Unfortunately
the test was written that under load the
system might be in a bad state. Let's
modify the code to check for a bgp version
change and then that the bgp state has
come back up
Donald Sharp [Wed, 28 Aug 2024 19:10:04 +0000 (15:10 -0400)]
tests: ospf_netns_vrf should give more time for coming up
Test fails:
test_func = partial(
topotest.router_json_cmp,
router,
"show ip ospf vrf {0}-ospf-cust1 json".format(rname),
expected,
)
_, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
assertmsg = '"{}" JSON output mismatches'.format(rname)
> assert diff is None, assertmsg
E AssertionError: "r1" JSON output mismatches
E assert Generated JSON diff error report:
E
E > $->r1-ospf-cust1->areas->0.0.0.0->nbrFullAdjacentCounter: output has element with value '1' but in expected it has value '2'
Support bundle has this data:
r1# show ip ospf vrf all neighbor
% 2024/08/28 14:55:54.763
VRF Name: r1-ospf-cust1
Neighbor ID Pri State Up Time Dead Time Address Interface RXmtL RqstL DBsmL
10.0.255.3 1 Full/DR 10.547s 39.456s 10.0.3.1 r1-eth1:10.0.3.2 0 0 0
10.0.255.2 1 Full/Backup 0.543s 38.378s 10.0.3.3 r1-eth1:10.0.3.2 1 0 0
So immediately after the test fails this test, the neighbor comes up.
Let's give the test a bit more time for failure to not happen
> #0 __pthread_kill_implementation (no_tid=0, signo=11, threadid=140486233631168) at ./nptl/pthread_kill.c:44
> #1 __pthread_kill_internal (signo=11, threadid=140486233631168) at ./nptl/pthread_kill.c:78
> #2 __GI___pthread_kill (threadid=140486233631168, signo=signo@entry=11) at ./nptl/pthread_kill.c:89
> #3 0x00007fc5802e9476 in __GI_raise (sig=11) at ../sysdeps/posix/raise.c:26
> #4 0x00007fc58076021f in core_handler (signo=11, siginfo=0x7ffd38d42470, context=0x7ffd38d42340) at lib/sigevent.c:248
> #5 <signal handler called>
> #6 0x000055c527f798c9 in isis_link_params_update_asla (circuit=0x55c52aaed3c0, ifp=0x55c52a1044e0) at isisd/isis_te.c:176
> #7 0x000055c527fb29da in isis_instance_flex_algo_create (args=0x7ffd38d43120) at isisd/isis_nb_config.c:2875
> #8 0x00007fc58072655b in nb_callback_create (context=0x55c52ab1d2f0, nb_node=0x55c529f72950, event=NB_EV_APPLY, dnode=0x55c52ab06230, resource=0x55c52ab189f8, errmsg=0x7ffd38d43750 "",
> errmsg_len=8192) at lib/northbound.c:1262
> #9 0x00007fc580727625 in nb_callback_configuration (context=0x55c52ab1d2f0, event=NB_EV_APPLY, change=0x55c52ab189c0, errmsg=0x7ffd38d43750 "", errmsg_len=8192) at lib/northbound.c:1662
> #10 0x00007fc580727c39 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x55c52ab1d2f0, errmsg=0x7ffd38d43750 "", errmsg_len=8192) at lib/northbound.c:1794
> #11 0x00007fc580725f77 in nb_candidate_commit_apply (transaction=0x55c52ab1d2f0, save_transaction=true, transaction_id=0x0, errmsg=0x7ffd38d43750 "", errmsg_len=8192)
> at lib/northbound.c:1131
> #12 0x00007fc5807260d1 in nb_candidate_commit (context=..., candidate=0x55c529f0a730, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffd38d43750 "", errmsg_len=8192)
> at lib/northbound.c:1164
> #13 0x00007fc58072d220 in nb_cli_classic_commit (vty=0x55c52a0fc6b0) at lib/northbound_cli.c:51
> #14 0x00007fc58072d839 in nb_cli_apply_changes_internal (vty=0x55c52a0fc6b0,
> xpath_base=0x7ffd38d477f0 "/frr-isisd:isis/instance[area-tag='1'][vrf='default']/flex-algos/flex-algo[flex-algo='129']", clear_pending=false) at lib/northbound_cli.c:178
> #15 0x00007fc58072dbcf in nb_cli_apply_changes (vty=0x55c52a0fc6b0, xpath_base_fmt=0x55c528014de0 "./flex-algos/flex-algo[flex-algo='%ld']") at lib/northbound_cli.c:234
> #16 0x000055c527fd3403 in flex_algo_magic (self=0x55c52804f1a0 <flex_algo_cmd>, vty=0x55c52a0fc6b0, argc=2, argv=0x55c52ab00ec0, algorithm=129, algorithm_str=0x55c52ab120d0 "129")
> at isisd/isis_cli.c:3752
> #17 0x000055c527fc97cb in flex_algo (self=0x55c52804f1a0 <flex_algo_cmd>, vty=0x55c52a0fc6b0, argc=2, argv=0x55c52ab00ec0) at ./isisd/isis_cli_clippy.c:6445
> #18 0x00007fc5806b9abc in cmd_execute_command_real (vline=0x55c52aaf78f0, vty=0x55c52a0fc6b0, cmd=0x0, up_level=0) at lib/command.c:984
> #19 0x00007fc5806b9c35 in cmd_execute_command (vline=0x55c52aaf78f0, vty=0x55c52a0fc6b0, cmd=0x0, vtysh=0) at lib/command.c:1043
> #20 0x00007fc5806ba1e5 in cmd_execute (vty=0x55c52a0fc6b0, cmd=0x55c52aae6bd0 "flex-algo 129\n", matched=0x0, vtysh=0) at lib/command.c:1209
> #21 0x00007fc580782ae1 in vty_command (vty=0x55c52a0fc6b0, buf=0x55c52aae6bd0 "flex-algo 129\n") at lib/vty.c:615
> #22 0x00007fc580784a05 in vty_execute (vty=0x55c52a0fc6b0) at lib/vty.c:1378
> #23 0x00007fc580787131 in vtysh_read (thread=0x7ffd38d4ab10) at lib/vty.c:2373
> #24 0x00007fc58077b605 in event_call (thread=0x7ffd38d4ab10) at lib/event.c:2011
> #25 0x00007fc5806f8976 in frr_run (master=0x55c529df9b30) at lib/libfrr.c:1212
> #26 0x000055c527f301bc in main (argc=5, argv=0x7ffd38d4ad58, envp=0x7ffd38d4ad88) at isisd/isis_main.c:350
> (gdb) f 6
> #6 0x000055c527f798c9 in isis_link_params_update_asla (circuit=0x55c52aaed3c0, ifp=0x55c52a1044e0) at isisd/isis_te.c:176
> 176 list_delete_all_node(ext->aslas);
> (gdb) p ext
> $1 = (struct isis_ext_subtlvs *) 0x0
Fixes: ae27101e6f ("isisd: fix building asla at first flex-algo config") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit cd81d28ae253e64665ce7c45e18e479e3fc2f90d)
Corey Siltala [Fri, 23 Aug 2024 18:04:26 +0000 (18:04 +0000)]
pimd: Fix crash in pimd
ifp->info is not always set in PIM. So add a guard here to stop
it from crashing when addresses are added to a non-PIM enabled interface
and PIM zebra debugging is enabled.
Louis Scalbert [Fri, 23 Aug 2024 14:05:45 +0000 (16:05 +0200)]
nhrpd: fix sending /32 shortcut
The remote spoke always sends a 32 prefix length to a shortcut request.
In the example, the remote spoke as the IP address 192.168.2.1/24.
spoke1# sh ip nhrp shortcut
Type Prefix Via Identity
dynamic 192.168.2.1/32 10.255.255.2
Do not deal with local routes in nhrpd. Now:
spoke1# sh ip nhrp shortcut
Type Prefix Via Identity
dynamic 192.168.2.0/24 10.255.255.2
Fixes: d4aa24ba7d ("*: Introduce Local Host Routes to FRR") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit af54901405474b0623bda1899424ec18a3240c71)
Dmytro Shytyi [Thu, 8 Aug 2024 13:42:40 +0000 (15:42 +0200)]
topotest: test_bgp_snmp_bgpv4v2_notification
This test checks the bgp crash on rt2 when 2 commands
launched consequently:
T0: rr, config -> router bgp 65004 -> neighbor 192.168.12.2 password 8888
T1: rt2, snmpwalk -v 2c -c public 127.0.0.1 .1.3.6.1.4.1.7336.4.2.1
T2: test if rt2 bgp is crashed.
Louis Scalbert [Tue, 20 Aug 2024 08:33:30 +0000 (10:33 +0200)]
bgpd: fix crash at no rpki
When 'no rpki' is requested and the rtrlib RPKI object was freed, bgpd
is crashing.
RPKI is configured in VRF red.
> ip l set red down
> ip l del red
> printf 'conf\n vrf red\n no rpki' | vtysh
> Core was generated by `/usr/bin/bgpd -A 127.0.0.1 -M snmp -M rpki -M bmp'.
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 __pthread_kill_implementation (no_tid=0, signo=11, threadid=140411103615424) at ./nptl/pthread_kill.c:44
> 44 ./nptl/pthread_kill.c: No such file or directory.
> [Current thread is 1 (Thread 0x7fb401f419c0 (LWP 190226))]
> (gdb) bt
> #0 __pthread_kill_implementation (no_tid=0, signo=11, threadid=140411103615424) at ./nptl/pthread_kill.c:44
> #1 __pthread_kill_internal (signo=11, threadid=140411103615424) at ./nptl/pthread_kill.c:78
> #2 __GI___pthread_kill (threadid=140411103615424, signo=signo@entry=11) at ./nptl/pthread_kill.c:89
> #3 0x00007fb4021ad476 in __GI_raise (sig=11) at ../sysdeps/posix/raise.c:26
> #4 0x00007fb4025ce22b in core_handler (signo=11, siginfo=0x7fff831b2d70, context=0x7fff831b2c40) at lib/sigevent.c:248
> #5 <signal handler called>
> #6 rtr_mgr_remove_group (config=0x55fe8789f750, preference=11) at /build/make-pkg/output/source/DIST_RTRLIB/rtrlib/rtrlib/rtr_mgr.c:607
> #7 0x00007fb40145f518 in rpki_delete_all_cache_nodes (rpki_vrf=0x55fe8789f4f0) at bgpd/bgp_rpki.c:442
> #8 0x00007fb401463098 in no_rpki_magic (self=0x7fb40146bba0 <no_rpki_cmd>, vty=0x55fe877f5130, argc=2, argv=0x55fe877fccd0) at bgpd/bgp_rpki.c:1732
> #9 0x00007fb40145c09a in no_rpki (self=0x7fb40146bba0 <no_rpki_cmd>, vty=0x55fe877f5130, argc=2, argv=0x55fe877fccd0) at ./bgpd/bgp_rpki_clippy.c:37
> #10 0x00007fb402527abc in cmd_execute_command_real (vline=0x55fe877fd150, vty=0x55fe877f5130, cmd=0x0, up_level=0) at lib/command.c:984
> #11 0x00007fb402527c35 in cmd_execute_command (vline=0x55fe877fd150, vty=0x55fe877f5130, cmd=0x0, vtysh=0) at lib/command.c:1043
> #12 0x00007fb4025281e5 in cmd_execute (vty=0x55fe877f5130, cmd=0x55fe877fb8c0 "no rpki\n", matched=0x0, vtysh=0) at lib/command.c:1209
> #13 0x00007fb4025f0aed in vty_command (vty=0x55fe877f5130, buf=0x55fe877fb8c0 "no rpki\n") at lib/vty.c:615
> #14 0x00007fb4025f2a11 in vty_execute (vty=0x55fe877f5130) at lib/vty.c:1378
> #15 0x00007fb4025f513d in vtysh_read (thread=0x7fff831b5fa0) at lib/vty.c:2373
> #16 0x00007fb4025e9611 in event_call (thread=0x7fff831b5fa0) at lib/event.c:2011
> #17 0x00007fb402566976 in frr_run (master=0x55fe871a14a0) at lib/libfrr.c:1212
> #18 0x000055fe857829fa in main (argc=9, argv=0x7fff831b6218) at bgpd/bgp_main.c:549
Fixes: 8156765abe ("bgpd: Add `no rpki` command") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit 4e053d65f1c7edbcc3391026300388513d4c31b0)
Mark Stapp [Fri, 9 Aug 2024 14:08:21 +0000 (10:08 -0400)]
isisd: fix memory handling in isis_adj_process_threeway()
The adj_process_threeway() api may call the adj_state_change()
api, which may delete the adj struct being examined. Change the
signature so that callers pass a ptr-to-ptr so that they will
see that deletion.
Donald Sharp [Sat, 10 Aug 2024 23:43:08 +0000 (19:43 -0400)]
zebra: Ensure non-equal id's are not same nhg's
The function zebra_nhg_hash_equal is only used
as a hash function for storage of NHG's and retrieval.
If you have say two nhg's:
31 (25/26)
32 (25/26)
This function would return them as being equal. Which
of course leads to the problem when you attempt to
hash_release 32 but release 31 from the hash. Then later
when you attempt to do hash comparisons 32 has actually
been freed leaving to use after free situations and shit
goes down hill fast.
This hash is only used as part of the hash comparison
function for nexthop group storage. Since this is so
let's always return the 31/32 nhg's are not equal at all.
We possibly have a different problem where we are creating
31 and 32 ( when 31 should have just been used instead of 32 )
but we need to prevent any type of hash release problem at all.
This supercedes any other issue( that should be tracked down
on it's own ). Since you can have use after free situation
that leads to a crash -vs- some possible nexthop group duplication
which is very minor in comparison.
Igor Ryzhov [Wed, 7 Aug 2024 21:40:51 +0000 (00:40 +0300)]
mgmtd: don't add implicit state data when reading config from file
When mgmt reads configuration from file, it shouldn't add implicit state
data to the candidate datastore. Configuration datastores like candidate
should never store state, otherwise they fail validation.
Igor Ryzhov [Wed, 7 Aug 2024 22:25:02 +0000 (01:25 +0300)]
ripd: fix show run output for distribute-list
CLI show callbacks should be defined in frr_ripd_cli_info instead of
frr_ripd_info, because only the former is loaded by mgmtd and only its
callbacks are getting called for config output.
Christian Hopps [Tue, 23 Jul 2024 21:42:07 +0000 (17:42 -0400)]
lib: mgmtd: fix too early daemon detach of mgmtd
Correct FRR startup counts on a daemon's vty socket to be open when the
parent process exits. The parent process waits for `frr_check_detach()`
to be called by the child before exiting. The problem is when the
`FRR_MANUAL_VTY_START` flag is set the vty socket was not opened but
`frr_check_detach()` was called anyway.
Instead add a bool option for `frr_check_detach()` to be called when the
socket is opened with `frr_vty_serv_start()`, and do so when "manually"
calling said function (i.e., when FRR_MANUAL_VTY_START is set).
The `FRR_MANUAL_VTY_START` flag is only set by mgmtd. The reason we
wait to open the vty socket is so that mgmtd can parse the various
daemon specific config files it has taken over, after the event loop has
started, but before we receive any possible new config from `vtysh`.
ospfd: fix internal ldp-sync state flags when feature is disabled
When enabling "mpls ldp-sync" under "router ospf" ospfd configures
SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG) so internally knowing
that the ldp-sync feature is enabled. However the flag is not cleared when
turning of the feature using "nompls ldp-sync"!
Louis Scalbert [Fri, 28 Jun 2024 11:22:36 +0000 (13:22 +0200)]
pimd: fix crash on non-existent interface
Fix the following crash when pim options are (un)configured on an
non-existent interface.
> r1(config)# int fgljdsf
> r1(config-if)# no ip pim unicast-bsm
> vtysh: error reading from pimd: Connection reset by peer (104)Warning: closing connection to pimd because of an I/O error!
> #0 raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1 0x00007f70c8f32249 in core_handler (signo=11, siginfo=0x7fffff88e4f0, context=0x7fffff88e3c0) at lib/sigevent.c:258
> #2 <signal handler called>
> #3 0x0000556cfdd9b16d in lib_interface_pim_address_family_unicast_bsm_modify (args=0x7fffff88f130) at pimd/pim_nb_config.c:1910
> #4 0x00007f70c8efdcb5 in nb_callback_modify (context=0x556d00032b60, nb_node=0x556cffeeb9b0, event=NB_EV_APPLY, dnode=0x556d00031670, resource=0x556d00032b48, errmsg=0x7fffff88f710 "", errmsg_len=8192)
> at lib/northbound.c:1538
> #5 0x00007f70c8efe949 in nb_callback_configuration (context=0x556d00032b60, event=NB_EV_APPLY, change=0x556d00032b10, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1888
> #6 0x00007f70c8efee82 in nb_transaction_process (event=NB_EV_APPLY, transaction=0x556d00032b60, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:2016
> #7 0x00007f70c8efd658 in nb_candidate_commit_apply (transaction=0x556d00032b60, save_transaction=true, transaction_id=0x0, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1356
> #8 0x00007f70c8efd78e in nb_candidate_commit (context=..., candidate=0x556cffeb0e80, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7fffff88f710 "", errmsg_len=8192) at lib/northbound.c:1389
> #9 0x00007f70c8f03e58 in nb_cli_classic_commit (vty=0x556d00025a80) at lib/northbound_cli.c:51
> #10 0x00007f70c8f043f8 in nb_cli_apply_changes_internal (vty=0x556d00025a80,
> xpath_base=0x7fffff893bb0 "/frr-interface:lib/interface[name='fgljdsf']/frr-pim:pim/address-family[address-family='frr-routing:ipv4']", clear_pending=false) at lib/northbound_cli.c:178
> #11 0x00007f70c8f0475d in nb_cli_apply_changes (vty=0x556d00025a80, xpath_base_fmt=0x556cfdde9fe0 "./frr-pim:pim/address-family[address-family='%s']") at lib/northbound_cli.c:234
> #12 0x0000556cfdd8298f in pim_process_no_unicast_bsm_cmd (vty=0x556d00025a80) at pimd/pim_cmd_common.c:3493
> #13 0x0000556cfddcf782 in no_ip_pim_ucast_bsm (self=0x556cfde40b20 <no_ip_pim_ucast_bsm_cmd>, vty=0x556d00025a80, argc=4, argv=0x556d00031500) at pimd/pim_cmd.c:4950
> #14 0x00007f70c8e942f0 in cmd_execute_command_real (vline=0x556d00032070, vty=0x556d00025a80, cmd=0x0, up_level=0) at lib/command.c:1002
> #15 0x00007f70c8e94451 in cmd_execute_command (vline=0x556d00032070, vty=0x556d00025a80, cmd=0x0, vtysh=0) at lib/command.c:1061
> #16 0x00007f70c8e9499f in cmd_execute (vty=0x556d00025a80, cmd=0x556d00030320 "no ip pim unicast-bsm", matched=0x0, vtysh=0) at lib/command.c:1227
> #17 0x00007f70c8f51e44 in vty_command (vty=0x556d00025a80, buf=0x556d00030320 "no ip pim unicast-bsm") at lib/vty.c:616
> #18 0x00007f70c8f53bdd in vty_execute (vty=0x556d00025a80) at lib/vty.c:1379
> #19 0x00007f70c8f55d59 in vtysh_read (thread=0x7fffff896600) at lib/vty.c:2374
> #20 0x00007f70c8f4b209 in event_call (thread=0x7fffff896600) at lib/event.c:2011
> #21 0x00007f70c8ed109e in frr_run (master=0x556cffdb4ea0) at lib/libfrr.c:1217
> #22 0x0000556cfdddec12 in main (argc=2, argv=0x7fffff896828, envp=0x7fffff896840) at pimd/pim_main.c:165
> (gdb) f 3
> #3 0x0000556cfdd9b16d in lib_interface_pim_address_family_unicast_bsm_modify (args=0x7fffff88f130) at pimd/pim_nb_config.c:1910
> 1910 pim_ifp->ucast_bsm_accept =
> (gdb) list
> 1905 case NB_EV_ABORT:
> 1906 break;
> 1907 case NB_EV_APPLY:
> 1908 ifp = nb_running_get_entry(args->dnode, NULL, true);
> 1909 pim_ifp = ifp->info;
> 1910 pim_ifp->ucast_bsm_accept =
> 1911 yang_dnode_get_bool(args->dnode, NULL);
> 1912
> 1913 break;
> 1914 }
> (gdb) p pim_ifp
> $1 = (struct pim_interface *) 0x0
Fixes: 3bb513c399 ("lib: adapt to version 2 of libyang") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit 6952bea5cdd38057bf8c0a5e9c0fbe916dc73953)
isisd: fix crash when calculating the neighbor spanning tree based on the fragmented LSP
1. When the root IS regenerates an LSP, it calls lsp_build() -> lsp_clear_data() to free the TLV memory of the first fragment and all other fragments. If the number of fragments in the regenerated LSP decreases or if no fragmentation is needed, the extra LSP fragments are not immediately deleted. Instead, lsp_seqno_update() -> lsp_purge() is called to set the remaining time to zero and start aging, while also notifying other IS nodes to age these fragments. lsp_purge() usually does not reset lsp->hdr.seqno to zero because the LSP might recover during the aging process.
2. When other IS nodes receive an LSP, they always call process_lsp() -> isis_unpack_tlvs() to allocate TLV memory for the LSP. This does not differentiate whether the received LSP has a remaining lifetime of zero. Therefore, it is rare for an LSP of a non-root IS to have empty TLVs. Of course, if an LSP with a remaining time of zero and already corrupted is received, lsp_update() -> lsp_purge() will be called to free the TLV memory of the LSP, but this scenario is rare.
3. In LFA calculations, neighbors of the root IS are traversed, and each neighbor is taken as a new root to compute the neighbor SPT. During this process, the old root IS will serve as a neighbor of the new root IS, triggering a call to isis_spf_process_lsp() to parse the LSP of the old root IS and obtain its IP vertices and neighboring IS vertices. However, isis_spf_process_lsp() only checks whether the TLVs in the first fragment of the LSP exist, and does not check the TLVs in the fragmented LSP. If the TLV memory of the fragmented LSP of the old root IS has been freed, it can lead to a null pointer access, causing the current crash.
Additionally, for the base SPT, there are only two places where the LSP of the root IS is parsed:
1. When obtaining the UP neighbors of the root IS via spf_adj_list_parse_lsp().
2. When preloading the IP vertices of the root IS via isis_lsp_iterate_ip_reach().
Both of these checks ensure that frag->tlvs is not null, and they do not subsequently call isis_spf_process_lsp() to parse the root IS's LSP. It is very rare for non-root IS LSPs to have empty TLVs unless they are corrupted LSPs awaiting deletion. If it happens, a crash will occur.
The backtrace is as follows:
(gdb) bt
#0 0x00007f3097281fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
#1 0x00007f30973a2972 in core_handler (signo=11, siginfo=0x7ffce66c2870, context=0x7ffce66c2740) at ../lib/sigevent.c:261
#2 <signal handler called>
#3 0x000055dfa805512b in isis_spf_process_lsp (spftree=0x55dfa950eee0, lsp=0x55dfa94cb590, cost=10, depth=1, root_sysid=0x55dfa950ef6c "", parent=0x55dfa952fca0)
at ../isisd/isis_spf.c:898
#4 0x000055dfa805743b in isis_spf_loop (spftree=0x55dfa950eee0, root_sysid=0x55dfa950ef6c "") at ../isisd/isis_spf.c:1688
#5 0x000055dfa805784f in isis_run_spf (spftree=0x55dfa950eee0) at ../isisd/isis_spf.c:1808
#6 0x000055dfa8037ff5 in isis_spf_run_neighbors (spftree=0x55dfa9474440) at ../isisd/isis_lfa.c:1259
#7 0x000055dfa803ac17 in isis_spf_run_lfa (area=0x55dfa9477510, spftree=0x55dfa9474440) at ../isisd/isis_lfa.c:2300
#8 0x000055dfa8057964 in isis_run_spf_with_protection (area=0x55dfa9477510, spftree=0x55dfa9474440) at ../isisd/isis_spf.c:1827
#9 0x000055dfa8057c15 in isis_run_spf_cb (thread=0x7ffce66c38e0) at ../isisd/isis_spf.c:1889
#10 0x00007f30973bbf04 in thread_call (thread=0x7ffce66c38e0) at ../lib/thread.c:1990
#11 0x00007f309735497b in frr_run (master=0x55dfa91733c0) at ../lib/libfrr.c:1198
#12 0x000055dfa8029d5d in main (argc=5, argv=0x7ffce66c3b08, envp=0x7ffce66c3b38) at ../isisd/isis_main.c:273
(gdb) f 3
#3 0x000055dfa805512b in isis_spf_process_lsp (spftree=0x55dfa950eee0, lsp=0x55dfa94cb590, cost=10, depth=1, root_sysid=0x55dfa950ef6c "", parent=0x55dfa952fca0)
at ../isisd/isis_spf.c:898
898 ../isisd/isis_spf.c: No such file or directory.
(gdb) p te_neighs
$1 = (struct isis_item_list *) 0x120
(gdb) p lsp->tlvs
$2 = (struct isis_tlvs *) 0x0
(gdb) p lsp->hdr
$3 = {pdu_len = 27, rem_lifetime = 0, lsp_id = "\000\000\000\000\000\001\000\001", seqno = 4, checksum = 59918, lsp_bits = 1 '\001'}
The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well.
I have reviewed the process for calculating the SPT based on the LSP, and isis_spf_process_lsp() is the only function that does not check whether the TLVs in the fragments are empty. Therefore, I believe that modifying this function alone should be sufficient. If the TLVs of the current fragment are already empty, we do not need to continue processing subsequent fragments. This is consistent with the behavior where we do not process fragments if the TLVs of the first fragment are empty.
Of course, one could argue that lsp_purge() should still retain the TLV memory, freeing it and then reallocating it if needed. However, this is a debatable point because in some scenarios, it is permissible for the LSP to have empty TLVs. For example, after receiving an SNP (Sequence Number PDU) message, an empty LSP (with lsp->hdr.seqno = 0) might be created by calling lsp_new. If the corresponding LSP message is discarded due to domain or area authentication failure, the TLV memory wouldn't be allocated.
Test scenario:
In an LFA network, importing a sufficient number of static routes to cause LSP fragmentation, and then rolling back the imported static routes so that the LSP is no longer fragmented, can easily result in this issue.
Use `vtysh` with this input file:
```
ip route A nh1
ip route A nh2
ip route B nh1
ip route B nh2
```
When running "ip route B" with "nh1" and "nh2", the procedure maybe is:
1) Create the two nexthops: "nh1" and "nh2".
2) Register "nh1" with `static_zebra_nht_register()`, then the states of both
"nh1" and "nht2" are set to "STATIC_SENT_TO_ZEBRA".
3) Register "nh2" with `static_zebra_nht_register()`, then only the routes with
nexthop of "STATIC_START" will be sent to zebra.
So, send the routes with the nexthop of "STATIC_SENT_TO_ZEBRA" to zebra.
Rajasekar Raja [Fri, 5 Jul 2024 23:02:12 +0000 (16:02 -0700)]
zebra: Fix to avoid two Vrfs with same table ids
During internal testing, when the following sequence is followed, two
non default vrfs end up pointing to the same table-id
- Initially vrf201 has table id 1002
- ip link add dev vrf202 type vrf table 1002
- ip link set dev vrf202 up
- ip link set dev <intrerface> master vrf202
This will ideally lead to zebra exit since this is a misconfiguration as
expected.
However if we perform a restart frr.service at this point, we end up
having two vrfs pointing to same table-id and bad things can happen.
This is because in the interface_vrf_change, we incorrectly check for
vrf_lookup_by_id() to evaluate if there is a misconfig. This works well
for a non restart case but not for the startup case.
root@mlx-3700-20:mgmt:/var/log/frr# sudo vtysh -c "sh vrf"
vrf mgmt id 37 table 1001
vrf vrf201 id 46 table 1002
vrf vrf202 id 59 table 1002 >>>>
Fix: in all cases of misconfiguration, exit zebra as expected.
Signed-off-by: Donald Sharp <sharpd@nvidia.com> Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
(cherry picked from commit c77e15710d6a3a9be71f41a9ce608f06b2795dfb)
anlan_cs [Sat, 15 Jun 2024 12:34:20 +0000 (20:34 +0800)]
ldpd: fix wrong gtsm count
In linux networking stack, the received mpls packets will be processed
by the host *twice*, one as mpls packet, the other as ip packet, so
its ttl decreased 1.
So, we need release the `IP_MINTTL` value if gtsm is enabled, it is for the
mpls packets of neighbor session caused by the command:
`label local advertise explicit-null`.
This change makes the gtsm mechanism a bit deviation.
zhou-run [Thu, 27 Jun 2024 03:51:02 +0000 (11:51 +0800)]
isisd: fix crash when obtaining the next hop to calculate LFA on LAN links
When a neighbor connection is disconnected, it may trigger LSP re-generation as a timer task, but this process may be delayed. As a result, the list of neighbors in area->adjacency_list may be inconsistent with the neighbors in lsp->tlvs->oldstyle_reach/extended_reach. For example, the area->adjacency_list may lack certain neighbors even though they are present in the LSP. When computing SPF, the call to isis_spf_build_adj_list() generates the spftree->sadj_list, which reflects the real neighbors in the area->adjacency_list. However, in the case of LAN links, spftree->sadj_list may include additional pseudo neighbors.
The pre-loading of tents through the call to isis_spf_preload_tent involves two steps:
1. isis_spf_process_lsp() is called to generate real neighbor vertices based on the root LSP and pseudo LSP.
2. isis_spf_add_local() is called to add corresponding next hops to the vertex->Adj_N list for the real neighbor vertices.
In the case of LAN links, the absence of corresponding real neighbors in the spftree->sadj_list prevents the execution of the second step. Consequently, the vertex->Adj_N list for the real neighbor vertices lacks corresponding next hops. This leads to a null pointer access when isis_lfa_compute() is called to calculate LFA.
As for P2P links, since there are no pseudo neighbors, only the second step is executed, which does not create real neighbor vertices and therefore does not encounter this issue.
The backtrace is as follows:
(gdb) bt
#0 0x00007fd065277fe1 in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
#1 0x00007fd065398972 in core_handler (signo=11, siginfo=0x7ffc5c0636b0, context=0x7ffc5c063580) at ../lib/sigevent.c:261
#2 <signal handler called>
#3 0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
#4 0x00005564d82f8d78 in isis_spf_run_lfa (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_lfa.c:2344
#5 0x00005564d8315964 in isis_run_spf_with_protection (area=0x5564d8b143f0, spftree=0x5564d8b06bf0) at ../isisd/isis_spf.c:1827
#6 0x00005564d8315c15 in isis_run_spf_cb (thread=0x7ffc5c064590) at ../isisd/isis_spf.c:1889
#7 0x00007fd0653b1f04 in thread_call (thread=0x7ffc5c064590) at ../lib/thread.c:1990
#8 0x00007fd06534a97b in frr_run (master=0x5564d88103c0) at ../lib/libfrr.c:1198
#9 0x00005564d82e7d5d in main (argc=5, argv=0x7ffc5c0647b8, envp=0x7ffc5c0647e8) at ../isisd/isis_main.c:273
(gdb) f 3
#3 0x00005564d82f8408 in isis_lfa_compute (area=0x5564d8b143f0, circuit=0x5564d8b21d10, spftree=0x5564d8b06bf0, resource=0x7ffc5c064410) at ../isisd/isis_lfa.c:2134
2134 ../isisd/isis_lfa.c: No such file or directory.
(gdb) p vadj_primary
$1 = (struct isis_vertex_adj *) 0x0
(gdb) p vertex->Adj_N->head
$2 = (struct listnode *) 0x0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->data
$8 = (struct isis_vertex *) 0x5564d8b5b240
(gdb) p $8->type
$9 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $8->N.id
$10 = "\000\000\000\000\000\002"
(gdb) p $8->Adj_N->count
$11 = 0
(gdb) p (struct isis_vertex *)spftree->paths->l.list->head->next->next->next->next->next->data
$12 = (struct isis_vertex *) 0x5564d8b73dd0
(gdb) p $12->type
$13 = VTYPE_NONPSEUDO_TE_IS
(gdb) p $12->N.id
$14 = "\000\000\000\000\000\003"
(gdb) p $12->Adj_N->count
$15 = 0
(gdb) p area->adjacency_list->count
$16 = 0
The backtrace provided above pertains to version 8.5.4, but it seems that the same issue exists in the code of the master branch as well.
The scenario where a vertex has no next hop is normal. For example, the "clear isis neighbor" command invokes isis_vertex_adj_del() to delete the next hop of a vertex. Upon reviewing all the instances where the vertex->Adj_N list is used, I found that only isis_lfa_compute() lacks a null check. Therefore, I believe that modifying this part will be sufficient. Additionally, the vertex->parents list for IP vertices is guaranteed not to be empty.
Test scenario:
Setting up LFA for LAN links and executing the "clear isis neighbor" command easily reproduces the issue.
isisd: Fix memory leaks when the transition of neighbor state from non-UP to DOWN
When receiving a hello packet, if the neighbor state transitions directly from a non-ISIS_ADJ_UP state (such as ISIS_ADJ_INITIALIZING) to ISIS_ADJ_DOWN state, the neighbor entry cannot be deleted. If the neighbor is removed or the neighbor's System ID changes, it may result in memory leakage in the neighbor entry.
Test Scenario:
LAN link between Router A and Router B is established. Router A does not configure neighbor authentication, while Router B is configured with neighbor authentication. When the neighbor entry on Router B ages out, the neighbor state on Router A transitions to INIT. If Router B is then removed, the neighbor state on Router A transitions to DOWN and persists.
Donatas Abraitis [Thu, 13 Jun 2024 06:00:21 +0000 (09:00 +0300)]
bgpd: Check if we have real stream data for tunnel encapsulation sub-tlvs
When the packet is malformed it can use whatever values it wants. Let's check
what the real data we have in a stream instead of relying on malformed values.
Donatas Abraitis [Mon, 24 Jun 2024 17:16:16 +0000 (20:16 +0300)]
bgpd: Relax OAD (One-Administration-Domain) for RFC8212
RFC 8212 defines leak prevention for eBGP peers, but BGP-OAD defines a new
peering type One Administrative Domain (OAD), where multiple ASNs could be used
inside a single administrative domain. OAD allows sending non-transitive attributes,
so this prevention should be relaxed too.
Donatas Abraitis [Thu, 13 Jun 2024 05:12:10 +0000 (08:12 +0300)]
bgpd: Check if we have really enough data before doing memcpy for FQDN capability
We advance data pointer (data++), but we do memcpy() with the length that is 1-byte
over, which is technically heap overflow.
```
==411461==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50600011da1a at pc 0xc4f45a9786f0 bp 0xffffed1e2740 sp 0xffffed1e1f30
READ of size 4 at 0x50600011da1a thread T0
0 0xc4f45a9786ec in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x3586ec) (BuildId: e794c5f796eee20c8973d7efb9bf5735e54d44cd)
1 0xc4f45abf15f8 in bgp_dynamic_capability_fqdn /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3457:4
2 0xc4f45abdd408 in bgp_capability_msg_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3911:4
3 0xc4f45abdbeb4 in bgp_capability_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3980:9
4 0xc4f45abde2cc in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4109:11
5 0xc4f45a9b6110 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3
```
Donatas Abraitis [Wed, 12 Jun 2024 19:54:45 +0000 (22:54 +0300)]
bgpd: Check if we have really enough data before doing memcpy for software version
If we receive CAPABILITY message (software-version), we SHOULD check if we really
have enough data before doing memcpy(), that could also lead to buffer overflow.
(data + len > end) is not enough, because after this check we do data++ and later
memcpy(..., data, len). That means we have one more byte.
Hit this through fuzzing by
```
0 0xaaaaaadf872c in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x35872c) (BuildId: 9c6e455d0d9a20f5a4d2f035b443f50add9564d7)
1 0xaaaaab06bfbc in bgp_dynamic_capability_software_version /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3713:3
2 0xaaaaab05ccb4 in bgp_capability_msg_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3839:4
3 0xaaaaab05c074 in bgp_capability_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3980:9
4 0xaaaaab05e48c in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4109:11
5 0xaaaaaae36150 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3
```
projects / mirror / frr.git / log