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

On Wed, May 20, 2026 at 01:36:48PM +0200, Stefano Brivio wrote:

On Wed, 20 May 2026 11:04:58 +1000 David Gibson wrote:

...

On Wed, May 20, 2026 at 02:37:02AM +0200, Stefano Brivio wrote:

...

On Mon, 18 May 2026 12:28:57 +1000 David Gibson wrote:

...

On Sat, May 16, 2026 at 05:46:11PM +0200, Stefano Brivio wrote:

...

On Wed, 13 May 2026 14:14:21 +1000 David Gibson 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().

So, I did think about that when wrote it, but went for the short version rather than saying clone() with CLONE_FILES doesn't count.

Now, I realised that we've both fallen for the trap again, forgetting that this has nothing to do with fork() or clone() and is, as it says right there in the name, about exec().

No, wait, I didn't fall for it, not this time. :) That's why I was mentioning that when we call clone() and execvp() later (which would be

Uh...? I'm pretty sure the only execve(2) in the entire program is where we spawn passt.avx2. That's essentially the very first thing we do, long before this point.

Well, grep would beg to differ, as we don't call execve() at all, but:

I meant the system call execve(2), which execv() and execvp() are library wrappers around.

$ grep execv *.c | grep -v qrap arch.c: execv(new_path, argv); pasta.c: execvp(a->exe, a->argv);

Ah, I did miss the one in pasta_spawn_cmd(). Of course, we definitely don't want to leak our internal fds into the spawned command, so CLOEXEC is what we want.

O_CLOEXEC (or lack thereof) also matters on execvp().

...

...

the only path that matters), we don't set CLONE_FILES anyway.

CLONE_FILES is irrelevant, it's lost during execve(2).

Yes, but if you first clone(), which we actually do before calling pasta_spawn_cmd(), and then execvp(), CLONE_FILES on clone() *would* matter, because the cloned process would inherit the open files, and the process started by execvp() would then get those files as well.

No, it doesn't matter. If you clone() without CLONE_FILES, the new thread/process gets a copy of the handles, which do or don't survive exec() depending on O_CLOEXEC. If you clone with CLONE_FILES, the new process shares the fd table. The fd table is unshared again as part of the exec(). From execve(2):

• The file descriptor table is unshared, undoing the effect of the CLONE_FILES flag of clone(2).

.. then the now copied files do or don't survive depending on O_CLOEXEC. Either way, O_CLOEXEC has the same final effect.

But as I was mentioning in that path we don't use CLONE_FILES anyway, so that's not relevant.

-- Stefano

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson