[PATCH] Consolidate & clean up fd-passing

Volker Lendecke Volker.Lendecke at SerNet.DE
Thu Jan 1 10:48:49 MST 2015


Hi!

Recently I've been playing with fd-passing a bit more. The CMSG style
macros mandated by standards are really, really ugly to use in my
opinion. Attached find a patchset that consolidates these macros from
unix_msg and aio_fork (along with minor cleanups), making them much
easier to use (at least IMHO).

Review&push appreciated!

Thanks,

Volker

-- 
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 at sernet.de
-------------- next part --------------
From 1aae9445ef535a47beee2b432948ebcffffad97b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 10:39:25 +0100
Subject: [PATCH 01/11] torture3: Fix a typo

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/torture/test_messaging_read.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/torture/test_messaging_read.c b/source3/torture/test_messaging_read.c
index 802b4fe..43b4367 100644
--- a/source3/torture/test_messaging_read.c
+++ b/source3/torture/test_messaging_read.c
@@ -506,7 +506,7 @@ static bool read4_child(int ready_fd)
 
 	printf("child: telling parent we're ready to receive messages\n");
 
-	/* Tell the parent we are ready to receive mesages. */
+	/* Tell the parent we are ready to receive messages. */
 	bytes = write(ready_fd, &c, 1);
 	if (bytes != 1) {
 		perror("child: failed to write to ready_fd");
-- 
1.9.1


From 0e1d2f88e48be981bb41e835f27c637b42dc32a5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 30 Dec 2014 12:26:16 +0100
Subject: [PATCH 02/11] lib: unix_dgram_msg does not need "num_fds"

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/unix_msg/unix_msg.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/source3/lib/unix_msg/unix_msg.c b/source3/lib/unix_msg/unix_msg.c
index 78b29c2..d85cde9 100644
--- a/source3/lib/unix_msg/unix_msg.c
+++ b/source3/lib/unix_msg/unix_msg.c
@@ -43,7 +43,6 @@ struct unix_dgram_msg {
 	int sock;
 	ssize_t sent;
 	int sys_errno;
-	size_t num_fds;
 	struct msghdr msg;
 	struct iovec iov;
 };
@@ -539,7 +538,6 @@ static int queue_msg(struct unix_dgram_send_queue *q,
 	}
 
 	msg->sock = q->sock;
-	msg->num_fds = num_fds;
 
 	data_buf = (uint8_t *)(msg + 1);
 
-- 
1.9.1


From 953cc2c38fee23779e6e265e5f10fe4fb9b4f4fd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 30 Dec 2014 13:36:46 +0000
Subject: [PATCH 03/11] lib: Add msghdr.[ch]

This is a little set of routines to deal with the ugly fd-passing macros.

This patch is the first step assisting the creation of msghrds for sending fds.
Receiving fd helpers will follow later.

The basic idea behind these routines is that they fill a variable-sized buffer.
They are supposed to be called twice per msghdr preparation. First with a
0-sized NULL output buffer to calculate the required bufsize, and then a second
time filling in the buffer as such.

This does not take care of the old msg_accrights way of passing file
descriptors. CMSG/SCM_RIGHTS is standardized for quite a while now, and I
believe this intreface can be made to also take care of msg_accrights if
needed.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/msghdr.c  | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++
 source3/lib/msghdr.h  |  38 +++++++++++++++
 source3/wscript_build |   5 ++
 3 files changed, 170 insertions(+)
 create mode 100644 source3/lib/msghdr.c
 create mode 100644 source3/lib/msghdr.h

diff --git a/source3/lib/msghdr.c b/source3/lib/msghdr.c
new file mode 100644
index 0000000..9d5f28b
--- /dev/null
+++ b/source3/lib/msghdr.c
@@ -0,0 +1,127 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Copyright (C) Volker Lendecke 2014
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "replace.h"
+#include "lib/msghdr.h"
+#include "lib/iov_buf.h"
+#include <sys/socket.h>
+
+ssize_t msghdr_prep_fds(struct msghdr *msg, uint8_t *buf, size_t bufsize,
+			const int *fds, size_t num_fds)
+{
+	size_t fds_size = sizeof(int) * MIN(num_fds, INT8_MAX);
+	size_t cmsg_len = CMSG_LEN(fds_size);
+	size_t cmsg_space = CMSG_SPACE(fds_size);
+	struct cmsghdr *cmsg;
+	void *fdptr;
+
+	if (num_fds == 0) {
+		if (msg != NULL) {
+			msg->msg_control = NULL;
+			msg->msg_controllen = 0;
+		}
+		return 0;
+	}
+	if (num_fds > INT8_MAX) {
+		return -1;
+	}
+	if (cmsg_space > bufsize) {
+		return cmsg_space;
+	}
+
+	msg->msg_control = buf;
+	msg->msg_controllen = cmsg_space;
+
+	cmsg = CMSG_FIRSTHDR(msg);
+	cmsg->cmsg_level = SOL_SOCKET;
+	cmsg->cmsg_type = SCM_RIGHTS;
+	cmsg->cmsg_len = cmsg_len;
+	fdptr = CMSG_DATA(cmsg);
+	memcpy(fdptr, fds, fds_size);
+	msg->msg_controllen = cmsg->cmsg_len;
+
+	return cmsg_space;
+}
+
+struct msghdr_buf {
+	struct msghdr msg;
+	struct sockaddr_storage addr;
+	struct iovec iov;
+	uint8_t buf[];
+};
+
+ssize_t msghdr_copy(struct msghdr_buf *msg, size_t msgsize,
+		    const void *addr, socklen_t addrlen,
+		    const struct iovec *iov, int iovcnt,
+		    const int *fds, size_t num_fds)
+{
+	size_t fd_len, iov_len, needed, bufsize;
+
+	bufsize = (msgsize > offsetof(struct msghdr_buf, buf)) ?
+		msgsize - offsetof(struct msghdr_buf, buf) : 0;
+
+	fd_len = msghdr_prep_fds(&msg->msg, msg->buf, bufsize, fds, num_fds);
+
+	if (bufsize >= fd_len) {
+		bufsize -= fd_len;
+	} else {
+		bufsize = 0;
+	}
+
+	if (msg != NULL) {
+
+		if (addr != NULL) {
+			if (addrlen > sizeof(struct sockaddr_storage)) {
+				errno = EMSGSIZE;
+				return -1;
+			}
+			memcpy(&msg->addr, addr, addrlen);
+			msg->msg.msg_name = &msg->addr;
+			msg->msg.msg_namelen = addrlen;
+		} else {
+			msg->msg.msg_name = NULL;
+			msg->msg.msg_namelen = 0;
+		}
+
+		msg->iov.iov_base = msg->buf + fd_len;
+		msg->iov.iov_len = iov_buf(
+			iov, iovcnt, msg->iov.iov_base, bufsize);
+		iov_len = msg->iov.iov_len;
+
+		msg->msg.msg_iov = &msg->iov;
+		msg->msg.msg_iovlen = 1;
+	} else {
+		iov_len = iov_buflen(iov, iovcnt);
+	}
+
+	needed = offsetof(struct msghdr_buf, buf) + fd_len;
+	if (needed < fd_len) {
+		return -1;
+	}
+	needed += iov_len;
+	if (needed < iov_len) {
+		return -1;
+	}
+
+	return needed;
+}
+
+struct msghdr *msghdr_buf_msghdr(struct msghdr_buf *msg)
+{
+	return &msg->msg;
+}
diff --git a/source3/lib/msghdr.h b/source3/lib/msghdr.h
new file mode 100644
index 0000000..af9506c
--- /dev/null
+++ b/source3/lib/msghdr.h
@@ -0,0 +1,38 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Copyright (C) Volker Lendecke 2014
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __LIB_MSGHDR_H__
+#define __LIB_MSGHDR_H__
+
+#include <stddef.h>
+#include <stdint.h>
+#include <sys/uio.h>
+#include <sys/socket.h>
+
+ssize_t msghdr_prep_fds(struct msghdr *msg, uint8_t *buf, size_t bufsize,
+			const int *fds, size_t num_fds);
+
+struct msghdr_buf;
+
+ssize_t msghdr_copy(struct msghdr_buf *msg, size_t msgsize,
+		    const void *addr, socklen_t addrlen,
+		    const struct iovec *iov, int iovcnt,
+		    const int *fds, size_t num_fds);
+struct msghdr *msghdr_buf_msghdr(struct msghdr_buf *msg);
+
+#endif
diff --git a/source3/wscript_build b/source3/wscript_build
index dc6b196..a6ef584 100755
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -774,6 +774,11 @@ bld.SAMBA3_SUBSYSTEM('tdb-wrap3',
                     source='lib/util_tdb.c',
                     deps='talloc samba3-util')
 
+bld.SAMBA3_LIBRARY('msghdr',
+                   source='lib/msghdr.c',
+                   deps='replace iov_buf',
+                   private_library=True)
+
 bld.SAMBA3_LIBRARY('samba3-util',
                    source='''lib/util_sec.c lib/util_str.c lib/adt_tree.c lib/util_malloc.c lib/namearray.c lib/file_id.c''',
                    deps='samba-util charset',
-- 
1.9.1


From ea535e77556a28574466e096acaadf0358d0e2c6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 30 Dec 2014 14:05:02 +0000
Subject: [PATCH 04/11] lib: Use msghdr in unix_msg

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/unix_msg/unix_msg.c    | 179 ++++++++++++-------------------------
 source3/lib/unix_msg/wscript_build |   2 +-
 2 files changed, 60 insertions(+), 121 deletions(-)

diff --git a/source3/lib/unix_msg/unix_msg.c b/source3/lib/unix_msg/unix_msg.c
index d85cde9..8d4960f 100644
--- a/source3/lib/unix_msg/unix_msg.c
+++ b/source3/lib/unix_msg/unix_msg.c
@@ -24,6 +24,7 @@
 #include "dlinklist.h"
 #include "pthreadpool/pthreadpool.h"
 #include "lib/iov_buf.h"
+#include "lib/msghdr.h"
 #include <fcntl.h>
 
 /*
@@ -43,8 +44,6 @@ struct unix_dgram_msg {
 	int sock;
 	ssize_t sent;
 	int sys_errno;
-	struct msghdr msg;
-	struct iovec iov;
 };
 
 struct unix_dgram_send_queue {
@@ -162,6 +161,22 @@ static void extract_fd_array_from_msghdr(struct msghdr *msg, int **fds,
 #endif
 }
 
+static size_t unix_dgram_msg_size(void)
+{
+	size_t msgsize = sizeof(struct unix_dgram_msg);
+	msgsize = (msgsize + 15) & ~15; /* align to 16 */
+	return msgsize;
+}
+
+static struct msghdr_buf *unix_dgram_msghdr(struct unix_dgram_msg *msg)
+{
+	/*
+	 * Not portable in C99, but "msg" is aligned and so is
+	 * unix_dgram_msg_size()
+	 */
+	return (struct msghdr_buf *)(((char *)msg) + unix_dgram_msg_size());
+}
+
 static void close_fd_array(int *fds, size_t num_fds)
 {
 	size_t i;
@@ -176,8 +191,10 @@ static void close_fd_array(int *fds, size_t num_fds)
 	}
 }
 
-static void close_fd_array_cmsg(struct msghdr *msg)
+static void close_fd_array_dgram_msg(struct unix_dgram_msg *dmsg)
 {
+	struct msghdr_buf *hdr = unix_dgram_msghdr(dmsg);
+	struct msghdr *msg = msghdr_buf_msghdr(hdr);
 	int *fds = NULL;
 	size_t num_fds = 0;
 
@@ -448,7 +465,7 @@ static void unix_dgram_send_queue_free(struct unix_dgram_send_queue *q)
 		struct unix_dgram_msg *msg;
 		msg = q->msgs;
 		DLIST_REMOVE(q->msgs, msg);
-		close_fd_array_cmsg(&msg->msg);
+		close_fd_array_dgram_msg(msg);
 		free(msg);
 	}
 	close(q->sock);
@@ -470,55 +487,17 @@ static struct unix_dgram_send_queue *find_send_queue(
 }
 
 static int queue_msg(struct unix_dgram_send_queue *q,
-		     const struct iovec *iov, int iovlen,
+		     const struct iovec *iov, int iovcnt,
 		     const int *fds, size_t num_fds)
 {
 	struct unix_dgram_msg *msg;
-	ssize_t data_len;
-	uint8_t *data_buf;
-	size_t msglen = sizeof(struct unix_dgram_msg);
-	int i;
-	size_t tmp;
-	int ret = -1;
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	size_t fds_size = sizeof(int) * MIN(num_fds, INT8_MAX);
+	struct msghdr_buf *hdr;
+	size_t msglen, needed;
+	ssize_t msghdrlen;
 	int fds_copy[MIN(num_fds, INT8_MAX)];
-	size_t cmsg_len = CMSG_LEN(fds_size);
-	size_t cmsg_space = CMSG_SPACE(fds_size);
-	char *cmsg_buf;
-
-	/*
-	 * Note: No need to check for overflow here,
-	 * since cmsg will store <= INT8_MAX fds.
-	 */
-	msglen += cmsg_space;
-
-#endif /*  HAVE_STRUCT_MSGHDR_MSG_CONTROL */
-
-	if (num_fds > INT8_MAX) {
-		return EINVAL;
-	}
-
-#ifndef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	if (num_fds > 0) {
-		return ENOSYS;
-	}
-#endif
-
-	data_len = iov_buflen(iov, iovlen);
-	if (data_len == -1) {
-		return EINVAL;
-	}
-
-	tmp = msglen + data_len;
-	if ((tmp < msglen) || (tmp < data_len)) {
-		/* overflow */
-		return EINVAL;
-	}
-	msglen = tmp;
+	int i, ret;
 
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	for (i = 0; i < num_fds; i++) {
+	for (i=0; i<num_fds; i++) {
 		fds_copy[i] = -1;
 	}
 
@@ -529,67 +508,37 @@ static int queue_msg(struct unix_dgram_send_queue *q,
 			goto fail;
 		}
 	}
-#endif
 
-	msg = malloc(msglen);
-	if (msg == NULL) {
-		ret = ENOMEM;
+	msglen = unix_dgram_msg_size();
+
+	msghdrlen = msghdr_copy(NULL, 0, NULL, 0, iov, iovcnt,
+				fds_copy, num_fds);
+	if (msghdrlen == -1) {
+		ret = EMSGSIZE;
 		goto fail;
 	}
 
-	msg->sock = q->sock;
-
-	data_buf = (uint8_t *)(msg + 1);
-
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	if (num_fds > 0) {
-		cmsg_buf = (char *)data_buf;
-		memset(cmsg_buf, 0, cmsg_space);
-		data_buf += cmsg_space;
-	} else {
-		cmsg_buf = NULL;
-		cmsg_space = 0;
+	needed = msglen + msghdrlen;
+	if (needed < msglen) {
+		ret = EMSGSIZE;
+		goto fail;
 	}
-#endif
-
-	msg->iov = (struct iovec) {
-		.iov_base = (void *)data_buf,
-		.iov_len = data_len,
-	};
-
-	msg->msg = (struct msghdr) {
-		.msg_iov = &msg->iov,
-		.msg_iovlen = 1,
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-		.msg_control = cmsg_buf,
-		.msg_controllen = cmsg_space,
-#endif
-	};
-
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	if (num_fds > 0) {
-		struct cmsghdr *cmsg;
-		void *fdptr;
 
-		cmsg = CMSG_FIRSTHDR(&msg->msg);
-		cmsg->cmsg_level = SOL_SOCKET;
-		cmsg->cmsg_type = SCM_RIGHTS;
-		cmsg->cmsg_len = cmsg_len;
-		fdptr = CMSG_DATA(cmsg);
-		memcpy(fdptr, fds_copy, fds_size);
-		msg->msg.msg_controllen = cmsg->cmsg_len;
+	msg = malloc(needed);
+	if (msg == NULL) {
+		ret = ENOMEM;
+		goto fail;
 	}
-#endif /*  HAVE_STRUCT_MSGHDR_MSG_CONTROL */
+	hdr = unix_dgram_msghdr(msg);
 
-	iov_buf(iov, iovlen, data_buf, data_len);
+	msg->sock = q->sock;
+	msghdr_copy(hdr, msghdrlen, NULL, 0, iov, iovcnt,
+		    fds_copy, num_fds);
 
 	DLIST_ADD_END(q->msgs, msg, struct unix_dgram_msg);
 	return 0;
-
 fail:
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
 	close_fd_array(fds_copy, num_fds);
-#endif
 	return ret;
 }
 
@@ -598,7 +547,9 @@ static void unix_dgram_send_job(void *private_data)
 	struct unix_dgram_msg *dmsg = private_data;
 
 	do {
-		dmsg->sent = sendmsg(dmsg->sock, &dmsg->msg, 0);
+		struct msghdr_buf *hdr = unix_dgram_msghdr(dmsg);
+		struct msghdr *msg = msghdr_buf_msghdr(hdr);
+		dmsg->sent = sendmsg(dmsg->sock, msg, 0);
 	} while ((dmsg->sent == -1) && (errno == EINTR));
 
 	if (dmsg->sent == -1) {
@@ -632,7 +583,7 @@ static void unix_dgram_job_finished(struct poll_watch *w, int fd, short events,
 
 	msg = q->msgs;
 	DLIST_REMOVE(q->msgs, msg);
-	close_fd_array_cmsg(&msg->msg);
+	close_fd_array_dgram_msg(msg);
 	free(msg);
 
 	if (q->msgs != NULL) {
@@ -653,13 +604,7 @@ static int unix_dgram_send(struct unix_dgram_ctx *ctx,
 {
 	struct unix_dgram_send_queue *q;
 	struct msghdr msg;
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	struct cmsghdr *cmsg;
-	size_t fds_size = sizeof(int) * num_fds;
-	size_t cmsg_len = CMSG_LEN(fds_size);
-	size_t cmsg_space = CMSG_SPACE(fds_size);
-	char cmsg_buf[cmsg_space];
-#endif /* HAVE_STRUCT_MSGHDR_MSG_CONTROL */
+	ssize_t fdlen;
 	int ret;
 	int i;
 
@@ -714,25 +659,19 @@ static int unix_dgram_send(struct unix_dgram_ctx *ctx,
 		.msg_iov = discard_const_p(struct iovec, iov),
 		.msg_iovlen = iovlen
 	};
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	if (num_fds > 0) {
-		void *fdptr;
 
-		memset(cmsg_buf, 0, cmsg_space);
+	fdlen = msghdr_prep_fds(&msg, NULL, 0, fds, num_fds);
+	if (fdlen == -1) {
+		return EINVAL;
+	}
+
+	{
+		uint8_t buf[fdlen];
+		msghdr_prep_fds(&msg, buf, 0, fds, num_fds);
 
-		msg.msg_control = cmsg_buf;
-		msg.msg_controllen = cmsg_space;
-		cmsg = CMSG_FIRSTHDR(&msg);
-		cmsg->cmsg_level = SOL_SOCKET;
-		cmsg->cmsg_type = SCM_RIGHTS;
-		cmsg->cmsg_len = cmsg_len;
-		fdptr = CMSG_DATA(cmsg);
-		memcpy(fdptr, fds, fds_size);
-		msg.msg_controllen = cmsg->cmsg_len;
+		ret = sendmsg(ctx->sock, &msg, 0);
 	}
-#endif /*  HAVE_STRUCT_MSGHDR_MSG_CONTROL */
 
-	ret = sendmsg(ctx->sock, &msg, 0);
 	if (ret >= 0) {
 		return 0;
 	}
diff --git a/source3/lib/unix_msg/wscript_build b/source3/lib/unix_msg/wscript_build
index e638ea6..b16d52c 100644
--- a/source3/lib/unix_msg/wscript_build
+++ b/source3/lib/unix_msg/wscript_build
@@ -2,7 +2,7 @@
 
 bld.SAMBA3_SUBSYSTEM('UNIX_MSG',
                      source='unix_msg.c',
-		     deps='replace PTHREADPOOL iov_buf')
+		     deps='replace PTHREADPOOL iov_buf msghdr')
 
 bld.SAMBA3_BINARY('unix_msg_test',
                   source='tests.c',
-- 
1.9.1


From 671a6c029e0029a265edd8973b2da6220c1fdabd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 13:03:24 +0100
Subject: [PATCH 05/11] smbd: Use msghdr_prep_fds in vfs_aio_fork

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/modules/vfs_aio_fork.c | 39 +++++++++------------------------------
 1 file changed, 9 insertions(+), 30 deletions(-)

diff --git a/source3/modules/vfs_aio_fork.c b/source3/modules/vfs_aio_fork.c
index 39334bc..3b6699b 100644
--- a/source3/modules/vfs_aio_fork.c
+++ b/source3/modules/vfs_aio_fork.c
@@ -28,6 +28,7 @@
 #include "lib/util/tevent_unix.h"
 #include "lib/sys_rw.h"
 #include "lib/sys_rw_data.h"
+#include "lib/msghdr.h"
 
 #if !defined(HAVE_STRUCT_MSGHDR_MSG_CONTROL) && !defined(HAVE_STRUCT_MSGHDR_MSG_ACCRIGHTS)
 # error Can not pass file descriptors
@@ -220,40 +221,18 @@ static ssize_t read_fd(int fd, void *ptr, size_t nbytes, int *recvfd)
 
 static ssize_t write_fd(int fd, void *ptr, size_t nbytes, int sendfd)
 {
-	struct msghdr	msg;
-	struct iovec	iov[1];
-
-#ifdef	HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	union {
-		struct cmsghdr	cm;
-		char control[CMSG_SPACE(sizeof(int))];
-	} control_un;
-	struct cmsghdr	*cmptr;
-
-	ZERO_STRUCT(msg);
-	ZERO_STRUCT(control_un);
-
-	msg.msg_control = control_un.control;
-	msg.msg_controllen = sizeof(control_un.control);
-
-	cmptr = CMSG_FIRSTHDR(&msg);
-	cmptr->cmsg_len = CMSG_LEN(sizeof(int));
-	cmptr->cmsg_level = SOL_SOCKET;
-	cmptr->cmsg_type = SCM_RIGHTS;
-	memcpy(CMSG_DATA(cmptr), &sendfd, sizeof(sendfd));
-#else
-	ZERO_STRUCT(msg);
-	msg.msg_accrights = (caddr_t) &sendfd;
-	msg.msg_accrightslen = sizeof(int);
-#endif
+	struct msghdr msg;
+	size_t bufsize = msghdr_prep_fds(NULL, NULL, 0, &sendfd, 1);
+	uint8_t buf[bufsize];
+	struct iovec iov;
 
+	msghdr_prep_fds(&msg, buf, bufsize, &sendfd, 1);
 	msg.msg_name = NULL;
 	msg.msg_namelen = 0;
 
-	ZERO_STRUCT(iov);
-	iov[0].iov_base = (void *)ptr;
-	iov[0].iov_len = nbytes;
-	msg.msg_iov = iov;
+	iov.iov_base = (void *)ptr;
+	iov.iov_len = nbytes;
+	msg.msg_iov = &iov;
 	msg.msg_iovlen = 1;
 
 	return (sendmsg(fd, &msg, 0));
-- 
1.9.1


From a61f7611807f17c12d6d12260b42b230c7222567 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 13:14:41 +0100
Subject: [PATCH 06/11] lib: Add msghdr_extract_fds

This is a copy of the extract_fd_array_from_msghdr routine in unix_msg.c, with
a similar use pattern: First call it without an output array to get the length
and then call it a second time to actually fill in the array.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/msghdr.c | 28 ++++++++++++++++++++++++++++
 source3/lib/msghdr.h |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/source3/lib/msghdr.c b/source3/lib/msghdr.c
index 9d5f28b..3449579 100644
--- a/source3/lib/msghdr.c
+++ b/source3/lib/msghdr.c
@@ -125,3 +125,31 @@ struct msghdr *msghdr_buf_msghdr(struct msghdr_buf *msg)
 {
 	return &msg->msg;
 }
+
+size_t msghdr_extract_fds(struct msghdr *msg, int *fds, size_t fds_size)
+{
+	struct cmsghdr *cmsg;
+	size_t num_fds;
+
+	for(cmsg = CMSG_FIRSTHDR(msg);
+	    cmsg != NULL;
+	    cmsg = CMSG_NXTHDR(msg, cmsg))
+	{
+		if ((cmsg->cmsg_type == SCM_RIGHTS) &&
+		    (cmsg->cmsg_level == SOL_SOCKET)) {
+			break;
+		}
+	}
+
+	if (cmsg == NULL) {
+		return 0;
+	}
+
+	num_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int);
+
+	if ((num_fds != 0) && (fds_size >= num_fds)) {
+		memcpy(fds, CMSG_DATA(cmsg), num_fds * sizeof(int));
+	}
+
+	return num_fds;
+}
diff --git a/source3/lib/msghdr.h b/source3/lib/msghdr.h
index af9506c..8882923 100644
--- a/source3/lib/msghdr.h
+++ b/source3/lib/msghdr.h
@@ -35,4 +35,6 @@ ssize_t msghdr_copy(struct msghdr_buf *msg, size_t msgsize,
 		    const int *fds, size_t num_fds);
 struct msghdr *msghdr_buf_msghdr(struct msghdr_buf *msg);
 
+size_t msghdr_extract_fds(struct msghdr *msg, int *fds, size_t num_fds);
+
 #endif
-- 
1.9.1


From 8f15ff8c56b8d5edc4b1ca4cba538e7faa319ca3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 13:33:48 +0100
Subject: [PATCH 07/11] lib: Use msghdr_extract_fds in unix_msg

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/unix_msg/unix_msg.c | 75 +++++++++++------------------------------
 1 file changed, 19 insertions(+), 56 deletions(-)

diff --git a/source3/lib/unix_msg/unix_msg.c b/source3/lib/unix_msg/unix_msg.c
index 8d4960f..c5f8143 100644
--- a/source3/lib/unix_msg/unix_msg.c
+++ b/source3/lib/unix_msg/unix_msg.c
@@ -135,32 +135,6 @@ static int prepare_socket(int sock)
 	return prepare_socket_cloexec(sock);
 }
 
-static void extract_fd_array_from_msghdr(struct msghdr *msg, int **fds,
-					 size_t *num_fds)
-{
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	struct cmsghdr *cmsg;
-
-	for(cmsg = CMSG_FIRSTHDR(msg);
-	    cmsg != NULL;
-	    cmsg = CMSG_NXTHDR(msg, cmsg))
-	{
-		void *data = CMSG_DATA(cmsg);
-
-		if (cmsg->cmsg_type != SCM_RIGHTS) {
-			continue;
-		}
-		if (cmsg->cmsg_level != SOL_SOCKET) {
-			continue;
-		}
-
-		*fds = (int *)data;
-		*num_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof (int);
-		break;
-	}
-#endif
-}
-
 static size_t unix_dgram_msg_size(void)
 {
 	size_t msgsize = sizeof(struct unix_dgram_msg);
@@ -195,14 +169,11 @@ static void close_fd_array_dgram_msg(struct unix_dgram_msg *dmsg)
 {
 	struct msghdr_buf *hdr = unix_dgram_msghdr(dmsg);
 	struct msghdr *msg = msghdr_buf_msghdr(hdr);
-	int *fds = NULL;
-	size_t num_fds = 0;
+	size_t num_fds = msghdr_extract_fds(msg, NULL, 0);
+	int fds[num_fds];
 
-	extract_fd_array_from_msghdr(msg, &fds, &num_fds);
+	msghdr_extract_fds(msg, fds, num_fds);
 
-	/*
-	 * TODO: caveat - side-effect - changing msg ???
-	 */
 	close_fd_array(fds, num_fds);
 }
 
@@ -304,8 +275,6 @@ static void unix_dgram_recv_handler(struct poll_watch *w, int fd, short events,
 #ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
 	char buf[CMSG_SPACE(sizeof(int)*INT8_MAX)] = { 0, };
 #endif /* HAVE_STRUCT_MSGHDR_MSG_CONTROL */
-	int *fds = NULL;
-	size_t i, num_fds = 0;
 
 	iov = (struct iovec) {
 		.iov_base = (void *)ctx->recv_buf,
@@ -342,32 +311,26 @@ static void unix_dgram_recv_handler(struct poll_watch *w, int fd, short events,
 		return;
 	}
 
-	extract_fd_array_from_msghdr(&msg, &fds, &num_fds);
-
-	for (i = 0; i < num_fds; i++) {
-		int err;
+	{
+		size_t num_fds = msghdr_extract_fds(&msg, NULL, 0);
+		int fds[num_fds];
+		int i;
 
-		err = prepare_socket_cloexec(fds[i]);
-		if (err != 0) {
-			goto cleanup_fds;
-		}
-	}
+		msghdr_extract_fds(&msg, fds, num_fds);
 
-	ctx->recv_callback(ctx, ctx->recv_buf, received,
-			   fds, num_fds, ctx->private_data);
+		for (i = 0; i < num_fds; i++) {
+			int err;
 
-	/*
-	 * Close those fds that the callback has not set to -1.
-	 */
-	close_fd_array(fds, num_fds);
-
-	return;
-
-cleanup_fds:
-	close_fd_array(fds, num_fds);
+			err = prepare_socket_cloexec(fds[i]);
+			if (err != 0) {
+				close_fd_array(fds, num_fds);
+				num_fds = 0;
+			}
+		}
 
-	ctx->recv_callback(ctx, ctx->recv_buf, received,
-			   NULL, 0, ctx->private_data);
+		ctx->recv_callback(ctx, ctx->recv_buf, received,
+				   fds, num_fds, ctx->private_data);
+	}
 }
 
 static void unix_dgram_job_finished(struct poll_watch *w, int fd, short events,
-- 
1.9.1


From 468603bb7500aaed2f2ffb5b6617d40107c5df0d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 14:18:59 +0100
Subject: [PATCH 08/11] lib: Add msghdr_prep_recv_fds

This will prepare a msghdr for receiving fd's. Same pattern as before: First
get the buffer size, then fill in msghdr.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/msghdr.c | 20 ++++++++++++++++++++
 source3/lib/msghdr.h |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/source3/lib/msghdr.c b/source3/lib/msghdr.c
index 3449579..9ed1444 100644
--- a/source3/lib/msghdr.c
+++ b/source3/lib/msghdr.c
@@ -126,6 +126,26 @@ struct msghdr *msghdr_buf_msghdr(struct msghdr_buf *msg)
 	return &msg->msg;
 }
 
+size_t msghdr_prep_recv_fds(struct msghdr *msg, uint8_t *buf, size_t bufsize,
+			    size_t num_fds)
+{
+	size_t ret = CMSG_SPACE(sizeof(int) * num_fds);
+
+	if (bufsize < ret) {
+		return ret;
+	}
+	if (msg != NULL) {
+		if (num_fds != 0) {
+			msg->msg_control = buf;
+			msg->msg_controllen = ret;
+		} else {
+			msg->msg_control = NULL;
+			msg->msg_controllen = 0;
+		}
+	}
+	return ret;
+}
+
 size_t msghdr_extract_fds(struct msghdr *msg, int *fds, size_t fds_size)
 {
 	struct cmsghdr *cmsg;
diff --git a/source3/lib/msghdr.h b/source3/lib/msghdr.h
index 8882923..c1676d2 100644
--- a/source3/lib/msghdr.h
+++ b/source3/lib/msghdr.h
@@ -35,6 +35,8 @@ ssize_t msghdr_copy(struct msghdr_buf *msg, size_t msgsize,
 		    const int *fds, size_t num_fds);
 struct msghdr *msghdr_buf_msghdr(struct msghdr_buf *msg);
 
+size_t msghdr_prep_recv_fds(struct msghdr *msg, uint8_t *buf, size_t bufsize,
+			    size_t num_fds);
 size_t msghdr_extract_fds(struct msghdr *msg, int *fds, size_t num_fds);
 
 #endif
-- 
1.9.1


From fb962d7b605bf20efb7f6e709310ba6d2bd7f0cf Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 14:19:13 +0100
Subject: [PATCH 09/11] lib: Use msghdr_prep_recv_fds in unix_msg

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/unix_msg/unix_msg.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/source3/lib/unix_msg/unix_msg.c b/source3/lib/unix_msg/unix_msg.c
index c5f8143..51bb0c6 100644
--- a/source3/lib/unix_msg/unix_msg.c
+++ b/source3/lib/unix_msg/unix_msg.c
@@ -272,9 +272,8 @@ static void unix_dgram_recv_handler(struct poll_watch *w, int fd, short events,
 	int flags = 0;
 	struct msghdr msg;
 	struct iovec iov;
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	char buf[CMSG_SPACE(sizeof(int)*INT8_MAX)] = { 0, };
-#endif /* HAVE_STRUCT_MSGHDR_MSG_CONTROL */
+	size_t bufsize = msghdr_prep_recv_fds(NULL, NULL, 0, INT8_MAX);
+	uint8_t buf[bufsize];
 
 	iov = (struct iovec) {
 		.iov_base = (void *)ctx->recv_buf,
@@ -284,12 +283,10 @@ static void unix_dgram_recv_handler(struct poll_watch *w, int fd, short events,
 	msg = (struct msghdr) {
 		.msg_iov = &iov,
 		.msg_iovlen = 1,
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-		.msg_control = buf,
-		.msg_controllen = sizeof(buf),
-#endif
 	};
 
+	msghdr_prep_recv_fds(&msg, buf, bufsize, INT8_MAX);
+
 #ifdef MSG_CMSG_CLOEXEC
 	flags |= MSG_CMSG_CLOEXEC;
 #endif
-- 
1.9.1


From d7ab379a4a0487d590460303248cab2d4115e24d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 14:26:43 +0100
Subject: [PATCH 10/11] smbd: Use msghdr.[ch] in vfs_aio_fork

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/modules/vfs_aio_fork.c | 62 ++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 42 deletions(-)

diff --git a/source3/modules/vfs_aio_fork.c b/source3/modules/vfs_aio_fork.c
index 3b6699b..7d2aff9 100644
--- a/source3/modules/vfs_aio_fork.c
+++ b/source3/modules/vfs_aio_fork.c
@@ -157,26 +157,10 @@ static ssize_t read_fd(int fd, void *ptr, size_t nbytes, int *recvfd)
 	struct msghdr msg;
 	struct iovec iov[1];
 	ssize_t n;
-#ifndef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	int newfd;
-
-	ZERO_STRUCT(msg);
-	msg.msg_accrights = (caddr_t) &newfd;
-	msg.msg_accrightslen = sizeof(int);
-#else
-
-	union {
-	  struct cmsghdr	cm;
-	  char				control[CMSG_SPACE(sizeof(int))];
-	} control_un;
-	struct cmsghdr	*cmptr;
-
-	ZERO_STRUCT(msg);
-	ZERO_STRUCT(control_un);
+	size_t bufsize = msghdr_prep_recv_fds(NULL, NULL, 0, 1);
+	uint8_t buf[bufsize];
 
-	msg.msg_control = control_un.control;
-	msg.msg_controllen = sizeof(control_un.control);
-#endif
+	msghdr_prep_recv_fds(&msg, buf, bufsize, 1);
 
 	msg.msg_name = NULL;
 	msg.msg_namelen = 0;
@@ -190,31 +174,25 @@ static ssize_t read_fd(int fd, void *ptr, size_t nbytes, int *recvfd)
 		return(n);
 	}
 
-#ifdef	HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	if ((cmptr = CMSG_FIRSTHDR(&msg)) != NULL
-	    && cmptr->cmsg_len == CMSG_LEN(sizeof(int))) {
-		if (cmptr->cmsg_level != SOL_SOCKET) {
-			DEBUG(10, ("control level != SOL_SOCKET"));
-			errno = EINVAL;
-			return -1;
-		}
-		if (cmptr->cmsg_type != SCM_RIGHTS) {
-			DEBUG(10, ("control type != SCM_RIGHTS"));
-			errno = EINVAL;
-			return -1;
+	{
+		size_t num_fds = msghdr_extract_fds(&msg, NULL, 0);
+		int fds[num_fds];
+
+		msghdr_extract_fds(&msg, fds, num_fds);
+
+		if (num_fds != 1) {
+			size_t i;
+
+			for (i=0; i<num_fds; i++) {
+				close(fds[i]);
+			}
+
+			*recvfd = -1;
+			return n;
 		}
-		memcpy(recvfd, CMSG_DATA(cmptr), sizeof(*recvfd));
-	} else {
-		*recvfd = -1;		/* descriptor was not passed */
-	}
-#else
-	if (msg.msg_accrightslen == sizeof(int)) {
-		*recvfd = newfd;
-	}
-	else {
-		*recvfd = -1;		/* descriptor was not passed */
+
+		*recvfd = fds[0];
 	}
-#endif
 
 	return(n);
 }
-- 
1.9.1


From 3fccbf4ff3173535be74cfe816e2849939f33402 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 31 Dec 2014 14:27:03 +0100
Subject: [PATCH 11/11] smbd: Properly handle EINTR in vfs_aio_fork

---
 source3/modules/vfs_aio_fork.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_aio_fork.c b/source3/modules/vfs_aio_fork.c
index 7d2aff9..bf29dd1 100644
--- a/source3/modules/vfs_aio_fork.c
+++ b/source3/modules/vfs_aio_fork.c
@@ -170,8 +170,12 @@ static ssize_t read_fd(int fd, void *ptr, size_t nbytes, int *recvfd)
 	msg.msg_iov = iov;
 	msg.msg_iovlen = 1;
 
-	if ( (n = recvmsg(fd, &msg, 0)) <= 0) {
-		return(n);
+	do {
+		n = recvmsg(fd, &msg, 0);
+	} while ((n == -1) && (errno == EINTR));
+
+	if (n <= 0) {
+		return n;
 	}
 
 	{
@@ -203,6 +207,7 @@ static ssize_t write_fd(int fd, void *ptr, size_t nbytes, int sendfd)
 	size_t bufsize = msghdr_prep_fds(NULL, NULL, 0, &sendfd, 1);
 	uint8_t buf[bufsize];
 	struct iovec iov;
+	ssize_t sent;
 
 	msghdr_prep_fds(&msg, buf, bufsize, &sendfd, 1);
 	msg.msg_name = NULL;
@@ -213,7 +218,11 @@ static ssize_t write_fd(int fd, void *ptr, size_t nbytes, int sendfd)
 	msg.msg_iov = &iov;
 	msg.msg_iovlen = 1;
 
-	return (sendmsg(fd, &msg, 0));
+	do {
+		sent = sendmsg(fd, &msg, 0);
+	} while ((sent == -1) && (errno == EINTR));
+
+	return sent;
 }
 
 static void aio_child_cleanup(struct tevent_context *event_ctx,
-- 
1.9.1



More information about the samba-technical mailing list