[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <D239DD38-BC1F-42BE-B854-7DED97F11D91@drivenets.com>
Date: Wed, 23 Nov 2022 16:15:41 +0000
From: Gilad Naaman <gnaaman@...venets.com>
To: Stephen Hemminger <stephen@...workplumber.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Lahav Daniel Schlesinger <lschlesinger@...venets.com>
Subject: [PATCH iproute2] libnetlink: Fix memory leak in __rtnl_talk_iov()
If `__rtnl_talk_iov()` fails then callers are not expected to free `answer`.
Currently if `NLMSG_ERROR` was received with an error then the netlink
buffer was stored in `answer`, while still returning an error
This leak can be observed by running this snippet over time.
This triggers an `NLMSG_ERROR` because for each neighbour update, `ip`
will try to query for the name of interface 9999 in the wrong netns.
(which in itself is a separate bug)
set -e
ip netns del test-a || true
ip netns add test-a
ip netns del test-b || true
ip netns add test-b
ip -n test-a netns set test-b auto
ip -n test-a link add veth_a index 9999 type veth peer name veth_b netns test-b
ip -n test-b link set veth_b up
ip -n test-a monitor link address prefix neigh nsid label all-nsid > /dev/null &
monitor_pid=$!
clean() {
kill $monitor_pid
ip netns del test-a
ip netns del test-b
}
trap clean EXIT
while true; do
ip -n test-b neigh add dev veth_b 1.2.3.4 lladdr AA:AA:AA:AA:AA:AA
ip -n test-b neigh del dev veth_b 1.2.3.4
done
Fixes: 55870df ("Improve batch and dump times by caching link lookups")
Signed-off-by: Lahav Schlesinger <lschlesinger@...venets.com>
Signed-off-by: Gilad Naaman <gnaaman@...venets.com>
---
lib/libnetlink.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 9af06232..001efc1d 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -1092,14 +1092,19 @@ next:
rtnl_talk_error(h, err, errfn);
}
- if (answer)
- *answer = (struct nlmsghdr *)buf;
- else
+ if (i < iovlen) {
free(buf);
-
- if (i < iovlen)
goto next;
- return error ? -i : 0;
+ }
+
+ if (error) {
+ free(buf);
+ return -i;
+ }
+
+ if (answer)
+ *answer = (struct nlmsghdr *)buf;
+ return 0;
}
if (answer) {
--
2.37.1
Powered by blists - more mailing lists