s3:unix_msg: don't fill cmsg buffer in unix_dgram_send_job()

Do this in queue_msg, instead.
This renders unix_dgram_send_job() as simple as it was before
we introduced fd-passing -- as it is intended.

This also changes struct unix_dgram_msg to not contain
the fd-array, but the correspondingly filled msghdr and cmsg buf.

Pair-Programmed-With: Volker Lendecke <vl@samba.org>
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

Signed-off-by: Michael Adam <obnox@samba.org>
Signed-off-by: Volker Lendecke <vl@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>

1 files changed, 61 insertions, 66 deletions

diff --git a/source3/lib/unix_msg/unix_msg.c b/source3/lib/unix_msg/unix_msg.c
index ca7f85dea39..be46a43e108 100644
--- a/source3/lib/unix_msg/unix_msg.c
+++ b/source3/lib/unix_msg/unix_msg.c
@@ -43,7 +43,7 @@ struct unix_dgram_msg {
 	ssize_t sent;
 	int sys_errno;
 	size_t num_fds;
-	int *fds;
+	struct msghdr msg;
 	struct iovec iov;
 };
 
@@ -443,7 +443,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(msg->fds, msg->num_fds);
+		close_fd_array_cmsg(&msg->msg);
 		free(msg);
 	}
 	close(q->sock);
@@ -472,19 +472,35 @@ static int queue_msg(struct unix_dgram_send_queue *q,
 	ssize_t data_len;
 	uint8_t *data_buf;
 	size_t msglen;
-	size_t fds_size = sizeof(int) * num_fds;
-	int fds_copy[MIN(num_fds, INT8_MAX)];
-	size_t fds_padding = 0;
 	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);
+	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;
+#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
+
 	msglen = sizeof(struct unix_dgram_msg);
 
+	/*
+	 * Note: No need to check for overflow here,
+	 * since cmsg will store <= INT8_MAX fds.
+	 */
+	msglen += cmsg_space;
+
 	data_len = iov_buflen(iov, iovlen);
 	if (data_len == -1) {
 		return EINVAL;
@@ -497,27 +513,7 @@ static int queue_msg(struct unix_dgram_send_queue *q,
 	}
 	msglen = tmp;
 
-	if (num_fds > 0) {
-		const size_t fds_align = sizeof(int) - 1;
-
-		tmp = msglen + fds_align;
-		if ((tmp < msglen) || (tmp < fds_align)) {
-			/* overflow */
-			return EINVAL;
-		}
-		tmp &= ~fds_align;
-
-		fds_padding = tmp - msglen;
-		msglen = tmp;
-
-		tmp = msglen + fds_size;
-		if ((tmp < msglen) || (tmp < fds_size)) {
-			/* overflow */
-			return EINVAL;
-		}
-		msglen = tmp;
-	}
-
+#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
 	for (i = 0; i < num_fds; i++) {
 		fds_copy[i] = -1;
 	}
@@ -529,6 +525,7 @@ static int queue_msg(struct unix_dgram_send_queue *q,
 			goto fail;
 		}
 	}
+#endif
 
 	msg = malloc(msglen);
 	if (msg == NULL) {
@@ -537,30 +534,55 @@ 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);
 
+#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;
+	}
+#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;
+	}
+#endif /*  HAVE_STRUCT_MSGHDR_MSG_CONTROL */
+
 	for (i=0; i<iovlen; i++) {
 		memcpy(data_buf, iov[i].iov_base, iov[i].iov_len);
 		data_buf += iov[i].iov_len;
 	}
 
-	msg->num_fds = num_fds;
-	if (msg->num_fds > 0) {
-		void *fds_ptr;
-		data_buf += fds_padding;
-		fds_ptr= (void *)data_buf;
-		memcpy(fds_ptr, fds_copy, fds_size);
-		msg->fds = (int *)fds_ptr;
-	} else {
-		msg->fds = NULL;
-	}
-
 	DLIST_ADD_END(q->msgs, msg, struct unix_dgram_msg);
 	return 0;
 
@@ -572,36 +594,9 @@ fail:
 static void unix_dgram_send_job(void *private_data)
 {
 	struct unix_dgram_msg *dmsg = private_data;
-	struct msghdr msg = {
-		.msg_iov = &dmsg->iov,
-		.msg_iovlen = 1,
-	};
-#ifdef HAVE_STRUCT_MSGHDR_MSG_CONTROL
-	struct cmsghdr *cmsg;
-	size_t fds_size = sizeof(int) * dmsg->num_fds;
-	size_t cmsg_len = CMSG_LEN(fds_size);
-	size_t cmsg_space = CMSG_SPACE(fds_size);
-	char cmsg_buf[cmsg_space];
-
-	if (dmsg->num_fds > 0) {
-		void *fdptr;
-
-		memset(cmsg_buf, 0, cmsg_space);
-
-		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, dmsg->fds, fds_size);
-		msg.msg_controllen = cmsg->cmsg_len;
-	}
-#endif /*  HAVE_STRUCT_MSGHDR_MSG_CONTROL */
 
 	do {
-		dmsg->sent = sendmsg(dmsg->sock, &msg, 0);
+		dmsg->sent = sendmsg(dmsg->sock, &dmsg->msg, 0);
 	} while ((dmsg->sent == -1) && (errno == EINTR));
 
 	if (dmsg->sent == -1) {
@@ -635,7 +630,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(msg->fds, msg->num_fds);
+	close_fd_array_cmsg(&msg->msg);
 	free(msg);
 
 	if (q->msgs != NULL) {