Donatas Abraitis [Tue, 23 May 2023 06:20:27 +0000 (09:20 +0300)]
ripd: Use argv_find to avoid buffer overflow when parsing allow-ecmp args
==13211==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000af158 at pc 0x55d48c5f1e38 bp 0x7fffd8a713d0 sp 0x7fffd8a713c0
READ of size 8 at 0x6020000af158 thread T0
#0 0x55d48c5f1e37 in rip_allow_ecmp ripd/rip_cli.c:98
#1 0x7f2ec125aa0f in cmd_execute_command_real lib/command.c:990
#2 0x7f2ec125ae90 in cmd_execute_command lib/command.c:1049
#3 0x7f2ec125b406 in cmd_execute lib/command.c:1217
#4 0x7f2ec137ca36 in vty_command lib/vty.c:551
#5 0x7f2ec137ce52 in vty_execute lib/vty.c:1314
#6 0x7f2ec1384f9e in vtysh_read lib/vty.c:2223
#7 0x7f2ec137041b in event_call lib/event.c:1995
#8 0x7f2ec12b54bf in frr_run lib/libfrr.c:1204
#9 0x55d48c5f0f32 in main ripd/rip_main.c:171
#10 0x7f2ec0ad9c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
#11 0x55d48c5f1349 in _start (/usr/lib/frr/ripd+0x3b349)
0x6020000af158 is located 0 bytes to the right of 8-byte region [0x6020000af150,0x6020000af158)
allocated by thread T0 here:
#0 0x7f2ec18ccb40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f2ec12d2e41 in qmalloc lib/memory.c:100
#2 0x7f2ec125a815 in cmd_execute_command_real lib/command.c:955
#3 0x7f2ec125ae90 in cmd_execute_command lib/command.c:1049
#4 0x7f2ec125b406 in cmd_execute lib/command.c:1217
#5 0x7f2ec137ca36 in vty_command lib/vty.c:551
#6 0x7f2ec137ce52 in vty_execute lib/vty.c:1314
#7 0x7f2ec1384f9e in vtysh_read lib/vty.c:2223
#8 0x7f2ec137041b in event_call lib/event.c:1995
#9 0x7f2ec12b54bf in frr_run lib/libfrr.c:1204
#10 0x55d48c5f0f32 in main ripd/rip_main.c:171
#11 0x7f2ec0ad9c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: heap-buffer-overflow ripd/rip_cli.c:98 in rip_allow_ecmp
Shadow bytes around the buggy address:
0x0c048000ddd0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fa
0x0c048000dde0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
0x0c048000ddf0: fa fa fd fa fa fa fd fd fa fa fd fd fa fa fd fd
0x0c048000de00: fa fa fd fa fa fa fd fd fa fa 00 03 fa fa fd fa
0x0c048000de10: fa fa fd fa fa fa 00 00 fa fa fd fa fa fa 00 03
=>0x0c048000de20: fa fa 00 03 fa fa fd fa fa fa 00[fa]fa fa fa fa
0x0c048000de30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c048000de40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c048000de50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c048000de60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c048000de70: 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
==13211==ABORTING
Rajasekar Raja [Mon, 22 May 2023 21:14:30 +0000 (14:14 -0700)]
bgpd: Using no pretty json output for l2vpn-Evpn routes
The output of show bgp all json is inconsistent across Address-families
i.e. ipv4/ipv6 is a no pretty format while l2vpn-evpn is in a pretty
format. For huge scale (lots of routes with lots of paths), it is better
to use no_pretty format.
Acee [Thu, 18 May 2023 14:43:52 +0000 (10:43 -0400)]
ospfd: OSPF P2MP Delayed Reflooding configuration
Currently, delayed reflooding on P2MP interfaces for LSAs received
from neighbors on the interface is unconditionally (see commit c706f0e32ba8aa8780a0618b6fbba364c383ae05). In some cases, this
change wasn't desirable and this feature makes delayed reflooding
configurable for P2MP interfaces via the CLI command:
"ip ospf network point-to-multipoint delay-reflood" in interface
submode.
Donald Sharp [Sun, 21 May 2023 23:39:39 +0000 (19:39 -0400)]
tests: Do not Cause test scripts to stop running when config load fails
When running the pytests in parallel, calling pytest.exit() causes
the entire test run to be aborted. Which.... Is frankly not cool.
Let's notice the failure and move on to the next tests.
Donald Sharp [Mon, 22 May 2023 12:25:38 +0000 (08:25 -0400)]
tests: Add `exit` stanzas to pre-generated config
A bunch of tests rely on pre-generated config from
json files. These tests were not putting `exit` stanzas
and a bunch of the tests as a result are silently failing
to configure properly at all, as commands were being sent
to the wrong daemons.
Donald Sharp [Fri, 19 May 2023 20:03:57 +0000 (16:03 -0400)]
tests: Slow bgp_default_originate test down slightly
The test is performing these steps:
a) get timestamp of route installed in zebra
b) <make changes>
c) get new timestamp of route installed in zebra
If < 1 second happens between A and C the test
assumes that something went wrong, as that it is
testing to see if the route was reinstalled <yes I know>.
Just sleep 1 second after a) happens so that if a reinstall
happens we can easily see it, and we also know that if a
reinstall doesn't happen then the new timestamp will
always be 1 second or greater.
Chirag Shah [Fri, 19 May 2023 05:43:08 +0000 (22:43 -0700)]
bgpd: fix aggregate route display
Based on RFC-4760, if NEXT_HOP attribute is not
suppose to be set if MP_REACH_NLRI NLRI is used.
for IPv4 aggregate route only NEXT_HOP attribute
with ipv4 prefixlen needs to be set.
Testing Done:
Before fix:
----------
aggregate route:
*> 184.123.0.0/16 ::(TORC11) 0 32768 i
After fix:
---------
aggregate route:
*> 184.123.0.0/16 0.0.0.0(TORC11) 0 32768 i
* i peerlink-3 0 100 0 i
* uplink1 0 4435 5546 i
184.123.1.0/24 0.0.0.0(TORC11) 0 32768 i
s> 184.123.8.0/22 0.0.0.0(TORC11) 0 32768 i
Chirag Shah [Fri, 19 May 2023 04:49:03 +0000 (21:49 -0700)]
bgpd: fix aggregate route best path select
In ebgp+ ibgp deployment aggregate summary-only route
selected path should always be locally originated
summary route.
When aggregate route summary-only config is removed
The selected path is iBGP peer as its lower cost
Upon reconfiguring aggregate route summary-only,
the locally originated is not selected due to
always choosing first path attribute and bailing
out as no change in route update.
Donald Sharp [Fri, 19 May 2023 16:51:09 +0000 (12:51 -0400)]
tests: pytest does not like return True from a test
From running the test:
bgp_remove_private_as/test_bgp_remove_private_as.py::test_bgp_remove_private_as
/home/sharpd/.local/lib/python3.10/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but bgp_remove_private_as/test_bgp_remove_private_as.py::test_bgp_remove_private_as returned True, which will be an error in a future version of pytest. Did you mean to use `assert` instead of `return`?
warnings.warn(
Donald Sharp [Thu, 18 May 2023 20:03:01 +0000 (16:03 -0400)]
zebra: On shutdown stop hook calls for fpm rmac updates
When shutting down zebra, the hook for the rmac update was
not being unregistered. As such it would be possible
to get into a condition where more rmacs are being
added to the queue for handling in the future after we
are told to shutdown.
Donald Sharp [Thu, 18 May 2023 19:59:43 +0000 (15:59 -0400)]
zebra: Properly handle zfpm_g->t_conn_down in zebra_fpm.c
The t_conn_down pointer was being set to NULL when it already
was. The t_conn_down pointer was being dropped( and leaving
a thread possibly running in the background ) which could
cause problems on shutdown. And finally when shutting down
the t_conn_down event was not being stopped at all.
Donald Sharp [Fri, 19 May 2023 13:54:05 +0000 (09:54 -0400)]
zebra: Do not allow old FPM to access freed memory after shutdown
On shutdown, the old FPM queues up dests to be sent to
the FPM listener. This is done through the rib_shutdown
hook. Which is called when the table that the routes are
stored in are being deleted. This dest has pointers
to the rnode. The rnode has pointers to the table it
is associated with as well as the table->info pointer for
the zebra data associated with this table.
The FPM after this attempts to tell this to it's listener
via events. Unfortunately the zvrf, table_id and nl_pid
was being grabbed from memory that had been freed! Since
all this can be grabbed from memory that has not been freed
on shutdown let's switch over to using that instead of freed
memory for gathering data.
Christian Hopps [Thu, 18 May 2023 02:26:49 +0000 (22:26 -0400)]
doc: configure: add configure option to generate .ccls file
`ccls` needs information from FRR build configuration to work,
so allow creation of a custom ccls config during autoconf.
Paraphrasing the doc entry: ccls is a very powerful tool that allows
dev environments to provide sophisticated IDE functionality, e.g.,
semantically aware jumps and code refactoring...
Christian Hopps [Wed, 17 May 2023 11:10:13 +0000 (07:10 -0400)]
tests: fix implicit config file and recently added logic error
- Restore default of looking for a daemon config underneath the router directory
if no config file was specified.
- Recent change for adding unified config support had a logic bug, fix
- Update the one test that conflicted with this default
- comment out asyncio option causing warnings if asyncio wasn't installed.
When FRR is built with the option `--disable-bfdd`, the build process
fails with the following error:
```
zebra/zebra_ptm.c: In function ‘zebra_ptm_init’:
zebra/zebra_ptm.c:119:35: error: ‘FRR_PTM_NAME’ undeclared (first use in this function)
119 | snprintf(buf, sizeof(buf), "%s", FRR_PTM_NAME);
| ^~~~~~~~~~~~
zebra/zebra_ptm.c:119:35: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [Makefile:10520: zebra/zebra_ptm.o] Error 1
```
The reason is that `FRR_PTM_NAME` is defined in `version.h` which is not
imported.
Donald Sharp [Wed, 17 May 2023 11:56:27 +0000 (07:56 -0400)]
tests: Change order of config files
Our CI test system is configuring interfaces like this:
int A
<ospfX config>
router ospfX
router-id Z.Y.M.Q
On sufficiently loaded systems, the router-id might not be respected because
the interface A neighbor might have come up by the time the router-id
command is read in. This is a problem for our topotests in that
the tests expect neighbors to be formed with certain router-id,
but the FRR code has stored the new router-id but not accepted
it's use since a neighbor relationship has formed.
Modify the ci test system code to put the 'router ospfX' commands
before the interface commands, thus forcing the router-id to be
read in *before* any possibility that the neighbor can have come
up.
Also, I've filed issue #13452 to address the ordering of commands
with regards to router-id in our ospf protocols. Once that is
done this should be backed out, to return to a more natural ordering
of commands.
Donald Sharp [Tue, 16 May 2023 20:46:35 +0000 (16:46 -0400)]
tests: Clean up commands that do nothing
Recently clear commands were added to the tests that do nothing
because they are using the wrong way to input the command.
Since these do nothing remove them:
Christian Hopps [Sat, 13 May 2023 00:32:54 +0000 (20:32 -0400)]
tests: improve bgp test determinism
don't grep the tail of a log file after running a previous test, there could be
(and have been) other items added to the log in between.
add before and after count of shutdown messages to very the actual message shows
up as the test intended, and keep the search for the shutdown message.
projects / mirror_frr.git / log