Yuan Yuan [Tue, 30 May 2023 19:20:09 +0000 (19:20 +0000)]
lib: fix vtysh core when handling questionmark
When issue vtysh command with ?, the initial buf size for the
element is 16. Then it would loop through each element in the cmd
output vector. If the required size for printing out the next
element is larger than the current buf size, realloc the buf memory
by doubling the current buf size regardless of the actual size
that's needed. This would cause vtysh core when the doubled size
is not enough for the next element.
Yuan Yuan [Tue, 30 May 2023 18:53:32 +0000 (18:53 +0000)]
bgpd: fix bgpd core when unintern attr
When the remote peer is neither EBGP nor confed, aspath is the
shadow copy of attr->aspath in bgp_packet_attribute(). Striping
AS4_PATH should not be done on the aspath directly, since
that would lead to bgpd core dump when unintern the attr.
Donald Sharp [Fri, 26 May 2023 11:44:11 +0000 (07:44 -0400)]
vtysh: Give actual pam error messages
Code was was written where the pam error message put out
was the result from a previous call to the pam modules
instead of the current call to the pam module.
The vrf check should use the carefully adjusted `vrfid`, which is
based on globally/reliable interface. We can't believe the
`bvrf->vrf->vrf_id` because the `/proc/sys/net/ipv4/udp_l3mdev_accept`
maybe is set "1" in VRF-lite backend even with security drawback.
Acee [Tue, 9 May 2023 20:51:03 +0000 (16:51 -0400)]
ospfd: OSPF opaque LSA stale processing fix and topotests.
1. Fix OSPF opaque LSA processing to preserve the stale opaque
LSAs in the Link State Database for 60 seconds consistent with
what is done for other LSA types.
2. Add a topotest that tests for cases where ospfd is restarted
and a stale OSPF opaque LSA exists in the OSPF routing domain
both when the LSA is purged and when the LSA is reoriginagted
with a more recent instance.
Donald Sharp [Tue, 9 May 2023 17:10:35 +0000 (13:10 -0400)]
ospfd: Respect loopback's cost that is set and set loopback costs to 0
When setting an loopback's cost, set the value to 0, unless the operator
has assigned a value for the loopback's cost.
RFC states:
If the state of the interface is Loopback, add a Type 3
link (stub network) as long as this is not an interface
to an unnumbered point-to-point network. The Link ID
should be set to the IP interface address, the Link Data
set to the mask 0xffffffff (indicating a host route),
and the cost set to 0.
FRR is going to allow this to be overridden if the operator specifically
sets a value too.
Sai Gomathi N [Fri, 17 Mar 2023 10:51:16 +0000 (03:51 -0700)]
pimd: PIM not sending register packets after changing from non DR to DR
When the router is non dr for an interface, it installs mroute to drop
the packets from directly connected source. This was done to avoid packets
coming to cpu as nocache hit. Later when it gets change from non-DR to DR,
these entries are not cleared. So the packets are still dropped.
This causes register packets not getting generated.
So cleaning up the mroute entries and channel oil without
upstream reference which was created to drop.
Co-authored-by: Saravanan K <saravanank@vmware.com> Signed-off-by: Sai Gomathi N <nsaigomathi@vmware.com>
(cherry picked from commit 1c883aef96013753f5467ba5e5028dee0f0a82c5)
Chirag Shah [Sat, 22 Oct 2022 23:00:14 +0000 (16:00 -0700)]
zebra:fix evpn dup detected local mac del event
The current local mac delete event send to flag with force
always which breaks the duplicate detected MACs where
it requires to be resynced from bgpd to earlier state.
Chirag Shah [Wed, 1 Dec 2021 04:42:01 +0000 (20:42 -0800)]
zebra: evpn handle del event for dup detected mac
Upon receiving local mobility event for MAC + NEIGH,
both are detected as duplicate upon hitting DAD threshold.
Duplicated detected ( freezed) MAC + NEIGH are not known
to bgpd.
If locally learnt MAC + NEIGH are deleted in kernel,
the MAC is marked as AUTO after sending delete event
to bgpd.
Bgpd only reinstalls best route for MAC_IP route (NEIGH)
but not for MAC event.
This puts a situation where MAC is AUTO state and
associated neigh as remote.
Fix:
DUPLICATE + LOCAL MAC deletion, set MAC delete request
as reinstall from bgpd.
Donald Sharp [Thu, 20 Apr 2023 20:27:20 +0000 (16:27 -0400)]
bgpd: Fix lcom->str string length to correctly cover aliases
If you have a very large number of large communities whose
string length happened to be greater than BUFSIZ FRR's bgpd
would crash. This is because bgpd would write beyond
the end of the string.
Originally the code auto-calculated the string size appropriately
but commit ed0e57e3f079352714c3a3a8a5b0dddf4aadfe1d modified
the string length to be a hard coded BUFSIZ. When a route-map
like this is added:
Modify the code to correctly determine the string length of the communities
and to also double check if the string has an alias and ensure that the
string is still sufficiently large enough. If not auto size it again.
bgpd: Fix for ain->attr corruption during path update
1. Consider a established L2VPN EVPN BGP peer with soft-reconfiguartion
inbound configured
2. When the interface of this directly connected BGP peer is shutdown,
bgp_soft_reconfig_table_update() is called, which memsets the evpn buffer
and calls bgp_update() with received attributes stored in ain table(ain->attr).
In bgp_update(), evpn_overlay attribute in ain->attr (which is an interned
attr) was modified by doing a memcpy
3. Above action causes 2 attributes in the attrhash (which were previously different)
to match!
4. Later during fsm change event of the peer, bgp_adj_in_remove() is called
to clean up the ain->attr. But, because 2 attrs in attrhash match, it causes
BGP to assert in bgp_attr_unintern()
tools: fix missing remote-as configuration when reload
From commit `411d1a2`, `bgp_delete_nbr_remote_as_line()` is added to
remove some specific bgp neighbors. But, when reloading the following
configuration, it will wrongly remove some good ones:
`neighbor 66.66.66.6 remote-as internal`:
Initially PIM nbr is down between FRR4----FRR2 from FRR2 side
Cisco is sending BSR packet to FRR4.
Problem Statement:
=================
No shutdown the PIM neighbor on FRR2 towards FRR4.
FRR2, receives BSR packet immediately as the new neighbor
comes up. This BSR packet is having no-forward bit set.
FRR2 is not able to process the BSR packet, and drop the
BSR packet.
Root Cause:
==========
When PIMD comes up, we start BSM timer for 60 seconds.
Here, the value accept_nofwd_bsm is setting to false.
FRR2, when receives no-forward BSR packet, it is getting
accept_nofwd_bsm value as false.
So, it drops, the no-forward BSM packet.
Fix:
===
Set accept_nofwd_bsm as false after first BSM packet received.
zebra: Install directly connected route after interface flap
Issue:
After vlan flap, zebra was not marking the selected/best route as installed.
As a result, when a static route was configured with nexthop as directly
connected interface's(vlan) IP, the static route was not being installed
in the kernel since its nexthop was unresolved. The nexthop was marked
unresolved because zebra failed to mark the best route as installed after
interface flap.
This was happening because, in dplane_route_update_internal() if the old and
new context type, and nexthop group id are the same, then zebra doesn't send
down a route replace request to kernel. But, the installed (ROUTE_ENTRY_INSTALLED)
flag is set when zebra receives a response from kernel. Since the
request to kernel was being skipped for the route entry, installed flag
was not being set
Fix:
In dplane_route_update_internal() if the old and new context type, and
nexthop group id are the same, then before returning, installed flag will
be set on the route-entry if it's not set already.
Mark Stapp [Tue, 11 Apr 2023 19:51:32 +0000 (15:51 -0400)]
zebra: fix race during shutdown
During shutdown, the main pthread stops the dplane pthread
before exiting. Don't try to clean up any events scheduled
to the dplane pthread at that point - just let the thread
exit and clean up. This is the 8.4 version.
Donald Sharp [Mon, 10 Apr 2023 18:04:27 +0000 (14:04 -0400)]
bgpd: Do not allow a `no router bgp XXX` when autoimport is happening
When we have these sequence of events causing a crash in
evpn_type5_test_topo1:
(A) no router bgp vrf RED 100
this schedules for deletion the vrf RED instance
(B) a l3vni change event from zebra
this creates a bgp instance for VRF RED in some cases
additionally it auto imports evpn routes into VRF RED
Please note this is desired behavior to allow for the
auto importation of evpn vrf routes
(C) no router bgp 100
The code was allowing the deletion of the default
instance and causing tests to crash.
Effectively the test in bgp_vty to allow/dissallow
the removal of the default instance was not correct
for the case when (B) happens.
Let's just not allow the command to succeed in this case as that
the test was wrong.
Donald Sharp [Mon, 10 Apr 2023 17:59:48 +0000 (13:59 -0400)]
bgpd: Do not allow l3vni changes when shutting down
When a `no router bgp XXX` is issued and the bgp instance
is in the process of shutting down, do not allow a l3vni
change coming up from zebra to do anything. We can just
safely ignore it at this point in time.
Donald Sharp [Wed, 5 Apr 2023 18:57:05 +0000 (14:57 -0400)]
bgpd: Limit flowspec to no attribute means a implicit withdrawal
All other parsing functions done from bgp_nlri_parse() assume
no attributes == an implicit withdrawal. Let's move
bgp_nlri_parse_flowspec() into the same alignment.
Reported-by: Matteo Memelli <mmemelli@amazon.it> Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit cfd04dcb3e689754a72507d086ba3b9709fc5ed8)
Trey Aspelund [Fri, 31 Mar 2023 21:46:21 +0000 (17:46 -0400)]
bgpd: fix ecommunity parsing for AS4
The parser for extended communities was incorrectly disallowing an
operator from configuring "Route Origin" extended communities
(e.g. RD/RT/SoO) with a 4-byte value matching BGP_AS4_MAX (UINT32_MAX)
and allowed the user to overflow UINT32_MAX. This updates the parser to
read the value as a uint64_t so that we can do proper checks on the
upper bounds (> BGP_AS4_MAX || errno).
before:
```
TORC11(config-router-af)# neighbor uplink-1 soo 4294967296:65
TORC11(config-router-af)# do sh run | include soo
neighbor uplink-1 soo 0:65
TORC11(config-router-af)# neighbor uplink-1 soo 4294967295:65
% Malformed SoO extended community
TORC11(config-router-af)#
```
after:
```
TORC11(config-router-af)# neighbor uplink-1 soo 4294967296:65
% Malformed SoO extended community
TORC11(config-router-af)# neighbor uplink-1 soo 4294967295:65
TORC11(config-router-af)# do sh run | include soo
neighbor uplink-1 soo 4294967295:65
TORC11(config-router-af)#
```
Donatas Abraitis [Tue, 28 Mar 2023 13:18:47 +0000 (16:18 +0300)]
bgpd: Do not announce routes immediatelly on filter updates
If we set `bgp route-map delay-timer X`, we should ignore starting to announce
routes immediately, and wait for delay timer to expire (or ignore at all if set
to zero).
ospfd: Fix for vitual-link crash in signal handler
Whenever OSPF virtual-link is created, a virtual interface is
associated with it. Name of the virtual interface is derived by
combining "VLINK" string with the value of vlink_count, which is a global
variable.
Problem:
Consider a scenario where 2 virtual links A and B are created in OSPF with
virtual interfaces VLINK0 and VLINK1 respectively. When virtual-link A is unconfigured
and reconfigured, new interface name derived for it will be VLINK1, which is already
associated with virtual-link B. Due to this, both virtual-links A and B will
point to the same interface, VLINK1.
During FRR restart when signal handler is called, OSPF goes through all the virtual
links and deletes the interface(oi) associated with it. During the deletion of interface
for virtual-link B,it accesses the interface which was deleted already(which was deleted
during deletion of virual-link A) and whose fields were set to NULL. This
leads to OSPF crash.
Fixed it by not decrementing vlink_count during unconfig/deletion for virtual-link.
Donald Sharp [Wed, 22 Mar 2023 22:24:56 +0000 (18:24 -0400)]
pimd: Fix use after free issue for ifp's moving vrfs
We have this valgrind trace:
==1125== Invalid read of size 4
==1125== at 0x170A7D: pim_if_delete (pim_iface.c:203)
==1125== by 0x170C01: pim_if_terminate (pim_iface.c:80)
==1125== by 0x174F34: pim_instance_terminate (pim_instance.c:68)
==1125== by 0x17535B: pim_vrf_terminate (pim_instance.c:260)
==1125== by 0x1941CF: pim_terminate (pimd.c:161)
==1125== by 0x1B476D: pim_sigint (pim_signals.c:44)
==1125== by 0x4910C22: frr_sigevent_process (sigevent.c:133)
==1125== by 0x49220A4: thread_fetch (thread.c:1777)
==1125== by 0x48DC8E2: frr_run (libfrr.c:1222)
==1125== by 0x15E12A: main (pim_main.c:176)
==1125== Address 0x6274d28 is 1,192 bytes inside a block of size 1,752 free'd
==1125== at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1125== by 0x174FF1: pim_vrf_delete (pim_instance.c:181)
==1125== by 0x4925480: vrf_delete (vrf.c:264)
==1125== by 0x4925480: vrf_delete (vrf.c:238)
==1125== by 0x49332C7: zclient_vrf_delete (zclient.c:2187)
==1125== by 0x4934319: zclient_read (zclient.c:4003)
==1125== by 0x492249C: thread_call (thread.c:2008)
==1125== by 0x48DC8D7: frr_run (libfrr.c:1223)
==1125== by 0x15E12A: main (pim_main.c:176)
==1125== Block was alloc'd at
==1125== at 0x4837B65: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1125== by 0x48E80AF: qcalloc (memory.c:116)
==1125== by 0x1750DA: pim_instance_init (pim_instance.c:90)
==1125== by 0x1750DA: pim_vrf_new (pim_instance.c:161)
==1125== by 0x4924FDC: vrf_get (vrf.c:183)
==1125== by 0x493334C: zclient_vrf_add (zclient.c:2157)
==1125== by 0x4934319: zclient_read (zclient.c:4003)
==1125== by 0x492249C: thread_call (thread.c:2008)
==1125== by 0x48DC8D7: frr_run (libfrr.c:1223)
==1125== by 0x15E12A: main (pim_main.c:176)
and you do this series of events:
a) Create a vrf, put an interface in it
b) Turn on pim on that interface and turn on pim in that vrf
c) Delete the vrf
d) Do anything with the interface, in this case shutdown the system
The move of the interface to a new vrf is leaving the pim_ifp->pim pointer pointing
at the old pim instance, which was just deleted, so the instance pointer was freed.
Let's clean up the pim pointer in the interface pointer as well.
rgirada [Mon, 20 Mar 2023 11:22:26 +0000 (11:22 +0000)]
ospfd: Fixing Summary origination after range configuration
Description:
After area range config, summary lsas are aggerated to configured
route but later it was being flushed instead of the actual summary
lsa. This was seen when prefix-id of the aggregated route is same
as one of the actual summary route.
Here, aggregated summary lsa need to be returned to set the flag
SUMMARY_APPROVE after originating aggregated summary lsa but its not.
Which is being cleaned up as part of unapproved summary cleanup.
Corrected this now.
Issue:
When a netns is deleted, since zebra doesn’t receive interface down/delete
notifications from kernel, it manually deletes the interface without removing
the association between zebra_l3vni and the interface that is being deleted
(i.e it deletes the interface without setting “zl3vni->vxlan_if” to NULL).
Later, during the deletion of netns, when zl3vni_rmac_uninstall() is called to
uninstall the remote RMAC from the kernel, zebra ends up accessing stale
“zl3vni->vxlan_if” pointer, which now points to freed memory.
This was causing heap use-after-free.
Fix:
Before zebra starts deleting the interfaces when it receives netns delete notification,
appropriate functions() are being called to remove the association between evpn structs
and interface and set “zl3vni->vxlan_if” to NULL. This ensures that when
zl3vni_rmac_uninstall() is called during netns deletion, it will bail because
“zl3vni->vxlan_if” is NULL.
Donald Sharp [Mon, 20 Mar 2023 20:07:20 +0000 (16:07 -0400)]
lib: on bfd peer shutdown actually stop event
When deleting a bfd peer during shutdown, let's ensure
that any scheduled events are actually stopped.
==7759== Invalid read of size 4
==7759== at 0x48BF700: _bfd_sess_valid (bfd.c:419)
==7759== by 0x48BF700: _bfd_sess_send (bfd.c:470)
==7759== by 0x492F79C: thread_call (thread.c:2008)
==7759== by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759== by 0x1C739B: main (bgp_main.c:550)
==7759== Address 0xfb687a4 is 4 bytes inside a block of size 272 free'd
==7759== at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7759== by 0x48BFA5A: bfd_sess_free (bfd.c:535)
==7759== by 0x2B7034: bgp_peer_remove_bfd (bgp_bfd.c:339)
==7759== by 0x29FF8A: peer_free (bgpd.c:1160)
==7759== by 0x29FF8A: peer_unlock_with_caller (bgpd.c:1192)
==7759== by 0x2A0506: peer_delete (bgpd.c:2633)
==7759== by 0x208190: bgp_stop (bgp_fsm.c:1639)
==7759== by 0x20C082: bgp_event_update (bgp_fsm.c:2751)
==7759== by 0x492F79C: thread_call (thread.c:2008)
==7759== by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759== by 0x1C739B: main (bgp_main.c:550)
==7759== Block was alloc'd at
==7759== at 0x4837B65: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7759== by 0x48F53AF: qcalloc (memory.c:116)
==7759== by 0x48BF98D: bfd_sess_new (bfd.c:397)
==7759== by 0x2B76DC: bgp_peer_configure_bfd (bgp_bfd.c:298)
==7759== by 0x2B76DC: bgp_peer_configure_bfd (bgp_bfd.c:279)
==7759== by 0x29BA06: peer_group2peer_config_copy (bgpd.c:2803)
==7759== by 0x2A3D96: peer_create_bind_dynamic_neighbor (bgpd.c:4107)
==7759== by 0x2A4195: peer_lookup_dynamic_neighbor (bgpd.c:4239)
==7759== by 0x21AB72: bgp_accept (bgp_network.c:422)
==7759== by 0x492F79C: thread_call (thread.c:2008)
==7759== by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759== by 0x1C739B: main (bgp_main.c:550)
tl;dr -> Effectively, in this test setup we have 300 dynamic bgp
sessions all of which are using bfd. When a peer collision is detected
or we remove the peers, if an event has been scheduled but not actually
executed yet the event event was not actually being stopped, leaving
the bsp pointer on the thread->arg and causing a crash when it is
executed.
Donald Sharp [Fri, 17 Mar 2023 19:40:33 +0000 (15:40 -0400)]
bgpd: Prevent Null pointer deref when outputting data
Crash:
(gdb) bt
0 0x00007fee27de15cb in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
1 0x00007fee280ecd9c in core_handler (signo=11, siginfo=0x7ffe56001bb0, context=<optimized out>) at lib/sigevent.c:264
2 <signal handler called>
3 0x0000555e321c41b2 in prefix_rd2str (prd=0x10, buf=buf@entry=0x7ffe56002080 "27.0.0.R\340\373\062\062^U", size=size@entry=28) at bgpd/bgp_rd.c:168
4 0x0000555e321c431a in printfrr_prd (buf=0x7ffe560021a0, ea=<optimized out>, ptr=<optimized out>) at bgpd/bgp_rd.c:224
5 0x00007fee2812069b in vbprintfrr (cb_in=cb_in@entry=0x7ffe56002330, fmt0=fmt0@entry=0x555e3229a3ad " RD: %pRD\n", ap=ap@entry=0x7ffe560023d8) at lib/printf/vfprintf.c:564
6 0x00007fee28122ef7 in vasnprintfrr (mt=mt@entry=0x7fee281cb5e0 <MTYPE_VTY_OUT_BUF>, out=out@entry=0x7ffe560023f0 " RD: : R\n", outsz=outsz@entry=1024, fmt=fmt@entry=0x555e3229a3ad " RD: %pRD\n", ap=ap@entry=0x7ffe560023d8) at lib/printf/glue.c:103
7 0x00007fee28103504 in vty_out (vty=vty@entry=0x555e33f82d10, format=format@entry=0x555e3229a3ad " RD: %pRD\n") at lib/vty.c:190
8 0x0000555e32185156 in bgp_evpn_es_show_entry_detail (vty=0x555e33f82d10, es=0x555e33c38420, json=<optimized out>) at bgpd/bgp_evpn_mh.c:2655
9 0x0000555e32188fe5 in bgp_evpn_es_show (vty=vty@entry=0x555e33f82d10, uj=false, detail=true) at bgpd/bgp_evpn_mh.c:2721
notice prd=0x10 in #3. This is because in bgp_evpn_mh.c we are sending &es->es_base_frag->prd.
There is one spot in the code where during output the es->es_base_frag is checked for non nullness
Let's just make sure it's right in all the places.
pbr-map map6 seq 1
match src-ip 2000::200:100:100:0/96
match dst-ip 2000::100:100:100:0/96
set nexthop-group group3
Before:
torc-12(config)# pbr-map map6 seq 1
torc-12(config-pbr-map)# no match src-ip 2000::200:100:100:0/96
Cannot mismatch families within match src/dst
After:
torc-12(config)# pbr-map map6 seq 1
torc-12(config-pbr-map)# no match src-ip 2000::200:100:100:0/96
torc-12(config-pbr-map)#
Donatas Abraitis [Fri, 17 Mar 2023 12:48:35 +0000 (14:48 +0200)]
lib: Adjust only `any` flag for prefix-list entries if destroying
Before this patch, if we destroy `any` flag for a prefix-list entry, we always
set destination as 0.0.0.0/0 and/or ::/0.
This means that, if we switch from `ip prefix-list r1-2 seq 5 deny any` to
`ip prefix-list r1-2 seq 5 permit 10.10.10.10/32` we will have
`permit any` eventually, which broke ACLs.
projects / mirror / frr.git / log