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

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(-)

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
>

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

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