PIMD: Fix pim_mroute_del crash while killing pimd. #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we kill pimd, pim_upstream_del() will be get called as part of cleaning process. Delete mroute by calling pim_mroute_del() which in result initialize the up->channel_oil to NULL after cleanup. Delete each ifchannel stored in the upstream ifchannel list one by one. As part of pim_ifchannel_delete(), it will check if it is the last ifchannel, then delete the corresponding upstream. So pim_upstream_del will be get called more than once as part of pim_upstream_terminate() and pim_ ifchannel_delete(). Similarly pim_mroute_del() will be again called and since there is no NULL check before accesing the data, crash is happening. Fix: Adding Null check in pim_mroute_del before accessing the data. and using the FLAG "PIM_UPSTREAM_FLAG_DEL" which will check if the deletion of pim upstream is already in progress then don’t call pim_upstream_del() from pim_ifchannel_delete(). Signed-off-by: Sarita Patra <saritap@vmware.com>
| @@ -230,6 +232,7 @@ struct pim_upstream *pim_upstream_del(struct pim_instance *pim, | |||
| __PRETTY_FUNCTION__, up->sg_str, buf); | |||
| } | |||
| pim_delete_tracked_nexthop(pim, &nht_p, up, NULL); | |||
| up->flags &= (~PIM_UPSTREAM_FLAG_DEL); | |||
There was a problem hiding this comment.
It seems, there is no need to reset the flag; as upstream is going to delete.
patrasar
pushed a commit
that referenced
this pull request
Nov 26, 2018
This commit introduces lib/id_alloc, which has facilities for both an ID number allocator, and less efficient ID holding pools. The pools are meant to be a temporary holding area for ID numbers meant to be re-used, and are implemented as a linked-list stack. The allocator itself is much more efficient with memory. Based on sizeof values on my 64 bit desktop, the allocator requires around 155 KiB per million IDs tracked. IDs are ultimately tracked in a bit-map split into many "pages." The allocator tracks a list of pages that have free bits, and which sections of each page have free IDs, so there isn't any scanning required to find a free ID. (The library utility ffs, or "Find First Set," is generally a single CPU instruction.) At the moment, totally empty pages will not be freed, so the memory utilization of this allocator will remain at the high water mark. The initial intended use case is for BGP's TX Addpath IDs to be pulled from an allocator that tracks which IDs are in use, rather than a free running counter. The allocator reserves ID #0 as a sentinel value for an invalid ID numbers, and BGP will want ID #1 reserved as well. To support this, the allocator allows for IDs to be explicitly reserved, though be aware this is only practical to use with low numbered IDs because the allocator must allocate pages in order. Signed-off-by Mitchell Skiba <mskiba@amazon.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 19, 2019
root@TORS1:~# net show bgp l2vpn evpn route rd 27.0.0.15:4 type multicast EVPN type-2 prefix: [2]:[ESI]:[EthTag]:[MAClen]:[MAC] EVPN type-3 prefix: [3]:[EthTag]:[IPlen]:[OrigIP] EVPN type-5 prefix: [5]:[ESI]:[EthTag]:[IPlen]:[IP] BGP routing table entry for 27.0.0.15:4:[3]:[0]:[32]:[27.0.0.15] Paths: (1 available, best #1) Advertised to non peer-group peers: MSP1(uplink-1) MSP2(uplink-2) Route [3]:[0]:[32]:[27.0.0.15] VNI 1003 Local 27.0.0.15 from 0.0.0.0 (27.0.0.15) Origin IGP, weight 32768, valid, sourced, local, bestpath-from-AS Local, best Extended Community: ET:8 RT:5550:1003 AddPath ID: RX 0, TX 10 Last update: Thu Feb 7 00:17:24 2019 PMSI Tunnel Type: Ingress Replication, label: 1003 >>>>>>>>>>>>> Displayed 1 prefixes (1 paths) with this RD (of requested type) root@TORS1:~# Ticket: CM-23790 Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 19, 2019
Consider the following topo VTEP1->SPINE1->VTEP2. ebgp is being used for evpn route exchange with SPINE just acting as a pass through. 1. VTEP1 was building the type-3 IMET route with the correct PMSI tunnel type (ingress-replication) and label (VNI) 2. Spine1 was however only parsing the tunnel-type in the attr (was skipping parsing of the label field altogether) - >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> root@MSP1:~# net show bgp l2vpn evpn route rd 27.0.0.15:4 type multicast EVPN type-2 prefix: [2]:[ESI]:[EthTag]:[MAClen]:[MAC] EVPN type-3 prefix: [3]:[EthTag]:[IPlen]:[OrigIP] EVPN type-5 prefix: [5]:[ESI]:[EthTag]:[IPlen]:[IP] BGP routing table entry for 27.0.0.15:4:[3]:[0]:[32]:[27.0.0.15] Paths: (1 available, best #1) Advertised to non peer-group peers: TORC11(downlink-1) TORC12(downlink-2) TORC21(downlink-3) TORC22(downlink-4) TORS1(downlink-5) TORS2(downlink-6) Route [3]:[0]:[32]:[27.0.0.15] 5550 27.0.0.15 from TORS1(downlink-5) (27.0.0.15) Origin IGP, valid, external, bestpath-from-AS 5550, best Extended Community: RT:5550:1003 ET:8 AddPath ID: RX 0, TX 227 Last update: Thu Feb 7 15:44:22 2019 PMSI Tunnel Type: Ingress Replication, label: 16777213 >>>>>>> Displayed 1 prefixes (1 paths) with this RD (of requested type) root@MSP1:~# >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3. So VTEP2 didn't rx the correct label. In an all FRR setup this doesn't have any functional consequence but some vendors are validating the content of the label field as well and ignoring the IMET route from FRR (say VTEP1 is FRR and VTEP2 is 3rd-party). The functional consequence of this VTEP2 ignores VTEP1's IMET route and doesn't add VTEP1 to the corresponding l2-vni flood list. This commit fixes up the PMSI attr parsing on spine-1 - >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> root@MSP1:~# net show bgp l2vpn evpn route rd 27.0.0.15:4 type multicast EVPN type-2 prefix: [2]:[ESI]:[EthTag]:[MAClen]:[MAC] EVPN type-3 prefix: [3]:[EthTag]:[IPlen]:[OrigIP] EVPN type-5 prefix: [5]:[ESI]:[EthTag]:[IPlen]:[IP] BGP routing table entry for 27.0.0.15:4:[3]:[0]:[32]:[27.0.0.15] Paths: (1 available, best #1) Advertised to non peer-group peers: TORC11(downlink-1) TORC12(downlink-2) TORC21(downlink-3) TORC22(downlink-4) TORS1(downlink-5) TORS2(downlink-6) Route [3]:[0]:[32]:[27.0.0.15] 5550 27.0.0.15 from TORS1(downlink-5) (27.0.0.15) Origin IGP, valid, external, bestpath-from-AS 5550, best Extended Community: RT:5550:1003 ET:8 AddPath ID: RX 0, TX 278 Last update: Thu Feb 7 00:17:40 2019 PMSI Tunnel Type: Ingress Replication, label: 1003 >>>>>>>>>>> Displayed 1 prefixes (1 paths) with this RD (of requested type) root@MSP1:~# >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ticket: CM-23790 Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 24, 2019
If path->net is NULL in the bgp_path_info_free() function, then bgpd would crash in bgp_addpath_free_info_data() with the following backtrace: (gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x00007ff7b267a42a in __GI_abort () at abort.c:89 #2 0x00007ff7b39c1ca0 in core_handler (signo=11, siginfo=0x7ffff66414f0, context=<optimized out>) at lib/sigevent.c:249 #3 <signal handler called> #4 idalloc_free_to_pool (pool_ptr=pool_ptr@entry=0x0, id=3) at lib/id_alloc.c:368 FRRouting#5 0x0000560096246688 in bgp_addpath_free_info_data (d=d@entry=0x560098665468, nd=0x0) at bgpd/bgp_addpath.c:100 FRRouting#6 0x00005600961bb522 in bgp_path_info_free (path=0x560098665400) at bgpd/bgp_route.c:252 FRRouting#7 bgp_path_info_unlock (path=0x560098665400) at bgpd/bgp_route.c:276 FRRouting#8 0x00005600961bb719 in bgp_path_info_reap (rn=rn@entry=0x5600986b2110, pi=pi@entry=0x560098665400) at bgpd/bgp_route.c:320 FRRouting#9 0x00005600961bf4db in bgp_process_main_one (safi=SAFI_MPLS_VPN, afi=AFI_IP, rn=0x5600986b2110, bgp=0x560098587320) at bgpd/bgp_route.c:2476 FRRouting#10 bgp_process_wq (wq=<optimized out>, data=0x56009869b8f0) at bgpd/bgp_route.c:2503 FRRouting#11 0x00007ff7b39d5fcc in work_queue_run (thread=0x7ffff6641e10) at lib/workqueue.c:294 FRRouting#12 0x00007ff7b39ce3b1 in thread_call (thread=thread@entry=0x7ffff6641e10) at lib/thread.c:1606 FRRouting#13 0x00007ff7b39a3538 in frr_run (master=0x5600980795b0) at lib/libfrr.c:1011 FRRouting#14 0x000056009618a5a3 in main (argc=3, argv=0x7ffff6642078) at bgpd/bgp_main.c:481 Add a null-check protection to fix this problem. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
patrasar
pushed a commit
that referenced
this pull request
Mar 21, 2019
After a router reboot the L3 network via it converges before the L2 network. This is because MLAG intentionally holds down bridge-access and vxlan-network ports for some time (MLAG init-delay) to prevent traffic from switching to a router that is not fully ready. This also means that routes (from vrf-peering sessions) that qualify for evpn type-5 advertisments are available long before the L3-VNI is available for that tenant VRF. In these windows bgpd was adding these evpn-type-5 routes with a L3-VNI of 0 (which was not fixed up after the L3-VNI became available) - BGP routing table entry for 100.0.0.1:2:[5]:[0]:[0]:[32]:[200.1.1.1] Paths: (1 available, best #1) Advertised to non peer-group peers: MSP1(uplink-1) MSP2(uplink-2) Route [5]:[0]:[0]:[32]:[200.1.1.1] VNI 0 >>>>>>>> 65001 65535 36.0.0.9 from 0.0.0.0 (27.0.0.9) Origin incomplete, metric 0, valid, sourced, local, bestpath-from-AS 65001, best Extended Community: ET:8 RT:5544:4001 Rmac:44:38:39:ff:ff:01 AddPath ID: RX 0, TX 327 Last update: Wed Feb 27 18:37:10 2019 Fix is to defer creating type-5 routes till the L3-VNI is available for that tenant VRF (this was already being done for most cases; fixup takes care of some that missed the check). Ticket: CM-24022 Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Apr 29, 2019
Fixes the following warning when running ripngd with valgrind: ==38== Syscall param sendmsg(msg.msg_control) points to uninitialised byte(s) ==38== at 0x5EA1E47: sendmsg (sendmsg.c:28) ==38== by 0x118C48: ripng_send_packet (ripngd.c:226) ==38== by 0x11D1D6: ripng_request (ripngd.c:1924) ==38== by 0x120BD8: ripng_interface_wakeup (ripng_interface.c:666) ==38== by 0x4ECB4B4: thread_call (thread.c:1601) ==38== by 0x4E8D9CE: frr_run (libfrr.c:1011) ==38== by 0x1121C8: main (ripng_main.c:180) ==38== Address 0xffefffc34 is on thread 1's stack ==38== in frame #1, created by ripng_send_packet (ripngd.c:172) Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
patrasar
pushed a commit
that referenced
this pull request
May 20, 2019
As part of detailed bgp route detail, include the reason why a route was selected as best path. robot# show bgp ipv4 uni 223.255.254.0 BGP routing table entry for 223.255.254.0/24 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: annie(192.168.201.136) 64539 15096 6939 7473 3758 55415 192.168.201.136 from annie(192.168.201.136) (192.168.201.136) Origin IGP, valid, external, bestpath-from-AS 64539, best (First path received) Last update: Wed May 15 21:15:48 2019 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Sep 4, 2019
PR FRRouting#3745 added EVPN feature to advertise individual SVI-IPs as MAC-IP routes. Fix a condition in zebra to send MAC and IP pair to bgpd when the feature is enabled. Testing Done: Originator VTEP: TORC11:~# ip -br addr show VxU-1002 VxU-1002 UP 45.0.2.2/24 2001:fee1:0:2::2/64 show bgp l2vpn evpn vni 1004 VNI: 1004 (known to the kernel) Type: L2 Tenant-Vrf: default RD: 27.0.0.11:3 Advertise-svi-macip : Yes Import Route Target: 10:1004 Export Route Target: 10:1004 Remote vtep evpn route output for 45.0.4.2: BGP routing table entry for 27.0.0.11:3:[2]:[0]:[48]:[00:02:00:00:00:2f]:[32]:[45.0.4.2] Paths: (2 available, best #1) Advertised to non peer-group peers: MSP1(uplink-1) MSP2(uplink-2) Route [2]:[0]:[48]:[00:02:00:00:00:2f]:[32]:[45.0.4.2] VNI 1004 64435 65546 36.0.0.11 from MSP1(uplink-1) (27.0.0.9) Origin IGP, valid, external, bestpath-from-AS 64435, best (First path received) Extended Community: RT:10:1004 ET:8 Last update: Thu Aug 8 18:09:13 2019 Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Sep 4, 2019
The `set as-path prepend last-as X` command had no, 'no' form of the command. Add this into the cli. Testing: ! route-map BLARBLE permit 10 set as-path prepend last-as 3 ! ! router bgp 9999 neighbor 10.50.12.118 remote-as external neighbor 10.50.12.118 ebgp-multihop 30 ! address-family ipv4 unicast neighbor 10.50.12.118 route-map BLARBLE in ! ! eva# show bgp ipv4 uni 4.4.4.4 BGP routing table entry for 4.4.4.4/32 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: 10.50.12.118 999 999 999 999 10.50.12.118 from 10.50.12.118 (10.50.12.118) Origin incomplete, metric 0, valid, external, best (First path received) Last update: Mon Aug 26 09:47:17 2019 eva# conf eva(config)# route-map BLARBLE permit 10 eva(config-route-map)# no set as-path prepend last-as 3 eva(config-route-map)# end eva# clear bgp ipv4 uni * eva# show bgp ipv4 uni 4.4.4.4 BGP routing table entry for 4.4.4.4/32 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: 10.50.12.118 999 10.50.12.118 from 10.50.12.118 (10.50.12.118) Origin incomplete, metric 0, valid, external, best (First path received) Last update: Mon Aug 26 09:48:31 2019 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 29, 2019
Our Address Sanitizer CI is finding this issue: error 09-Oct-2019 19:28:33 r4: bgpd triggered an exception by AddressSanitizer error 09-Oct-2019 19:28:33 ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdd425b060 at pc 0x00000068575f bp 0x7ffdd4258550 sp 0x7ffdd4258540 error 09-Oct-2019 19:28:33 READ of size 1 at 0x7ffdd425b060 thread T0 error 09-Oct-2019 19:28:33 #0 0x68575e in prefix_cmp lib/prefix.c:776 error 09-Oct-2019 19:28:33 #1 0x5889f5 in rfapiItBiIndexSearch bgpd/rfapi/rfapi_import.c:2230 error 09-Oct-2019 19:28:33 #2 0x5889f5 in rfapiBgpInfoFilteredImportVPN bgpd/rfapi/rfapi_import.c:3520 error 09-Oct-2019 19:28:33 #3 0x58b909 in rfapiProcessWithdraw bgpd/rfapi/rfapi_import.c:4071 error 09-Oct-2019 19:28:33 #4 0x4c459b in bgp_withdraw bgpd/bgp_route.c:3736 error 09-Oct-2019 19:28:33 FRRouting#5 0x484122 in bgp_nlri_parse_vpn bgpd/bgp_mplsvpn.c:237 error 09-Oct-2019 19:28:33 FRRouting#6 0x497f52 in bgp_nlri_parse bgpd/bgp_packet.c:315 error 09-Oct-2019 19:28:33 FRRouting#7 0x49d06d in bgp_update_receive bgpd/bgp_packet.c:1598 error 09-Oct-2019 19:28:33 FRRouting#8 0x49d06d in bgp_process_packet bgpd/bgp_packet.c:2274 error 09-Oct-2019 19:28:33 FRRouting#9 0x6b9f54 in thread_call lib/thread.c:1531 error 09-Oct-2019 19:28:33 FRRouting#10 0x657037 in frr_run lib/libfrr.c:1052 error 09-Oct-2019 19:28:33 FRRouting#11 0x42d268 in main bgpd/bgp_main.c:486 error 09-Oct-2019 19:28:33 FRRouting#12 0x7f806032482f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f) error 09-Oct-2019 19:28:33 FRRouting#13 0x42bcc8 in _start (/usr/lib/frr/bgpd+0x42bcc8) error 09-Oct-2019 19:28:33 error 09-Oct-2019 19:28:33 Address 0x7ffdd425b060 is located in stack of thread T0 at offset 240 in frame error 09-Oct-2019 19:28:33 #0 0x483945 in bgp_nlri_parse_vpn bgpd/bgp_mplsvpn.c:103 error 09-Oct-2019 19:28:33 error 09-Oct-2019 19:28:33 This frame has 5 object(s): error 09-Oct-2019 19:28:33 [32, 36) 'label' error 09-Oct-2019 19:28:33 [96, 108) 'rd_as' error 09-Oct-2019 19:28:33 [160, 172) 'rd_ip' error 09-Oct-2019 19:28:33 [224, 240) 'prd' <== Memory access at offset 240 overflows this variable error 09-Oct-2019 19:28:33 [288, 336) 'p' error 09-Oct-2019 19:28:33 HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext error 09-Oct-2019 19:28:33 (longjmp and C++ exceptions *are* supported) error 09-Oct-2019 19:28:33 SUMMARY: AddressSanitizer: stack-buffer-overflow lib/prefix.c:776 prefix_cmp error 09-Oct-2019 19:28:33 Shadow bytes around the buggy address: error 09-Oct-2019 19:28:33 0x10003a8435b0: 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 error 09-Oct-2019 19:28:33 0x10003a8435c0: 00 00 00 00 00 00 00 00 00 00 f3 f3 f3 f3 f3 f3 error 09-Oct-2019 19:28:33 0x10003a8435d0: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 error 09-Oct-2019 19:28:33 0x10003a8435e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 error 09-Oct-2019 19:28:33 0x10003a8435f0: f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 04 f4 f4 f2 f2 error 09-Oct-2019 19:28:33 =>0x10003a843600: f2 f2 00 04 f4 f4 f2 f2 f2 f2 00 00[f4]f4 f2 f2 error 09-Oct-2019 19:28:33 0x10003a843610: f2 f2 00 00 00 00 00 00 f4 f4 f3 f3 f3 f3 00 00 error 09-Oct-2019 19:28:33 0x10003a843620: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 error 09-Oct-2019 19:28:33 0x10003a843630: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 02 f4 error 09-Oct-2019 19:28:33 0x10003a843640: f4 f4 f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2 00 00 error 09-Oct-2019 19:28:33 0x10003a843650: f4 f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00 error 09-Oct-2019 19:28:33 Shadow byte legend (one shadow byte represents 8 application bytes): error 09-Oct-2019 19:28:33 Addressable: 00 error 09-Oct-2019 19:28:33 Partially addressable: 01 02 03 04 05 06 07 error 09-Oct-2019 19:28:33 Heap left redzone: fa error 09-Oct-2019 19:28:33 Heap right redzone: fb error 09-Oct-2019 19:28:33 Freed heap region: fd error 09-Oct-2019 19:28:33 Stack left redzone: f1 error 09-Oct-2019 19:28:33 Stack mid redzone: f2 error 09-Oct-2019 19:28:33 Stack right redzone: f3 error 09-Oct-2019 19:28:33 Stack partial redzone: f4 error 09-Oct-2019 19:28:33 Stack after return: f5 error 09-Oct-2019 19:28:33 Stack use after scope: f8 error 09-Oct-2019 19:28:33 Global redzone: f9 error 09-Oct-2019 19:28:33 Global init order: f6 error 09-Oct-2019 19:28:33 Poisoned by user: f7 error 09-Oct-2019 19:28:33 Container overflow: fc error 09-Oct-2019 19:28:33 Array cookie: ac error 09-Oct-2019 19:28:33 Intra object redzone: bb error 09-Oct-2019 19:28:33 ASan internal: fe error 09-Oct-2019 19:28:36 r3: Daemon bgpd not running This is the result of this code pattern in rfapi/rfapi_import.c: prefix_cmp((struct prefix *)&bpi_result->extra->vnc.import.rd, (struct prefix *)prd)) Effectively prd or vnc.import.rd are `struct prefix_rd` which are being typecast to a `struct prefix`. Not a big deal except commit 1315d74 modified the prefix_cmp function to allow for a sorted prefix_cmp. In prefix_cmp we were looking at the offset and shift. In the case of vnc we were passing a prefix length of 64 which is the exact length of the remaining data structure for struct prefix_rd. So we calculated a offset of 8 and a shift of 0. The data structures for the prefix portion happened to be equal to 64 bits of data. So we checked that with the memcmp got a 0 and promptly read off the end of the data structure for the numcmp. The fix is if shift is 0 that means thei the memcmp has checked everything and there is nothing to do. Please note: We will still crash if we set the prefixlen > then ~312 bits currently( ie if the prefixlen specifies a bit length longer than the prefix length ). I do not think there is anything to do here( nor am I sure how to correct this either ) as that we are going to have some severe problems when we muck up the prefixlen. Fixes: FRRouting#5025 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 29, 2019
When a type 2/3 or 5 route is received, verified and the resulting route generated is pushed into the appropriate vrf the vni's associated with the route are also passed in. This is showing up as a Remote label when you dump the route in bgp: BGP routing table entry for 0.0.0.0/0^M Paths: (1 available, best #1, table third) Advertised to non peer-group peers: 10.10.120.22 42001 42005 42006 42055 10.10.120.22 from 10.10.120.22 (10.10.255.193) Origin IGP, valid, external, bestpath-from-AS 42001, best Remote label: 62750 AddPath ID: RX 0, TX 2 Last update: Fri Oct 11 12:59:56 2019 The `Remote label: 62750` is the mpls label version of the vni passed in. This is meaningless and confusing to the end user. Do not display this information. Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Nov 19, 2019
This code is called from the zebra main pthread during shutdown but the thread event is scheduled via the zebra dplane pthread. Hence, we should be using the `thread_cancel_async()` API to cancel the thread event on a different pthread. This is only ever hit in the rare case that we still have work left to do on the update queue during shutdown. Found via zebra crash: ``` (gdb) bt \#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 \#1 0x00007f4e4d3f7535 in __GI_abort () at abort.c:79 \#2 0x00007f4e4d3f740f in __assert_fail_base (fmt=0x7f4e4d559ee0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7f4e4d9071d0 "master->owner == pthread_self()", file=0x7f4e4d906cf8 "lib/thread.c", line=1185, function=<optimized out>) at assert.c:92 \#3 0x00007f4e4d405102 in __GI___assert_fail (assertion=assertion@entry=0x7f4e4d9071d0 "master->owner == pthread_self()", file=file@entry=0x7f4e4d906cf8 "lib/thread.c", line=line@entry=1185, function=function@entry=0x7f4e4d906b68 <__PRETTY_FUNCTION__.15817> "thread_cancel") at assert.c:101 \#4 0x00007f4e4d8d095a in thread_cancel (thread=0x55b40d01a640) at lib/thread.c:1185 \FRRouting#5 0x000055b40c291845 in zebra_dplane_shutdown () at zebra/zebra_dplane.c:3274 \FRRouting#6 0x000055b40c27ee13 in zebra_finalize (dummy=<optimized out>) at zebra/main.c:202 \FRRouting#7 0x00007f4e4d8d1416 in thread_call (thread=thread@entry=0x7ffcbbc08870) at lib/thread.c:1599 \FRRouting#8 0x00007f4e4d8a1ef8 in frr_run (master=0x55b40ce35510) at lib/libfrr.c:1024 \FRRouting#9 0x000055b40c270916 in main (argc=8, argv=0x7ffcbbc08c78) at zebra/main.c:483 (gdb) down \#4 0x00007f4e4d8d095a in thread_cancel (thread=0x55b40d01a640) at lib/thread.c:1185 1185 assert(master->owner == pthread_self()); (gdb) ``` Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Dec 20, 2019
Address Sanitizer is reporting this issue:
==26177==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000238d8 at pc 0x7f88f7c4fa93 bp 0x7fff9a641830 sp 0x7fff9a641820
READ of size 8 at 0x6120000238d8 thread T0
#0 0x7f88f7c4fa92 in if_delete lib/if.c:290
#1 0x42192e in ospf_vl_if_delete ospfd/ospf_interface.c:912
#2 0x42192e in ospf_vl_delete ospfd/ospf_interface.c:990
#3 0x4a6208 in no_ospf_area_vlink ospfd/ospf_vty.c:1227
#4 0x7f88f7c1553d in cmd_execute_command_real lib/command.c:1073
FRRouting#5 0x7f88f7c19b1e in cmd_execute_command lib/command.c:1132
FRRouting#6 0x7f88f7c19e8e in cmd_execute lib/command.c:1288
FRRouting#7 0x7f88f7cd7523 in vty_command lib/vty.c:516
FRRouting#8 0x7f88f7cd79ff in vty_execute lib/vty.c:1285
FRRouting#9 0x7f88f7cde4f9 in vtysh_read lib/vty.c:2119
FRRouting#10 0x7f88f7ccb845 in thread_call lib/thread.c:1549
FRRouting#11 0x7f88f7c5d6a7 in frr_run lib/libfrr.c:1093
FRRouting#12 0x412976 in main ospfd/ospf_main.c:221
FRRouting#13 0x7f88f73b082f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
FRRouting#14 0x413c78 in _start (/usr/local/master/sbin/ospfd+0x413c78)
Effectively we are in a shutdown phase and as part of shutdown we delete the
ospf interface pointer ( ifp->info ). The interface deletion code
was modified in the past year to pass in the address of operator
to allow us to NULL out the holding pointer. The catch here
is that we free the oi and then delete the interface passing
in the address of the oi->ifp pointer, causing a use after free.
Fixes: FRRouting#5555
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 5, 2020
Running with --enable-address-sanitizer I am seeing this:
=================================================================
==19520==ERROR: AddressSanitizer: heap-use-after-free on address 0x6020003ef850 at pc 0x7fe9b8f7b57b bp 0x7fffbac6f9c0 sp 0x7fffbac6f170
READ of size 6 at 0x6020003ef850 thread T0
#0 0x7fe9b8f7b57a (/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
#1 0x55e33d1071e5 in bgp_process_mac_rescan_table bgpd/bgp_mac.c:159
#2 0x55e33d107c09 in bgp_mac_rescan_evpn_table bgpd/bgp_mac.c:252
#3 0x55e33d107e39 in bgp_mac_rescan_all_evpn_tables bgpd/bgp_mac.c:266
#4 0x55e33d108270 in bgp_mac_remove_ifp_internal bgpd/bgp_mac.c:291
FRRouting#5 0x55e33d108893 in bgp_mac_del_mac_entry bgpd/bgp_mac.c:351
FRRouting#6 0x55e33d21412d in bgp_ifp_down bgpd/bgp_zebra.c:257
FRRouting#7 0x7fe9b8cbf3be in if_down_via_zapi lib/if.c:198
FRRouting#8 0x7fe9b8db303a in zclient_interface_down lib/zclient.c:1549
FRRouting#9 0x7fe9b8db8a06 in zclient_read lib/zclient.c:2693
FRRouting#10 0x7fe9b8d7b95a in thread_call lib/thread.c:1599
FRRouting#11 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
FRRouting#12 0x55e33d09d463 in main bgpd/bgp_main.c:477
FRRouting#13 0x7fe9b879409a in __libc_start_main ../csu/libc-start.c:308
FRRouting#14 0x55e33d09c189 in _start (/usr/lib/frr/bgpd+0x168189)
0x6020003ef850 is located 0 bytes inside of 16-byte region [0x6020003ef850,0x6020003ef860)
freed by thread T0 here:
#0 0x7fe9b8fabfb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0)
#1 0x7fe9b8ce4ea9 in qfree lib/memory.c:129
#2 0x55e33d10825c in bgp_mac_remove_ifp_internal bgpd/bgp_mac.c:289
#3 0x55e33d108893 in bgp_mac_del_mac_entry bgpd/bgp_mac.c:351
#4 0x55e33d21412d in bgp_ifp_down bgpd/bgp_zebra.c:257
FRRouting#5 0x7fe9b8cbf3be in if_down_via_zapi lib/if.c:198
FRRouting#6 0x7fe9b8db303a in zclient_interface_down lib/zclient.c:1549
FRRouting#7 0x7fe9b8db8a06 in zclient_read lib/zclient.c:2693
FRRouting#8 0x7fe9b8d7b95a in thread_call lib/thread.c:1599
FRRouting#9 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
FRRouting#10 0x55e33d09d463 in main bgpd/bgp_main.c:477
FRRouting#11 0x7fe9b879409a in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
#0 0x7fe9b8fac518 in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0xe9518)
#1 0x7fe9b8ce4d93 in qcalloc lib/memory.c:110
#2 0x55e33d106b29 in bgp_mac_hash_alloc bgpd/bgp_mac.c:96
#3 0x7fe9b8cb8350 in hash_get lib/hash.c:149
#4 0x55e33d10845b in bgp_mac_add_mac_entry bgpd/bgp_mac.c:303
FRRouting#5 0x55e33d226757 in bgp_ifp_create bgpd/bgp_zebra.c:2644
FRRouting#6 0x7fe9b8cbf1e6 in if_new_via_zapi lib/if.c:176
FRRouting#7 0x7fe9b8db2d3b in zclient_interface_add lib/zclient.c:1481
FRRouting#8 0x7fe9b8db87f8 in zclient_read lib/zclient.c:2659
FRRouting#9 0x7fe9b8d7b95a in thread_call lib/thread.c:1599
FRRouting#10 0x7fe9b8cd824e in frr_run lib/libfrr.c:1024
FRRouting#11 0x55e33d09d463 in main bgpd/bgp_main.c:477
FRRouting#12 0x7fe9b879409a in __libc_start_main ../csu/libc-start.c:308
Effectively we are passing to bgp_mac_remove_ifp_internal the macaddr
that is associated with the bsm data structure. There exists a path
where the bsm is freed and then we immediately pass the macaddr into
bgp_mac_rescan_all_evpn_tables. So just make a copy of the macaddr
data structure before we free the bsm
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 5, 2020
We were not connecting the default zebra_ns to the default ns->info at namespace initialization in zebra. Thus, when we tried to use the `ns_walk_func()` it would ignore the default zebra_ns since there is no pointer to it from the ns struct. Fix this by connecting them in `zebra_ns_init()` and, if the default ns is not found, exit with failure since this is not recoverable. This was found during a crash where we fail to cancel the kernel_read thread at termination (via the `ns_walk_func()`) and then we get a netlink notification trying to use the zns struct that has already been freed. ``` (gdb) bt \#0 0x00007fc1134dc7bb in raise () from /lib/x86_64-linux-gnu/libc.so.6 \#1 0x00007fc1134c7535 in abort () from /lib/x86_64-linux-gnu/libc.so.6 \#2 0x00007fc113996f8f in core_handler (signo=11, siginfo=0x7ffe5429d070, context=<optimized out>) at lib/sigevent.c:254 \#3 <signal handler called> \#4 0x0000561880e15449 in if_lookup_by_index_per_ns (ns=0x0, ifindex=174) at zebra/interface.c:269 \FRRouting#5 0x0000561880e1642c in if_up (ifp=ifp@entry=0x561883076c50) at zebra/interface.c:1043 \FRRouting#6 0x0000561880e10723 in netlink_link_change (h=0x7ffe5429d8f0, ns_id=<optimized out>, startup=<optimized out>) at zebra/if_netlink.c:1384 \FRRouting#7 0x0000561880e17e68 in netlink_parse_info (filter=filter@entry=0x561880e17680 <netlink_information_fetch>, nl=nl@entry=0x561882497238, zns=zns@entry=0x7ffe542a5940, count=count@entry=5, startup=startup@entry=0) at zebra/kernel_netlink.c:932 \FRRouting#8 0x0000561880e186a5 in kernel_read (thread=<optimized out>) at zebra/kernel_netlink.c:406 \FRRouting#9 0x00007fc1139a4416 in thread_call (thread=thread@entry=0x7ffe542a5b70) at lib/thread.c:1599 \FRRouting#10 0x00007fc113974ef8 in frr_run (master=0x5618823c9510) at lib/libfrr.c:1024 \FRRouting#11 0x0000561880e0b916 in main (argc=8, argv=0x7ffe542a5f78) at zebra/main.c:483 ``` Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 5, 2020
=================================================================
==13611==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe9e5c8694 at pc 0x0000004d18ac bp 0x7ffe9e5c8330 sp 0x7ffe9e5c7ae0
WRITE of size 17 at 0x7ffe9e5c8694 thread T0
#0 0x4d18ab in __asan_memcpy (/usr/lib/frr/zebra+0x4d18ab)
#1 0x7f16f04bd97f in stream_get2 /home/qlyoung/frr/lib/stream.c:277:2
#2 0x6410ec in zebra_vxlan_remote_macip_del /home/qlyoung/frr/zebra/zebra_vxlan.c:7718:4
#3 0x68fa98 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
#4 0x556add in main /home/qlyoung/frr/zebra/main.c:309:2
FRRouting#5 0x7f16eea3bb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
FRRouting#6 0x431249 in _start (/usr/lib/frr/zebra+0x431249)
This decode is the result of a buffer overflow because we are
not checking ipa_len.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 5, 2020
=================================================================
==3058==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7f5bf3ef7477 bp 0x7ffdfaa20d40 sp 0x7ffdfaa204c8 T0)
==3058==The signal is caused by a READ memory access.
==3058==Hint: address points to the zero page.
#0 0x7f5bf3ef7476 in memcpy /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:134
#1 0x4d158a in __asan_memcpy (/usr/lib/frr/zebra+0x4d158a)
#2 0x7f5bf58da8ad in stream_put /home/qlyoung/frr/lib/stream.c:605:3
#3 0x67d428 in zsend_ipset_entry_notify_owner /home/qlyoung/frr/zebra/zapi_msg.c:851:2
#4 0x5c70b3 in zebra_pbr_add_ipset_entry /home/qlyoung/frr/zebra/zebra_pbr.c
FRRouting#5 0x68e1bb in zread_ipset_entry /home/qlyoung/frr/zebra/zapi_msg.c:2465:4
FRRouting#6 0x68f958 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
FRRouting#7 0x55666d in main /home/qlyoung/frr/zebra/main.c:309:2
FRRouting#8 0x7f5bf3e5db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
FRRouting#9 0x4311d9 in _start (/usr/lib/frr/zebra+0x4311d9)
the ipset->backpointer was NULL as that the hash lookup failed to find
anything. Prevent this crash from happening.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 5, 2020
==25402==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x533302 in calloc (/usr/lib/frr/zebra+0x533302)
#1 0x7fee84cdc80b in qcalloc /home/qlyoung/frr/lib/memory.c:110:27
#2 0x5a3032 in create_label_chunk /home/qlyoung/frr/zebra/label_manager.c:188:3
#3 0x5a3c2b in assign_label_chunk /home/qlyoung/frr/zebra/label_manager.c:354:8
#4 0x5a2a38 in label_manager_get_chunk /home/qlyoung/frr/zebra/label_manager.c:424:9
FRRouting#5 0x5a1412 in hook_call_lm_get_chunk /home/qlyoung/frr/zebra/label_manager.c:60:1
FRRouting#6 0x5a1412 in lm_get_chunk_call /home/qlyoung/frr/zebra/label_manager.c:81:2
FRRouting#7 0x72a234 in zread_get_label_chunk /home/qlyoung/frr/zebra/zapi_msg.c:2026:2
FRRouting#8 0x72a234 in zread_label_manager_request /home/qlyoung/frr/zebra/zapi_msg.c:2073:4
FRRouting#9 0x73150c in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2688:2
When creating label chunk that has a specified base, we eventually are
calling assign_specific_label_chunk. This function finds the appropriate
list node and deletes it from the lbl_mgr.lc_list but since
the function uses list_delete_node() the deletion function that is
specified for lbl_mgr.lc_list is not called thus dropping the memory.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Feb 5, 2020
The only two safi's that are usable for zebra for installation
of routes into the rib are SAFI_UNICAST and SAFI_MULTICAST.
The acceptance of other safi's is causing a memory leak:
Direct leak of 56 byte(s) in 1 object(s) allocated from:
#0 0x5332f2 in calloc (/usr/lib/frr/zebra+0x5332f2)
#1 0x7f594adc29db in qcalloc /opt/build/frr/lib/memory.c:110:27
#2 0x686849 in zebra_vrf_get_table_with_table_id /opt/build/frr/zebra/zebra_vrf.c:390:11
#3 0x65a245 in rib_add_multipath /opt/build/frr/zebra/zebra_rib.c:2591:10
#4 0x7211bc in zread_route_add /opt/build/frr/zebra/zapi_msg.c:1616:8
FRRouting#5 0x73063c in zserv_handle_commands /opt/build/frr/zebra/zapi_msg.c:2682:2
Collapse
Sequence of events:
Upon vrf creation there is a zvrf->table[afi][safi] data structure
that tables are auto created for. These tables only create SAFI_UNICAST
and SAFI_MULTICAST tables. Since these are the only safi types that
are zebra can actually work on. zvrf data structures also have a
zvrf->otable data structure that tracks in a RB tree other tables
that are created ( say you have routes stuck in any random table
in the 32bit route table space in linux ). This data structure is
only used if the lookup in zvrf->table[afi][safi] fails.
After creation if we pass a route down from an upper level protocol
that has non unicast or multicast safi *but* has the actual
tableid of the vrf we are in, the initial lookup will always
return NULL leaving us to look in the otable. This will create
a data structure to track this data.
If after this event you pass in a second route with the same
afi/safi/table_id, the otable will be created and attempted
to be stored, but the RB_TREE_UNIQ data structure when it sees
this will return the original otable returned and the lookup function
zebra_vrf_get_table_with_table_id will just drop the second otable.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Mar 13, 2020
Upper level clients ask for default routes of a particular family
This change ensures that they only receive the family that they
have asked for.
Discovered when testing in ospf `default-information originate`
=================================================================
==246306==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffa2e8 at pc 0x7ffff73c44e2 bp 0x7fffffffa090 sp 0x7fffffffa088
READ of size 16 at 0x7fffffffa2e8 thread T0
#0 0x7ffff73c44e1 in prefix_copy lib/prefix.c:310
#1 0x7ffff741c0aa in route_node_lookup lib/table.c:255
#2 0x5555556cd263 in ospf_external_info_delete ospfd/ospf_asbr.c:178
#3 0x5555556a47cc in ospf_zebra_read_route ospfd/ospf_zebra.c:852
#4 0x7ffff746f5d8 in zclient_read lib/zclient.c:3028
FRRouting#5 0x7ffff742fc91 in thread_call lib/thread.c:1549
FRRouting#6 0x7ffff7374642 in frr_run lib/libfrr.c:1093
FRRouting#7 0x5555555bfaef in main ospfd/ospf_main.c:235
FRRouting#8 0x7ffff70a2bba in __libc_start_main ../csu/libc-start.c:308
FRRouting#9 0x5555555bf499 in _start (/usr/lib/frr/ospfd+0x6b499)
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Mar 31, 2020
Show if this malformed under `show [ip] bgp <prefix>`: ``` eva# sh ip bgp 103.79.124.0/22 BGP routing table entry for 103.79.124.0/22 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: 192.168.201.136 64539 15096 6939 7545 7545 136001, (aggregated by 0(malformed) 0.0.0.0) 192.168.201.136 from 192.168.201.136 (192.168.201.136) Origin IGP, valid, external, best (First path received) Last update: Thu Mar 26 10:02:07 2020 ``` Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
patrasar
pushed a commit
that referenced
this pull request
Apr 21, 2020
Implement tests to verify BGP link-bandwidth and weighted ECMP functionality. These tests validate one of the primary use cases for weighted ECMP (a.k.a. Unequal cost multipath) using BGP link-bandwidth: https://tools.ietf.org/html/draft-mohanty-bess-ebgp-dmz The included tests are: Test #1: Test BGP link-bandwidth advertisement based on number of multipaths Test #2: Test cumulative link-bandwidth propagation Test #3: Test weighted ECMP - multipath with next hop weights Test #4: Test weighted ECMP rebalancing upon change (link flap) Test FRRouting#5: Test weighted ECMP for a second anycast IP Test FRRouting#6: Test paths with and without link-bandwidth - receiver should resort to regular ECMP Test FRRouting#7: Test different options for processing link-bandwidth on the receiver Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 9, 2020
This problem was reported by the sanitizer -
=================================================================
==24764==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d0000115c8 at pc 0x55cb9cfad312 bp 0x7fffa0552140 sp 0x7fffa0552138
READ of size 8 at 0x60d0000115c8 thread T0
#0 0x55cb9cfad311 in zebra_evpn_remote_es_flush zebra/zebra_evpn_mh.c:2041
#1 0x55cb9cfad311 in zebra_evpn_es_cleanup zebra/zebra_evpn_mh.c:2234
#2 0x55cb9cf6ae78 in zebra_vrf_disable zebra/zebra_vrf.c:205
#3 0x7fc8d478f114 in vrf_delete lib/vrf.c:229
#4 0x7fc8d478f99a in vrf_terminate lib/vrf.c:541
FRRouting#5 0x55cb9ceba0af in sigint zebra/main.c:176
FRRouting#6 0x55cb9ceba0af in sigint zebra/main.c:130
FRRouting#7 0x7fc8d4765d20 in quagga_sigevent_process lib/sigevent.c:103
FRRouting#8 0x7fc8d4787e8c in thread_fetch lib/thread.c:1396
FRRouting#9 0x7fc8d4708782 in frr_run lib/libfrr.c:1092
FRRouting#10 0x55cb9ce931d8 in main zebra/main.c:488
FRRouting#11 0x7fc8d43ee09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
FRRouting#12 0x55cb9ce94c09 in _start (/usr/lib/frr/zebra+0x8ac09)
=================================================================
Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 9, 2020
Current code in bgp bestpath selection would accept the newest locally originated path as the best path. Making the selection non-deterministic. Modify the code to always come to the same bestpath conclusion when you have multiple locally originated paths in bestpath selection. Before: eva# conf eva(config)# router bgp 323 eva(config-router)# address-family ipv4 uni eva(config-router-af)# redistribute connected eva(config-router-af)# network 192.168.161.0/24 eva(config-router-af)# do show bgp ipv4 uni 192.168.161.0 BGP routing table entry for 192.168.161.0/24 Paths: (2 available, best #1, table default) Not advertised to any peer Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin IGP, metric 0, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (Origin) Last update: Wed Sep 16 15:03:03 2020 Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin incomplete, metric 0, weight 32768, valid, sourced Last update: Wed Sep 16 15:02:52 2020 eva(config-router-af)# no redistribute connected eva(config-router-af)# do show bgp ipv4 uni 192.168.161.0 BGP routing table entry for 192.168.161.0/24 Paths: (1 available, best #1, table default) Not advertised to any peer Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin IGP, metric 0, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (First path received) Last update: Wed Sep 16 15:03:03 2020 eva(config-router-af)# redistribute connected eva(config-router-af)# do show bgp ipv4 uni 192.168.161.0 BGP routing table entry for 192.168.161.0/24 Paths: (2 available, best #2, table default) Not advertised to any peer Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin incomplete, metric 0, weight 32768, valid, sourced Last update: Wed Sep 16 15:03:32 2020 Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin IGP, metric 0, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (Origin) Last update: Wed Sep 16 15:03:03 2020 eva(config-router-af)# Notice the route choosen depends on order received Fixed behavior: eva# conf eva(config)# router bgp 323 eva(config-router)# address-family ipv4 uni eva(config-router-af)# redistribute connected eva(config-router-af)# network 192.168.161.0/24 eva(config-router-af)# do show bgp ipv4 uni 192.168.161.0 BGP routing table entry for 192.168.161.0/24 Paths: (2 available, best #1, table default) Not advertised to any peer Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin IGP, metric 0, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (Origin) Last update: Wed Sep 16 15:03:03 2020 Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin incomplete, metric 0, weight 32768, valid, sourced Last update: Wed Sep 16 15:02:52 2020 eva(config-router-af)# no redistribute connected eva(config-router-af)# do show bgp ipv4 uni 192.168.161.0 BGP routing table entry for 192.168.161.0/24 Paths: (1 available, best #1, table default) Not advertised to any peer Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin IGP, metric 0, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (First path received) Last update: Wed Sep 16 15:03:03 2020 eva(config-router-af)# redistribute connected eva(config-router-af)# do show bgp ipv4 uni 192.168.161.0 BGP routing table entry for 192.168.161.0/24 Paths: (2 available, best #2, table default) Not advertised to any peer Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin incomplete, metric 0, weight 32768, valid, sourced Last update: Wed Sep 16 15:03:32 2020 Local 0.0.0.0(eva) from 0.0.0.0 (192.168.161.245) Origin IGP, metric 0, weight 32768, valid, sourced, local, bestpath-from-AS Local, best (Origin) Last update: Wed Sep 16 15:03:03 2020 eva(config-router-af)# Ticket: CM-31490 Found-by: Trey Aspelund <taspelund@nvidia.com> Signed-off-by: Donald Sharp <sharpd@nvidia.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 9, 2020
The bgp_l3vpn_to_bgp_vrf test is looking for a prefix on multiple routers that the ordered received is non-deterministic. As such the regex's are failing occassionaly when the route is received in an unexpected order. One possible order: (FRRouting#89) scripts/check_routes.py:120 COMMAND:ce3:vtysh -c "show bgp ipv4 uni 6.0.1.0":2 available, best .*192.168.1.1.* Local.* 99.0.0.3 from 0.0.0.0 .99.0.0.3.* Origin IGP, metric 200, localpref 50, weight 32768, valid, sourced, local, best .Weight.* Community: 0:67.* Extended Community: RT:89:123.* Large Community: 12:34:56.* Local.* 192.168.1.1 from 192.168.1.1 .192.168.1.1.* Origin IGP, metric 98, localpref 123, valid, internal.* Community: 0:67.* Extended Community: RT:52:100 RT:89:123.* Large Community: 12:34:56:pass:Redundant route 1 details c: COMMAND OUTPUT:BGP routing table entry for 6.0.1.0/24^M Paths: (2 available, best #1, table default)^M Advertised to non peer-group peers:^M 192.168.1.1^M Local^M 99.0.0.3 from 0.0.0.0 (99.0.0.3)^M Origin IGP, metric 200, localpref 50, weight 32768, valid, sourced, local, best (Weight)^M Community: 0:67^M Extended Community: RT:89:123^M Large Community: 12:34:56^M Last update: Wed Oct 7 11:12:22 2020^M Local^M 192.168.1.1 from 192.168.1.1 (192.168.1.1)^M Origin IGP, metric 98, localpref 123, valid, internal^M Community: 0:67^M Extended Community: RT:52:100 RT:89:123^M Large Community: 12:34:56^M Last update: Wed Oct 7 11:12:41 2020: R:89 ce3 Redundant route 1 details c 1 0 Second possible order: (FRRouting#89) scripts/check_routes.py:120 COMMAND:ce3:vtysh -c "show bgp ipv4 uni 6.0.1.0":2 available, best .*192.168.1.1.* Local.* 99.0.0.3 from 0.0.0.0 .99.0.0.3.* Origin IGP, metric 200, localpref 50, weight 32768, valid, sourced, local, best .Weight.* Community: 0:67.* Extended Community: RT:89:123.* Large Community: 12:34:56.* Local.* 192.168.1.1 from 192.168.1.1 .192.168.1.1.* Origin IGP, metric 98, localpref 123, valid, internal.* Community: 0:67.* Extended Community: RT:52:100 RT:89:123.* Large Community: 12:34:56:pass:Redundant route 1 details c: COMMAND OUTPUT:BGP routing table entry for 6.0.1.0/24^M Paths: (2 available, best #2, table default)^M Advertised to non peer-group peers:^M 192.168.1.1^M Local^M 192.168.1.1 from 192.168.1.1 (192.168.1.1)^M Origin IGP, metric 98, localpref 123, valid, internal^M Community: 0:67^M Extended Community: RT:52:100 RT:89:123^M Large Community: 12:34:56^M Last update: Wed Oct 7 11:14:45 2020^M Local^M 99.0.0.3 from 0.0.0.0 (99.0.0.3)^M Origin IGP, metric 200, localpref 50, weight 32768, valid, sourced, local, best (Weight)^M Community: 0:67^M Extended Community: RT:89:123^M Large Community: 12:34:56^M Last update: Wed Oct 7 11:14:27 2020: R:89 ce3 Redundant route 1 details c 0 1 BGP displays the paths in the order received since it's just a linked list. For this test modify/add the luCommands to track that we may receive the paths in a non-deterministic order. Signed-off-by: Donald Sharp <sharpd@nvidia.com> Signed-off-by: Quentin Young <qlyoung@nvidia.com>
patrasar
pushed a commit
that referenced
this pull request
Jul 10, 2023
The `bgp_vrf->vrf_prd_pretty` string was not properly freed, leading to a memory leak.
This commit resolves the memory leak by freeing the memory allocated for `bgp_vrf->vrf_prd_pretty` before returning from the function.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in evpn_type5_test_topo1.test_evpn_type5_topo1/e1.asan.bgpd.17689
=================================================================
==17689==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7fdd94fc0538 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x77538)
#1 0x55e28d9c4c6c in qstrdup lib/memory.c:117
#2 0x55e28d6c0d27 in evpn_configure_vrf_rd bgpd/bgp_evpn_vty.c:2297
#3 0x55e28d6c0d27 in bgp_evpn_vrf_rd bgpd/bgp_evpn_vty.c:6271
#4 0x55e28d94c155 in cmd_execute_command_real lib/command.c:994
FRRouting#5 0x55e28d94c622 in cmd_execute_command lib/command.c:1053
FRRouting#6 0x55e28d94ca99 in cmd_execute lib/command.c:1221
FRRouting#7 0x55e28da6d7d4 in vty_command lib/vty.c:591
FRRouting#8 0x55e28da6dc6e in vty_execute lib/vty.c:1354
FRRouting#9 0x55e28da7644d in vtysh_read lib/vty.c:2362
FRRouting#10 0x55e28da616e2 in event_call lib/event.c:1995
FRRouting#11 0x55e28d9a7a65 in frr_run lib/libfrr.c:1213
FRRouting#12 0x55e28d63ef00 in main bgpd/bgp_main.c:505
FRRouting#13 0x7fdd93883c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 15 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Jul 10, 2023
`bmnc->nh` was not properly freed, leading to a memory leak.
The commit adds a check to ensure that the `bmnc->nh` member variable is freed if it exists.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in bgp_vpnv4_asbr.test_bgp_vpnv4_asbr/r2.asan.bgpd.6382
=================================================================
==6382==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 720 byte(s) in 5 object(s) allocated from:
#0 0x7f6a80d02d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x55c9afd7c81c in qcalloc lib/memory.c:105
#2 0x55c9afd9166b in nexthop_new lib/nexthop.c:358
#3 0x55c9afd93aaa in nexthop_dup lib/nexthop.c:843
#4 0x55c9afad39bb in bgp_mplsvpn_nh_label_bind_register_local_label bgpd/bgp_mplsvpn.c:4259
FRRouting#5 0x55c9afb1c5e9 in bgp_mplsvpn_handle_label_allocation bgpd/bgp_route.c:3239
FRRouting#6 0x55c9afb1c5e9 in bgp_process_main_one bgpd/bgp_route.c:3339
FRRouting#7 0x55c9afb1d2c1 in bgp_process_wq bgpd/bgp_route.c:3591
FRRouting#8 0x55c9afe33df9 in work_queue_run lib/workqueue.c:266
FRRouting#9 0x55c9afe198e2 in event_call lib/event.c:1995
FRRouting#10 0x55c9afd5fc6f in frr_run lib/libfrr.c:1213
FRRouting#11 0x55c9af9f6f00 in main bgpd/bgp_main.c:505
FRRouting#12 0x7f6a7f55ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Indirect leak of 16 byte(s) in 2 object(s) allocated from:
#0 0x7f6a80d02d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x55c9afd7c81c in qcalloc lib/memory.c:105
#2 0x55c9afd91ce8 in nexthop_add_labels lib/nexthop.c:536
#3 0x55c9afd93754 in nexthop_copy_no_recurse lib/nexthop.c:802
#4 0x55c9afd939fb in nexthop_copy lib/nexthop.c:821
FRRouting#5 0x55c9afd93abb in nexthop_dup lib/nexthop.c:845
FRRouting#6 0x55c9afad39bb in bgp_mplsvpn_nh_label_bind_register_local_label bgpd/bgp_mplsvpn.c:4259
FRRouting#7 0x55c9afb1c5e9 in bgp_mplsvpn_handle_label_allocation bgpd/bgp_route.c:3239
FRRouting#8 0x55c9afb1c5e9 in bgp_process_main_one bgpd/bgp_route.c:3339
FRRouting#9 0x55c9afb1d2c1 in bgp_process_wq bgpd/bgp_route.c:3591
FRRouting#10 0x55c9afe33df9 in work_queue_run lib/workqueue.c:266
FRRouting#11 0x55c9afe198e2 in event_call lib/event.c:1995
FRRouting#12 0x55c9afd5fc6f in frr_run lib/libfrr.c:1213
FRRouting#13 0x55c9af9f6f00 in main bgpd/bgp_main.c:505
FRRouting#14 0x7f6a7f55ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 736 byte(s) leaked in 7 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Jul 10, 2023
When `dplane_fpm_nl` receives a route, it allocates memory for a dplane context and calls `netlink_route_change_read_unicast_internal` without initializing the `intf_extra_list` contained in the dplane context. If `netlink_route_change_read_unicast_internal` is not able to process the route, we call `dplane_ctx_fini` to free the dplane context. This causes a crash because `dplane_ctx_fini` attempts to access the intf_extra_list which is not initialized. To solve this issue, we can call `dplane_ctx_route_init`to initialize the dplane route context properly, just after the dplane context allocation. (gdb) bt #0 0x0000555dd5ceae80 in dplane_intf_extra_list_pop (h=0x7fae1c007e68) at ../zebra/zebra_dplane.c:427 #1 dplane_ctx_free_internal (ctx=0x7fae1c0074b0) at ../zebra/zebra_dplane.c:724 #2 0x0000555dd5cebc99 in dplane_ctx_free (pctx=0x7fae2aa88c98) at ../zebra/zebra_dplane.c:869 #3 dplane_ctx_free (pctx=0x7fae2aa88c98, pctx@entry=0x7fae2aa78c28) at ../zebra/zebra_dplane.c:855 #4 dplane_ctx_fini (pctx=pctx@entry=0x7fae2aa88c98) at ../zebra/zebra_dplane.c:890 FRRouting#5 0x00007fae31e93f29 in fpm_read (t=) at ../zebra/dplane_fpm_nl.c:605 FRRouting#6 0x00007fae325191dd in thread_call (thread=thread@entry=0x7fae2aa98da0) at ../lib/thread.c:2006 FRRouting#7 0x00007fae324c42b8 in fpt_run (arg=0x555dd74777c0) at ../lib/frr_pthread.c:309 FRRouting#8 0x00007fae32405ea7 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 FRRouting#9 0x00007fae32325a2f in clone () from /lib/x86_64-linux-gnu/libc.so.6 Fixes: FRRouting#13754 Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
patrasar
pushed a commit
that referenced
this pull request
Jul 20, 2023
In the function ospf_lsa_translated_nssa_new the newly created lsa is lock however, the return lsa from ospf_lsa_new already has a lock. Therefore removing the addition lock resolve the leak below.
ospf_basic_functionality.test_ospf_nssa#r3.asan.ospfd.5456
=================================================================
==5456==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 640 byte(s) in 5 object(s) allocated from:
#0 0x7f294f354a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f294eeed562 in qcalloc ../lib/memory.c:105
#2 0x561a16004f60 in ospf_lsa_new ../ospfd/ospf_lsa.c:186
#3 0x561a160051a1 in ospf_lsa_new_and_data ../ospfd/ospf_lsa.c:205
#4 0x561a1600f21d in ospf_exnl_lsa_prepare_and_flood ../ospfd/ospf_lsa.c:1762
FRRouting#5 0x561a1600fd71 in ospf_external_lsa_new ../ospfd/ospf_lsa.c:1863
FRRouting#6 0x561a160107d7 in ospf_lsa_translated_nssa_new ../ospfd/ospf_lsa.c:1985
FRRouting#7 0x561a16011cfb in ospf_translated_nssa_refresh ../ospfd/ospf_lsa.c:2152
FRRouting#8 0x561a16014bb2 in ospf_external_lsa_install ../ospfd/ospf_lsa.c:2871
FRRouting#9 0x561a1601596b in ospf_lsa_install ../ospfd/ospf_lsa.c:3076
FRRouting#10 0x561a16168b3c in ospf_flood ../ospfd/ospf_flood.c:482
FRRouting#11 0x561a160462f8 in ospf_ls_upd ../ospfd/ospf_packet.c:2115
FRRouting#12 0x561a1604c66c in ospf_read_helper ../ospfd/ospf_packet.c:3198
FRRouting#13 0x561a1604c88e in ospf_read ../ospfd/ospf_packet.c:3229
FRRouting#14 0x7f294efd6c33 in event_call ../lib/event.c:1995
FRRouting#15 0x7f294eec134a in frr_run ../lib/libfrr.c:1213
FRRouting#16 0x561a15fd3b6d in main ../ospfd/ospf_main.c:249
FRRouting#17 0x7f294e998d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Objects leaked above:
0x60c000062800 (128 bytes)
0x60c000062c80 (128 bytes)
0x60c0000631c0 (128 bytes)
0x60c000063700 (128 bytes)
0x60c000063d00 (128 bytes)
Direct leak of 640 byte(s) in 5 object(s) allocated from:
#0 0x7f294f354a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f294eeed562 in qcalloc ../lib/memory.c:105
#2 0x561a16004f60 in ospf_lsa_new ../ospfd/ospf_lsa.c:186
#3 0x561a160051a1 in ospf_lsa_new_and_data ../ospfd/ospf_lsa.c:205
#4 0x561a1600f21d in ospf_exnl_lsa_prepare_and_flood ../ospfd/ospf_lsa.c:1762
FRRouting#5 0x561a1600fd71 in ospf_external_lsa_new ../ospfd/ospf_lsa.c:1863
FRRouting#6 0x561a160107d7 in ospf_lsa_translated_nssa_new ../ospfd/ospf_lsa.c:1985
FRRouting#7 0x561a16010e10 in ospf_translated_nssa_originate ../ospfd/ospf_lsa.c:2034
FRRouting#8 0x561a16136559 in ospf_abr_translate_nssa ../ospfd/ospf_abr.c:668
FRRouting#9 0x561a161383da in ospf_abr_process_nssa_translates ../ospfd/ospf_abr.c:968
FRRouting#10 0x561a1613f9b8 in ospf_abr_nssa_task ../ospfd/ospf_abr.c:2054
FRRouting#11 0x561a161402e5 in ospf_abr_task_timer ../ospfd/ospf_abr.c:2168
FRRouting#12 0x7f294efd6c33 in event_call ../lib/event.c:1995
FRRouting#13 0x7f294eec134a in frr_run ../lib/libfrr.c:1213
FRRouting#14 0x561a15fd3b6d in main ../ospfd/ospf_main.c:249
FRRouting#15 0x7f294e998d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Objects leaked above:
0x60c00003e380 (128 bytes)
0x60c00003e740 (128 bytes)
0x60c00003eb00 (128 bytes)
0x60c00005fd40 (128 bytes)
0x60c00005ff80 (128 bytes)
Indirect leak of 180 byte(s) in 5 object(s) allocated from:
#0 0x7f294f354a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f294eeed562 in qcalloc ../lib/memory.c:105
#2 0x561a16005a43 in ospf_lsa_data_new ../ospfd/ospf_lsa.c:296
#3 0x561a160051b1 in ospf_lsa_new_and_data ../ospfd/ospf_lsa.c:206
#4 0x561a1600f21d in ospf_exnl_lsa_prepare_and_flood ../ospfd/ospf_lsa.c:1762
FRRouting#5 0x561a1600fd71 in ospf_external_lsa_new ../ospfd/ospf_lsa.c:1863
FRRouting#6 0x561a160107d7 in ospf_lsa_translated_nssa_new ../ospfd/ospf_lsa.c:1985
FRRouting#7 0x561a16011cfb in ospf_translated_nssa_refresh ../ospfd/ospf_lsa.c:2152
FRRouting#8 0x561a16014bb2 in ospf_external_lsa_install ../ospfd/ospf_lsa.c:2871
FRRouting#9 0x561a1601596b in ospf_lsa_install ../ospfd/ospf_lsa.c:3076
FRRouting#10 0x561a16168b3c in ospf_flood ../ospfd/ospf_flood.c:482
FRRouting#11 0x561a160462f8 in ospf_ls_upd ../ospfd/ospf_packet.c:2115
FRRouting#12 0x561a1604c66c in ospf_read_helper ../ospfd/ospf_packet.c:3198
FRRouting#13 0x561a1604c88e in ospf_read ../ospfd/ospf_packet.c:3229
FRRouting#14 0x7f294efd6c33 in event_call ../lib/event.c:1995
FRRouting#15 0x7f294eec134a in frr_run ../lib/libfrr.c:1213
FRRouting#16 0x561a15fd3b6d in main ../ospfd/ospf_main.c:249
FRRouting#17 0x7f294e998d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Objects leaked above:
0x60400003f890 (36 bytes)
0x60400003f990 (36 bytes)
0x60400003fa50 (36 bytes)
0x60400003fb10 (36 bytes)
0x60400003fbd0 (36 bytes)
Indirect leak of 180 byte(s) in 5 object(s) allocated from:
#0 0x7f294f354a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f294eeed562 in qcalloc ../lib/memory.c:105
#2 0x561a16005a43 in ospf_lsa_data_new ../ospfd/ospf_lsa.c:296
#3 0x561a160051b1 in ospf_lsa_new_and_data ../ospfd/ospf_lsa.c:206
#4 0x561a1600f21d in ospf_exnl_lsa_prepare_and_flood ../ospfd/ospf_lsa.c:1762
FRRouting#5 0x561a1600fd71 in ospf_external_lsa_new ../ospfd/ospf_lsa.c:1863
FRRouting#6 0x561a160107d7 in ospf_lsa_translated_nssa_new ../ospfd/ospf_lsa.c:1985
FRRouting#7 0x561a16010e10 in ospf_translated_nssa_originate ../ospfd/ospf_lsa.c:2034
FRRouting#8 0x561a16136559 in ospf_abr_translate_nssa ../ospfd/ospf_abr.c:668
FRRouting#9 0x561a161383da in ospf_abr_process_nssa_translates ../ospfd/ospf_abr.c:968
FRRouting#10 0x561a1613f9b8 in ospf_abr_nssa_task ../ospfd/ospf_abr.c:2054
FRRouting#11 0x561a161402e5 in ospf_abr_task_timer ../ospfd/ospf_abr.c:2168
FRRouting#12 0x7f294efd6c33 in event_call ../lib/event.c:1995
FRRouting#13 0x7f294eec134a in frr_run ../lib/libfrr.c:1213
FRRouting#14 0x561a15fd3b6d in main ../ospfd/ospf_main.c:249
FRRouting#15 0x7f294e998d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Objects leaked above:
0x60400003c6d0 (36 bytes)
0x60400003c790 (36 bytes)
0x60400003c810 (36 bytes)
0x60400003c890 (36 bytes)
0x60400003c910 (36 bytes)
SUMMARY: AddressSanitizer: 1640 byte(s) leaked in 20 allocation(s).
Signed-off-by: ryndia <dindyalsarvesh@gmail.com>
patrasar
pushed a commit
that referenced
this pull request
Jul 20, 2023
A route-map applied on incoming BGP updates is not able to exclude the unwanted as segments, based on an AS path access-list. The below configuration illustrates the case: router bgp 65001 address-family ipv4 unicast neighbor 192.168.1.2 route-map rule_2 in exit-address-family bgp as-path access-list RULE permit ^65 route-map rule_2 permit 10 set as-path exclude as-path-access-list RULE ``` BGP routing table entry for 10.10.10.10/32, version 13 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: 192.168.10.65 65000 1 2 3 123 192.168.10.65 from 192.168.10.65 (10.10.10.11) Origin IGP, metric 0, valid, external, best (First path received) ``` After: ``` do show ip bgp 10.10.10.10/32 BGP routing table entry for 10.10.10.10/32, version 15 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: 192.168.10.65 2 3 123 192.168.10.65 from 192.168.10.65 (10.10.10.11) Origin IGP, metric 0, valid, external, best (First path received) ``` Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 16, 2023
A route-map applied on incoming BGP updates is not able to replace an unwanted as segments by another one. unwanted as segment are based on an AS path access-list. The below configuration illustrates the case: router bgp 65001 address-family ipv4 unicast neighbor 192.168.1.2 route-map rule_2 in exit-address-family bgp as-path access-list RULE permit ^65 route-map rule_2 permit 10 set as-path replace as-path-access-list RULE 6000 ``` BGP routing table entry for 10.10.10.10/32, version 13 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: 192.168.10.65 65000 1 2 3 123 192.168.10.65 from 192.168.10.65 (10.10.10.11) Origin IGP, metric 0, valid, external, best (First path received) ``` After: ``` do show ip bgp 10.10.10.10/32 BGP routing table entry for 10.10.10.10/32, version 15 Paths: (1 available, best #1, table default) Advertised to non peer-group peers: 192.168.10.65 6000 1 2 3 123 192.168.10.65 from 192.168.10.65 (10.10.10.11) Origin IGP, metric 0, valid, external, best (First path received) ``` Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
patrasar
added a commit
that referenced
this pull request
Aug 16, 2023
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050
READ of size 4 at 0x6160000801f0 thread T0
#0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340
#1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405
#2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324
#3 0x7fb78e68e1a7 in event_call lib/event.c:1995
#4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#5 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
FRRouting#7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9)
0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8)
freed by thread T0 here:
#0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
#1 0x7fb78e5eff7c in qfree lib/memory.c:130
#2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84
#3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199
#4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167
FRRouting#5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225
FRRouting#6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155
FRRouting#7 0x7fb78e68e1a7 in event_call lib/event.c:1995
FRRouting#8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#9 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Root Cause:
-----------
This issue arises because the "source->source_channel_oil" pointer is deleted
within the "igmp_source_forward_stop()" API.
However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340
is called, it attempts to reference the "source->source_channel_oil" pointer,
which now containes garbage value.
Fix:
----
After deletion of "source_channel_oil" in igmp_source_forward_stop() API,
it is necessary to set the "source->source_channel_oil" pointer to NULL
value.
This will prevent any issue that could arise from referencing in future.
Issue: FRRouting#14195
Signed-off-by: Sarita Patra <saritap@vmware.com>
patrasar
added a commit
that referenced
this pull request
Aug 16, 2023
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050
READ of size 4 at 0x6160000801f0 thread T0
#0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340
#1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405
#2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324
#3 0x7fb78e68e1a7 in event_call lib/event.c:1995
#4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#5 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
FRRouting#7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9)
0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8)
freed by thread T0 here:
#0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
#1 0x7fb78e5eff7c in qfree lib/memory.c:130
#2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84
#3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199
#4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167
FRRouting#5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225
FRRouting#6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155
FRRouting#7 0x7fb78e68e1a7 in event_call lib/event.c:1995
FRRouting#8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#9 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Root Cause:
-----------
This issue arises because the "source->source_channel_oil" pointer is deleted
within the "igmp_source_forward_stop()" API.
However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340
is called, it attempts to reference the "source->source_channel_oil" pointer,
which now containes garbage value.
Fix:
----
After deletion of "source_channel_oil" in igmp_source_forward_stop() API,
it is necessary to set the "source->source_channel_oil" pointer to NULL
value.
This will prevent any issue that could arise from referencing in future.
Issue: FRRouting#14195
Signed-off-by: Sarita Patra <saritap@vmware.com>
patrasar
added a commit
that referenced
this pull request
Aug 16, 2023
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050
READ of size 4 at 0x6160000801f0 thread T0
#0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340
#1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405
#2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324
#3 0x7fb78e68e1a7 in event_call lib/event.c:1995
#4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#5 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
FRRouting#7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9)
0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8)
freed by thread T0 here:
#0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
#1 0x7fb78e5eff7c in qfree lib/memory.c:130
#2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84
#3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199
#4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167
FRRouting#5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225
FRRouting#6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155
FRRouting#7 0x7fb78e68e1a7 in event_call lib/event.c:1995
FRRouting#8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#9 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Root Cause:
-----------
This issue arises because the "source->source_channel_oil" pointer is deleted
within the "igmp_source_forward_stop()" API.
However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340
is called, it attempts to reference the "source->source_channel_oil" pointer,
which now containes garbage value.
Fix:
----
After deletion of "source_channel_oil" in igmp_source_forward_stop() API,
it is necessary to set the "source->source_channel_oil" pointer to NULL
value.
This will prevent any issue that could arise from referencing in future.
Issue: FRRouting#14195
Signed-off-by: Sarita Patra <saritap@vmware.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
Bug is reporoduced in case of switching interfaces betwean VRFs.
ospf6d is enabled and configured in each VRF.
'dest' can be removed from the route node in the time when the same
route node waiting processing in another sub-queue.
A route node must only be in one sub-queue at a time.
Details:
1. Config:
interface if0
ipv6 address 2001:db8:cafe:2::2/64
ipv6 nat inside
ipv6 ospf6 area 0.0.0.51
ipv6 ospf6 cost 10
vrf test2
exit
!
interface if1
ipv6 address 2001:db8:cafe:4::1/64
ipv6 nat outside
ipv6 ospf6 area 0.0.0.0
ipv6 ospf6 cost 10
vrf test2
exit
!
router ospf6
ospf6 router-id 2.2.2.2
exit
!
router ospf6 vrf test1
ospf6 router-id 2.2.2.2
exit
!
router ospf6 vrf test2
ospf6 router-id 2.2.2.2
exit
I just quickly switched interfaces between different VRFs (default/test1/test2).
2. Log messages:
Aug 02 16:51:56 ubuntu zebra[386985]: [MFYWV-KH3MC] process_subq_early_route_add: (0:?):2001:db8:cafe:2::/64: Inserting route rn 0x56267593de90, re 0x56267595ae40 (connected) existing 0x0, same_count 0
Aug 02 16:51:56 ubuntu zebra[386985]: [Q4T2G-E2SQF] process_subq_early_route_add: dumping RE entry 0x56267595ae40 for 2001:db8:cafe:2::/64 vrf default(0)
Aug 02 16:51:56 ubuntu zebra[386985]: [GCGMT-SQR82] rib_link: (0:?):2001:db8:cafe:2::/64: rn 0x56267593de90 adding dest
Aug 02 16:51:56 ubuntu zebra[386985]: [JF0K0-DVHWH] rib_meta_queue_add: (0:254):2001:db8:cafe:2::/64: queued rn 0x56267593de90 into sub-queue Connected Routes
Aug 02 16:51:56 ubuntu zebra[386985]: [QE6V0-J8BG5] rib_delnode: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90, re 0x56267595ae40, removing
Aug 02 16:51:56 ubuntu zebra[386985]: [KMPGN-JBRKW] rib_meta_queue_add: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90 is already queued in sub-queue Connected Routes
Aug 02 16:51:56 ubuntu zebra[386985]: [MFYWV-KH3MC] process_subq_early_route_add: (0:254):2001:db8:cafe:2::/64: Inserting route rn 0x56267593de90, re 0x56267595abf0 (ospf6) existing 0x0, same_count 1
Aug 02 16:51:56 ubuntu zebra[386985]: [Q4T2G-E2SQF] process_subq_early_route_add: dumping RE entry 0x56267595abf0 for 2001:db8:cafe:2::/64 vrf default(0)
Aug 02 16:51:56 ubuntu zebra[386985]: [KMPGN-JBRKW] rib_meta_queue_add: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90 is already queued in sub-queue Connected Routes
Aug 02 16:51:56 ubuntu zebra[386985]: [YEYFX-TDSC2] process_subq_early_route_add: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90, removing unneeded re 0x56267595ae40
Aug 02 16:51:56 ubuntu zebra[386985]: [Y53JX-CBC5H] rib_unlink: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90, re 0x56267595ae40
Aug 02 16:51:56 ubuntu zebra[386985]: [QE6V0-J8BG5] rib_delnode: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90, re 0x56267595abf0, removing
Aug 02 16:51:56 ubuntu zebra[386985]: [JF0K0-DVHWH] rib_meta_queue_add: (0:254):2001:db8:cafe:2::/64: queued rn 0x56267593de90 into sub-queue RIP/OSPF/ISIS/EIGRP/NHRP Routes
Aug 02 16:51:56 ubuntu zebra[386985]: [NZNZ4-7P54Y] default(0:254):2001:db8:cafe:2::/64: Processing rn 0x56267593de90
Aug 02 16:51:56 ubuntu zebra[386985]: [ZJVZ4-XEGPF] default(0:254):2001:db8:cafe:2::/64: Examine re 0x56267595abf0 (ospf6) status: Removed Changed flags: None dist 110 metric 10
Aug 02 16:51:56 ubuntu zebra[386985]: [NM15X-X83N9] rib_process: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90, removing re 0x56267595abf0
Aug 02 16:51:56 ubuntu zebra[386985]: [Y53JX-CBC5H] rib_unlink: (0:254):2001:db8:cafe:2::/64: rn 0x56267593de90, re 0x56267595abf0
Aug 02 16:51:56 ubuntu zebra[386985]: [KT8QQ-45WQ0] rib_gc_dest: (0:?):2001:db8:cafe:2::/64: removing dest from table
Aug 02 16:51:56 ubuntu zebra[386985]: [HH6N2-PDCJS] default(0:0):2001:db8:cafe:2::/64 rn 0x56267593de90 dequeued from sub-queue Connected Routes
3. ...and then assert:
(gdb) bt
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=140662163115136) at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=140662163115136) at ./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=140662163115136, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3 0x00007fee76753476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4 0x00007fee767397f3 in __GI_abort () at ./stdlib/abort.c:79
FRRouting#5 0x00007fee76a420fd in _zlog_assert_failed () from target:/usr/lib/x86_64-linux-gnu/frr/libfrr.so.0
FRRouting#6 0x0000562674efe0f0 in process_subq_route (qindex=7 '\a', lnode=0x562675940c60) at zebra/zebra_rib.c:2540
FRRouting#7 process_subq (qindex=META_QUEUE_NOTBGP, subq=0x562675574580) at zebra/zebra_rib.c:3055
FRRouting#8 meta_queue_process (dummy=<optimized out>, data=0x56267556d430) at zebra/zebra_rib.c:3091
FRRouting#9 0x00007fee76a386e8 in work_queue_run () from target:/usr/lib/x86_64-linux-gnu/frr/libfrr.so.0
FRRouting#10 0x00007fee76a31c91 in thread_call () from target:/usr/lib/x86_64-linux-gnu/frr/libfrr.so.0
FRRouting#11 0x00007fee769ee528 in frr_run () from target:/usr/lib/x86_64-linux-gnu/frr/libfrr.so.0
FRRouting#12 0x0000562674e97ec5 in main (argc=5, argv=0x7ffd1e275958) at zebra/main.c:478
(gdb) print lnode->data
$10 = (void *) 0x56267593de90
(gdb) p/x *(struct route_node *)0x56267593de90
$11 = {
p = {
family = 0xa,
prefixlen = 0x40,
u = {
prefix = 0x20,
prefix4 = {
s_addr = 0xb80d0120
},
prefix6 = {
__in6_u = {
__u6_addr8 = {0x20, 0x1, 0xd, 0xb8, 0xca, 0xfe, 0x0, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0},
__u6_addr16 = {0x120, 0xb80d, 0xfeca, 0x200, 0x0, 0x0, 0x0, 0x0},
__u6_addr32 = {0xb80d0120, 0x200feca, 0x0, 0x0}
}
},
...
table = 0x5626755ae010,
parent = 0x5626755ae070,
link = {0x0, 0x0},
lock = 0x4,
nodehash = {
hi = {
next = 0x5626755ae0d0,
hashval = 0xebe8bdbf
}
},
info = 0x0
3. What's happen:
We removed unneeded re 0x56267595ae40 while adding re 0x56267595abf0. It was the last connected re,
but rn 0x56267593de90 is still in the connected sub-queue.
Then rib_delnode was called for 0x56267595abf0. (rn 0x56267593de90 is still in the connected sub-queue).
rib_delnode have called rib_meta_queue_add which have checked, that rn is absent in sub-queue RIP/OSPF/ISIS/EIGRP/NHRP
and have added rn in the second sub-queue.
Fixes: d7ac4c4 ("zebra: Introduce early route processing on the MetaQ")
Signed-off-by: Pavel Ivashchenko <pivashchenko@nfware.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
Properly free the dynamically allocated memory held by `str` after its use.
The change also maintains the return value of `nb_cli_apply_changes` by using 'ret' variable.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in bgp_set_aspath_replace.test_bgp_set_aspath_replace/r1.asan.bgpd.11586
=================================================================
==11586==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 92 byte(s) in 3 object(s) allocated from:
#0 0x7f4e2951db40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f4e28f19ea2 in qmalloc lib/memory.c:100
#2 0x7f4e28edbb08 in frrstr_join lib/frrstr.c:89
#3 0x7f4e28e9a601 in argv_concat lib/command.c:183
#4 0x56519adf8413 in set_aspath_replace_access_list_magic bgpd/bgp_routemap.c:6174
FRRouting#5 0x56519adf8942 in set_aspath_replace_access_list bgpd/bgp_routemap_clippy.c:683
FRRouting#6 0x7f4e28e9d548 in cmd_execute_command_real lib/command.c:993
FRRouting#7 0x7f4e28e9da0c in cmd_execute_command lib/command.c:1051
FRRouting#8 0x7f4e28e9de8b in cmd_execute lib/command.c:1218
FRRouting#9 0x7f4e28fc4f1c in vty_command lib/vty.c:591
FRRouting#10 0x7f4e28fc53c7 in vty_execute lib/vty.c:1354
FRRouting#11 0x7f4e28fcdc8d in vtysh_read lib/vty.c:2362
FRRouting#12 0x7f4e28fb8c8b in event_call lib/event.c:1979
FRRouting#13 0x7f4e28efd445 in frr_run lib/libfrr.c:1213
FRRouting#14 0x56519ac85d81 in main bgpd/bgp_main.c:510
FRRouting#15 0x7f4e27f40c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 92 byte(s) leaked in 3 allocation(s).
***********************************************************************************
***********************************************************************************
Address Sanitizer Error detected in bgp_set_aspath_exclude.test_bgp_set_aspath_exclude/r1.asan.bgpd.10385
=================================================================
==10385==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 55 byte(s) in 2 object(s) allocated from:
#0 0x7f6814fdab40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f68149d6ea2 in qmalloc lib/memory.c:100
#2 0x7f6814998b08 in frrstr_join lib/frrstr.c:89
#3 0x7f6814957601 in argv_concat lib/command.c:183
#4 0x5570e05117a1 in set_aspath_exclude_access_list_magic bgpd/bgp_routemap.c:6327
FRRouting#5 0x5570e05119da in set_aspath_exclude_access_list bgpd/bgp_routemap_clippy.c:836
FRRouting#6 0x7f681495a548 in cmd_execute_command_real lib/command.c:993
FRRouting#7 0x7f681495aa0c in cmd_execute_command lib/command.c:1051
FRRouting#8 0x7f681495ae8b in cmd_execute lib/command.c:1218
FRRouting#9 0x7f6814a81f1c in vty_command lib/vty.c:591
FRRouting#10 0x7f6814a823c7 in vty_execute lib/vty.c:1354
FRRouting#11 0x7f6814a8ac8d in vtysh_read lib/vty.c:2362
FRRouting#12 0x7f6814a75c8b in event_call lib/event.c:1979
FRRouting#13 0x7f68149ba445 in frr_run lib/libfrr.c:1213
FRRouting#14 0x5570e03a0d81 in main bgpd/bgp_main.c:510
FRRouting#15 0x7f68139fdc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 55 byte(s) leaked in 2 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
Properly free the dynamically allocated memory held by `str` after its use.
The change also maintains the return value of `nb_cli_apply_changes` by using `ret` variable.
The ASan leak log for reference:
```
Direct leak of 55 byte(s) in 2 object(s) allocated from:
#0 0x7f16f285f867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x7f16f23fda11 in qmalloc ../lib/memory.c:100
#2 0x7f16f23a01a0 in frrstr_join ../lib/frrstr.c:89
#3 0x7f16f23418c7 in argv_concat ../lib/command.c:183
#4 0x55aba24731f2 in set_aspath_exclude_access_list_magic ../bgpd/bgp_routemap.c:6327
FRRouting#5 0x55aba2455cf4 in set_aspath_exclude_access_list bgpd/bgp_routemap_clippy.c:836
FRRouting#6 0x7f16f2345d61 in cmd_execute_command_real ../lib/command.c:993
FRRouting#7 0x7f16f23460ee in cmd_execute_command ../lib/command.c:1052
FRRouting#8 0x7f16f2346dc0 in cmd_execute ../lib/command.c:1218
FRRouting#9 0x7f16f24f7197 in vty_command ../lib/vty.c:591
FRRouting#10 0x7f16f24fc07c in vty_execute ../lib/vty.c:1354
FRRouting#11 0x7f16f250247a in vtysh_read ../lib/vty.c:2362
FRRouting#12 0x7f16f24e72f4 in event_call ../lib/event.c:1979
FRRouting#13 0x7f16f23d1828 in frr_run ../lib/libfrr.c:1213
FRRouting#14 0x55aba2269e52 in main ../bgpd/bgp_main.c:510
FRRouting#15 0x7f16f1dbfd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
In scenarios where no backup paths are available, ensure proper
memory management by deleting `q_space->vertex_list`. This prevents
memory leaks.
The ASan leak log for reference:
```
Direct leak of 80 byte(s) in 2 object(s) allocated from:
#0 0x7fcf8c70aa37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7fcf8c2a8a45 in qcalloc ../lib/memory.c:105
#2 0x7fcf8c27d0cc in list_new ../lib/linklist.c:49
#3 0x55d6e8385e35 in ospf_spf_init ../ospfd/ospf_spf.c:540
#4 0x55d6e838c30d in ospf_spf_calculate ../ospfd/ospf_spf.c:1736
FRRouting#5 0x55d6e83933cf in ospf_ti_lfa_generate_q_spaces ../ospfd/ospf_ti_lfa.c:673
FRRouting#6 0x55d6e8394214 in ospf_ti_lfa_generate_p_space ../ospfd/ospf_ti_lfa.c:812
FRRouting#7 0x55d6e8394c63 in ospf_ti_lfa_generate_p_spaces ../ospfd/ospf_ti_lfa.c:923
FRRouting#8 0x55d6e8396390 in ospf_ti_lfa_compute ../ospfd/ospf_ti_lfa.c:1101
FRRouting#9 0x55d6e838ca48 in ospf_spf_calculate_area ../ospfd/ospf_spf.c:1811
FRRouting#10 0x55d6e838cd73 in ospf_spf_calculate_areas ../ospfd/ospf_spf.c:1840
FRRouting#11 0x55d6e838cfb0 in ospf_spf_calculate_schedule_worker ../ospfd/ospf_spf.c:1871
FRRouting#12 0x7fcf8c3922e4 in event_call ../lib/event.c:1979
FRRouting#13 0x7fcf8c27c828 in frr_run ../lib/libfrr.c:1213
FRRouting#14 0x55d6e82eeb6d in main ../ospfd/ospf_main.c:249
FRRouting#15 0x7fcf8bd59d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
This commit ensures that sequence data
and associated structures are correctly deleted to prevent memory leaks
The ASan leak log for reference:
```
Direct leak of 432 byte(s) in 1 object(s) allocated from:
#0 0x7f911ebaba37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f911e749a4e in qcalloc ../lib/memory.c:105
#2 0x564fd444b2d3 in pbrms_get ../pbrd/pbr_map.c:527
#3 0x564fd443a82d in pbr_map ../pbrd/pbr_vty.c:90
#4 0x7f911e691d61 in cmd_execute_command_real ../lib/command.c:993
FRRouting#5 0x7f911e6920ee in cmd_execute_command ../lib/command.c:1052
FRRouting#6 0x7f911e692dc0 in cmd_execute ../lib/command.c:1218
FRRouting#7 0x7f911e843197 in vty_command ../lib/vty.c:591
FRRouting#8 0x7f911e84807c in vty_execute ../lib/vty.c:1354
FRRouting#9 0x7f911e84e47a in vtysh_read ../lib/vty.c:2362
FRRouting#10 0x7f911e8332f4 in event_call ../lib/event.c:1979
FRRouting#11 0x7f911e71d828 in frr_run ../lib/libfrr.c:1213
FRRouting#12 0x564fd4425795 in main ../pbrd/pbr_main.c:168
FRRouting#13 0x7f911e2e1d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
This commit ensures proper cleanup by clearing the `algo->pdst` pointer if it points to a path that is being deleted.
It addresses memory leaks by freeing memory held by `algo->pdst` that might not have been released during the cleanup of processed paths.
The ASan leak log for reference:
```
Direct leak of 96 byte(s) in 1 object(s) allocated from:
#0 0x7fbffcec9a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7fbffca67a81 in qcalloc ../lib/memory.c:105
#2 0x7fbffc9d1a54 in cpath_new ../lib/cspf.c:44
#3 0x7fbffc9d2829 in cspf_init ../lib/cspf.c:256
#4 0x7fbffc9d295d in cspf_init_v4 ../lib/cspf.c:287
FRRouting#5 0x5601dcd34d3f in show_sharp_cspf_magic ../sharpd/sharp_vty.c:1262
FRRouting#6 0x5601dcd2c2be in show_sharp_cspf sharpd/sharp_vty_clippy.c:1869
FRRouting#7 0x7fbffc9afd61 in cmd_execute_command_real ../lib/command.c:993
FRRouting#8 0x7fbffc9b00ee in cmd_execute_command ../lib/command.c:1052
FRRouting#9 0x7fbffc9b0dc0 in cmd_execute ../lib/command.c:1218
FRRouting#10 0x7fbffcb611c7 in vty_command ../lib/vty.c:591
FRRouting#11 0x7fbffcb660ac in vty_execute ../lib/vty.c:1354
FRRouting#12 0x7fbffcb6c4aa in vtysh_read ../lib/vty.c:2362
FRRouting#13 0x7fbffcb51324 in event_call ../lib/event.c:1979
FRRouting#14 0x7fbffca3b872 in frr_run ../lib/libfrr.c:1213
FRRouting#15 0x5601dcd11c6f in main ../sharpd/sharp_main.c:177
FRRouting#16 0x7fbffc5ffd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Indirect leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7fbffcec9a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7fbffca67a81 in qcalloc ../lib/memory.c:105
#2 0x7fbffca3c108 in list_new ../lib/linklist.c:49
#3 0x7fbffc9d1acc in cpath_new ../lib/cspf.c:47
#4 0x7fbffc9d2829 in cspf_init ../lib/cspf.c:256
FRRouting#5 0x7fbffc9d295d in cspf_init_v4 ../lib/cspf.c:287
FRRouting#6 0x5601dcd34d3f in show_sharp_cspf_magic ../sharpd/sharp_vty.c:1262
FRRouting#7 0x5601dcd2c2be in show_sharp_cspf sharpd/sharp_vty_clippy.c:1869
FRRouting#8 0x7fbffc9afd61 in cmd_execute_command_real ../lib/command.c:993
FRRouting#9 0x7fbffc9b00ee in cmd_execute_command ../lib/command.c:1052
FRRouting#10 0x7fbffc9b0dc0 in cmd_execute ../lib/command.c:1218
FRRouting#11 0x7fbffcb611c7 in vty_command ../lib/vty.c:591
FRRouting#12 0x7fbffcb660ac in vty_execute ../lib/vty.c:1354
FRRouting#13 0x7fbffcb6c4aa in vtysh_read ../lib/vty.c:2362
FRRouting#14 0x7fbffcb51324 in event_call ../lib/event.c:1979
FRRouting#15 0x7fbffca3b872 in frr_run ../lib/libfrr.c:1213
FRRouting#16 0x5601dcd11c6f in main ../sharpd/sharp_main.c:177
FRRouting#17 0x7fbffc5ffd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
Previously when updating vertices, edges and subnets, when no update was required
due to existing value matching the new one, memory associated with the new object
was not being freed leading to memory leaks. This commit fixes memory leak by
freeing memory associated with new object when update is unnecessary.
The ASan leak log for reference:
```
Direct leak of 312 byte(s) in 3 object(s) allocated from:
#0 0x7faf3afbfa37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7faf3ab5dbcf in qcalloc ../lib/memory.c:105
#2 0x7faf3ab42e00 in ls_parse_prefix ../lib/link_state.c:1323
#3 0x7faf3ab43c87 in ls_parse_msg ../lib/link_state.c:1373
#4 0x7faf3ab476a5 in ls_stream2ted ../lib/link_state.c:1885
FRRouting#5 0x564e045046aa in sharp_opaque_handler ../sharpd/sharp_zebra.c:792
FRRouting#6 0x7faf3aca35a9 in zclient_read ../lib/zclient.c:4410
FRRouting#7 0x7faf3ac47474 in event_call ../lib/event.c:1979
FRRouting#8 0x7faf3ab318b4 in frr_run ../lib/libfrr.c:1213
FRRouting#9 0x564e044fdc6f in main ../sharpd/sharp_main.c:177
FRRouting#10 0x7faf3a6f4d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: 312 byte(s) leaked in 3 allocation(s).
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Aug 31, 2023
…Discarded
The newly created LSA `new` is now properly freed to prevent memory leaks when
a non-self-originated Grace LSA which is not in LSDB is received.
The ASan leak log for reference:
```
Direct leak of 400 byte(s) in 2 object(s) allocated from:
#0 0x7f70e984bd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7f70e92481c5 in qcalloc lib/memory.c:105
#2 0x55b35068c975 in ospf6_lsa_alloc ospf6d/ospf6_lsa.c:710
#3 0x55b35068c9f9 in ospf6_lsa_create ospf6d/ospf6_lsa.c:725
#4 0x55b35065ab2c in ospf6_receive_lsa ospf6d/ospf6_flood.c:912
FRRouting#5 0x55b3506a1413 in ospf6_lsupdate_recv ospf6d/ospf6_message.c:1621
FRRouting#6 0x55b3506a1413 in ospf6_read_helper ospf6d/ospf6_message.c:1896
FRRouting#7 0x55b3506a1413 in ospf6_receive ospf6d/ospf6_message.c:1925
FRRouting#8 0x7f70e92e6ccb in event_call lib/event.c:1979
FRRouting#9 0x7f70e922b488 in frr_run lib/libfrr.c:1213
FRRouting#10 0x55b35064345e in main ospf6d/ospf6_main.c:250
FRRouting#11 0x7f70e8843c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Indirect leak of 72 byte(s) in 2 object(s) allocated from:
#0 0x7f70e984bb40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f70e9247ee5 in qmalloc lib/memory.c:100
#2 0x55b35068c987 in ospf6_lsa_alloc ospf6d/ospf6_lsa.c:711
#3 0x55b35068c9f9 in ospf6_lsa_create ospf6d/ospf6_lsa.c:725
#4 0x55b35065ab2c in ospf6_receive_lsa ospf6d/ospf6_flood.c:912
FRRouting#5 0x55b3506a1413 in ospf6_lsupdate_recv ospf6d/ospf6_message.c:1621
FRRouting#6 0x55b3506a1413 in ospf6_read_helper ospf6d/ospf6_message.c:1896
FRRouting#7 0x55b3506a1413 in ospf6_receive ospf6d/ospf6_message.c:1925
FRRouting#8 0x7f70e92e6ccb in event_call lib/event.c:1979
FRRouting#9 0x7f70e922b488 in frr_run lib/libfrr.c:1213
FRRouting#10 0x55b35064345e in main ospf6d/ospf6_main.c:250
FRRouting#11 0x7f70e8843c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 472 byte(s) leaked in 4 allocation(s).
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
added a commit
that referenced
this pull request
Aug 31, 2023
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050
READ of size 4 at 0x6160000801f0 thread T0
#0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340
#1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405
#2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324
#3 0x7fb78e68e1a7 in event_call lib/event.c:1995
#4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#5 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
FRRouting#7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9)
0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8)
freed by thread T0 here:
#0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
#1 0x7fb78e5eff7c in qfree lib/memory.c:130
#2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84
#3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199
#4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167
FRRouting#5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225
FRRouting#6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155
FRRouting#7 0x7fb78e68e1a7 in event_call lib/event.c:1995
FRRouting#8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#9 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Root Cause:
-----------
This issue arises because the "source->source_channel_oil" pointer is deleted
within the "igmp_source_forward_stop()" API.
However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340
is called, it attempts to reference the "source->source_channel_oil" pointer,
which now containes garbage value.
Fix:
----
After deletion of "source_channel_oil" in igmp_source_forward_stop() API,
it is necessary to set the "source->source_channel_oil" pointer to NULL
value.
This will prevent any issue that could arise from referencing in future.
Issue: FRRouting#14195
Signed-off-by: Sarita Patra <saritap@vmware.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
Addressed a memory leak in OSPF by fixing the improper deallocation of
area range nodes when removed from the table. Introducing a new function,
`ospf_range_table_node_destroy` for proper node cleanup, resolved the issue.
The ASan leak log for reference:
```
Direct leak of 56 byte(s) in 2 object(s) allocated from:
#0 0x7faf661d1d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7faf65bce1e9 in qcalloc lib/memory.c:105
#2 0x55a66e0b61cd in ospf_area_range_new ospfd/ospf_abr.c:43
#3 0x55a66e0b61cd in ospf_area_range_set ospfd/ospf_abr.c:195
#4 0x55a66e07f2eb in ospf_area_range ospfd/ospf_vty.c:631
FRRouting#5 0x7faf65b51548 in cmd_execute_command_real lib/command.c:993
FRRouting#6 0x7faf65b51f79 in cmd_execute_command_strict lib/command.c:1102
FRRouting#7 0x7faf65b51fd8 in command_config_read_one_line lib/command.c:1262
FRRouting#8 0x7faf65b522bf in config_from_file lib/command.c:1315
FRRouting#9 0x7faf65c832df in vty_read_file lib/vty.c:2605
FRRouting#10 0x7faf65c83409 in vty_read_config lib/vty.c:2851
FRRouting#11 0x7faf65bb0341 in frr_config_read_in lib/libfrr.c:977
FRRouting#12 0x7faf65c6cceb in event_call lib/event.c:1979
FRRouting#13 0x7faf65bb1488 in frr_run lib/libfrr.c:1213
FRRouting#14 0x55a66dfb28c4 in main ospfd/ospf_main.c:249
FRRouting#15 0x7faf651c9c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 56 byte(s) leaked in 2 allocation(s).
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
This commit frees dynamically allocated memory associated
with `pbrms->nhgrp_name` and `pbrms->dst` which were causing memory leaks.
The ASan leak log for reference:
```
=================================================================
==107458==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 56 byte(s) in 1 object(s) allocated from:
#0 0x7f87d644ca37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x7f87d5feaa37 in qcalloc ../lib/memory.c:105
#2 0x7f87d6054ffd in prefix_new ../lib/prefix.c:1180
#3 0x55722f3c2885 in pbr_map_match_dst_magic ../pbrd/pbr_vty.c:302
#4 0x55722f3b5c24 in pbr_map_match_dst pbrd/pbr_vty_clippy.c:228
FRRouting#5 0x7f87d5f32d61 in cmd_execute_command_real ../lib/command.c:993
FRRouting#6 0x7f87d5f330ee in cmd_execute_command ../lib/command.c:1052
FRRouting#7 0x7f87d5f33dc0 in cmd_execute ../lib/command.c:1218
FRRouting#8 0x7f87d60e4177 in vty_command ../lib/vty.c:591
FRRouting#9 0x7f87d60e905c in vty_execute ../lib/vty.c:1354
FRRouting#10 0x7f87d60ef45a in vtysh_read ../lib/vty.c:2362
FRRouting#11 0x7f87d60d42d4 in event_call ../lib/event.c:1979
FRRouting#12 0x7f87d5fbe828 in frr_run ../lib/libfrr.c:1213
FRRouting#13 0x55722f3ac795 in main ../pbrd/pbr_main.c:168
FRRouting#14 0x7f87d5b82d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Direct leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x7f87d63f39a7 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:454
#1 0x7f87d5feaafc in qstrdup ../lib/memory.c:117
#2 0x55722f3da139 in pbr_nht_set_seq_nhg ../pbrd/pbr_nht.c:551
#3 0x55722f3c693f in pbr_map_nexthop_group_magic ../pbrd/pbr_vty.c:1140
#4 0x55722f3bdaae in pbr_map_nexthop_group pbrd/pbr_vty_clippy.c:1284
FRRouting#5 0x7f87d5f32d61 in cmd_execute_command_real ../lib/command.c:993
FRRouting#6 0x7f87d5f330ee in cmd_execute_command ../lib/command.c:1052
FRRouting#7 0x7f87d5f33dc0 in cmd_execute ../lib/command.c:1218
FRRouting#8 0x7f87d60e4177 in vty_command ../lib/vty.c:591
FRRouting#9 0x7f87d60e905c in vty_execute ../lib/vty.c:1354
FRRouting#10 0x7f87d60ef45a in vtysh_read ../lib/vty.c:2362
FRRouting#11 0x7f87d60d42d4 in event_call ../lib/event.c:1979
FRRouting#12 0x7f87d5fbe828 in frr_run ../lib/libfrr.c:1213
FRRouting#13 0x55722f3ac795 in main ../pbrd/pbr_main.c:168
FRRouting#14 0x7f87d5b82d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: 58 byte(s) leaked in 2 allocation(s).
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
Fixes a memory leak in ospfd where the external aggregator
was not released after its associated route node is deleted.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in ospf_basic_functionality.test_ospf_asbr_summary_topo1/r0.asan.ospfd.31502
=================================================================
==31502==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 200 byte(s) in 5 object(s) allocated from:
#0 0x7fdb30665d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7fdb300620da in qcalloc lib/memory.c:105
#2 0x55e53c2da5fa in ospf_external_aggregator_new ospfd/ospf_asbr.c:396
#3 0x55e53c2dead3 in ospf_asbr_external_aggregator_set ospfd/ospf_asbr.c:1123
#4 0x55e53c27c921 in ospf_external_route_aggregation ospfd/ospf_vty.c:10264
FRRouting#5 0x7fdb2ffe5428 in cmd_execute_command_real lib/command.c:993
FRRouting#6 0x7fdb2ffe58ec in cmd_execute_command lib/command.c:1051
FRRouting#7 0x7fdb2ffe5d6b in cmd_execute lib/command.c:1218
FRRouting#8 0x7fdb3010ce2a in vty_command lib/vty.c:591
FRRouting#9 0x7fdb3010d2d5 in vty_execute lib/vty.c:1354
FRRouting#10 0x7fdb30115b9b in vtysh_read lib/vty.c:2362
FRRouting#11 0x7fdb30100b99 in event_call lib/event.c:1979
FRRouting#12 0x7fdb30045379 in frr_run lib/libfrr.c:1213
FRRouting#13 0x55e53c1ccab4 in main ospfd/ospf_main.c:249
FRRouting#14 0x7fdb2f65dc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7fdb30665d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7fdb300620da in qcalloc lib/memory.c:105
#2 0x55e53c2da5fa in ospf_external_aggregator_new ospfd/ospf_asbr.c:396
#3 0x55e53c2dedd3 in ospf_asbr_external_rt_no_advertise ospfd/ospf_asbr.c:1182
#4 0x55e53c27cf10 in ospf_external_route_aggregation_no_adrvertise ospfd/ospf_vty.c:10626
FRRouting#5 0x7fdb2ffe5428 in cmd_execute_command_real lib/command.c:993
FRRouting#6 0x7fdb2ffe58ec in cmd_execute_command lib/command.c:1051
FRRouting#7 0x7fdb2ffe5d6b in cmd_execute lib/command.c:1218
FRRouting#8 0x7fdb3010ce2a in vty_command lib/vty.c:591
FRRouting#9 0x7fdb3010d2d5 in vty_execute lib/vty.c:1354
FRRouting#10 0x7fdb30115b9b in vtysh_read lib/vty.c:2362
FRRouting#11 0x7fdb30100b99 in event_call lib/event.c:1979
FRRouting#12 0x7fdb30045379 in frr_run lib/libfrr.c:1213
FRRouting#13 0x55e53c1ccab4 in main ospfd/ospf_main.c:249
FRRouting#14 0x7fdb2f65dc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 240 byte(s) leaked in 6 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
CID 1566378 (#1-4 of 4): Use after free (USE_AFTER_FREE)76. use_after_free: Using freed pointer cur_seg. now the prev_seg pointer is set with always existaing values. Link: https://scan7.scan.coverity.com/reports.htm#v39104/p13747/fileInstanceId=146858993&defectInstanceId=18968273&mergedDefectId=1566378&fileStart=1376&fileEnd=1625 Fixes: 4685db4 (bgpd: add set as-path exclude acl-list command) Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
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: 8b531b1 ("bgpd: store and send bgp link-state attributes") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
Fix comparaison of link state attributes pointers in link_state_hash_cmp(). > CID 1568379 (#1 of 1): Logically dead code (DEADCODE) > dead_error_line: Execution cannot reach this statement: return false;. Fixes: 8b531b1 ("bgpd: store and send bgp link-state attributes") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
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] Fixes: 7e0d9ff ("bgpd: display link-state prefixes detail") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 4, 2023
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: 8b531b1 ("bgpd: store and send bgp link-state attributes") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
patrasar
added a commit
that referenced
this pull request
Oct 4, 2023
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050
READ of size 4 at 0x6160000801f0 thread T0
#0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340
#1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405
#2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324
#3 0x7fb78e68e1a7 in event_call lib/event.c:1995
#4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#5 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
FRRouting#7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9)
0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8)
freed by thread T0 here:
#0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
#1 0x7fb78e5eff7c in qfree lib/memory.c:130
#2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84
#3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199
#4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167
FRRouting#5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225
FRRouting#6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155
FRRouting#7 0x7fb78e68e1a7 in event_call lib/event.c:1995
FRRouting#8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#9 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Root Cause:
-----------
This issue arises because the "source->source_channel_oil" pointer is deleted
within the "igmp_source_forward_stop()" API.
However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340
is called, it attempts to reference the "source->source_channel_oil" pointer,
which now containes garbage value.
Fix:
----
After deletion of "source_channel_oil" in igmp_source_forward_stop() API,
it is necessary to set the "source->source_channel_oil" pointer to NULL
value.
This will prevent any issue that could arise from referencing in future.
Issue: FRRouting#14195
Signed-off-by: Sarita Patra <saritap@vmware.com>
patrasar
pushed a commit
that referenced
this pull request
Oct 11, 2023
`ng` was not properly freed, leading to a memory leak.
The commit calls `nexthop_group_delete` to free memory associated with `ng`.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in isis_topo1.test_isis_topo1/r5.asan.zebra.24308
=================================================================
==24308==ERROR: LeakSanitizer: detected memory leaks
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
FRRouting#5 0x562ded787c35 in rib_addnode zebra/zebra_rib.c:3993
FRRouting#6 0x562ded787c35 in process_subq_early_route_add zebra/zebra_rib.c:2860
FRRouting#7 0x562ded787c35 in process_subq_early_route zebra/zebra_rib.c:3138
FRRouting#8 0x562ded787c35 in process_subq zebra/zebra_rib.c:3178
FRRouting#9 0x562ded787c35 in meta_queue_process zebra/zebra_rib.c:3228
FRRouting#10 0x7f4f475f7118 in work_queue_run lib/workqueue.c:266
FRRouting#11 0x7f4f475dc7f2 in event_call lib/event.c:1969
FRRouting#12 0x7f4f4751f347 in frr_run lib/libfrr.c:1213
FRRouting#13 0x562ded69e818 in main zebra/main.c:486
FRRouting#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
FRRouting#5 0x562ded6a3a1c in zebra_add_import_table_entry zebra/redistribute.c:682
FRRouting#6 0x562ded787c35 in rib_link zebra/zebra_rib.c:3972
FRRouting#7 0x562ded787c35 in rib_addnode zebra/zebra_rib.c:3993
FRRouting#8 0x562ded787c35 in process_subq_early_route_add zebra/zebra_rib.c:2860
FRRouting#9 0x562ded787c35 in process_subq_early_route zebra/zebra_rib.c:3138
FRRouting#10 0x562ded787c35 in process_subq zebra/zebra_rib.c:3178
FRRouting#11 0x562ded787c35 in meta_queue_process zebra/zebra_rib.c:3228
FRRouting#12 0x7f4f475f7118 in work_queue_run lib/workqueue.c:266
FRRouting#13 0x7f4f475dc7f2 in event_call lib/event.c:1969
FRRouting#14 0x7f4f4751f347 in frr_run lib/libfrr.c:1213
FRRouting#15 0x562ded69e818 in main zebra/main.c:486
FRRouting#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).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
patrasar
added a commit
that referenced
this pull request
Oct 11, 2023
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050
READ of size 4 at 0x6160000801f0 thread T0
#0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340
#1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405
#2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324
#3 0x7fb78e68e1a7 in event_call lib/event.c:1995
#4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#5 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
FRRouting#7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9)
0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8)
freed by thread T0 here:
#0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
#1 0x7fb78e5eff7c in qfree lib/memory.c:130
#2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84
#3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199
#4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167
FRRouting#5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225
FRRouting#6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155
FRRouting#7 0x7fb78e68e1a7 in event_call lib/event.c:1995
FRRouting#8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#9 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Root Cause:
-----------
This issue arises because the "source->source_channel_oil" pointer is deleted
within the "igmp_source_forward_stop()" API.
However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340
is called, it attempts to reference the "source->source_channel_oil" pointer,
which now containes garbage value.
Fix:
----
After deletion of "source_channel_oil" in igmp_source_forward_stop() API,
it is necessary to set the "source->source_channel_oil" pointer to NULL
value.
This will prevent any issue that could arise from referencing in future.
Issue: FRRouting#14195
Signed-off-by: Sarita Patra <saritap@vmware.com>
patrasar
added a commit
that referenced
this pull request
Aug 29, 2024
ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050
READ of size 4 at 0x6160000801f0 thread T0
#0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340
#1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405
#2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324
#3 0x7fb78e68e1a7 in event_call lib/event.c:1995
#4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#5 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
FRRouting#7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9)
0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8)
freed by thread T0 here:
#0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8)
#1 0x7fb78e5eff7c in qfree lib/memory.c:130
#2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84
#3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199
#4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167
FRRouting#5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225
FRRouting#6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155
FRRouting#7 0x7fb78e68e1a7 in event_call lib/event.c:1995
FRRouting#8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213
FRRouting#9 0x5598c27c781d in main pimd/pim_main.c:162
FRRouting#10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Root Cause:
-----------
This issue arises because the "source->source_channel_oil" pointer is deleted
within the "igmp_source_forward_stop()" API.
However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340
is called, it attempts to reference the "source->source_channel_oil" pointer,
which now containes garbage value.
Fix:
----
After deletion of "source_channel_oil" in igmp_source_forward_stop() API,
it is necessary to set the "source->source_channel_oil" pointer to NULL
value.
This will prevent any issue that could arise from referencing in future.
Issue: FRRouting#14195
Signed-off-by: Sarita Patra <saritap@vmware.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we kill pimd, pim_upstream_del() will be get called as part of cleaning process.
Delete mroute by calling pim_mroute_del() which in result initialize the up->channel_oil
to NULL after cleanup. Delete each ifchannel stored in the upstream ifchannel list one
by one. As part of pim_ifchannel_delete(), it will check if it is the last ifchannel,
then delete the corresponding upstream. So pim_upstream_del will be get called more than
once as part of pim_upstream_terminate() and pim_ ifchannel_delete(). Similarly pim_mroute_del()
will be again called and since there is no NULL check before accesing the data, crash is happening.
Fix:
Adding Null check in pim_mroute_del before accessing the data.
and using the FLAG "PIM_UPSTREAM_FLAG_DEL" which will check if the deletion of pim upstream is already
in progress then don’t call pim_upstream_del() from pim_ifchannel_delete().
Signed-off-by: Sarita Patra saritap@vmware.com