Re: [PATCH 1/3] treewide: Add SOCK_CLOEXEC to accept() calls that are missing it

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 13 May 2026 14:14:21 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Generally we try to set the O_CLOEXEC flag on every fd we create.  This
> seems to be generally accepted security best practice these days, and we
> never fork(), so certainly have no need to pass fds to children.

But we do clone() with CLONE_FILES (even though when we clone() to call
execvp() later, we don't set CLONE_FILES), so, even though I don't see
a reason to skip O_CLOEXEC for c->fd_tap, this conclusion shouldn't be
automatic from the fact we don't fork().

I spent some time on it and I really couldn't find a reason why we
don't have O_CLOEXEC there, so probably there isn't any, and I think
this patch is fine.

I would just change this paragraph to "[...] these days, and we don't
need to pass file descriptors to children."

> A handful of accept4() calls on Unix sockets are missing the SOCK_CLOEXEC
> flag to set this though.  Add the missing flag.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  repair.c | 5 +++--
>  tap.c    | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/repair.c b/repair.c
> index 69c53077..3e0e3e0a 100644
> --- a/repair.c
> +++ b/repair.c
> @@ -87,7 +87,7 @@ int repair_listen_handler(struct ctx *c, uint32_t events)
>  	/* Another client is already connected: accept and close right away. */
>  	if (c->fd_repair != -1) {
>  		int discard = accept4(c->fd_repair_listen, NULL, NULL,
> -				      SOCK_NONBLOCK);
> +				      SOCK_NONBLOCK | SOCK_CLOEXEC);
>  
>  		if (discard == -1)
>  			return errno;
> @@ -99,7 +99,8 @@ int repair_listen_handler(struct ctx *c, uint32_t events)
>  		return EEXIST;
>  	}
>  
> -	if ((c->fd_repair = accept4(c->fd_repair_listen, NULL, NULL, 0)) < 0) {
> +	if ((c->fd_repair = accept4(c->fd_repair_listen, NULL, NULL,
> +				    SOCK_CLOEXEC)) < 0) {
>  		rc = errno;
>  		debug_perror("accept4() on TCP_REPAIR helper listening socket");
>  		return rc;
> diff --git a/tap.c b/tap.c
> index 0920a325..e7cac9df 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1477,7 +1477,7 @@ void tap_listen_handler(struct ctx *c, uint32_t events)
>  	/* Another client is already connected: accept and close right away. */
>  	if (c->fd_tap != -1) {
>  		int discard = accept4(c->fd_tap_listen, NULL, NULL,
> -				      SOCK_NONBLOCK);
> +				      SOCK_NONBLOCK | SOCK_CLOEXEC);
>  
>  		if (discard == -1)
>  			return;
> @@ -1490,7 +1490,7 @@ void tap_listen_handler(struct ctx *c, uint32_t events)
>  		return;
>  	}
>  
> -	c->fd_tap = accept4(c->fd_tap_listen, NULL, NULL, 0);
> +	c->fd_tap = accept4(c->fd_tap_listen, NULL, NULL, SOCK_CLOEXEC);
>  
>  	if (!getsockopt(c->fd_tap, SOL_SOCKET, SO_PEERCRED, &ucred, &len))
>  		info("accepted connection from PID %i", ucred.pid);

-- 
Stefano