[PATCH 1/4] hurd: Don't pass fd flags in CMSG_DATA

[PATCH 1/4] hurd: Don't pass fd flags in CMSG_DATA

[Sergey Bugaev]

The only valid flag defined here is FD_CLOEXEC. It is of no concern to
the receiving process whether or not the sender process wants to close
its copy of sent file descriptor upon exec, and it should not influence
whether or not the received file descriptor gets the FD_CLOEXEC flag
set in the receiving process.

The latter should in fact be dependent on the MSG_CMSG_CLOEXEC flag
being passed to the recvmsg () call, which is going to be implemented
in the following commit.

Fixes 344e755248ce02c0f8d095d11cc49e340703d926
"hurd: Support sending file descriptors over Unix sockets"

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/recvmsg.c | 3 +--
 sysdeps/mach/hurd/sendmsg.c | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/sysdeps/mach/hurd/recvmsg.c b/sysdeps/mach/hurd/recvmsg.c
index 39de86f6..5e8b18c6 100644
--- a/sysdeps/mach/hurd/recvmsg.c
+++ b/sysdeps/mach/hurd/recvmsg.c
@@ -189,7 +189,6 @@ __libc_recvmsg (int fd, struct msghdr *message, int flags)
     if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS)
       {
 	/* SCM_RIGHTS support.  */
-	/* The fd's flags are passed in the control data.  */
 	int *fds = (int *) CMSG_DATA (cmsg);
 	nfds = (cmsg->cmsg_len - CMSG_ALIGN (sizeof (struct cmsghdr)))
 	       / sizeof (int);
@@ -200,7 +199,7 @@ __libc_recvmsg (int fd, struct msghdr *message, int flags)
 	    if (err)
 	      goto cleanup;
 	    fds[j] = opened_fds[newfds] = _hurd_intern_fd (newports[newfds],
-							   fds[j], 0);
+							   0, 0);
 	    if (fds[j] == -1)
 	      {
 		err = errno;
diff --git a/sysdeps/mach/hurd/sendmsg.c b/sysdeps/mach/hurd/sendmsg.c
index 5871d1d8..77a720fb 100644
--- a/sysdeps/mach/hurd/sendmsg.c
+++ b/sysdeps/mach/hurd/sendmsg.c
@@ -138,8 +138,8 @@ __libc_sendmsg (int fd, const struct msghdr *message, int flags)
 					     0, 0, 0, 0);
 		   if (! err)
 		     nports++;
-		   /* We pass the flags in the control data.  */
-		   fds[i] = descriptor->flags;
+		   /* We just pass 0 in the control data.  */
+		   fds[i] = 0;
 		   err;
 		 }));
 
-- 
2.39.2

[PATCH 2/4] hurd: Implement MSG_CMSG_CLOEXEC

[Sergey Bugaev]

This is a new flag that can be passed to recvmsg () to make it
atomically set the CLOEXEC flag on all the file descriptors received
using the SCM_RIGHTS mechanism. This is useful for all the same reasons
that the other XXX_CLOEXEC flags are useful: namely, it provides
atomicity with respect to another thread of the same process calling
(fork and then) exec at the same time.

This flag is already supported on Linux and FreeBSD. The flag's value,
0x40000, is choosen to match FreeBSD's.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/bits/socket.h | 5 ++++-
 sysdeps/mach/hurd/recvmsg.c     | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h
index 2d78a916..c2392bed 100644
--- a/sysdeps/mach/hurd/bits/socket.h
+++ b/sysdeps/mach/hurd/bits/socket.h
@@ -197,8 +197,11 @@ enum
 #define MSG_WAITALL MSG_WAITALL
     MSG_DONTWAIT	= 0x80,	/* This message should be nonblocking.  */
 #define MSG_DONTWAIT MSG_DONTWAIT
-    MSG_NOSIGNAL	= 0x0400	/* Do not generate SIGPIPE on EPIPE.  */
+    MSG_NOSIGNAL	= 0x0400,	/* Do not generate SIGPIPE on EPIPE.  */
 #define MSG_NOSIGNAL MSG_NOSIGNAL
+    MSG_CMSG_CLOEXEC	= 0x40000	/* Atomically set close-on-exec flag
+					   for file descriptors in SCM_RIGHTS.  */
+#define MSG_CMSG_CLOEXEC MSG_CMSG_CLOEXEC
   };
 
 
diff --git a/sysdeps/mach/hurd/recvmsg.c b/sysdeps/mach/hurd/recvmsg.c
index 5e8b18c6..56c163fe 100644
--- a/sysdeps/mach/hurd/recvmsg.c
+++ b/sysdeps/mach/hurd/recvmsg.c
@@ -195,11 +195,12 @@ __libc_recvmsg (int fd, struct msghdr *message, int flags)
 
 	for (j = 0; j < nfds; j++)
 	  {
+	    int fd_flags = (flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0;
 	    err = reauthenticate (ports[i], &newports[newfds]);
 	    if (err)
 	      goto cleanup;
 	    fds[j] = opened_fds[newfds] = _hurd_intern_fd (newports[newfds],
-							   0, 0);
+							   fd_flags, 0);
 	    if (fds[j] == -1)
 	      {
 		err = errno;
-- 
2.39.2

[PATCH 3/4] hurd: Only deallocate addrport when it's valid

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/recv.c     | 3 ++-
 sysdeps/mach/hurd/recvfrom.c | 3 ++-
 sysdeps/mach/hurd/recvmsg.c  | 3 ++-
 sysdeps/mach/hurd/sendmsg.c  | 5 +++--
 sysdeps/mach/hurd/sendto.c   | 2 +-
 5 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/sysdeps/mach/hurd/recv.c b/sysdeps/mach/hurd/recv.c
index 3bd5c16f..1783e38d 100644
--- a/sysdeps/mach/hurd/recv.c
+++ b/sysdeps/mach/hurd/recv.c
@@ -54,7 +54,8 @@ __recv (int fd, void *buf, size_t n, int flags)
   if (err)
     return __hurd_sockfail (fd, flags, err);
 
-  __mach_port_deallocate (__mach_task_self (), addrport);
+  if (MACH_PORT_VALID (addrport))
+    __mach_port_deallocate (__mach_task_self (), addrport);
   __vm_deallocate (__mach_task_self (), (vm_address_t) cdata, clen);
 
   if (bufp != buf)
diff --git a/sysdeps/mach/hurd/recvfrom.c b/sysdeps/mach/hurd/recvfrom.c
index 1cd5f917..6f2c927a 100644
--- a/sysdeps/mach/hurd/recvfrom.c
+++ b/sysdeps/mach/hurd/recvfrom.c
@@ -94,7 +94,8 @@ __recvfrom (int fd, void *buf, size_t n, int flags, __SOCKADDR_ARG addrarg,
   else if (addr_len != NULL)
     *addr_len = 0;
 
-  __mach_port_deallocate (__mach_task_self (), addrport);
+  if (MACH_PORT_VALID (addrport))
+    __mach_port_deallocate (__mach_task_self (), addrport);
 
   /* Toss control data; we don't care.  */
   __vm_deallocate (__mach_task_self (), (vm_address_t) cdata, clen);
diff --git a/sysdeps/mach/hurd/recvmsg.c b/sysdeps/mach/hurd/recvmsg.c
index 56c163fe..2b898c72 100644
--- a/sysdeps/mach/hurd/recvmsg.c
+++ b/sysdeps/mach/hurd/recvmsg.c
@@ -135,7 +135,8 @@ __libc_recvmsg (int fd, struct msghdr *message, int flags)
   else if (message->msg_name != NULL)
     message->msg_namelen = 0;
 
-  __mach_port_deallocate (__mach_task_self (), aport);
+  if (MACH_PORT_VALID (aport))
+    __mach_port_deallocate (__mach_task_self (), aport);
 
   if (buf == data)
     buf += len;
diff --git a/sysdeps/mach/hurd/sendmsg.c b/sysdeps/mach/hurd/sendmsg.c
index 77a720fb..80eee6b1 100644
--- a/sysdeps/mach/hurd/sendmsg.c
+++ b/sysdeps/mach/hurd/sendmsg.c
@@ -195,8 +195,9 @@ __libc_sendmsg (int fd, const struct msghdr *message, int flags)
 						   message->msg_controllen,
 						   &amount);
 			      LIBC_CANCEL_RESET (cancel_oldtype);
-			      __mach_port_deallocate (__mach_task_self (),
-						      aport);
+			      if (MACH_PORT_VALID (aport))
+				__mach_port_deallocate (__mach_task_self (),
+							aport);
 			    }
 			  err;
 			}));
diff --git a/sysdeps/mach/hurd/sendto.c b/sysdeps/mach/hurd/sendto.c
index 5a960de8..777af1c4 100644
--- a/sysdeps/mach/hurd/sendto.c
+++ b/sysdeps/mach/hurd/sendto.c
@@ -94,7 +94,7 @@ __sendto (int fd,
 			  err;
 			}));
 
-  if (aport != MACH_PORT_NULL)
+  if (MACH_PORT_VALID (aport))
     __mach_port_deallocate (__mach_task_self (), aport);
 
   return err ? __hurd_sockfail (fd, flags, err) : wrote;
-- 
2.39.2

[RFC PATCH 4/4] socket: Add a test for MSG_CMSG_CLOEXEC

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---

This is an attempt to write a test, roughly based on
sysdeps/unix/sysv/linux/tst-scm_rights.c

Please tell me if this needs any changes. For instance, I opted to do
my own error checking for sendmsg/recvmsg/socketpair, but I could've
alternatively added xsendmsg/xrecvmsg/xsocketpair. This is also looser
on the coding style side, e.g. variables are defined mid-function
instead of upfront, since that's what I saw in tst-scm_rights.c; I can
get rid of that if that's not intended.

I placed this into the generic socket/ since it should also work on
Linux (though I only tested it on i686-gnu myself). Please tell me if
this would be better suited for sysdeps/mach/hurd/ instead.

 socket/Makefile           |   1 +
 socket/tst-cmsg_cloexec.c | 124 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+)
 create mode 100644 socket/tst-cmsg_cloexec.c

diff --git a/socket/Makefile b/socket/Makefile
index fffed7dd..94951ae3 100644
--- a/socket/Makefile
+++ b/socket/Makefile
@@ -35,6 +35,7 @@ tests := \
   tst-accept4 \
   tst-sockopt \
   tst-cmsghdr \
+  tst-cmsg_cloexec \
   # tests
 
 tests-internal := \
diff --git a/socket/tst-cmsg_cloexec.c b/socket/tst-cmsg_cloexec.c
new file mode 100644
index 00000000..237c3db2
--- /dev/null
+++ b/socket/tst-cmsg_cloexec.c
@@ -0,0 +1,124 @@
+/* Smoke test for MSG_CMSG_CLOEXEC.
+   Copyright (C) 2021-2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xunistd.h>
+#include <support/check.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <string.h>
+#include <fcntl.h>
+
+static void
+send_fd (int sockfd, int fd)
+{
+  /* const */ char data[] = "hello";
+  struct iovec iov = { .iov_base = data, .iov_len = sizeof (data) };
+
+  union
+  {
+    struct cmsghdr header;
+    char bytes[CMSG_SPACE (sizeof (fd))];
+  } cmsg_storage;
+
+  struct msghdr msg =
+    {
+      .msg_iov = &iov,
+      .msg_iovlen = 1,
+      .msg_control = cmsg_storage.bytes,
+      .msg_controllen = CMSG_LEN (sizeof (fd))
+    };
+
+  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
+  cmsg->cmsg_level = SOL_SOCKET;
+  cmsg->cmsg_type = SCM_RIGHTS;
+  cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
+  memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
+
+  ssize_t nsent = sendmsg (sockfd, &msg, 0);
+  if (nsent < 0)
+    FAIL_EXIT1 ("sendmsg (%d): %m", sockfd);
+  TEST_COMPARE (nsent, sizeof (data));
+}
+
+static int
+recv_fd (int sockfd, int flags)
+{
+  char buffer[100];
+  struct iovec iov = { .iov_base = buffer, .iov_len = sizeof (buffer) };
+
+  union
+  {
+    struct cmsghdr header;
+    char bytes[100];
+  } cmsg_storage;
+
+  struct msghdr msg =
+    {
+      .msg_iov = &iov,
+      .msg_iovlen = 1,
+      .msg_control = cmsg_storage.bytes,
+      .msg_controllen = sizeof (cmsg_storage)
+    };
+
+  ssize_t nrecv = recvmsg (sockfd, &msg, flags);
+  if (nrecv < 0)
+    FAIL_EXIT1 ("recvmsg (%d): %m", sockfd);
+
+  TEST_COMPARE (msg.msg_controllen, CMSG_LEN (sizeof (int)));
+  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
+  TEST_COMPARE (cmsg->cmsg_level, SOL_SOCKET);
+  TEST_COMPARE (cmsg->cmsg_type, SCM_RIGHTS);
+  TEST_COMPARE (cmsg->cmsg_len, CMSG_LEN (sizeof (int)));
+
+  int fd;
+  memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
+  return fd;
+}
+
+static int
+do_test (void)
+{
+  int sockfd[2];
+  int newfd;
+  int flags;
+  int rc = socketpair (AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0, sockfd);
+  if (rc < 0)
+    FAIL_EXIT1 ("socketpair: %m");
+
+  send_fd (sockfd[1], STDIN_FILENO);
+  newfd = recv_fd (sockfd[0], 0);
+  TEST_VERIFY_EXIT (newfd > 0);
+  flags = fcntl (newfd, F_GETFD, 0);
+  TEST_VERIFY_EXIT (flags != -1);
+  TEST_VERIFY (!(flags & FD_CLOEXEC));
+  xclose (newfd);
+
+  send_fd (sockfd[1], STDIN_FILENO);
+  newfd = recv_fd (sockfd[0], MSG_CMSG_CLOEXEC);
+  TEST_VERIFY_EXIT (newfd > 0);
+  flags = fcntl (newfd, F_GETFD, 0);
+  TEST_VERIFY_EXIT (flags != -1);
+  TEST_VERIFY (flags & FD_CLOEXEC);
+  xclose (newfd);
+
+  xclose (sockfd[0]);
+  xclose (sockfd[1]);
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.39.2

[RFC PATCH v2 4/4] socket: Add a test for MSG_CMSG_CLOEXEC

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---

And now it passes on x86_64-linux as well.

 socket/Makefile           |   1 +
 socket/tst-cmsg_cloexec.c | 124 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+)
 create mode 100644 socket/tst-cmsg_cloexec.c

diff --git a/socket/Makefile b/socket/Makefile
index fffed7dd..94951ae3 100644
--- a/socket/Makefile
+++ b/socket/Makefile
@@ -35,6 +35,7 @@ tests := \
   tst-accept4 \
   tst-sockopt \
   tst-cmsghdr \
+  tst-cmsg_cloexec \
   # tests
 
 tests-internal := \
diff --git a/socket/tst-cmsg_cloexec.c b/socket/tst-cmsg_cloexec.c
new file mode 100644
index 00000000..9351a052
--- /dev/null
+++ b/socket/tst-cmsg_cloexec.c
@@ -0,0 +1,124 @@
+/* Smoke test for MSG_CMSG_CLOEXEC.
+   Copyright (C) 2021-2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xunistd.h>
+#include <support/check.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <string.h>
+#include <fcntl.h>
+
+static void
+send_fd (int sockfd, int fd)
+{
+  /* const */ char data[] = "hello";
+  struct iovec iov = { .iov_base = data, .iov_len = sizeof (data) };
+
+  union
+  {
+    struct cmsghdr header;
+    char bytes[CMSG_SPACE (sizeof (fd))];
+  } cmsg_storage;
+
+  struct msghdr msg =
+    {
+      .msg_iov = &iov,
+      .msg_iovlen = 1,
+      .msg_control = cmsg_storage.bytes,
+      .msg_controllen = CMSG_LEN (sizeof (fd))
+    };
+
+  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
+  cmsg->cmsg_level = SOL_SOCKET;
+  cmsg->cmsg_type = SCM_RIGHTS;
+  cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
+  memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
+
+  ssize_t nsent = sendmsg (sockfd, &msg, 0);
+  if (nsent < 0)
+    FAIL_EXIT1 ("sendmsg (%d): %m", sockfd);
+  TEST_COMPARE (nsent, sizeof (data));
+}
+
+static int
+recv_fd (int sockfd, int flags)
+{
+  char buffer[100];
+  struct iovec iov = { .iov_base = buffer, .iov_len = sizeof (buffer) };
+
+  union
+  {
+    struct cmsghdr header;
+    char bytes[100];
+  } cmsg_storage;
+
+  struct msghdr msg =
+    {
+      .msg_iov = &iov,
+      .msg_iovlen = 1,
+      .msg_control = cmsg_storage.bytes,
+      .msg_controllen = sizeof (cmsg_storage)
+    };
+
+  ssize_t nrecv = recvmsg (sockfd, &msg, flags);
+  if (nrecv < 0)
+    FAIL_EXIT1 ("recvmsg (%d): %m", sockfd);
+
+  TEST_COMPARE (msg.msg_controllen, CMSG_SPACE (sizeof (int)));
+  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
+  TEST_COMPARE (cmsg->cmsg_level, SOL_SOCKET);
+  TEST_COMPARE (cmsg->cmsg_type, SCM_RIGHTS);
+  TEST_COMPARE (cmsg->cmsg_len, CMSG_LEN (sizeof (int)));
+
+  int fd;
+  memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
+  return fd;
+}
+
+static int
+do_test (void)
+{
+  int sockfd[2];
+  int newfd;
+  int flags;
+  int rc = socketpair (AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0, sockfd);
+  if (rc < 0)
+    FAIL_EXIT1 ("socketpair: %m");
+
+  send_fd (sockfd[1], STDIN_FILENO);
+  newfd = recv_fd (sockfd[0], 0);
+  TEST_VERIFY_EXIT (newfd > 0);
+  flags = fcntl (newfd, F_GETFD, 0);
+  TEST_VERIFY_EXIT (flags != -1);
+  TEST_VERIFY (!(flags & FD_CLOEXEC));
+  xclose (newfd);
+
+  send_fd (sockfd[1], STDIN_FILENO);
+  newfd = recv_fd (sockfd[0], MSG_CMSG_CLOEXEC);
+  TEST_VERIFY_EXIT (newfd > 0);
+  flags = fcntl (newfd, F_GETFD, 0);
+  TEST_VERIFY_EXIT (flags != -1);
+  TEST_VERIFY (flags & FD_CLOEXEC);
+  xclose (newfd);
+
+  xclose (sockfd[0]);
+  xclose (sockfd[1]);
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.39.2

Re: [RFC PATCH 4/4] socket: Add a test for MSG_CMSG_CLOEXEC

[Adhemerval Zanella Netto]

On 17/04/23 10:39, Sergey Bugaev via Libc-alpha wrote:
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>

Add some description of what the test is accomplishing here.
Some small nits below, the patch LGTM.

> ---
> 
> This is an attempt to write a test, roughly based on
> sysdeps/unix/sysv/linux/tst-scm_rights.c
> 
> Please tell me if this needs any changes. For instance, I opted to do
> my own error checking for sendmsg/recvmsg/socketpair, but I could've
> alternatively added xsendmsg/xrecvmsg/xsocketpair. This is also looser
> on the coding style side, e.g. variables are defined mid-function
> instead of upfront, since that's what I saw in tst-scm_rights.c; I can
> get rid of that if that's not intended.
> 
> I placed this into the generic socket/ since it should also work on
> Linux (though I only tested it on i686-gnu myself). Please tell me if
> this would be better suited for sysdeps/mach/hurd/ instead.
> 
>  socket/Makefile           |   1 +
>  socket/tst-cmsg_cloexec.c | 124 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 125 insertions(+)
>  create mode 100644 socket/tst-cmsg_cloexec.c
> 
> diff --git a/socket/Makefile b/socket/Makefile
> index fffed7dd..94951ae3 100644
> --- a/socket/Makefile
> +++ b/socket/Makefile
> @@ -35,6 +35,7 @@ tests := \
>    tst-accept4 \
>    tst-sockopt \
>    tst-cmsghdr \
> +  tst-cmsg_cloexec \
>    # tests
>  
>  tests-internal := \
> diff --git a/socket/tst-cmsg_cloexec.c b/socket/tst-cmsg_cloexec.c
> new file mode 100644
> index 00000000..237c3db2
> --- /dev/null
> +++ b/socket/tst-cmsg_cloexec.c
> @@ -0,0 +1,124 @@
> +/* Smoke test for MSG_CMSG_CLOEXEC.
> +   Copyright (C) 2021-2023 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <string.h>
> +#include <fcntl.h>
> +
> +static void
> +send_fd (int sockfd, int fd)
> +{
> +  /* const */ char data[] = "hello";

Just remove the 'const' comment here.

> +  struct iovec iov = { .iov_base = data, .iov_len = sizeof (data) };
> +
> +  union
> +  {
> +    struct cmsghdr header;
> +    char bytes[CMSG_SPACE (sizeof (fd))];
> +  } cmsg_storage;
> +
> +  struct msghdr msg =
> +    {
> +      .msg_iov = &iov,
> +      .msg_iovlen = 1,
> +      .msg_control = cmsg_storage.bytes,
> +      .msg_controllen = CMSG_LEN (sizeof (fd))
> +    };
> +
> +  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
> +  cmsg->cmsg_level = SOL_SOCKET;
> +  cmsg->cmsg_type = SCM_RIGHTS;
> +  cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
> +  memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
> +
> +  ssize_t nsent = sendmsg (sockfd, &msg, 0);
> +  if (nsent < 0)
> +    FAIL_EXIT1 ("sendmsg (%d): %m", sockfd);
> +  TEST_COMPARE (nsent, sizeof (data));
> +}
> +
> +static int
> +recv_fd (int sockfd, int flags)
> +{
> +  char buffer[100];
> +  struct iovec iov = { .iov_base = buffer, .iov_len = sizeof (buffer) };
> +
> +  union
> +  {
> +    struct cmsghdr header;
> +    char bytes[100];
> +  } cmsg_storage;
> +
> +  struct msghdr msg =
> +    {
> +      .msg_iov = &iov,
> +      .msg_iovlen = 1,
> +      .msg_control = cmsg_storage.bytes,
> +      .msg_controllen = sizeof (cmsg_storage)
> +    };
> +
> +  ssize_t nrecv = recvmsg (sockfd, &msg, flags);
> +  if (nrecv < 0)
> +    FAIL_EXIT1 ("recvmsg (%d): %m", sockfd);
> +
> +  TEST_COMPARE (msg.msg_controllen, CMSG_LEN (sizeof (int)));
> +  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
> +  TEST_COMPARE (cmsg->cmsg_level, SOL_SOCKET);
> +  TEST_COMPARE (cmsg->cmsg_type, SCM_RIGHTS);
> +  TEST_COMPARE (cmsg->cmsg_len, CMSG_LEN (sizeof (int)));
> +
> +  int fd;
> +  memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
> +  return fd;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  int sockfd[2];
> +  int newfd;
> +  int flags;
> +  int rc = socketpair (AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0, sockfd);
> +  if (rc < 0)
> +    FAIL_EXIT1 ("socketpair: %m");
> +
> +  send_fd (sockfd[1], STDIN_FILENO);
> +  newfd = recv_fd (sockfd[0], 0);
> +  TEST_VERIFY_EXIT (newfd > 0);
> +  flags = fcntl (newfd, F_GETFD, 0);
> +  TEST_VERIFY_EXIT (flags != -1);
> +  TEST_VERIFY (!(flags & FD_CLOEXEC));
> +  xclose (newfd);
> +
> +  send_fd (sockfd[1], STDIN_FILENO);
> +  newfd = recv_fd (sockfd[0], MSG_CMSG_CLOEXEC);
> +  TEST_VERIFY_EXIT (newfd > 0);
> +  flags = fcntl (newfd, F_GETFD, 0);
> +  TEST_VERIFY_EXIT (flags != -1);
> +  TEST_VERIFY (flags & FD_CLOEXEC);
> +  xclose (newfd);
> +
> +  xclose (sockfd[0]);
> +  xclose (sockfd[1]);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Re: [RFC PATCH 4/4] socket: Add a test for MSG_CMSG_CLOEXEC

[Sergey Bugaev]

Hello,

On Tue, Apr 18, 2023 at 3:13 PM Adhemerval Zanella Netto wrote:
> On 17/04/23 10:39, Sergey Bugaev via Libc-alpha wrote:
> > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
>
> Add some description of what the test is accomplishing here.

Do you mean in the commit message? Done.

Also changed:
* in send_fd, memset the cmsg_storage to zero before filling it
* in send_fd, set msg_controllen to sizeof (cmsg_storage) (aka
  CMSG_SPACE (sizeof (fd))), not CMSG_LEN (sizeof (fd))
* dropped the /* const */

And here's the test passing on x86_64-linux:

socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, [3, 4]) = 0
sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=6}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[0]}], msg_controllen=24, msg_flags=0}, 0) = 6
recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=100}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[5]}], msg_controllen=24, msg_flags=0}, 0) = 6
fcntl(5, F_GETFD)                       = 0
close(5)                                = 0
sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=6}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[0]}], msg_controllen=24, msg_flags=0}, 0) = 6
recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=100}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[5]}], msg_controllen=24, msg_flags=MSG_CMSG_CLOEXEC}, MSG_CMSG_CLOEXEC) = 6
fcntl(5, F_GETFD)                       = 0x1 (flags FD_CLOEXEC)
close(5)                                = 0
close(3)                                = 0
close(4)                                = 0

Sergey

---- >8 ----

From 256a774a16e1bb3d721e2be82dc4bbab119bf6ea Mon Sep 17 00:00:00 2001
From: Sergey Bugaev <bugaevc@gmail.com>
Date: Mon, 17 Apr 2023 16:28:35 +0300
Subject: [RFC PATCH v3 4/4] socket: Add a test for MSG_CMSG_CLOEXEC

This checks that:
* We can send and receive fds over Unix domain sockets using SCM_RIGHTS;
* msg_controllen, cmsg_level, cmsg_type, cmsg_len are all filled in
  correctly on receive;
* Most importantly, the received fd has or has not the close-on-exec
  flag set depending on whether we pass MSG_CMSG_CLOEXEC to recvmsg ().

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 socket/Makefile           |   1 +
 socket/tst-cmsg_cloexec.c | 126 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)
 create mode 100644 socket/tst-cmsg_cloexec.c

diff --git a/socket/Makefile b/socket/Makefile
index fffed7dd..94951ae3 100644
--- a/socket/Makefile
+++ b/socket/Makefile
@@ -35,6 +35,7 @@ tests := \
   tst-accept4 \
   tst-sockopt \
   tst-cmsghdr \
+  tst-cmsg_cloexec \
   # tests
 
 tests-internal := \
diff --git a/socket/tst-cmsg_cloexec.c b/socket/tst-cmsg_cloexec.c
new file mode 100644
index 00000000..a18f4912
--- /dev/null
+++ b/socket/tst-cmsg_cloexec.c
@@ -0,0 +1,126 @@
+/* Smoke test for MSG_CMSG_CLOEXEC.
+   Copyright (C) 2021-2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xunistd.h>
+#include <support/check.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <string.h>
+#include <fcntl.h>
+
+static void
+send_fd (int sockfd, int fd)
+{
+  char data[] = "hello";
+  struct iovec iov = { .iov_base = data, .iov_len = sizeof (data) };
+
+  union
+  {
+    struct cmsghdr header;
+    char bytes[CMSG_SPACE (sizeof (fd))];
+  } cmsg_storage;
+
+  struct msghdr msg =
+    {
+      .msg_iov = &iov,
+      .msg_iovlen = 1,
+      .msg_control = cmsg_storage.bytes,
+      .msg_controllen = sizeof (cmsg_storage)
+    };
+
+  memset (&cmsg_storage, 0, sizeof (cmsg_storage));
+
+  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
+  cmsg->cmsg_level = SOL_SOCKET;
+  cmsg->cmsg_type = SCM_RIGHTS;
+  cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
+  memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
+
+  ssize_t nsent = sendmsg (sockfd, &msg, 0);
+  if (nsent < 0)
+    FAIL_EXIT1 ("sendmsg (%d): %m", sockfd);
+  TEST_COMPARE (nsent, sizeof (data));
+}
+
+static int
+recv_fd (int sockfd, int flags)
+{
+  char buffer[100];
+  struct iovec iov = { .iov_base = buffer, .iov_len = sizeof (buffer) };
+
+  union
+  {
+    struct cmsghdr header;
+    char bytes[100];
+  } cmsg_storage;
+
+  struct msghdr msg =
+    {
+      .msg_iov = &iov,
+      .msg_iovlen = 1,
+      .msg_control = cmsg_storage.bytes,
+      .msg_controllen = sizeof (cmsg_storage)
+    };
+
+  ssize_t nrecv = recvmsg (sockfd, &msg, flags);
+  if (nrecv < 0)
+    FAIL_EXIT1 ("recvmsg (%d): %m", sockfd);
+
+  TEST_COMPARE (msg.msg_controllen, CMSG_SPACE (sizeof (int)));
+  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
+  TEST_COMPARE (cmsg->cmsg_level, SOL_SOCKET);
+  TEST_COMPARE (cmsg->cmsg_type, SCM_RIGHTS);
+  TEST_COMPARE (cmsg->cmsg_len, CMSG_LEN (sizeof (int)));
+
+  int fd;
+  memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
+  return fd;
+}
+
+static int
+do_test (void)
+{
+  int sockfd[2];
+  int newfd;
+  int flags;
+  int rc = socketpair (AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0, sockfd);
+  if (rc < 0)
+    FAIL_EXIT1 ("socketpair: %m");
+
+  send_fd (sockfd[1], STDIN_FILENO);
+  newfd = recv_fd (sockfd[0], 0);
+  TEST_VERIFY_EXIT (newfd > 0);
+  flags = fcntl (newfd, F_GETFD, 0);
+  TEST_VERIFY_EXIT (flags != -1);
+  TEST_VERIFY (!(flags & FD_CLOEXEC));
+  xclose (newfd);
+
+  send_fd (sockfd[1], STDIN_FILENO);
+  newfd = recv_fd (sockfd[0], MSG_CMSG_CLOEXEC);
+  TEST_VERIFY_EXIT (newfd > 0);
+  flags = fcntl (newfd, F_GETFD, 0);
+  TEST_VERIFY_EXIT (flags != -1);
+  TEST_VERIFY (flags & FD_CLOEXEC);
+  xclose (newfd);
+
+  xclose (sockfd[0]);
+  xclose (sockfd[1]);
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.39.2

Re: [RFC PATCH 4/4] socket: Add a test for MSG_CMSG_CLOEXEC

[Adhemerval Zanella Netto]

On 18/04/23 12:37, Sergey Bugaev wrote:
> Hello,
> 
> On Tue, Apr 18, 2023 at 3:13 PM Adhemerval Zanella Netto wrote:
>> On 17/04/23 10:39, Sergey Bugaev via Libc-alpha wrote:
>>> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
>>
>> Add some description of what the test is accomplishing here.
> 
> Do you mean in the commit message? Done.
> 
> Also changed:
> * in send_fd, memset the cmsg_storage to zero before filling it
> * in send_fd, set msg_controllen to sizeof (cmsg_storage) (aka
>   CMSG_SPACE (sizeof (fd))), not CMSG_LEN (sizeof (fd))
> * dropped the /* const */
> 
> And here's the test passing on x86_64-linux:
> 
> socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, [3, 4]) = 0
> sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=6}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[0]}], msg_controllen=24, msg_flags=0}, 0) = 6
> recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=100}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[5]}], msg_controllen=24, msg_flags=0}, 0) = 6
> fcntl(5, F_GETFD)                       = 0
> close(5)                                = 0
> sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=6}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[0]}], msg_controllen=24, msg_flags=0}, 0) = 6
> recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="hello\0", iov_len=100}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[5]}], msg_controllen=24, msg_flags=MSG_CMSG_CLOEXEC}, MSG_CMSG_CLOEXEC) = 6
> fcntl(5, F_GETFD)                       = 0x1 (flags FD_CLOEXEC)
> close(5)                                = 0
> close(3)                                = 0
> close(4)                                = 0
> 
> Sergey
> 
> ---- >8 ----
> 
> From 256a774a16e1bb3d721e2be82dc4bbab119bf6ea Mon Sep 17 00:00:00 2001
> From: Sergey Bugaev <bugaevc@gmail.com>
> Date: Mon, 17 Apr 2023 16:28:35 +0300
> Subject: [RFC PATCH v3 4/4] socket: Add a test for MSG_CMSG_CLOEXEC
> 
> This checks that:
> * We can send and receive fds over Unix domain sockets using SCM_RIGHTS;
> * msg_controllen, cmsg_level, cmsg_type, cmsg_len are all filled in
>   correctly on receive;
> * Most importantly, the received fd has or has not the close-on-exec
>   flag set depending on whether we pass MSG_CMSG_CLOEXEC to recvmsg ().
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  socket/Makefile           |   1 +
>  socket/tst-cmsg_cloexec.c | 126 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 127 insertions(+)
>  create mode 100644 socket/tst-cmsg_cloexec.c
> 
> diff --git a/socket/Makefile b/socket/Makefile
> index fffed7dd..94951ae3 100644
> --- a/socket/Makefile
> +++ b/socket/Makefile
> @@ -35,6 +35,7 @@ tests := \
>    tst-accept4 \
>    tst-sockopt \
>    tst-cmsghdr \
> +  tst-cmsg_cloexec \
>    # tests
>  
>  tests-internal := \
> diff --git a/socket/tst-cmsg_cloexec.c b/socket/tst-cmsg_cloexec.c
> new file mode 100644
> index 00000000..a18f4912
> --- /dev/null
> +++ b/socket/tst-cmsg_cloexec.c
> @@ -0,0 +1,126 @@
> +/* Smoke test for MSG_CMSG_CLOEXEC.
> +   Copyright (C) 2021-2023 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <string.h>
> +#include <fcntl.h>
> +
> +static void
> +send_fd (int sockfd, int fd)
> +{
> +  char data[] = "hello";
> +  struct iovec iov = { .iov_base = data, .iov_len = sizeof (data) };
> +
> +  union
> +  {
> +    struct cmsghdr header;
> +    char bytes[CMSG_SPACE (sizeof (fd))];
> +  } cmsg_storage;
> +
> +  struct msghdr msg =
> +    {
> +      .msg_iov = &iov,
> +      .msg_iovlen = 1,
> +      .msg_control = cmsg_storage.bytes,
> +      .msg_controllen = sizeof (cmsg_storage)
> +    };
> +
> +  memset (&cmsg_storage, 0, sizeof (cmsg_storage));
> +
> +  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
> +  cmsg->cmsg_level = SOL_SOCKET;
> +  cmsg->cmsg_type = SCM_RIGHTS;
> +  cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
> +  memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
> +
> +  ssize_t nsent = sendmsg (sockfd, &msg, 0);
> +  if (nsent < 0)
> +    FAIL_EXIT1 ("sendmsg (%d): %m", sockfd);
> +  TEST_COMPARE (nsent, sizeof (data));
> +}
> +
> +static int
> +recv_fd (int sockfd, int flags)
> +{
> +  char buffer[100];
> +  struct iovec iov = { .iov_base = buffer, .iov_len = sizeof (buffer) };
> +
> +  union
> +  {
> +    struct cmsghdr header;
> +    char bytes[100];
> +  } cmsg_storage;
> +
> +  struct msghdr msg =
> +    {
> +      .msg_iov = &iov,
> +      .msg_iovlen = 1,
> +      .msg_control = cmsg_storage.bytes,
> +      .msg_controllen = sizeof (cmsg_storage)
> +    };
> +
> +  ssize_t nrecv = recvmsg (sockfd, &msg, flags);
> +  if (nrecv < 0)
> +    FAIL_EXIT1 ("recvmsg (%d): %m", sockfd);
> +
> +  TEST_COMPARE (msg.msg_controllen, CMSG_SPACE (sizeof (int)));
> +  struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msg);
> +  TEST_COMPARE (cmsg->cmsg_level, SOL_SOCKET);
> +  TEST_COMPARE (cmsg->cmsg_type, SCM_RIGHTS);
> +  TEST_COMPARE (cmsg->cmsg_len, CMSG_LEN (sizeof (int)));
> +
> +  int fd;
> +  memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
> +  return fd;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  int sockfd[2];
> +  int newfd;
> +  int flags;
> +  int rc = socketpair (AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0, sockfd);
> +  if (rc < 0)
> +    FAIL_EXIT1 ("socketpair: %m");
> +
> +  send_fd (sockfd[1], STDIN_FILENO);
> +  newfd = recv_fd (sockfd[0], 0);
> +  TEST_VERIFY_EXIT (newfd > 0);
> +  flags = fcntl (newfd, F_GETFD, 0);
> +  TEST_VERIFY_EXIT (flags != -1);
> +  TEST_VERIFY (!(flags & FD_CLOEXEC));
> +  xclose (newfd);
> +
> +  send_fd (sockfd[1], STDIN_FILENO);
> +  newfd = recv_fd (sockfd[0], MSG_CMSG_CLOEXEC);
> +  TEST_VERIFY_EXIT (newfd > 0);
> +  flags = fcntl (newfd, F_GETFD, 0);
> +  TEST_VERIFY_EXIT (flags != -1);
> +  TEST_VERIFY (flags & FD_CLOEXEC);
> +  xclose (newfd);
> +
> +  xclose (sockfd[0]);
> +  xclose (sockfd[1]);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Re: [PATCH 1/4] hurd: Don't pass fd flags in CMSG_DATA

[Samuel Thibault]

Sergey Bugaev, le lun. 17 avril 2023 16:38:59 +0300, a ecrit:
> The only valid flag defined here is FD_CLOEXEC. It is of no concern to
> the receiving process whether or not the sender process wants to close
> its copy of sent file descriptor upon exec,

Ok, but couldn't there be some flags that we could want to transfer, in
the future? I'd better keep the infrastructure, even if it is not
actually useful for now. So that people who end up needing something see
that passing it is already supported.

Samuel

> and it should not influence
> whether or not the received file descriptor gets the FD_CLOEXEC flag
> set in the receiving process.
> 
> The latter should in fact be dependent on the MSG_CMSG_CLOEXEC flag
> being passed to the recvmsg () call, which is going to be implemented
> in the following commit.
> 
> Fixes 344e755248ce02c0f8d095d11cc49e340703d926
> "hurd: Support sending file descriptors over Unix sockets"
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/recvmsg.c | 3 +--
>  sysdeps/mach/hurd/sendmsg.c | 4 ++--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/recvmsg.c b/sysdeps/mach/hurd/recvmsg.c
> index 39de86f6..5e8b18c6 100644
> --- a/sysdeps/mach/hurd/recvmsg.c
> +++ b/sysdeps/mach/hurd/recvmsg.c
> @@ -189,7 +189,6 @@ __libc_recvmsg (int fd, struct msghdr *message, int flags)
>      if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS)
>        {
>  	/* SCM_RIGHTS support.  */
> -	/* The fd's flags are passed in the control data.  */
>  	int *fds = (int *) CMSG_DATA (cmsg);
>  	nfds = (cmsg->cmsg_len - CMSG_ALIGN (sizeof (struct cmsghdr)))
>  	       / sizeof (int);
> @@ -200,7 +199,7 @@ __libc_recvmsg (int fd, struct msghdr *message, int flags)
>  	    if (err)
>  	      goto cleanup;
>  	    fds[j] = opened_fds[newfds] = _hurd_intern_fd (newports[newfds],
> -							   fds[j], 0);
> +							   0, 0);
>  	    if (fds[j] == -1)
>  	      {
>  		err = errno;
> diff --git a/sysdeps/mach/hurd/sendmsg.c b/sysdeps/mach/hurd/sendmsg.c
> index 5871d1d8..77a720fb 100644
> --- a/sysdeps/mach/hurd/sendmsg.c
> +++ b/sysdeps/mach/hurd/sendmsg.c
> @@ -138,8 +138,8 @@ __libc_sendmsg (int fd, const struct msghdr *message, int flags)
>  					     0, 0, 0, 0);
>  		   if (! err)
>  		     nports++;
> -		   /* We pass the flags in the control data.  */
> -		   fds[i] = descriptor->flags;
> +		   /* We just pass 0 in the control data.  */
> +		   fds[i] = 0;
>  		   err;
>  		 }));
>  
> -- 
> 2.39.2
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

Re: [PATCH 1/4] hurd: Don't pass fd flags in CMSG_DATA

[Sergey Bugaev]

On Fri, Apr 21, 2023 at 12:14 AM Samuel Thibault
<samuel.thibault@gnu.org> wrote:
> Sergey Bugaev, le lun. 17 avril 2023 16:38:59 +0300, a ecrit:
> > The only valid flag defined here is FD_CLOEXEC. It is of no concern to
> > the receiving process whether or not the sender process wants to close
> > its copy of sent file descriptor upon exec,
>
> Ok, but couldn't there be some flags that we could want to transfer, in
> the future?

Unlikely -- it's been years (I don't know how old FD_CLOEXEC is
exactly, but it surely predates O_CLOEXEC by many years) and AFAIK
nobody came up with any ideas for more fd flags, other than
FD_CLOFORK, but that wouldn't be transferable either.

And the whole idea of fd flags (as opposed to flags applied to the
open file itself, the peropen in Hurd terms, like O_NONBLOCK and
O_ASYNC) is that they apply to that single file descriptor, and are
not carried over when it's dup'ed. sendmsg+recvmsg is like a remote
dup in this sense.

> I'd better keep the infrastructure, even if it is not
> actually useful for now. So that people who end up needing something see
> that passing it is already supported.

I understand, but also it's not like there's a lot of infrastructure
that I'm removing here. You could think of it that way: the
infrastructure for passing an integer value along with the port is
still there, but currently no valid flags for it are defined, and so 0
is always used. We could spell it as

fds[i] = descriptor->flags & ~FD_CLOEXEC;

if you would prefer; but that would still always evaluate to 0 (but
the compiler wouldn't be aware and would generate the extra
instructions).

Sergey

Re: [PATCH 1/4] hurd: Don't pass fd flags in CMSG_DATA

[Samuel Thibault]

Sergey Bugaev, le ven. 21 avril 2023 00:47:43 +0300, a ecrit:
> You could think of it that way: the
> infrastructure for passing an integer value along with the port is
> still there, but currently no valid flags for it are defined, and so 0
> is always used. We could spell it as
> 
> fds[i] = descriptor->flags & ~FD_CLOEXEC;
> 
> if you would prefer;

Yes, I'd prefer.

> but that would still always evaluate to 0 (but the compiler wouldn't
> be aware and would generate the extra instructions).

Yes, sure, but that still looks preferable to me maintenance-wise.

Samuel