From: Roy Marples <roy@marples.name>
Date: Thu, 7 Apr 2016 20:35:57 +0000 (+0000)
Subject: Linux netlink nlmsg_pid is not process id - only the first socket opened
X-Git-Tag: v6.10.2~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d306a8f2df27b629cac51cd6df648262046dcd20;p=thirdparty%2Fdhcpcd.git

Linux netlink nlmsg_pid is not process id - only the first socket opened
has that. So after opening the link socket, open a persistent route socket
and record the nlmsg_pid the kernel creates which is guaranteed unique
and won't clash with a process id which can then be ignored by testing it
directly and not that it's some large number.
---

diff --git a/dhcpcd.c b/dhcpcd.c
index ac5720a4..9486a9c3 100644
--- a/dhcpcd.c
+++ b/dhcpcd.c
@@ -1974,17 +1974,7 @@ exit1:
 		eloop_event_delete(ctx.eloop, ctx.link_fd);
 		close(ctx.link_fd);
 	}
-	if (ctx.pf_inet_fd != -1)
-		close(ctx.pf_inet_fd);
-#if defined(INET6) && defined(BSD)
-	if (ctx.pf_inet6_fd != -1)
-		close(ctx.pf_inet6_fd);
-#endif
-#ifdef IFLR_ACTIVE
-	if (ctx.pf_link_fd != -1)
-		close(ctx.pf_link_fd);
-#endif
-
+	if_closesockets(&ctx);
 	free_options(ifo);
 	free_globals(&ctx);
 	ipv4_ctxfree(&ctx);
diff --git a/dhcpcd.h b/dhcpcd.h
index 65c93f9f..08c7723b 100644
--- a/dhcpcd.h
+++ b/dhcpcd.h
@@ -117,12 +117,10 @@ struct dhcpcd_ctx {
 	struct if_head *ifaces;
 
 	int pf_inet_fd;
-#if defined(INET6) && defined(BSD)
-	int pf_inet6_fd;
-#endif
 #ifdef IFLR_ACTIVE
 	int pf_link_fd;
 #endif
+	void *priv;
 	int link_fd;
 	int seq;	/* route message sequence no */
 	int sseq;	/* successful seq no sent */
diff --git a/if-bsd.c b/if-bsd.c
index aa7e89ae..eb988623 100644
--- a/if-bsd.c
+++ b/if-bsd.c
@@ -105,6 +105,10 @@
 #  define CLLADDR(s) ((const char *)((s)->sdl_data + (s)->sdl_nlen))
 #endif
 
+struct priv {
+	int pf_inet6_fd;
+};
+
 int
 if_init(__unused struct interface *iface)
 {
@@ -120,10 +124,35 @@ if_conf(__unused struct interface *iface)
 }
 
 int
-if_openlinksocket(void)
+if_opensockets_os(struct dhcpcd_ctx *ctx)
+{
+	struct priv *priv;
+
+	if ((priv = malloc(sizeof(*priv))) == NULL)
+		return -1;
+	ctx->priv = priv;
+
+#ifdef INET6
+	priv->pf_inet6_fd = xsocket(PF_INET6, SOCK_DGRAM, 0, SOCK_CLOEXEC);
+	/* Don't return an error so we at least work on kernels witout INET6
+	 * even though we expect INET6 support.
+	 * We will fail noisily elsewhere anyway. */
+#else
+	priv->pf_inet6_fd = -1;
+#endif
+
+	ctx->pf_link_fd = xsocket(PF_LINK, SOCK_DGRAM, 0, SOCK_CLOEXEC);
+	return ctx->pf_link_fd == -1 ? -1 : 0;
+}
+
+void
+if_closesockets_os(struct dhcpcd_ctx *ctx)
 {
+	struct priv *priv;
 
-	return xsocket(PF_ROUTE, SOCK_RAW, 0, SOCK_NONBLOCK | SOCK_CLOEXEC);
+	priv = (struct priv *)ctx->priv;
+	if (priv->pf_inet6_fd != -1)
+		close(priv->pf_inet6_fd);
 }
 
 #if defined(INET) || defined(INET6)
diff --git a/if-linux.c b/if-linux.c
index dc1bb383..a8dbb2d2 100644
--- a/if-linux.c
+++ b/if-linux.c
@@ -91,6 +91,11 @@ int if_getssid_wext(const char *ifname, uint8_t *ssid);
 
 #include "bpf-filter.h"
 
+struct priv {
+	int route_fd;
+	uint32_t route_pid;
+};
+
 /* Broadcast address for IPoIB */
 static const uint8_t ipv4_bcast_addr[] = {
 	0x00, 0xff, 0xff, 0xff,
@@ -259,10 +264,15 @@ _open_link_socket(struct sockaddr_nl *nl, int protocol)
 }
 
 int
-if_openlinksocket(void)
+if_opensockets_os(struct dhcpcd_ctx *ctx)
 {
+	struct priv *priv;
 	struct sockaddr_nl snl;
+	socklen_t len;
 
+	/* Open the link socket first so it gets pid() for the socket.
+	 * Then open our persistent route socket so we get a unique
+	 * pid that doesn't clash with a process id for after we fork. */
 	memset(&snl, 0, sizeof(snl));
 	snl.nl_groups = RTMGRP_LINK;
 
@@ -273,7 +283,34 @@ if_openlinksocket(void)
 	snl.nl_groups |= RTMGRP_IPV6_ROUTE | RTMGRP_IPV6_IFADDR | RTMGRP_NEIGH;
 #endif
 
-	return _open_link_socket(&snl, NETLINK_ROUTE);
+	ctx->link_fd = _open_link_socket(&snl, NETLINK_ROUTE);
+	if (ctx->link_fd == -1)
+		return -1;
+
+	if ((priv = calloc(1, sizeof(*priv))) == NULL)
+		return -1;
+
+	ctx->priv = priv;
+	memset(&snl, 0, sizeof(snl));
+	priv->route_fd = _open_link_socket(&snl, NETLINK_ROUTE);
+	if (priv->route_fd == -1)
+		return -1;
+	len = sizeof(snl);
+	if (getsockname(priv->route_fd, (struct sockaddr *)&snl, &len) == -1)
+		return -1;
+	priv->route_pid = snl.nl_pid;
+	return 0;
+}
+
+void
+if_closesockets_os(struct dhcpcd_ctx *ctx)
+{
+	struct priv *priv;
+
+	if (ctx->priv != NULL) {
+		priv = (struct priv *)ctx->priv;
+		close(priv->route_fd);
+	}
 }
 
 static int
@@ -330,7 +367,6 @@ get_netlink(struct dhcpcd_ctx *ctx, struct interface *ifp, int fd, int flags,
 				goto eexit;
 			buf = nbuf;
 		}
-		nladdr.nl_pid = 0;
 		nladdr_len = sizeof(nladdr);
 		bytes = recvfrom(fd, buf, buflen, flags,
 		    (struct sockaddr *)&nladdr, &nladdr_len);
@@ -355,9 +391,8 @@ get_netlink(struct dhcpcd_ctx *ctx, struct interface *ifp, int fd, int flags,
 			r = err_netlink(nlm);
 			if (r == -1)
 				goto eexit;
-			if (r)
-				continue;
-			if (callback) {
+			
+			if (r == 0 && callback) {
 				r = callback(ctx, ifp, nlm);
 				if (r != 0)
 					goto eexit;
@@ -531,18 +566,6 @@ if_copyrt6(struct dhcpcd_ctx *ctx, struct rt6 *rt, struct nlmsghdr *nlm)
 }
 #endif
 
-/* Work out the maximum pid size */
-static inline long long
-get_max_pid_t()
-{
-
-	if (sizeof(pid_t) == sizeof(short))		return SHRT_MAX;
-	if (sizeof(pid_t) == sizeof(int))		return INT_MAX;
-	if (sizeof(pid_t) == sizeof(long))		return LONG_MAX;
-	if (sizeof(pid_t) == sizeof(long long))		return LLONG_MAX;
-	abort();
-}
-
 static int
 link_route(struct dhcpcd_ctx *ctx, __unused struct interface *ifp,
     struct nlmsghdr *nlm)
@@ -550,6 +573,7 @@ link_route(struct dhcpcd_ctx *ctx, __unused struct interface *ifp,
 	size_t len;
 	struct rtmsg *rtm;
 	int cmd;
+	struct priv *priv;
 #ifdef INET
 	struct rt rt;
 #endif
@@ -573,12 +597,10 @@ link_route(struct dhcpcd_ctx *ctx, __unused struct interface *ifp,
 		return -1;
 	}
 
-	/* Ignore messages generated by us.
-	 * For some reason we get messages generated by us
-	 * with a very large value in nlmsg_pid that seems to be
-	 * sequentially changing. Is there a better test for this? */
-	if (nlm->nlmsg_pid > get_max_pid_t())
-		return 1;
+	/* Ignore messages we sent. */
+	priv = (struct priv *)ctx->priv;
+	if (nlm->nlmsg_pid == priv->route_pid)
+		return 0;
 
 	rtm = NLMSG_DATA(nlm);
 	switch (rtm->rtm_family) {
@@ -605,6 +627,7 @@ link_addr(struct dhcpcd_ctx *ctx, struct interface *ifp, struct nlmsghdr *nlm)
 	size_t len;
 	struct rtattr *rta;
 	struct ifaddrmsg *ifa;
+	struct priv *priv;
 #ifdef INET
 	struct in_addr addr, net, dest;
 #endif
@@ -621,12 +644,10 @@ link_addr(struct dhcpcd_ctx *ctx, struct interface *ifp, struct nlmsghdr *nlm)
 		return -1;
 	}
 
-	/* Ignore messages generated by us.
-	 * For some reason we get messages generated by us
-	 * with a very large value in nlmsg_pid that seems to be
-	 * sequentially changing. Is there a better test for this? */
-	if (nlm->nlmsg_pid > get_max_pid_t())
-		return 1;
+	/* Ignore messages we sent. */
+	priv = (struct priv*)ctx->priv;
+	if (nlm->nlmsg_pid == priv->route_pid)
+		return 0;
 
 	ifa = NLMSG_DATA(nlm);
 	if ((ifp = if_findindex(ctx->ifaces, ifa->ifa_index)) == NULL) {
@@ -878,14 +899,24 @@ send_netlink(struct dhcpcd_ctx *ctx, struct interface *ifp,
 	struct msghdr msg;
 
 	memset(&snl, 0, sizeof(snl));
-	if ((s = _open_link_socket(&snl, protocol)) == -1)
-		return -1;
-	memset(&iov, 0, sizeof(iov));
-	iov.iov_base = hdr;
-	iov.iov_len = hdr->nlmsg_len;
+	snl.nl_family = AF_NETLINK;
+
+	if (protocol == NETLINK_ROUTE) {
+		struct priv *priv;
+
+		priv = (struct priv *)ctx->priv;
+		s = priv->route_fd;
+	} else {
+		if ((s = _open_link_socket(&snl, protocol)) == -1)
+			return -1;
+	}
+
 	memset(&msg, 0, sizeof(msg));
 	msg.msg_name = &snl;
 	msg.msg_namelen = sizeof(snl);
+	memset(&iov, 0, sizeof(iov));
+	iov.iov_base = hdr;
+	iov.iov_len = hdr->nlmsg_len;
 	msg.msg_iov = &iov;
 	msg.msg_iovlen = 1;
 	/* Request a reply */
@@ -893,13 +924,13 @@ send_netlink(struct dhcpcd_ctx *ctx, struct interface *ifp,
 	hdr->nlmsg_seq = (uint32_t)++ctx->seq;
 	if ((unsigned int)ctx->seq > UINT32_MAX)
 		ctx->seq = 0;
-
 	if (sendmsg(s, &msg, 0) != -1) {
 		ctx->sseq = ctx->seq;
 		r = get_netlink(ctx, ifp, s, 0, callback);
 	} else
 		r = -1;
-	close(s);
+	if (protocol != NETLINK_ROUTE)
+		close(s);
 	return r;
 }
 
diff --git a/if.c b/if.c
index a3b1e02d..d482a755 100644
--- a/if.c
+++ b/if.c
@@ -89,7 +89,7 @@ int
 if_opensockets(struct dhcpcd_ctx *ctx)
 {
 
-	if ((ctx->link_fd = if_openlinksocket()) == -1)
+	if (if_opensockets_os(ctx) == -1)
 		return -1;
 
 	/* We use this socket for some operations without INET. */
@@ -113,6 +113,23 @@ if_opensockets(struct dhcpcd_ctx *ctx)
 	return 0;
 }
 
+void
+if_closesockets(struct dhcpcd_ctx *ctx)
+{
+
+	if (ctx->pf_inet_fd != -1)
+		close(ctx->pf_inet_fd);
+#ifdef IFLR_ACTIVE
+	if (ctx->pf_link_fd != -1)
+		close(ctx->pf_link_fd);
+#endif
+
+	if (ctx->priv) {
+		if_closesockets_os(ctx);
+		free(ctx->priv);
+	}
+}
+
 int
 if_carrier(struct interface *ifp)
 {
diff --git a/if.h b/if.h
index 62de36f3..fdb73c45 100644
--- a/if.h
+++ b/if.h
@@ -101,7 +101,9 @@ int if_init(struct interface *);
 int if_getssid(struct interface *);
 int if_vimaster(const struct dhcpcd_ctx *ctx, const char *);
 int if_opensockets(struct dhcpcd_ctx *);
-int if_openlinksocket(void);
+int if_opensockets_os(struct dhcpcd_ctx *);
+void if_closesockets(struct dhcpcd_ctx *);
+void if_closesockets_os(struct dhcpcd_ctx *);
 int if_managelink(struct dhcpcd_ctx *);
 
 /* dhcpcd uses the same routing flags as BSD.