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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 20 May 2026 11:04:58 +1000
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au> 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 <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().    
> > > 
> > > 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:

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

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.

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

-- 
Stefano