Re: vfork() and posix_spawn() [was: Re: CVS commit: src/lib/libc/sys]

[Joerg Sonnenberger <joerg%bec.de@localhost>]

On Mon, Jun 14, 2021 at 01:33:51AM +0700, Robert Elz wrote:
>     Date:        Sat, 12 Jun 2021 23:13:54 +0200
>     From:        Joerg Sonnenberger <joerg%bec.de@localhost>
>     Message-ID:  <YMUjkhwh91De57eC%bec.de@localhost>
> 
> Sorry, missed this message when I was cherry picking messages to read in
> a timely fashion.
> 
>   | On Wed, Jun 09, 2021 at 01:03:20AM +0700, Robert Elz wrote:
>   | > after a vfork() the child can do anything
> 
>   | This is not true. After vfork(), it is only supposed to call async
>   | signal safe functions.
> 
> What you said, and what I said are not contradictory.   Note I said "can
> do" and you said "is only supposed to".
> 
> What is supposed to work and what actually does work (or can be made to
> work) are two different things.

Sure, but the set of what works reliably has actually been shrinking
over time and more bugs in various programs using vfork specifically
have been discovered. This is even more true for multi-threaded
applications (where POSIX explicitly suggests that requirement). On the
specific topic, I'm somewhat puzzled by the claim that fork is
async-signal-safe since most implementations will require
synchronisation for pthread_atfork and that seems to place quite a
burden on the implementation...

> I would note though our man page doesn't make that requirement, or not
> that I can see anyway, and vfork() is not in POSIX of course - and while
> I don't have a copy to check, I'd suspect not in the C standards either),
> so while that sounds like a reasonable requirement to impose for safer
> operation, I'm not sure that anyone or anything actually does that.

It most likely should. The main reason is that much of the system can
and often do depend on things like mutexes to ensure correctness and all
that is essentially UB after vfork(). That's even ignoring the stronger
issue of mutating state. vfork use really should die...

> 
>   | That said, a flag for the double fork semantic might be useful.
> 
> If the "that said" relates to fork() or vfork() not being async signal safe,
> so a double fork() (when the first is vfork anyway) would not be condoned,
> that doesn't matter, there to be an async-signal-safe _Fork() call added to
> the next version of POSIX, so even with the "only async signal safe"
> requirement for vfork() a:

No, it relates to one common pattern for used by or for daemon.

> Please don't see posix_spawn() as being (or ever becoming) a panacea that
> can replace all fork() (or even just vfork()) calls - even just in the
> cases where an exec() follows soon after.   It works fine for most simple
> cases, but there are lots of not-so-simple situations that it cannot handle,
> and burdening the interface with the ability to deal with all of those
> would reduce the "efficiently implementable" goal for little real benefit.

There are non-trivial uses of fork, yes. There are much less non-trivial
uses of vfork as the latter already has quite a few problems with spooky
actions otherwise. Supporting something like a double fork flag has very
little impact on the complexity of the implementation and even less
impact on the efficiency. We certainly are at the point where we can
start analyzing the remaining blockers for (v)fork+exec users.

> Another example is made obvious by the bug Martin committed a fix for
> in the past few days ... the order of operations was incorrect in our
> implementation.  But that this problem can exist shows that there
> are ordering issues - a process that wants to open files using its
> current privs, then reset those before (perhaps) opening more files,
> and then doing the exec() part cannot do that with posix_spawn().
> Again, this is rare enough that adding the complexity required to allow
> it to work just isn't worth it.   [In this area also note that the
> POSIX_SPAWN_RESETIDS flag causes (effectively) setuid(getuid()), there's
> no similar operation to do setuid(geteuid()) ... again too rare to bother.]

I quite disagree here, actually. The design-level issue is that
POSIX_SPAWN_RESETIDS is a flag and not an action. This means it can't be
sequenced and that is the reason for the limitation. There is an obvious
parallel with the semantics of the chdir action here--that needs to be
that, an action and not just a flag. The separate concern is of course
that we need more testing for posix_spawn, but that is hopefully also
going to become better as side effect of the non-GSoC project.

Joerg