Chirag Shah [Wed, 19 Jun 2024 00:21:49 +0000 (17:21 -0700)]
zebra: fix evpn mh bond member proto reinstall
In case of EVPN MH bond, a member port going in
protodown state due to external reason (one case being linkflap),
frr updates the state correctly but upon manually
clearing external reason trigger FRR to reinstate
protodown without any reason code.
Fix is to ensure if the protodown reason was external
and new state is to have protodown 'off' then do no reinstate
protodown.
Ticket: #3947432
Testing:
switch:#ip link show swp1
4: swp1: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 9216 qdisc
pfifo_fast master bond1 state DOWN mode DEFAULT group default qlen
1000
link/ether 1c:34:da:2c:aa:68 brd ff:ff:ff:ff:ff:ff protodown on
protodown_reason <linkflap>
switch:#ip link set swp1 protodown off protodown_reason linkflap off
switch:#ip link show swp1
4: swp1: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 9216 qdisc
pfifo_fast master bond1 state DOWN mode DEFAULT group default qlen
1000
link/ether 1c:34:da:2c:aa:68 brd ff:ff:ff:ff:ff:ff
bgpd
"default-originate" shouldn't withdraw non-default routes
Aggr summary-only suppressed export to evpn
Allow using optional table id for negative `no set table x` command
Arrange peer notification to after zebra announce
Check bgp evpn instance presence in soo
Convert the bgp_advertise_attr->adv to a fifo
Do not show tcp mss if the socket is broken
Ensure bgp does not stop monitoring nexthops
Ensure community data is freed in some cases.
Ensure that the correct aspath is free'd
Fix `match peer` when switching between ipv4/ipv6/interface
Fix `no set as-path prepend asnum...`
Fix bgp_best_selection heap-use-after-free
Fix crash when deleting the srv6 locator
Fix display when using `missing-as-worst`
Fix dynamic peer graceful restart race condition
Fix ecommunity_fill_pbr_action heap-buffer-overflow
Fix error handling when receiving bgp prefix sid attribute
Fix errors handling for mp/gr capabilities as dynamic capability
Fix format overflow for graceful-restart debug logs
Fix logging message when receiving a software version capability
Fix no bgp as-path access-list issue
Fix route-map match probability deconfiguration callback
Fix srv6 memory leak detection
Fix the order of null check and zapi decode
Fix vrf leaking with 'no bgp network import-check
Free memory for srv6 functions and locator chunks
Ignore validating the attribute flags if path-attribute is configured
Include unsuppress-map as a valid outgoing policy
Lttng tp add evpn route events
Make `suppress-fib-pending` clear peering
Note when receiving but not understanding a route notification
Prevent from one more cve triggering this place
Set correct ttl for the dynamic neighbor peers
Update default-originate route-map actual map structure
Revert "Fix pointer arithmetic in bgp snmp module"
doc
Add param range for graceful-restart helper supported-grace-time
Remove duplicated show route-map
isisd
Fix _isis_spftree_del heap-use-after-free
Fix dislaying lsp id
Fix heap-after-free with prefix sid
Fix ip/ipv6 reachability tlvs
lib
Check for not being a blackhole route
Fix show route map json output
Do not convert evpn prefixes into ipv4/ipv6 if not needed
Replace deprecated ares_gethostbyname
Replace deprecated ares_process()
ospf6d
Ospfv3 route change comparision fixed for asbr-only change
Prevent heap-buffer-overflow with unknown type
ospfd
Add support for "no router-info [<area|as>] command"
Can not delete "segment-routing node-msd" when sr if off
Correct lsa parser which fulfill the ted
Correct opaque lsa extended parser
Correct sid check size
Fix ospf dead-interval minimal hello-multiplier param range
Fix the bug where ip_ospf_dead-interval_minimal_hello-multiplier did not reset hello timer
Protect call to get_edge() in ospf_te.c
Solved crash in ospf te parsing
Solved crash in ri parsing with ospf te
Revert "Fix some dicey pointer arith in snmp module"
pbrd
Fix map seq installed flag in json
Fix pbr handling for last rule deletion
pimd
Fix crash unconfiguring rp keepalive timer
Fix crash when configuring ssmpingd
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
staticd
Fix changing to source auto in bfd monitor
tests
Check for 0.0.0.0/1 in bgp_default_route
Check if ibgp session can drop invalid aigp attribute
Extend tests for aspath exclude
Update ospf te topotests
tools
Apply black formatting for tools/frr-reload.py
Fix frr-reload interface desc cmd
Fix frr-reload multiple no description cmds
Fix frr-reload multiple no description cmds
Use error log level when failing to execute commands via frr-reload.py
topotests
Do not check table version
Redispatch tests in bfd_topo3
Test wrong bfd source in bfd_topo3
Vpnv4 route leaking with no import-check
vtysh
Show `ip ospf network ...` even if it's not the same as the interface type
zebra
Add missing whitespace when printing route entry status
Deny the routes if ip protocol cli refers to an undefined rmap
Don't deref vxlan-vni array
Fix crash if macvlan link in another netns
Fix crash on macvlan link down/up
Fix evpn svd based remote nh neigh del
Fix mpls command
Fix route deletion during zebra shutdown
The dplane_fpm_nl return path leaks memory
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#
```
anlan_cs [Thu, 16 May 2024 08:44:45 +0000 (16:44 +0800)]
zebra: fix mpls command
Configured with "mpls label bind 1.1.1.1/32 explicit-null", the running
configuration is:
```
!
mpls label bind 1.1.1.1/32 IPv4 Explicit Null
!
```
After this commit, the running configuration is:
```
!
mpls label bind 1.1.1.1/32 explicit-null
!
```
And add the support for the "no" form:
```
anlan(config)# mpls label bind 1.1.1.1/32 explicit-null
anlan(config)# no mpls label bind 1.1.1.1/32 explicit-null
```
> topo: STEP 8: 'Check if we can see 172.16.1.2/32 after R4 (dynamic peer) was killed'
R4 neighbor is deleted on R2 because it fails to re-connect:
> 14:33:40.128048 BGP: [HKWM3-ZC5QP] 192.168.3.1 fd -1 went from Established to Clearing
> 14:33:40.128154 BGP: [MJ1TJ-HEE3V] 192.168.3.1(r4) graceful restart timer expired
> 14:33:40.128158 BGP: [ZTA2J-YRKGY] 192.168.3.1(r4) graceful restart stalepath timer stopped
> 14:33:40.128162 BGP: [H917J-25EWN] 192.168.3.1(r4) Long-lived stale timer (IPv4 Unicast) started for 20 sec
> 14:33:40.128168 BGP: [H5X66-NXP9S] 192.168.3.1(r4) Long-lived set stale community (LLGR_STALE) for: 172.16.1.2/32
> 14:33:40.128220 BGP: [H5X66-NXP9S] 192.168.3.1(r4) Long-lived set stale community (LLGR_STALE) for: 192.168.3.0/24
> [...]
> 14:33:41.138869 BGP: [RGGAC-RJ6WG] 192.168.3.1 [Event] Connect failed 111(Connection refused)
> 14:33:41.138906 BGP: [ZWCSR-M7FG9] 192.168.3.1 [FSM] TCP_connection_open_failed (Connect->Active), fd 23
> 14:33:41.138912 BGP: [JA9RP-HSD1K] 192.168.3.1 (dynamic neighbor) deleted (bgp_connect_fail)
> 14:33:41.139126 BGP: [P98A2-2RDFE] 192.168.3.1(r4) graceful restart stalepath timer stopped
af8496af08 ("bgpd: Do not delete BGP dynamic peers if graceful restart
kicks in") forgot to modify bgp_connect_fail()
Do not delete the peer in bgp_connect_fail() if Non-Stop-Forwarding is
in progress.
Fixes: af8496af08 ("bgpd: Do not delete BGP dynamic peers if graceful restart kicks in") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e446308d76b4ac8f26149db982bf5ed5ac93d5f5)
Igor Ryzhov [Mon, 1 Apr 2024 21:56:17 +0000 (00:56 +0300)]
isisd: fix ip/ipv6 reachability tlvs
Don't allocate subtlvs container if there's nothing to add to it. If the
container is allocated, the "sub-TLVs presence" bit is set in the TLVs
even if there's no actual sub-TLVs, what violates the RFC.
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.
Chirag Shah [Fri, 3 May 2024 03:58:50 +0000 (20:58 -0700)]
zebra: fix EVPN svd based remote nh neigh del
In the context of SVD (Single VxLAN Device) for L3VNI,
the remote VTEP's nexthop is programmed neighbor entry against
SVD along with neighbor entry against SVI.
However, when L3VNI is removed or the VRF is disabled, all SVI
based remote nexthop neighbors are uninstalled and deleted.
The SVD based neigh entries remains in Zebra and the Kernel.
Subsequently, when reconfiguring L3VNI and relearning the same nexthop,
the neighbor entry is not programmed is because it is not removed
from Zebra SVD neighbor hash table, leading to the failure to
reprogram the entry.
With this fix, the SVD nexthop neigh entry is uninstalled
and deleted from Zebra and Kernel.
borderleaf:# ip neigh show 2.2.2.2
2.2.2.2 dev vlan2560_l3 lladdr 00:01:00:00:1d:09 extern_learn NOARP proto zebra
2.2.2.2 dev vxlan99 lladdr 00:01:00:00:1d:09 extern_learn NOARP proto zebra
With the fix:
Zebra log shows both enties SVD (vxlan99) and SVI (vlan2560_l3)
neighbor entries are deleted.
2024/05/03 18:41:33.527125 ZEBRA: [NH6N7-54CD1] Tx RTM_DELNEIGH family
ipv4 IF vxlan99(16) Neigh 2.2.2.2 MAC null flags 0x10 state 0x0
ext_flags 0x0
2024/05/03 18:41:33.527128 ZEBRA: [NH6N7-54CD1] Tx RTM_DELNEIGH family
ipv4 IF vlan2560_l3(18) Neigh 2.2.2.2 MAC null flags 0x10 state 0x0
ext_flags 0x0
When BGP receives an SRV6_LOCATOR_ADD message from zebra, it calls the
`bgp_zebra_process_srv6_locator_add()` function to process the message.
`bgp_zebra_process_srv6_locator_add()` decodes the message first, and
then if the pointer to the default BGP instance is NULL (i.e. the
default BGP instance is not configured yet), it returns early without
doing anything and without using the decoded message information.
This commit fixes the order of the operations executed by
`bgp_zebra_process_srv6_locator_add()`. We first ensure that the default
BGP instance is ready and we return early if it is not. Then, we decode
the message and do something with the information contained in it.
pimd crashs while unconfigure of rp ka timer as we are trying to access
a yand dnode(suppress timer) which does not exist at the moment.
User just configured rp keepalive timer and not suppress timer,
the yang dnode would not be present. Instead of directly accessing
yang_dnode_get_unit16, first check the yang node exist using
the xpath.
Before:
------
tor-11(config)# no ip pim rp keep-alive-timer 3000
vtysh: error reading from pimd: Success (0)Warning: closing connection to pimd because of an I/O error!
Broadcast message from root@tor-11 (somewhere) (Mon Apr 22 17:29:12 2024):
cumulus-core: Running cl-support for core files "pimd.25467.1713806952.core"
After:
-----
tor-11(config)# no ip pim rp keep-alive-timer 3000
tor-11(config)#
Signed-off-by: Donald Sharp <sharpd@nvidia.com> Signed-off-by: Vijayalaxmi Basavaraj <vbasavaraj@nvidia.com>
(cherry picked from commit 0c4b7c1f1f359c1d7647fea34f62378af8b640a5)
Donald Sharp [Fri, 19 Apr 2024 16:13:32 +0000 (12:13 -0400)]
lib, zebra: Check for not being a blackhole route
In zebra_interface_nhg_reinstall zebra is checking that the
nhg is a singleton and not a blackhole nhg. This was originally
done with checking that the nexthop is a NEXTHOP_TYPE_IFINDEX,
NEXTHOP_TYPE_IPV4_IFINDEX and NEXTHOP_TYPE_IPV6_IFINDEX. This
was excluding NEXTHOP_TYPE_IPV4 and NEXTHOP_TYPE_IPV6. These
were both possible to be received and maintained from the upper
level protocol for when a route is being recursively resolved.
If we have gotten to this point in zebra_interface_nhg_reinstall
the nexthop group has already been installed at least once
and we *know* that it is actually a valid nexthop. What the
test is really trying to do is ensure that we are not reinstalling
a blackhole nexthop group( Which is not possible to even be
here by the way, but safety first! ). So let's change
to test for that instead.
Donald Sharp [Thu, 11 Apr 2024 14:46:46 +0000 (10:46 -0400)]
bgpd: Fix display when using `missing-as-worst`
The usage of the `bgp bestpath med missing-as-worst` command
was being accepted and applied during bestpath, but during output
of the routes affected by this it would not give any indication
that this was happening or what med value was being used.
Donald Sharp [Thu, 28 Mar 2024 16:27:38 +0000 (12:27 -0400)]
bgpd: Arrange peer notification to after zebra announce
Currently BGP attempts to send route change information
to it's peers *before* the route is installed into zebra.
This creates a bug in suppress-fib-pending in the following
scenario:
a) bgp suppress-fib-pending and bgp has a route with
2 way ecmp.
b) bgp receives a route withdraw from peer 1. BGP
will send the route to zebra and mark the route as
FIB_INSTALL_PENDING.
c) bgp receives a route withdraw from peer 2. BGP
will see the route has the FIB_INSTALL_PENDING and
not send the withdrawal of the route to the peer.
bgp will then send the route deletion to zebra and
clean up the bgp_path_info's.
At this point BGP is stuck where it has not sent
a route withdrawal to downstream peers.
Let's modify the code in bgp_process_main_one to
send the route notification to zebra first before
attempting to announce the route. The route withdrawal
will remove the FIB_INSTALL_PENDING flag from the dest
and this will allow group_announce_route to believe
it can send the route withdrawal.
For the master branch this is ok because the recent
backpressure commits are in place and nothing is going
to change from an ordering perspective in that regards.
Ostensibly this fix is also for operators of Sonic and
will be backported to the 8.5 branch as well. This will
change the order of the send to peers to be after the
zebra installation but sonic users are using suppress-fib-pending
anyways so updates won't go out until rib ack has been
received anyways.
Donald Sharp [Thu, 28 Mar 2024 16:25:05 +0000 (12:25 -0400)]
bgpd: Note when receiving but not understanding a route notification
When BGP has been asked to wait for FIB installation, on route
removal a return call is likely to not have the dest since BGP
will have cleaned up the node, entirely. Let's just note that
the prefix cannot be found if debugs are turned on and move on.
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.
Philippe Guibert [Fri, 29 Mar 2024 07:35:34 +0000 (08:35 +0100)]
bgpd: fix srv6 memory leak detection
The asan memory leak has been detected:
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
> #0 0x7f9066dadd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
> #1 0x7f9066779b5d in qcalloc lib/memory.c:105
> #2 0x556d6ca527c2 in vpn_leak_zebra_vrf_sid_update_per_af bgpd/bgp_mplsvpn.c:389
> #3 0x556d6ca530e1 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:451
> #4 0x556d6ca64b3b in vpn_leak_postchange bgpd/bgp_mplsvpn.h:311
> #5 0x556d6ca64b3b in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3751
> #6 0x556d6cb9f116 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3337
> #7 0x7f906685a6b6 in zclient_read lib/zclient.c:4490
> #8 0x7f9066826a32 in event_call lib/event.c:2011
> #9 0x7f906675c444 in frr_run lib/libfrr.c:1217
> #10 0x556d6c980d52 in main bgpd/bgp_main.c:545
> #11 0x7f9065784c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Fix this by freeing the previous memory chunk.
Fixes: b72c9e14756f ("bgpd: cli for SRv6 SID alloc to redirect to vrf (step4)") Fixes: 527588aa78b2 ("bgpd: add support for per-VRF SRv6 SID") Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
(cherry picked from commit eea8a8ae248ed726449878c7a74705d779469fda)
Donald Sharp [Fri, 6 Oct 2023 16:48:38 +0000 (12:48 -0400)]
bgpd: Convert the bgp_advertise_attr->adv to a fifo
BGP is storing outgoing updates in a couple of different
fifo's. This is to ensure proper packet packing of
all bgp_dests that happen to use the same attribute.
How it's all put together currently: On initial update
BGP walks through all the bgp_dest's in a table. For each
path being sent a bgp_advertise is created. This bgp_advertise
is placed in fifo order on the bgp_synchronize->update queue.
The bgp_advertise has a pointer to the bgp_advertise_attr which
is associated iwth the actual attribute that is being sent to
it's peer. In turn this bgp_advertise is placed in a fifo off
of the bgp_advertise_attr structure. As such as we have paths
that share an attribute, the path/dest is placed on the
bgp_syncrhonize->update fifo as well as being placed on the fifo
associated with the advertised attribute.
On actual creation of a packet. The first item in the
bgp_synchronize->update fifo is popped. The bgp_advertise_attr
pointer is grabbed, we fill out the nlri part of the bgp packet
and then walk the bgp_advertise_attr fifo to place paths/dests in
the packet. As each path/dest is placed in the packet it is removed
from both the bgp_synchronize->update fifo and the bgp_advertise_attr
fifo.
The whole point of this change is to switch the *next, *prev
pointers in the bgp_advertise structure with a typesafe data
structure.
Tomi Salminen [Wed, 13 Dec 2023 09:37:21 +0000 (11:37 +0200)]
zebra: Fix crash on macvlan link down/up
Whenever a link up change was detected on a macvlan device where
the linked device wasn't visible in the namespace zebra was
running in, the linked zebra interface was NULL. This was already
handled in the event of a link down, but was ommitted from the
upside. Added the same null check to the up-side.
Donatas Abraitis [Fri, 15 Mar 2024 11:49:06 +0000 (13:49 +0200)]
bgpd: Update default-originate route-map actual map structure
If using with `bgp listen range ... peer-group x`, default_rmap[afi][safi] is not
updated, and after the hard-reset in other side, this is flushed and never updated
again without restarting the sender BGP daemon.
Split zebra's vrf_terminate() into disable() and delete() stages.
The former enqueues all events for the dplane thread.
Memory freeing is performed in the second stage.
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.
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.
Louis Scalbert [Thu, 15 Feb 2024 12:28:02 +0000 (13:28 +0100)]
bgpd: fix 6vpe nexthop
6vPE enables the announcement of IPv6 VPN prefixes through an IPv4 BGP
session. In this scenario, the next hop addresses for these prefixes are
represented in an IPv4-mapped IPv6 format, noted as ::ffff:[IPv4]. This
format indicates to the peer that it should route these IPv6 addresses
using information from the IPv4 nexthop. For example:
> Path Attribute - MP_REACH_NLRI
> [...]
> Address family identifier (AFI): IPv6 (2)
> Subsequent address family identifier (SAFI): Labeled VPN Unicast (128)
> Next hop: RD=0:0 IPv6=::ffff:192.0.2.5 RD=0:0 Link-local=fe80::501d:42ff:feef:b021
> Number of Subnetwork points of attachment (SNPA): 0
This rule is set out in RFC4798:
> The IPv4 address of the egress 6PE router MUST be encoded as an
> IPv4-mapped IPv6 address in the BGP Next Hop field.
However, in some situations, bgpd sends a standard nexthop IPv6 address
instead of an IPv4-mapped IPv6 address because the outgoing interface for
the BGP session has a valid IPv6 address. This is problematic because
the peer router may not be able to route the nexthop IPv6 address (ie.
if the outgoing interface has not IPv6).
Fix the issue by always sending a IPv4-mapped IPv6 address as nexthop
when the BGP session is on IPv4 and address family IPv6.
Philippe Guibert [Mon, 13 Mar 2023 09:47:16 +0000 (10:47 +0100)]
topotests: add an ebgp 6vpe test
This test uses the connected ipv4 mapped ipv6 prefix
to resolve the received BGP routes.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com> Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com> Signed-off-by: François Dumontet <francois.dumontet@6wind.com>
(cherry picked from commit 4d7df91752d7414d9719a361a2fd4cc30943dc96)
projects / mirror / frr.git / log