Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7f4f47b43d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7f4f4753c0a8 in qcalloc lib/memory.c:105
#2 0x7f4f47559526 in nexthop_group_new lib/nexthop_group.c:270
#3 0x562ded6a39d4 in zebra_add_import_table_entry zebra/redistribute.c:681
#4 0x562ded787c35 in rib_link zebra/zebra_rib.c:3972
#5 0x562ded787c35 in rib_addnode zebra/zebra_rib.c:3993
#6 0x562ded787c35 in process_subq_early_route_add zebra/zebra_rib.c:2860
#7 0x562ded787c35 in process_subq_early_route zebra/zebra_rib.c:3138
#8 0x562ded787c35 in process_subq zebra/zebra_rib.c:3178
#9 0x562ded787c35 in meta_queue_process zebra/zebra_rib.c:3228
#10 0x7f4f475f7118 in work_queue_run lib/workqueue.c:266
#11 0x7f4f475dc7f2 in event_call lib/event.c:1969
#12 0x7f4f4751f347 in frr_run lib/libfrr.c:1213
#13 0x562ded69e818 in main zebra/main.c:486
#14 0x7f4f468ffc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Indirect leak of 152 byte(s) in 1 object(s) allocated from:
#0 0x7f4f47b43d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7f4f4753c0a8 in qcalloc lib/memory.c:105
#2 0x7f4f475510ad in nexthop_new lib/nexthop.c:376
#3 0x7f4f475539c5 in nexthop_dup lib/nexthop.c:914
#4 0x7f4f4755b27a in copy_nexthops lib/nexthop_group.c:444
#5 0x562ded6a3a1c in zebra_add_import_table_entry zebra/redistribute.c:682
#6 0x562ded787c35 in rib_link zebra/zebra_rib.c:3972
#7 0x562ded787c35 in rib_addnode zebra/zebra_rib.c:3993
#8 0x562ded787c35 in process_subq_early_route_add zebra/zebra_rib.c:2860
#9 0x562ded787c35 in process_subq_early_route zebra/zebra_rib.c:3138
#10 0x562ded787c35 in process_subq zebra/zebra_rib.c:3178
#11 0x562ded787c35 in meta_queue_process zebra/zebra_rib.c:3228
#12 0x7f4f475f7118 in work_queue_run lib/workqueue.c:266
#13 0x7f4f475dc7f2 in event_call lib/event.c:1969
#14 0x7f4f4751f347 in frr_run lib/libfrr.c:1213
#15 0x562ded69e818 in main zebra/main.c:486
#16 0x7f4f468ffc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 184 byte(s) leaked in 2 allocation(s).
***********************************************************************************
```
anlan_cs [Fri, 4 Mar 2022 13:52:27 +0000 (21:52 +0800)]
*: remove ZEBRA_INTERFACE_VRF_UPDATE
Currently when one interface changes its VRF, zebra will send these messages to
all daemons in *order*:
1) `ZEBRA_INTERFACE_DELETE` ( notify them delete from old VRF )
2) `ZEBRA_INTERFACE_VRF_UPDATE` ( notify them move from old to new VRF )
3) `ZEBRA_INTERFACE_ADD` ( notify them added into new VRF )
When daemons deal with `VRF_UPDATE`, they use
`zebra_interface_vrf_update_read()->if_lookup_by_name()`
to check the interface exist or not in old VRF. This check will always return
*NULL* because `DELETE` ( deleted from old VRF ) is already done, so can't
find this interface in old VRF.
Send `VRF_UPDATE` is redundant and unuseful. `DELETE` and `ADD` are enough,
they will deal with RB tree, so don't send this `VRF_UPDATE` message when
vrf changes.
Since all daemons have good mechanism to deal with changing vrf, and don't
use this `VRF_UPDATE` mechanism. So, it is safe to completely remove
all the code with `VRF_UPDATE`.
Mark Stapp [Thu, 5 Oct 2023 15:02:42 +0000 (11:02 -0400)]
lib,*: add vrf id to pbr rule results zapi message
The iprule/pbr rule object has a vrf id, and zebra uses
that internally, but the vrf id isn't returned to clients
who install rules and are waiting for results. Include the
vrf_id sent by the client in the zapi result notification
message; update the existing clients so they decode the id.
Mark Stapp [Thu, 5 Oct 2023 17:47:17 +0000 (13:47 -0400)]
tests: locate nhg id from json output in all_proto_startup
Don't hard-code a sharpd nhg id: those values aren't stable
if the daemons/protos/route-types change. Use json show output
to find the id in the 'resilient' nhg test case in
the all_protocol_startup suite.
Indicating the configured PIM Rendezvous Point (RP) in the MSDP SA
message
The RFC-3618, section 12.2.1, describes the fields included in the MSDP
SA message. The "RP address" field is "the address of the RP in the
domain the source has become active in".
In the most common case, we will establish an MSDP connection from RP to
RP. However, there are cases where we want to establish a MSDP
connection from an interface/address that is not the RP. Section 3 of
RFC-3618 describes that scenario as "intermediate MSDP peer". Moreover,
the RP could be another router in the PIM domain - not the one
establishing the MSDP connection.
The current implementation could be problematic even with a single
router per PIM domain. Consider the following scenario:
* There are two PIM domains, each one with a single router.
* The two routers are connected via two independent networks. Let's say
that is to provide redundancy.
* The routers are configured to establish two MSDP connections, one on
each network (redundancy again).
* A multicast source becomes active on the router 1. It will be
communicated to router 2 via two independent MSDP SA messages, one per
MSDP connection.
* Without these changes, each MSDP SA message will indicate a different
RP.
* Both RP addresses will pass the RPF check, and both MSDP sources will
be accepted.
* If the router has clients interested in that multicast group, it will
send PIM Join messages to both RPs and start receiving the multicast
traffic from both.
With the changes included in this commit, the multicast source available
in router 1 would still be communicated to router 2 twice. But both MSDP
SA messages would indicate the same RP, and one of them would be
discarded due to failure in the RPF-check failure. Also, the changes
allow us to define the RP that will be included in the MSDP SA message,
and it could be one of the interfaces used to establish the MSDP
connection, some other interface on the router, a loopback interface, or
another router in the PIM domain.
These changes should not create compatibility issues. As I mentioned, we
usually establish MSDP connections from RP to RP. In this case, the
result will be the same. We would still indicate the address used to
establish the MSDP connection if the RP is not set - I wonder if that
should even be a valid configuration.
Signed-off-by: Adriano Marto Reis <adrianomarto@gmail.com>
Philippe Guibert [Tue, 27 Jun 2023 19:36:01 +0000 (21:36 +0200)]
bgpd: add 'match community-list any' function
There is no match mechanism to match one community from the
incoming community-list. Add the 'any' keyword to the 'match
route-map' command of communit-list and large-community-list.
> match community-list AAA any
> match large-community-list AAA any
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Mark Stapp [Fri, 29 Sep 2023 16:08:17 +0000 (12:08 -0400)]
zebra: be more careful removing 'installed' flag from nhgs
When interface addresses change, we examine nhgs associated
with the interface in case they need to be reinstalled. As
part of that, we may need to reinstall ecmp nhgs that use the
interface being examined - but not always.
fmihoub6WIND [Fri, 25 Nov 2022 10:17:07 +0000 (11:17 +0100)]
zebra: delete label chunk upon release
In zebra/label_manager.c the releasing of the label chunk is done by
disowning the chunk to the system. The presence of this system label
chunk will cause label assignment to fail for this use case example:
Christian Hopps [Thu, 28 Sep 2023 16:10:31 +0000 (12:10 -0400)]
lib: use XREALLOC over realloc avoid coverity warning
I believe coverity is complaining that the current code does not handle the
realloc fail case, in which case the original pointer is not freed, but NULL is
returned. The code assert()s it's not failed but that is not strong enough it
needs to abort which XREALLOC does and is a better integration into FRR-inrfa
anyway.
Louis Scalbert [Thu, 28 Sep 2023 14:55:43 +0000 (16:55 +0200)]
bgpd: fix insecure data write with area addresses
Fix an issue where an attacker may inject a tainted length value to
corrupt the memory.
> CID 1568380 (#1 of 1): Untrusted value as argument (TAINTED_SCALAR)
> 9. tainted_data: Passing tainted expression length to bgp_linkstate_nlri_value_display, which uses it as an offset
Fixes: 8b531b1107 ("bgpd: store and send bgp link-state attributes") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Louis Scalbert [Thu, 28 Sep 2023 14:53:35 +0000 (16:53 +0200)]
bgpd: fix insecure data write with ip addresses
Fix issues where an attacker may inject a tainted length value to
corrupt the memory.
> CID 1568378 (#1-6 of 6): Untrusted value as argument (TAINTED_SCALAR)
> 16. tainted_data: Passing tainted expression length to bgp_linkstate_tlv_attribute_value_display, which uses it as an offset. [show details]
Quentin Young [Thu, 28 Sep 2023 14:49:37 +0000 (10:49 -0400)]
doc: unpin sphinx from 4.0.2
requirements.txt was pinning sphinx at a very old version. This version
doesn't work in recent versions of Python; the new RTD configuration
made RTD respect our requirements file, breaking the build.
Signed-off-by: Quentin Young <qlyoung@qlyoung.net>
Louis Scalbert [Thu, 28 Sep 2023 13:27:27 +0000 (15:27 +0200)]
bgpd: fix illegal memory access in bgp_ls_tlv_check_size()
Fix illegal memory access bgp_ls_tlv_check_size() if type is 1253.
> CID 1568377 (#4 of 4): Out-of-bounds read (OVERRUN)
> 5. overrun-local: Overrunning array bgp_linkstate_tlv_infos of 1253 16-byte elements at element index 1253 (byte offset 20063) using index type (which evaluates to 1253).
Louis Scalbert [Thu, 28 Sep 2023 13:08:23 +0000 (15:08 +0200)]
bgpd: do not check attr in bgp_packet_attribute
Fix the following coverity issue. attr cannot be NULL.
> CID 1568376 (#1 of 1): Dereference before null check (REVERSE_INULL)
> check_after_deref: Null-checking attr suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
Fixes: 8b531b1107 ("bgpd: store and send bgp link-state attributes") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Igor Ryzhov [Wed, 27 Sep 2023 23:45:05 +0000 (02:45 +0300)]
vtysh: fix entering configuration node in file-lock mode
When the config node is entered in file-lock mode, we should actually
remember it to correctly apply the workaround in `vtysh_exit`.
Otherwise, the file-lock mode is dropped once we exit any node one level
below the config node.
Igor Ryzhov [Wed, 27 Sep 2023 23:41:16 +0000 (02:41 +0300)]
vty: fix working in file-lock mode
When the configuration node is entered in file-lock mode, candidate
and running datastores are locked. Any configuration change is followed
by an implicit commit which leads to a crash of mgmtd, because double
lock is prohibited by an assert. When working in file-lock mode, we
shouldn't do implicit commits which is disabled by allowing pending
configuration changes.
When we accept a connection, we try to set TTL for the socket, but the socket
is not yet created/assigned and we are trying to set it on the wrong socket fd.
```
[Event] connection from 127.0.0.1 fd 25, active peer status 3 fd -1
can't set sockopt IP_TTL 255 to socket -1
bgp_set_socket_ttl: Can't set TxTTL on peer (rtrid 0.0.0.0) socket, err = 9
Unable to set min/max TTL on peer 127.0.0.1, Continuing
```
David Lamparter [Sun, 24 Sep 2023 18:12:42 +0000 (20:12 +0200)]
lib: assert for VTY_PASSFD expectations
Coverity is complaining that vty->state could be VTY_PASSFD here. It
can't, it really shouldn't, and if it actually is then something went
seriously wrong somewhere earlier so assert()ing out is the best thing
to do.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>