Igor Ryzhov [Fri, 6 Oct 2023 12:01:16 +0000 (15:01 +0300)]
mgmt, lib: differentiate DELETE and REMOVE operations
Currently, there's a single operation type which doesn't return error
if the object doesn't exists. To be compatible with NETCONF/RESTCONF,
we should support differentiate between DELETE (fails when object
doesn't exist) and REMOVE (doesn't fail if the object doesn't exist).
Igor Ryzhov [Thu, 5 Oct 2023 23:58:58 +0000 (02:58 +0300)]
mgmtd, lib: implement CREATE_EXCL operation
Currently, there's no difference between CREATE and MODIFY operations.
To be compatible with NETCONF/RESTCONF, add new CREATE_EXCL operation
that throws an error if the configuration data already exists.
Igor Ryzhov [Thu, 5 Oct 2023 21:13:16 +0000 (00:13 +0300)]
lib: split nb_operation into two types
Currently, nb_operation enum means two different things - edit operation
type (frontend part), and callback type (backend part). These types
overlap, but they are not identical. We need to add more operation
types to support NETCONF/RESTCONF integration, so it's better to have
separate enums to identify different entities.
Edwin Brossette [Fri, 8 Dec 2023 15:02:11 +0000 (16:02 +0100)]
lib: use snmp's large fd sets for agentx
The maximum number of file descriptors in an fd set is limited by
FD_SETSIZE. This limitation is important because the libc macros
FD_SET(), FD_CLR() and FD_ISSET() will invoke a sigabort if the size of
the fd set given to them is above FD_SETSIZE.
We ran into such a sigabort with bgpd because snmp can return an fd set
of size higher than FD_SETSIZE when calling snmp_select_info(). An
unfortunate FD_ISSET() call later causes the following abort:
Received signal 6 at 1701115534 (si_addr 0xb94, PC 0x7ff289a16a7c); aborting...
/lib/x86_64-linux-gnu/libfrr.so.0(zlog_backtrace_sigsafe+0xb3) [0x7ff289d62bba]
/lib/x86_64-linux-gnu/libfrr.so.0(zlog_signal+0x1b4) [0x7ff289d62a1f]
/lib/x86_64-linux-gnu/libfrr.so.0(+0x102860) [0x7ff289da4860]
/lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7ff2899c2520]
/lib/x86_64-linux-gnu/libc.so.6(pthread_kill+0x12c) [0x7ff289a16a7c]
/lib/x86_64-linux-gnu/libc.so.6(raise+0x16) [0x7ff2899c2476]
/lib/x86_64-linux-gnu/libc.so.6(abort+0xd3) [0x7ff2899a87f3]
/lib/x86_64-linux-gnu/libc.so.6(+0x896f6) [0x7ff289a096f6]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x2a) [0x7ff289ab676a]
/lib/x86_64-linux-gnu/libc.so.6(+0x1350c6) [0x7ff289ab50c6]
/lib/x86_64-linux-gnu/libc.so.6(+0x1366ab) [0x7ff289ab66ab]
/lib/x86_64-linux-gnu/libfrrsnmp.so.0(+0x36f5) [0x7ff2897736f5]
/lib/x86_64-linux-gnu/libfrrsnmp.so.0(+0x3c27) [0x7ff289773c27]
/lib/x86_64-linux-gnu/libfrr.so.0(thread_call+0x1c2) [0x7ff289dbe105]
/lib/x86_64-linux-gnu/libfrr.so.0(frr_run+0x257) [0x7ff289d56e69]
/usr/bin/bgpd(main+0x4f4) [0x560965c40488]
/lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7ff2899a9d90]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7ff2899a9e40]
/usr/bin/bgpd(_start+0x25) [0x560965c3e965]
in thread agentx_timeout scheduled from /build/make-pkg/output/_packages/cp-routing/src/lib/agentx.c:122 agentx_events_update()
Also, the following error is logged by snmp just before the abort:
snmp[err]: Use snmp_sess_select_info2() for processing large file descriptors
snmp uses a custom struct netsnmp_large_fd_set to work above the limit
imposed by FD_SETSIZE. It is noteworthy that, when calling
snmp_select_info() instead of snmp_select_info2(), snmp uses the same
code working with its custom, large structs, and copy/paste the result
to a regular, libc compatible fd_set. So there should be no downside
working with snmp_select_info2() instead of snmp_select_info().
Replace every use of the libc file descriptors sets by snmp's extended
file descriptors sets in agentx to acommodate for the high number of
file descriptors that can come out of snmp. This should prevent the
abort seen above.
Donald Sharp [Fri, 5 Jan 2024 18:24:59 +0000 (13:24 -0500)]
zebra: SA incorrectly believes a NULL pointer
SA has decided that old_re could be a NULL pointer
even though the zebra_redistribute_check function
checks for NULL and returns false that would
not allow a NULL pointer deref.
Donald Sharp [Fri, 5 Jan 2024 18:02:16 +0000 (13:02 -0500)]
bgpd: data is set but never used
I've kept the assignment in a comment because I am concerned
about new code being added later that the data pointer would
not be set correctly. Next coder can see the commented
out line and uncomment it.
bgpd: Change printing format `show debugging` for `debug bgp updates`
Before:
```
BGP debugging status:
BGP updates debugging is on (inbound) for 127.0.0.1 with prefix-list rn-dummy 192.168.10.18 with prefix-list rn-dummy 1.1.1.1
```
After:
```
BGP debugging status:
BGP updates debugging is on (inbound) for:
127.0.0.1 with prefix-list rn-dummy
192.168.10.18 with prefix-list rn-dummy
192.168.10.20
```
bgpd: Fix memory leak for default-originate with route-map
```
Direct leak of 40 byte(s) in 1 object(s) allocated from:
0 0x7fc4b81eed28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
1 0x7fc4b7bd60bb in qcalloc lib/memory.c:105
2 0x56221dc19207 in aspath_dup bgpd/bgp_aspath.c:689
3 0x56221daacd42 in route_set_aspath_prepend bgpd/bgp_routemap.c:2283
4 0x7fc4b7c3891a in route_map_apply_ext lib/routemap.c:2687
5 0x56221dace552 in subgroup_default_originate bgpd/bgp_updgrp_adv.c:906
6 0x56221dabf79c in update_group_default_originate_route_map_walkcb bgpd/bgp_updgrp.c:2105
7 0x56221dabde4e in update_group_walkcb bgpd/bgp_updgrp.c:1721
8 0x7fc4b7b9d398 in hash_walk lib/hash.c:270
9 0x56221dac94cb in update_group_af_walk bgpd/bgp_updgrp.c:2062
10 0x56221dac9b0f in update_group_walk bgpd/bgp_updgrp.c:2071
11 0x56221dac9fd5 in update_group_refresh_default_originate_route_map bgpd/bgp_updgrp.c:2118
12 0x7fc4b7c7fc54 in event_call lib/event.c:1974
13 0x7fc4b7bb9276 in frr_run lib/libfrr.c:1214
14 0x56221d9217fd in main bgpd/bgp_main.c:510
15 0x7fc4b6bf2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
```
tmp_pi.attr should be flushed since it's already interned (new_attr) or the
origin value is used (attr).
Christian Hopps [Sat, 6 Jan 2024 09:45:29 +0000 (09:45 +0000)]
lib: mgmtd: implement full XPath 1.0 predicate functionality
Allow user to specify full YANG compatible XPath 1.0 predicates. This
allows for trimming results of generic queries using functions and other
non-key predicates from XPath 1.0
Donald Sharp [Sun, 7 Jan 2024 01:41:20 +0000 (20:41 -0500)]
lib: Use event name instead of thread
The thread system has been renamed, let's use the proper
terminology now in the show commands. Also realign
output a tiny bit for stuff that was missing.
Donald Sharp [Sun, 7 Jan 2024 01:26:14 +0000 (20:26 -0500)]
bgpd: bgp_sync_label_manager failure case
There are several problems with the bgp_sync_label_manager
function:
a) It is possible that a request in the lp->requests
fifo will be unable to be filled at this point in time
and the lf will be leaked and not ever fullfilled.
b) The bgp_sync_label_manager runs one time a second
irrelevant if there is work to do or not.
To fix (a) just add the request back to the requests
fifo and set the timer to pop in the future.
To fix (b) just every time something is put into
the request pool start a timer to run in 1 second
and do not restart it if all the work is done.
Donald Sharp [Thu, 4 Jan 2024 19:20:40 +0000 (14:20 -0500)]
lib: HAVE_GLIBC_BACKTRACE does not belong in zebra.h
The backtrace functionality has been abstracted over
to zlog_backtrace(). Now that every place uses this
move the inclusion for HAVE_GLIBC_BACKTRACE into
the appropriate files instead of having everyone
pay for this costly include.
> ==3901635==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020003a5940 at pc 0x56260067bb48 bp 0x7ffe8a4f3840 sp 0x7ffe8a4f3838
> READ of size 4 at 0x6020003a5940 thread T0
> #0 0x56260067bb47 in ecommunity_fill_pbr_action bgpd/bgp_ecommunity.c:1587
> #1 0x5626007a246e in bgp_pbr_build_and_validate_entry bgpd/bgp_pbr.c:939
> #2 0x5626007b25e6 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2933
> #3 0x562600909d18 in bgp_zebra_announce bgpd/bgp_zebra.c:1351
> #4 0x5626007d5efd in bgp_process_main_one bgpd/bgp_route.c:3528
> #5 0x5626007d6b43 in bgp_process_wq bgpd/bgp_route.c:3641
> #6 0x7f450f34c2cc in work_queue_run lib/workqueue.c:266
> #7 0x7f450f327a27 in event_call lib/event.c:1970
> #8 0x7f450f21a637 in frr_run lib/libfrr.c:1213
> #9 0x56260062fc04 in main bgpd/bgp_main.c:540
> #10 0x7f450ee2dd09 in __libc_start_main ../csu/libc-start.c:308
> #11 0x56260062ca29 in _start (/usr/lib/frr/bgpd+0x2e3a29)
>
> 0x6020003a5940 is located 0 bytes to the right of 16-byte region [0x6020003a5930,0x6020003a5940)
> allocated by thread T0 here:
> #0 0x7f450f6aa1f8 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164
> #1 0x7f450f244f8a in qrealloc lib/memory.c:112
> #2 0x562600673313 in ecommunity_add_val_internal bgpd/bgp_ecommunity.c:143
> #3 0x5626006735bc in ecommunity_uniq_sort_internal bgpd/bgp_ecommunity.c:193
> #4 0x5626006737e3 in ecommunity_parse_internal bgpd/bgp_ecommunity.c:228
> #5 0x562600673890 in ecommunity_parse bgpd/bgp_ecommunity.c:236
> #6 0x562600640469 in bgp_attr_ext_communities bgpd/bgp_attr.c:2674
> #7 0x562600646eb3 in bgp_attr_parse bgpd/bgp_attr.c:3893
> #8 0x562600791b7e in bgp_update_receive bgpd/bgp_packet.c:2141
> #9 0x56260079ba6b in bgp_process_packet bgpd/bgp_packet.c:3406
> #10 0x7f450f327a27 in event_call lib/event.c:1970
> #11 0x7f450f21a637 in frr_run lib/libfrr.c:1213
> #12 0x56260062fc04 in main bgpd/bgp_main.c:540
> #13 0x7f450ee2dd09 in __libc_start_main ../csu/libc-start.c:308
Fixes: dacf6ec120 ("bgpd: utility routine to convert flowspec actions into pbr actions") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
*** CID 1575079: Null pointer dereferences (REVERSE_INULL)
/zebra/zebra_dplane.c: 5950 in dplane_srv6_encap_srcaddr_set()
5944 if (ret == AOK)
5945 result = ZEBRA_DPLANE_REQUEST_QUEUED;
5946 else {
5947 atomic_fetch_add_explicit(&zdplane_info
5948 .dg_srv6_encap_srcaddr_set_errors,
5949 1, memory_order_relaxed);
CID 1575079: Null pointer dereferences (REVERSE_INULL)
Null-checking "ctx" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
5950 if (ctx)
5951 dplane_ctx_free(&ctx);
5952 }
5953 return result;
5954 }
5955
Remove the pointer check for `ctx`. At this point in the
function it has to be non null since we deref'ed it.
Additionally the alloc function that creates it cannot
fail.
Christian Hopps [Tue, 2 Jan 2024 11:34:34 +0000 (11:34 +0000)]
lib: don't count on scheme key nodes being first children
Heard back from libyang folks that this is not something they consider part of
the API and/or are going to guarantee. So we cannot count on it. Expect keys at
any location on the child sibling list.
projects / mirror / frr.git / log