lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240918165030.3914855-1-nicolas.dichtel@6wind.com>
Date: Wed, 18 Sep 2024 18:49:41 +0200
From: Nicolas Dichtel <nicolas.dichtel@...nd.com>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: David Ahern <dsahern@...il.com>,
	netdev@...r.kernel.org,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	Alexandre Ferrieux <alexandre.ferrieux@...nge.com>
Subject: [PATCH iproute2 v2] iplink: fix fd leak when playing with netns

The command 'ip link set foo netns mynetns' opens a file descriptor to fill
the netlink attribute IFLA_NET_NS_FD. This file descriptor is never closed.
When batch mode is used, the number of file descriptor may grow greatly and
reach the maximum file descriptor number that can be opened.

This fd can be closed only after the netlink answer. Moreover, a second
fd could be opened because some (struct link_util)->parse_opt() handlers
call iplink_parse().

Let's add a helper to manage these fds:
 - open_fds_add() stores a fd, up to 5 (arbitrary choice, it seems enough);
 - open_fds_close() closes all stored fds.

Fixes: 0dc34c7713bb ("iproute2: Add processless network namespace support")
Reported-by: Alexandre Ferrieux <alexandre.ferrieux@...nge.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>
---

v1 -> v2:
 - use a global variable to store fds

 include/utils.h |  3 +++
 ip/iplink.c     |  6 +++++-
 lib/utils.c     | 23 +++++++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/utils.h b/include/utils.h
index a2a98b9bf17d..f044b3401320 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -395,4 +395,7 @@ const char *proto_n2a(unsigned short id, char *buf, int len,
 
 FILE *generic_proc_open(const char *env, const char *name);
 
+int open_fds_add(int fd);
+void open_fds_close(void);
+
 #endif /* __UTILS_H__ */
diff --git a/ip/iplink.c b/ip/iplink.c
index 3bc75d243719..0dd83ff44846 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -622,9 +622,11 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 			if (netns != -1)
 				duparg("netns", *argv);
 			netns = netns_get_fd(*argv);
-			if (netns >= 0)
+			if (netns >= 0) {
+				open_fds_add(netns);
 				addattr_l(&req->n, sizeof(*req), IFLA_NET_NS_FD,
 					  &netns, 4);
+			}
 			else if (get_integer(&netns, *argv, 0) == 0)
 				addattr_l(&req->n, sizeof(*req),
 					  IFLA_NET_NS_PID, &netns, 4);
@@ -1088,6 +1090,8 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 	else
 		ret = rtnl_talk(&rth, &req.n, NULL);
 
+	open_fds_close();
+
 	if (ret)
 		return -2;
 
diff --git a/lib/utils.c b/lib/utils.c
index deb7654a0b01..98c06ab6a652 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -40,6 +40,9 @@ int timestamp_short;
 int pretty;
 const char *_SL_ = "\n";
 
+static int open_fds[5];
+static int open_fds_cnt;
+
 static int af_byte_len(int af);
 static void print_time(char *buf, int len, __u32 time);
 static void print_time64(char *buf, int len, __s64 time);
@@ -2017,3 +2020,23 @@ FILE *generic_proc_open(const char *env, const char *name)
 
 	return fopen(p, "r");
 }
+
+int open_fds_add(int fd)
+{
+	if (open_fds_cnt >= ARRAY_SIZE(open_fds))
+		return -1;
+
+	open_fds[open_fds_cnt++] = fd;
+	return 0;
+}
+
+
+void open_fds_close(void)
+{
+	int i;
+
+	for (i = 0; i < open_fds_cnt; i++)
+		close(open_fds[i]);
+
+	open_fds_cnt = 0;
+}
-- 
2.43.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ