From: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
Date: Tue, 23 Aug 2022 21:27:47 +0000 (+0200)
Subject: lib: Fix memory leak in `zclient_send_localsid()`
X-Git-Tag: base_8.4~86^2~6
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=d0c775e3eb749e03f8414478b5c1fd26b9a5e726;p=matthieu%2Ffrr.git

lib: Fix memory leak in `zclient_send_localsid()`

Running `bgp_srv6l3vpn_to_bgp_vrf` and `bgp_srv6l3vpn_to_bgp_vrf2`
topotests with `--valgrind-memleaks` gives several memory leak errors.
This is due to the way FRR daemons pass local SIDs to zebra: to send a
local SID to zebra, FRR daemons call the `zclient_send_localsid()`
function.

The `zclient_send_localsid()` function performs the following sequence
of operations:
* create a temporary `struct nexthop`;
* call `nexthop_add_srv6_seg6local()` to fill the `struct nexthop` with
the proper local SID information;
* create a `struct zapi_route` and call `zapi_nexthop_from_nexthop()` to
copy the information from the `struct nexthop` to the
`struct zapi_route`;
* send the `struct zapi_route` to zebra through the ZAPI.

The `nexthop_add_srv6_seg6local()` function uses `XCALLOC()` to allocate
memory for the SRv6 nexthop. This memory is never freed.

Creating a temporary `struct nexthop` is unnecessary, as the local SID
information can be pushed directly to the `struct zapi_route`. This
patch simplifies the implementation of `zclient_send_localsid()` by
avoiding using the temporary `struct nexthop`. This eliminates the need
to use `nexthop_add_srv6_seg6local()` to fill the `struct nexthop` and
consequently fixes the memory leak.

Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
---

diff --git a/lib/zclient.c b/lib/zclient.c
index e556b768ac..8ec82ab7bb 100644
--- a/lib/zclient.c
+++ b/lib/zclient.c
@@ -447,7 +447,7 @@ enum zclient_send_status zclient_send_localsid(struct zclient *zclient,
 {
 	struct prefix_ipv6 p = {};
 	struct zapi_route api = {};
-	struct nexthop nh = {};
+	struct zapi_nexthop *znh;
 
 	p.family = AF_INET6;
 	p.prefixlen = IPV6_MAX_BITLEN;
@@ -465,12 +465,16 @@ enum zclient_send_status zclient_send_localsid(struct zclient *zclient,
 	SET_FLAG(api.flags, ZEBRA_FLAG_ALLOW_RECURSION);
 	SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
 
-	nh.type = NEXTHOP_TYPE_IFINDEX;
-	nh.ifindex = oif;
-	SET_FLAG(nh.flags, ZAPI_NEXTHOP_FLAG_SEG6LOCAL);
-	nexthop_add_srv6_seg6local(&nh, action, context);
+	znh = &api.nexthops[0];
+
+	memset(znh, 0, sizeof(*znh));
+
+	znh->type = NEXTHOP_TYPE_IFINDEX;
+	znh->ifindex = oif;
+	SET_FLAG(znh->flags, ZAPI_NEXTHOP_FLAG_SEG6LOCAL);
+	znh->seg6local_action = action;
+	memcpy(&znh->seg6local_ctx, context, sizeof(struct seg6local_context));
 
-	zapi_nexthop_from_nexthop(&api.nexthops[0], &nh);
 	api.nexthop_num = 1;
 
 	return zclient_route_send(ZEBRA_ROUTE_ADD, zclient, &api);