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: 55870dfe7f8b ("Improve batch and dump times by caching link lookups")
Signed-off-by: Lahav Schlesinger <lschlesinger@drivenets.com>
Signed-off-by: Gilad Naaman <gnaaman@drivenets.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 9af0623282e2baa59a69d3ffb9d13641362ecd46..001efc1d7f2461416652bb31ae74c8c072d1f27a 100644 (file)
--- 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) {