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>

diff --git a/source3/lib/unix_msg/unix_msg.c b/source3/lib/unix_msg/unix_msg.c
index ca7f85dea398b06d451c44780c4462a7f4714420..be46a43e1089644b228f83f94c8c9b0706569f90 100644 (file)
--- 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) {