[PATCH] Patch for bug 11714: Listening for network interface changes doesn't work

[Ralph Boehme]

On Fri, Feb 05, 2016 at 03:02:14PM +1100, Martin Schwenke wrote:
> On Thu, 4 Feb 2016 18:20:20 +0100, Ralph Boehme <rb at sernet.de> wrote:
> 
> > Hi!
> > 
> > <https://bugzilla.samba.org/show_bug.cgi?id=11714>
> > 
> > winbindd is watching for network interface address changes by virtue
> > of our addrchange_send/recv() API that uses netlink to query the
> > kernel for this information.
> > 
> > Log:
> > 
> > [2016/02/04 11:43:31.723123, 10, pid=17692, effective(0, 0), real(0,0)] ../source3/lib/addrchange.c:154(addrchange_done)
> >   tdgram_recvfrom_recv returned Operation not supported
> > [2016/02/04 11:43:31.723176, 10, pid=17692, effective(0, 0), real(0,0), class=winbind] ../source3/winbindd/winbindd.c:1486(winbindd_addr_changed)
> >   addrchange_recv failed: NT_STATUS_NOT_SUPPORTED, stop listening
> > 
> > strace shows a failing ioctl(fd, FIONREAD, ...) returning
> > EOPNOTSUPP. We use the ioctl to determine the amount of pending data
> > so we can allocate an appropriate buffer.
> > 
> > This is a regression introduced in 0d161e42d9aeb155eae2b04eccec497b21de8029.
> > 
> > What can we do?
> > 
> > We could use MSG_PEEK, but that's not portable and we'd have to call
> > recvmsg() twice every time the fd is reported readable.
> 
> Just out interest, what doesn't support MSG_PEEK?  It is in the POSIX
> standard that I have from 2001.  :-)

Typo, MSG_TRUNC, not MSG_PEEK. The recvmsg() request flag is Linux
only.

Attached is a new patch, that takes a different approach:

netlink is Linux only, so we can assume we have a working MSG_TRUNC as
well. We now remember the fact that a socket is AF_NETLINK and when
trying to determine pending bytes, we use a new function uses recvmsg
with MSG_PEEK|MSG_TRUNC.

Please review & push if ok. Thanks!
-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From d9557a327bfc6f6d9811e9438bd4b659d7b9442e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 Feb 2016 15:35:06 +0100
Subject: [PATCH] lib/tsocket: workaround sockets not supporting FIONREAD

Netlink sockets don't support querying pending bytes with ioctl(fd,
FIONREAD, ...) and would return EOPNOTSUPP, so use recvmsg() with
MSG_PEEK|MSG_TRUNC as a fallback.

The MSG_TRUNC flag to recvmsg() is Linux only, but netlink is as well,
so we're safe for now.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11714

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 lib/tsocket/tsocket_bsd.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/lib/tsocket/tsocket_bsd.c b/lib/tsocket/tsocket_bsd.c
index ac0617d..26bddf4 100644
--- a/lib/tsocket/tsocket_bsd.c
+++ b/lib/tsocket/tsocket_bsd.c
@@ -132,6 +132,43 @@ static int tsocket_bsd_common_prepare_fd(int fd, bool high_fd)
 	return -1;
 }
 
+#ifdef HAVE_LINUX_RTNETLINK_H
+/**
+ * Get the amount of pending bytes from a netlink socket
+ *
+ * For some reason netlink sockets don't support querying the amount of pending
+ * data via ioctl with FIONREAD, which is what we use in tsocket_bsd_pending()
+ * below.
+ *
+ * We know we are on Linux as we're usign netlink, which means we have a working
+ * MSG_TRUNC flag to recvmsg() as well, so we use that together with MSG_PEEK.
+ **/
+static ssize_t tsocket_bsd_netlink_pending(int fd)
+{
+	struct iovec iov;
+	struct msghdr msg;
+	char buf[1];
+
+	iov = (struct iovec) {
+		.iov_base = buf,
+		.iov_len = sizeof(buf)
+	};
+
+	msg = (struct msghdr) {
+		.msg_iov = &iov,
+		.msg_iovlen = 1
+	};
+
+	return recvmsg(fd, &msg, MSG_PEEK | MSG_TRUNC);
+}
+#else
+static ssize_t tsocket_bsd_netlink_pending(int fd)
+{
+	errno = ENOSYS;
+	return -1;
+}
+#endif
+
 static ssize_t tsocket_bsd_pending(int fd)
 {
 	int ret, error;
@@ -640,6 +677,7 @@ struct tdgram_bsd {
 	void *event_ptr;
 	struct tevent_fd *fde;
 	bool optimize_recvfrom;
+	bool netlink;
 
 	void *readable_private;
 	void (*readable_handler)(void *private_data);
@@ -892,7 +930,12 @@ static void tdgram_bsd_recvfrom_handler(void *private_data)
 	int err;
 	bool retry;
 
-	ret = tsocket_bsd_pending(bsds->fd);
+	if (bsds->netlink) {
+		ret = tsocket_bsd_netlink_pending(bsds->fd);
+	} else {
+		ret = tsocket_bsd_pending(bsds->fd);
+	}
+
 	if (state->first_try && ret == 0) {
 		state->first_try = false;
 		/* retry later */
@@ -1395,6 +1438,11 @@ int _tdgram_bsd_existing_socket(TALLOC_CTX *mem_ctx,
 {
 	struct tdgram_context *dgram;
 	struct tdgram_bsd *bsds;
+	int result;
+#ifdef HAVE_LINUX_RTNETLINK_H
+	struct sockaddr sa;
+	socklen_t sa_len = sizeof(struct sockaddr);
+#endif
 
 	dgram = tdgram_context_create(mem_ctx,
 				      &tdgram_bsd_ops,
@@ -1409,6 +1457,18 @@ int _tdgram_bsd_existing_socket(TALLOC_CTX *mem_ctx,
 	talloc_set_destructor(bsds, tdgram_bsd_destructor);
 
 	*_dgram = dgram;
+
+#ifdef HAVE_LINUX_RTNETLINK_H
+	/*
+	 * Try to determine the protocol family and remember if it's
+	 * AF_NETLINK. We don't care if this fails.
+	 */
+	result = getsockname(fd, &sa, &sa_len);
+	if (result == 0 && sa.sa_family == AF_NETLINK) {
+		bsds->netlink = true;
+	}
+#endif
+
 	return 0;
 }
 
-- 
2.5.0