Donald Sharp [Fri, 14 Feb 2025 12:55:09 +0000 (07:55 -0500)]
bgpd: When removing the prefix list drop the pointer
We are very very rarely seeing this crash:
0 0x7f36ba48e389 in prefix_list_apply_ext lib/plist.c:789
1 0x55eff3fa4126 in subgroup_announce_check bgpd/bgp_route.c:2334
2 0x55eff3fa858e in subgroup_process_announce_selected bgpd/bgp_route.c:3440
3 0x55eff4016488 in subgroup_announce_table bgpd/bgp_updgrp_adv.c:808
4 0x55eff401664e in subgroup_announce_route bgpd/bgp_updgrp_adv.c:861
5 0x55eff40111df in peer_af_announce_route bgpd/bgp_updgrp.c:2223
6 0x55eff3f884cb in bgp_announce_route_timer_expired bgpd/bgp_route.c:5892
7 0x7f36ba4ec239 in event_call lib/event.c:2019
8 0x7f36ba41a22a in frr_run lib/libfrr.c:1295
9 0x55eff3e668b7 in main bgpd/bgp_main.c:557
10 0x7f36b9e2d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
11 0x7f36b9e2d304 in __libc_start_main_impl ../csu/libc-start.c:360
12 0x55eff3e64a30 in _start (/home/ci/cibuild.1407/frr-source/bgpd/.libs/bgpd+0x2fda30)
0x608000037038 is located 24 bytes inside of 88-byte region [0x608000037020,0x608000037078)
freed by thread T0 here:
0 0x7f36ba8b76a8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
1 0x7f36ba439bd7 in qfree lib/memory.c:131
2 0x7f36ba48d3a3 in prefix_list_free lib/plist.c:156
3 0x7f36ba48d3a3 in prefix_list_delete lib/plist.c:247
4 0x7f36ba48fbef in prefix_bgp_orf_remove_all lib/plist.c:1516
5 0x55eff3f679c4 in bgp_route_refresh_receive bgpd/bgp_packet.c:2841
6 0x55eff3f70bab in bgp_process_packet bgpd/bgp_packet.c:4069
7 0x7f36ba4ec239 in event_call lib/event.c:2019
8 0x7f36ba41a22a in frr_run lib/libfrr.c:1295
9 0x55eff3e668b7 in main bgpd/bgp_main.c:557
10 0x7f36b9e2d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
previously allocated by thread T0 here:
0 0x7f36ba8b83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
1 0x7f36ba4392e4 in qcalloc lib/memory.c:106
2 0x7f36ba48d0de in prefix_list_new lib/plist.c:150
3 0x7f36ba48d0de in prefix_list_insert lib/plist.c:186
4 0x7f36ba48d0de in prefix_list_get lib/plist.c:204
5 0x7f36ba48f9df in prefix_bgp_orf_set lib/plist.c:1479
6 0x55eff3f67ba6 in bgp_route_refresh_receive bgpd/bgp_packet.c:2920
7 0x55eff3f70bab in bgp_process_packet bgpd/bgp_packet.c:4069
8 0x7f36ba4ec239 in event_call lib/event.c:2019
9 0x7f36ba41a22a in frr_run lib/libfrr.c:1295
10 0x55eff3e668b7 in main bgpd/bgp_main.c:557
11 0x7f36b9e2d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Let's just stop trying to save the pointer around in the peer->orf_plist
data structure. There are other design problems but at least lets
stop the crash from possibly happening.
David Lamparter [Fri, 7 Feb 2025 12:22:25 +0000 (13:22 +0100)]
lib: crash handlers must be allowed on threads
Blocking all signals on non-main threads is not the way to go, at least
the handlers for SIGSEGV, SIGBUS, SIGILL, SIGABRT and SIGFPE need to run
so we get backtraces. Otherwise the process just exits.
David Lamparter [Wed, 22 Jan 2025 10:19:04 +0000 (11:19 +0100)]
lib: guard against padding garbage in ZAPI read
When reading in a nexthop from ZAPI, only set the fields that actually
have meaning. While it shouldn't happen to begin with, we can otherwise
carry padding garbage into the unused leftover union bytes.
David Lamparter [Wed, 22 Jan 2025 10:17:21 +0000 (11:17 +0100)]
zebra: guard against junk in nexthop->rmap_src
rmap_src wasn't initialized, so for IPv4 the unused 12 bytes would
contain whatever junk is on the stack on function entry. Also move
the IPv4 parse before the IPv6 parse so if it's successful we can be
sure the other bytes haven't been touched.
David Lamparter [Wed, 22 Jan 2025 10:13:21 +0000 (11:13 +0100)]
bgpd: don't reuse nexthop variable in loop/switch
While the loop is currently exited in all cases after using nexthop, it
is a footgun to have "nh" around to be reused in another iteration of
the loop. This would leave nexthop with partial data from the previous
use. Make it local where needed instead.
Donald Sharp [Mon, 27 Jan 2025 15:34:31 +0000 (10:34 -0500)]
tests: Add a test that shows the v6 recursive nexthop problem
Currently FRR does not handle v6 recurisive resolution properly
when the route being recursed through changes and the most
significant bits of the route are not changed.
Louis Scalbert [Thu, 9 Jan 2025 17:28:53 +0000 (18:28 +0100)]
bgpd: fix crash in displaying json orf prefix-list
bgpd crashes when there is several entries in the prefix-list. No
backtrace is provided because the issue was catched from a code review.
Fixes: 856ca177c4 ("Added json formating support to show-...-neighbors-... bgp commands.") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit 8ccf60921b85893d301186a0f8156fb702da379f)
Louis Scalbert [Thu, 9 Jan 2025 17:24:39 +0000 (18:24 +0100)]
bgpd: fix bgp orf prefix-list json prefix
0x<address>FX was displayed instead of the prefix.
Fixes: b219dda129 ("lib: Convert usage of strings to %pFX and %pRN") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit b7e843d7e8afe57d3815dbb44e30307654e73711)
- bgpd
- Fix for CVE-2024-44070
- Fix crash at no rpki
- isisd
- Fix update link params after circuit is up
- tools
- Ignore errors for frr reload stuff
Igor Zhukov [Fri, 4 Oct 2024 06:16:02 +0000 (13:16 +0700)]
zebra: Fix crash during reconnect
fpm_enqueue_rmac_table expects an fpm_rmac_arg* as its argument.
The issue can be reproduced by dropping the TCP session using:
ss -K dst 127.0.0.1 dport = 2620
I used Fedora 40 and frr 9.1.2 and I got the gdb backtrace:
(gdb) bt
0 0x00007fdd7d6997ea in fpm_enqueue_rmac_table (bucket=0x2134dd0, arg=0x2132b60) at zebra/dplane_fpm_nl.c:1217
1 0x00007fdd7dd1560d in hash_iterate (hash=0x21335f0, func=0x7fdd7d6997a0 <fpm_enqueue_rmac_table>, arg=0x2132b60) at lib/hash.c:252
2 0x00007fdd7dd1560d in hash_iterate (hash=0x1e5bf10, func=func@entry=0x7fdd7d698900 <fpm_enqueue_l3vni_table>,
arg=arg@entry=0x7ffed983bef0) at lib/hash.c:252
3 0x00007fdd7d698b5c in fpm_rmac_send (t=<optimized out>) at zebra/dplane_fpm_nl.c:1262
4 0x00007fdd7dd6ce22 in event_call (thread=thread@entry=0x7ffed983c010) at lib/event.c:1970
5 0x00007fdd7dd20758 in frr_run (master=0x1d27f10) at lib/libfrr.c:1213
6 0x0000000000425588 in main (argc=10, argv=0x7ffed983c2e8) at zebra/main.c:492
Donald Sharp [Wed, 25 Sep 2024 16:09:40 +0000 (12:09 -0400)]
zebra: Correctly report metrics
Report the routes metric in IPFORWARDMETRIC1 and return
-1 for the other metrics as required by the IP-FORWARD-MIB.
inetCidrRouteMetric2 OBJECT-TYPE
SYNTAX Integer32
MAX-ACCESS read-create
STATUS current
DESCRIPTION
"An alternate routing metric for this route. The
semantics of this metric are determined by the routing-
protocol specified in the route's inetCidrRouteProto
value. If this metric is not used, its value should be
set to -1."
DEFVAL { -1 }
::= { inetCidrRouteEntry 13 }
I've included metric2 but it's the same for all of them.
Donald Sharp [Wed, 25 Sep 2024 16:06:29 +0000 (12:06 -0400)]
zebra: Fix snmp walk of zebra rib
The snmp walk of the zebra rib was skipping entries
because in_addr_cmp was replaced with a prefix_cmp
which worked slightly differently causing parts
of the zebra rib tree to be skipped.
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 [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
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().
bgpd
Check mandatory attributes more carefully for update message
Do not explicitly print maxttl value for ebgp-multihop vty output
Do not process nlris if the attribute length is zero
Don't read the first byte of orf header if we are ahead of stream
Ensure community data is freed in some cases.
Ensure that the correct aspath is free'd
Evpn code was not properly unlocking rd_dest
Fix error handling when receiving bgp prefix sid attribute
Fix null argument warning
Fix session reset issue caused by malformed core attributes
Fix use beyond end of stream of labeled unicast parsing
Handle mp_reach_nlri malformed packets with session reset
Ignore handling nlris if we received mp_unreach_nlri
Include unsuppress-map as a valid outgoing policy
Prevent from one more cve triggering this place
Treat eor as withdrawn to avoid unwanted handling of malformed attrs
Use enum bgp_create_error_code as argument in header
Use treat-as-withdraw for tunnel encapsulation attribute
isisd
Fix heap-after-free with prefix sid
need to link directly against libyang
lib
Fix evpn nexthop config order
Allow unsetting walltime-warning and cpu-warning
Make cmd_element->attr a bitmask & clarify
Replace deprecated ares_gethostbyname
Replace deprecated ares_process()
nhrpd
Fix nhrp_peer leak
Fix core dump on shutdown
ospf6d
Fix crash because neighbor structure was freed
Fix uninitialized warnings
Ospfv3 route change comparision fixed for asbr-only change
Stop crash in ospf6_write
Prevent heap-buffer-overflow with unknown type
ospfd
Check for nulls in vty code
Correct opaque lsa extended parser
Prevent use after free( and crash of ospf ) when no router ospf
Protect call to get_edge() in ospf_te.c
Solved crash in ospf te parsing
Solved crash in ri parsing with ospf te
pimd
Fix dr-priority range
Fix null register before aging out reg-stop
Fix order of operations for evaluating join
Re-evaluated s,g oils upon rp changes and for empty sg upstream oils
Fix crash when mixing ssm/any-source joins
ripd
Revert "cleanup memory allocations on shutdown"
ripngd
Revert "cleanup memory allocations on shutdown"
vtysh
Print uniq lines when parsing `no service ...`
zebra
Deny the routes if ip protocol cli refers to an undefined rmap
Fix connected route deletion when multiple entry exists
Keelan10 [Wed, 5 Jun 2024 10:34:18 +0000 (13:34 +0300)]
nhrpd: Fix nhrp_peer leak
- Addressed memory leak by removing `&c->peer_notifier` from the notifier list on termination. Retaining it caused the notifier list to stay active, preventing the deletion of `c->cur.peer`
thereby causing a memory leak.
- Reordered termination steps to call `vrf_terminate` before `nhrp_vc_terminate`, preventing a heap-use-after-free issue when `nhrp_vc_notify_del` is invoked in `nhrp_peer_check_delete`.
- Added an if statement to avoid passing NULL as hash to `hash_release`, which leads to a SIGSEGV.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r1.asan.nhrpd.20265
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x7f80270c9b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f8026ac1eb8 in qmalloc lib/memory.c:100
#2 0x560fd648f0a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
#3 0x7f8026a88d3f in hash_get lib/hash.c:147
#4 0x560fd6490a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
#5 0x560fd648a51a in nhrp_nhs_resolve_cb nhrpd/nhrp_nhs.c:297
#6 0x7f80266b000f in resolver_cb_literal lib/resolver.c:234
#7 0x7f8026b62e0e in event_call lib/event.c:1969
#8 0x7f8026aa5437 in frr_run lib/libfrr.c:1213
#9 0x560fd6488b4f in main nhrpd/nhrp_main.c:166
#10 0x7f8025eb2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
***********************************************************************************
Address Sanitizer Error detected in nhrp_topo.test_nhrp_topo/r2.asan.nhrpd.20400
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x7fb6e3ca5b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7fb6e369deb8 in qmalloc lib/memory.c:100
#2 0x562652de40a6 in nhrp_peer_create nhrpd/nhrp_peer.c:175
#3 0x7fb6e3664d3f in hash_get lib/hash.c:147
#4 0x562652de5a5d in nhrp_peer_get nhrpd/nhrp_peer.c:228
#5 0x562652de1e8e in nhrp_packet_recvraw nhrpd/nhrp_packet.c:325
#6 0x7fb6e373ee0e in event_call lib/event.c:1969
#7 0x7fb6e3681437 in frr_run lib/libfrr.c:1213
#8 0x562652dddb4f in main nhrpd/nhrp_main.c:166
#9 0x7fb6e2a8ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 112 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```
Iggy Frankovic [Thu, 30 May 2024 11:59:54 +0000 (07:59 -0400)]
ospf6d: Prevent heap-buffer-overflow with unknown type
When parsing a osf6 grace lsa field and we receive an
unknown tlv type, ospf6d was not incrementing the pointer
to get beyond the tlv. Leaving a situation where ospf6d
would parse the packet incorrectly.
There is no reason to call `igmp_anysource_forward_stop()` inside a call to
`igmp_get_source_by_addr()`; not only it is not expected for a "get" function
to perform such an action, but also the decision to start/stop forwarding is
already handled correctly by pim outside `igmp_get_source_by_addr()`.
That call was left there from the days pim was initially imported into the sources.
The problem/crash was happening because `igmp_find_source_by_addr()` would fail to
find the group/source combo when mixing `(*, G)` and `(S, G)`. When having an existing
flow `(*, G)`, and a new `(S, G)` igmp is received, a new entry is correctly created.
`igmp_anysource_forward_stop(group)` always stops and eventually frees `(*, G)`, even
when the new igmp is `(S, G)`, leaving a bad state. I.e, the new entry for `(S, G)`
causes `(*, G)` to be deleted.
Tested the fix with multiple receivers on the same interface with several ssm and
any source senders and receivers with various combination of start/stop orders and
they all worked correctly.
Acee [Tue, 28 May 2024 14:02:27 +0000 (10:02 -0400)]
ospf6d: OSPFv3 route change comparision fixed for ASBR-only change
When a router route already exists in the area border routers table
as an ABR and it solely changes its ABR or ASBR status, the change
was missed and border route is not updated. This fixes the comparison
for the router_bits in the ospf6_path structure.
This fixes issue https://github.com/FRRouting/frr/issues/16053 although
the actual problem is not the computing router (r2) and not the OSPFv3
redistribution (r3).
During fuzzing, Iggy Frankovic discovered that get_edge() function in ospf_te.c
could return null pointer, in particular when the link_id or advertised router
IP addresses are fuzzed. As the null pointer returned by get_edge() function is
not handlei by calling functions, this could cause ospfd crash.
This patch introduces new verification of returned pointer by get_edge()
function and stop the processing in case of null pointer. In addition, link ID
and advertiser router ID are validated before calling ls_find_edge_by_key() to
avoid the creation of a new edge with an invalid key.
Iggy Frankovic discovered another ospfd crash when performing fuzzing of OSPF
LSA packets. The crash occurs in ospf_te_parse_ext_link() function when
attemping to read Segment Routing Adjacency SID subTLVs. The original code
doesn't check if the size of the Extended Link TLVs and subTLVs have the correct
length. In presence of erronous LSA, this will cause a buffer overflow and ospfd
crashes.
This patch introduces new verification of the subTLVs size for Extended Link
TLVs and subTLVs. Similar check has been also introduced for the Extended
Prefix TLV.
Iggy Frankovic discovered another ospfd crash when performing fuzzing of OSPF
LSA packets. The crash occurs in ospf_te_parse_ri() function when attemping to
read Segment Routing subTLVs. The original code doesn't check if the size of
the SR subTLVs have the correct length. In presence of erronous LSA, this will
cause a buffer overflow and ospfd crash.
This patch introduces new verification of the subTLVs size for Router
Information TLV.
Louis Scalbert [Thu, 16 May 2024 14:44:03 +0000 (16:44 +0200)]
isisd: fix heap-after-free with prefix sid
> ==2334217==ERROR: AddressSanitizer: heap-use-after-free on address 0x61000001d0a0 at pc 0x563828c8de6f bp 0x7fffbdaee560 sp 0x7fffbdaee558
> READ of size 1 at 0x61000001d0a0 thread T0
> #0 0x563828c8de6e in prefix_sid_cmp isisd/isis_spf.c:187
> #1 0x7f84b8204f71 in hash_get lib/hash.c:142
> #2 0x7f84b82055ec in hash_lookup lib/hash.c:184
> #3 0x563828c8e185 in isis_spf_prefix_sid_lookup isisd/isis_spf.c:209
> #4 0x563828c90642 in isis_spf_add2tent isisd/isis_spf.c:598
> #5 0x563828c91cd0 in process_N isisd/isis_spf.c:824
> #6 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
> #7 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
> #8 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
> #9 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
> #10 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
> #11 0x7f84b835c72d in event_call lib/event.c:2011
> #12 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
> #13 0x563828c21918 in main isisd/isis_main.c:346
> #14 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
> #15 0x563828c20df9 in _start (/usr/lib/frr/isisd+0xf5df9)
>
> 0x61000001d0a0 is located 96 bytes inside of 184-byte region [0x61000001d040,0x61000001d0f8)
> freed by thread T0 here:
> #0 0x7f84b88a9b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
> #1 0x7f84b8263bae in qfree lib/memory.c:130
> #2 0x563828c8e433 in isis_vertex_del isisd/isis_spf.c:249
> #3 0x563828c91c95 in process_N isisd/isis_spf.c:811
> #4 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
> #5 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
> #6 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
> #7 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
> #8 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
> #9 0x7f84b835c72d in event_call lib/event.c:2011
> #10 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
> #11 0x563828c21918 in main isisd/isis_main.c:346
> #12 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> previously allocated by thread T0 here:
> #0 0x7f84b88aa037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
> #1 0x7f84b8263a6c in qcalloc lib/memory.c:105
> #2 0x563828c8e262 in isis_vertex_new isisd/isis_spf.c:225
> #3 0x563828c904db in isis_spf_add2tent isisd/isis_spf.c:588
> #4 0x563828c91cd0 in process_N isisd/isis_spf.c:824
> #5 0x563828c93852 in isis_spf_process_lsp isisd/isis_spf.c:1041
> #6 0x563828c98dde in isis_spf_loop isisd/isis_spf.c:1821
> #7 0x563828c998de in isis_run_spf isisd/isis_spf.c:1983
> #8 0x563828c99c7b in isis_run_spf_with_protection isisd/isis_spf.c:2009
> #9 0x563828c9a60d in isis_run_spf_cb isisd/isis_spf.c:2090
> #10 0x7f84b835c72d in event_call lib/event.c:2011
> #11 0x7f84b8236d93 in frr_run lib/libfrr.c:1217
> #12 0x563828c21918 in main isisd/isis_main.c:346
> #13 0x7f84b7e4fd09 in __libc_start_main ../csu/libc-start.c:308
>
> SUMMARY: AddressSanitizer: heap-use-after-free isisd/isis_spf.c:187 in prefix_sid_cmp
> Shadow bytes around the buggy address:
> 0x0c207fffb9c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
> 0x0c207fffb9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
> 0x0c207fffb9e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
> 0x0c207fffb9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
> 0x0c207fffba00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
> =>0x0c207fffba10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fa
> 0x0c207fffba20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
> 0x0c207fffba30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
> 0x0c207fffba40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
> 0x0c207fffba50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
> 0x0c207fffba60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> Shadow gap: cc
> ==2334217==ABORTING
Fixes: 2f7cc7bcd3 ("isisd: detect Prefix-SID collisions and handle them appropriately") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e697de58431474cdb06eff79bcbc70de4215e222)
zebra: Deny the routes if ip protocol CLI refers to an undefined rmap
Currently zebra does not deny the routes if `ip protocol <proto> route-map
FOO`
commmand is configured with reference to an undefined route-map (FOO in
this case).
However, on FRR restart, in zebra_route_map_check() routes get denied
if route-map name is available but the route-map is not defined. This
change was introduced in fd303a4ba14c762550db972317e1e88528768005.
Fix:
When `ip protocol <proto> route-map FOO` CLI is configured with reference to an
undefined route-map FOO, let the processing in ip_protocol_rm_add() and
ip_protocol_rm_del() go through so that zebra can deny the routes instead
of simply returning. This will result in consistent behavior.
Testing Done:
Before fix:
```
spine-1# configure
spine-1(config)# ip protocol bgp route-map rmap7
spine-1(config)# do show ip route
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, A - Babel, D - SHARP, F - PBR, f - OpenFabric,
Z - FRR,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
C>* 27.0.0.1/32 is directly connected, lo, 02:27:45
B>* 27.0.0.3/32 [20/0] via fe80::202:ff:fe00:21, downlink_1, weight 1, 02:27:35
B>* 27.0.0.4/32 [20/0] via fe80::202:ff:fe00:29, downlink_2, weight 1, 02:27:40
B>* 27.0.0.5/32 [20/0] via fe80::202:ff:fe00:31, downlink_3, weight 1, 02:27:40
B>* 27.0.0.6/32 [20/0] via fe80::202:ff:fe00:39, downlink_4, weight 1, 02:27:40
```
After fix:
```
spine-1(config)# ip protocol bgp route-map route-map67
spine-1(config)# do show ip route
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, A - Babel, D - SHARP, F - PBR, f - OpenFabric,
Z - FRR,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
C>* 27.0.0.1/32 is directly connected, lo, 00:35:03
B 27.0.0.3/32 [20/0] via fe80::202:ff:fe00:21, downlink_1 inactive, weight 1, 00:34:58
B 27.0.0.4/32 [20/0] via fe80::202:ff:fe00:29, downlink_2 inactive, weight 1, 00:34:57
B 27.0.0.5/32 [20/0] via fe80::202:ff:fe00:31, downlink_3 inactive, weight 1, 00:34:57
B 27.0.0.6/32 [20/0] via fe80::202:ff:fe00:39, downlink_4 inactive, weight 1, 00:34:58
spine-1(config)#
root@spine-1:mgmt:/var/home/cumulus# ip route show
root@spine-1:mgmt:/var/home/cumulus#
```
David Lamparter [Mon, 12 Dec 2022 16:50:59 +0000 (17:50 +0100)]
pimd: fix order of operations for evaluating join
join_desired looks at whether up->channel_oil is empty. up->channel_oil
is updated from pim_forward_stop(), calling pim_channel_del_oif(). But
that was being called *after* updating join_desired, so join_desired saw
a non-empty OIL. Pull up the pim_forward_stop() call to before updating
join_desired.
David Lamparter [Mon, 17 Apr 2023 09:47:08 +0000 (11:47 +0200)]
pimd: fix null register before aging out reg-stop
It looks like the code was trying to do this with the null_register
parameter on pim_upstream_start_register_stop_timer(), but that didn't
quite work right. Restructure a bit to get it right.
Donatas Abraitis [Wed, 27 Mar 2024 17:08:38 +0000 (19:08 +0200)]
bgpd: Prevent from one more CVE triggering this place
If we receive an attribute that is handled by bgp_attr_malformed(), use
treat-as-withdraw behavior for unknown (or missing to add - if new) attributes.
Donatas Abraitis [Wed, 27 Mar 2024 16:42:56 +0000 (18:42 +0200)]
bgpd: Fix error handling when receiving BGP Prefix SID attribute
Without this patch, we always set the BGP Prefix SID attribute flag without
checking if it's malformed or not. RFC8669 says that this attribute MUST be discarded.
Also, this fixes the bgpd crash when a malformed Prefix SID attribute is received,
with malformed transitive flags and/or TLVs.
Olivier Dugeon [Mon, 26 Feb 2024 09:40:34 +0000 (10:40 +0100)]
ospfd: Solved crash in OSPF TE parsing
Iggy Frankovic discovered an ospfd crash when perfomring fuzzing of OSPF LSA
packets. The crash occurs in ospf_te_parse_te() function when attemping to
create corresponding egde from TE Link parameters. If there is no local
address, an edge is created but without any attributes. During parsing, the
function try to access to this attribute fields which has not been created
causing an ospfd crash.
The patch simply check if the te parser has found a valid local address. If not
found, we stop the parser which avoid the crash.
Donatas Abraitis [Sun, 29 Oct 2023 20:44:45 +0000 (22:44 +0200)]
bgpd: Ignore handling NLRIs if we received MP_UNREACH_NLRI
If we receive MP_UNREACH_NLRI, we should stop handling remaining NLRIs if
no mandatory path attributes received.
In other words, if MP_UNREACH_NLRI received, the remaining NLRIs should be handled
as a new data, but without mandatory attributes, it's a malformed packet.
In normal case, this MUST not happen at all, but to avoid crashing bgpd, we MUST
handle that.
Donatas Abraitis [Fri, 27 Oct 2023 08:56:45 +0000 (11:56 +0300)]
bgpd: Treat EOR as withdrawn to avoid unwanted handling of malformed attrs
Treat-as-withdraw, otherwise if we just ignore it, we will pass it to be
processed as a normal UPDATE without mandatory attributes, that could lead
to harmful behavior. In this case, a crash for route-maps with the configuration
such as:
Donatas Abraitis [Tue, 13 Jun 2023 13:08:24 +0000 (16:08 +0300)]
bgpd: Use enum bgp_create_error_code as argument in header
```
bgpd/bgp_vty.c:865:5: warning: conflicting types for ‘bgp_vty_return’ due to enum/integer mismatch; have ‘int(struct vty *, enum bgp_create_error_code)’ [-Wenum-int-mismatch]
865 | int bgp_vty_return(struct vty *vty, enum bgp_create_error_code ret)
| ^~~~~~~~~~~~~~
In file included from ./bgpd/bgp_mplsvpn.h:15,
from bgpd/bgp_vty.c:48:
./bgpd/bgp_vty.h:148:12: note: previous declaration of ‘bgp_vty_return’ with type ‘int(struct vty *, int)’
148 | extern int bgp_vty_return(struct vty *vty, int ret);
| ^~~~~~~~~~~~~~
```
David Lamparter [Sat, 16 Sep 2023 12:17:24 +0000 (14:17 +0200)]
ospf6d: fix uninitialized warnings
GCC 13.2.0 complains:
```
ospf6d/ospf6_intra.c:139:25: error: ‘json_arr’ may be used uninitialized [-Werror=maybe-uninitialized]
ospf6d/ospf6_intra.c:485:20: error: ‘json_arr’ may be used uninitialized [-Werror=maybe-uninitialized]
```
Donatas Abraitis [Mon, 23 Oct 2023 20:34:10 +0000 (23:34 +0300)]
bgpd: Check mandatory attributes more carefully for UPDATE message
If we send a crafted BGP UPDATE message without mandatory attributes, we do
not check if the length of the path attributes is zero or not. We only check
if attr->flag is at least set or not. Imagine we send only unknown transit
attribute, then attr->flag is always 0. Also, this is true only if graceful-restart
capability is received.
Thread 1 "bgpd" received signal SIGSEGV, Segmentation fault.
0x00005555556e37be in route_set_aspath_prepend (rule=0x555555aac0d0, prefix=0x7fffffffe050,
object=0x7fffffffdb00) at bgpd/bgp_routemap.c:2282
2282 if (path->attr->aspath->refcnt)
(gdb)
```
Donald Sharp [Sat, 2 Mar 2024 14:50:38 +0000 (09:50 -0500)]
bgpd: Ensure community data is freed in some cases.
Customer has this valgrind trace:
Direct leak of 2829120 byte(s) in 70728 object(s) allocated from:
0 in community_new ../bgpd/bgp_community.c:39
1 in community_uniq_sort ../bgpd/bgp_community.c:170
2 in route_set_community ../bgpd/bgp_routemap.c:2342
3 in route_map_apply_ext ../lib/routemap.c:2673
4 in subgroup_announce_check ../bgpd/bgp_route.c:2367
5 in subgroup_process_announce_selected ../bgpd/bgp_route.c:2914
6 in group_announce_route_walkcb ../bgpd/bgp_updgrp_adv.c:199
7 in hash_walk ../lib/hash.c:285
8 in update_group_af_walk ../bgpd/bgp_updgrp.c:2061
9 in group_announce_route ../bgpd/bgp_updgrp_adv.c:1059
10 in bgp_process_main_one ../bgpd/bgp_route.c:3221
11 in bgp_process_wq ../bgpd/bgp_route.c:3221
12 in work_queue_run ../lib/workqueue.c:282
The above leak detected by valgrind was from a screenshot so I copied it
by hand. Any mistakes in line numbers are purely from my transcription.
Additionally this is against a slightly modified 8.5.1 version of FRR.
Code inspection of 8.5.1 -vs- latest master shows the same problem
exists. Code should be able to be followed from there to here.
What is happening:
There is a route-map being applied that modifes the outgoing community
to a peer. This is saved in the attr copy created in
subgroup_process_announce_selected. This community pointer is not
interned. So the community->refcount is still 0. Normally when
a prefix is announced, the attr and the prefix are placed on a
adjency out structure where the attribute is interned. This will
cause the community to be saved in the community hash list as well.
In a non-normal operation when the decision to send is aborted after
the route-map application, the attribute is just dropped and the
pointer to the community is just dropped too, leading to situations
where the memory is leaked. The usage of bgp suppress-fib would
would be a case where the community is caused to be leaked.
Additionally the previous commit where an unsuppress-map is used
to modify the outgoing attribute but since unsuppress-map was
not considered part of outgoing policy the attribute would be dropped as
well. This pointer drop also extends to any dynamically allocated
memory saved by the attribute pointer that was not interned yet as well.
So let's modify the return case where the decision is made to
not send the prefix to the peer to always just flush the attribute
to ensure memory is not leaked.
Fixes: #15459 Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Donald Sharp [Wed, 13 Mar 2024 14:26:58 +0000 (10:26 -0400)]
bgpd: Ensure that the correct aspath is free'd
Currently in subgroup_default_originate the attr.aspath
is set in bgp_attr_default_set, which hashs the aspath
and creates a refcount for it. If this is a withdraw
the subgroup_announce_check and bgp_adj_out_set_subgroup
is called which will intern the attribute. This will
cause the the attr.aspath to be set to a new value
finally at the bottom of the function it intentionally
uninterns the aspath which is not the one that was
created for this function. This reduces the other
aspath's refcount by 1 and if a clear bgp * is issued
fast enough the aspath for that will be removed
and the system will crash.
pimd: re-evaluated S,G OILs upon RP changes and for empty SG upstream oils
Topology:
TOR11 (FHR) --- LEAF-11---SPINE1 (RP)MSDP SPINE-2(RP)MSDP --- LEAF-12 -- TOR12 (LHR)
| | | | |
| -----------------------------------------------------(ECMP) |
| | | |
-----------------------------------------------------------------------(ECMP)
Issue:
In some triggers, S,G upstream is preserved even with the PP timer expiry, resulting
in S,G with NULL OILS. This could be because we create a dummy S,G upstream and
dummy channel_oif for *,G, where RPF is UNKNOWN. As a result, PIM+VXLAN traffic is never
forwarded downstream to LHR.
Fix:
when the S,G stream is running, Determine if a reevaluation of the outgoing interface
list (OIL) is required. S,G upstream should then inherit the OIL from *,G.
David Lamparter [Thu, 16 Mar 2023 10:00:02 +0000 (11:00 +0100)]
bgpd: fix NULL argument warning
gcc 12.2.0 complains `error: ‘%s’ directive argument is null`, even
though all enum values are covered with a string. Let's just go with a
`???` default.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
zebra: Fix connected route deletion when multiple entry exists
When multiple interfaces have addresses in the same network, deleting
one of them may cause the wrong connected route being deleted.
For example:
ip link add veth1 type veth peer veth2
ip link set veth1 up
ip link set veth2 up
ip addr add dev veth1 192.168.0.1/24
ip addr add dev veth2 192.168.0.2/24
ip addr flush dev veth1
Zebra deletes the route of interface veth2 rather than veth1.
Should match nexthop against ere->re_nhe instead of ere->re->nhe.
Donald Sharp [Wed, 30 Aug 2023 14:33:29 +0000 (10:33 -0400)]
ospfd: Prevent use after free( and crash of ospf ) when no router ospf
Consider this config:
router ospf
redistribute kernel
Then you issue:
no router ospf
ospf will crash with a use after free.
The problem is that the event's associated with the
ospf pointer were shut off then the ospf_external_delete
was called which rescheduled the event. Let's just move
event deletion to the end of the no router ospf.
projects / mirror / frr.git / log