[Bug libc/28730] Extend posix_spawnattr_* interface with more useful features

["adhemerval.zanella at linaro dot org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=28730

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adhemerval.zanella at linaro dot o
                   |                            |rg

--- Comment #2 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Cristian Rodríguez from comment #0)
> I have reviewed a number of software to possible make them use posix_spawn
> interfaces and found a few missing features, (other than the terminal
> control stuff whicb I am aware it is being worked on) I do not expect all of
> this suggestions to be implemented and I know it is not intended as a 100%
> fork/execve replacement
> 
> The first one .. if I:
> 
> posix_spawn(&child_pid, "/bin/sleep" , NULL, NULL, (char *const[]) {
> "sleep", "1000", NULL}, environ)
> 
> and kill the process the invoked the function, the orphan keeps running as
> good ole unix behaviour mandates, but this behaviour is highly undersirable
> except when you are spawning long running process or you are sure it will
> not do anything evil unsupervised by the calling process. 
> 
> Therefore I propose a linux only extension (unless the hurd has it too?)
> with prototype:
> 
> posix_spawnattr_setpdeathsig_np(posix_spawnattr_t *attr, int deathsignal)
> 
> that escentially calls 
> 
> prctl(PR_SET_PDEATHSIG, deathsignal);

I think it makes sense, although it has the the inherent issue with the
set-user-ID or set-group-ID binary and it would require a lot of funcionality
to make it work (for instance, a possible solution is described
https://stackoverflow.com/questions/42496478/prctlpr-set-pdeathsig-race-condition),
that is also out of the scope of posix_spawn.

I checked systemd code and it does seem to use PR_SET_PDEATHSIG as default. 
However, it also implements a 'safe_fork' that issues clone directly with extra
arguments to handle the pid and filesystem namespace creation.  It makes me
wonder if usual posix_spawn usage will really benefit with a Linux specific
feature, since it aims to be a potable solution (although other POSIX systems
do add a lot of extensions).

> 
> in the "child" process.
> 
> Unless standards say otherwise, there shall be no default death signal
> configured.
> 
> 
> Second feature request will be having an equivalent to pthread_setname_np as 
> posix_spawnattr_setname_np which does the same as pthread_setname_np but on
> this case for the "child" process..
> 
> Unless the standards say otherwise there shall be no default name set, but
> it will be nice if glibc tags its own created processes with for example a
> (system), (popen), (wordexp) name, but this is just a nice to have
> conveniance for humans that are looking the process table..

It would require to alloc/free the name itself (to avoid waste space on
posix_spawnattr_t), but I think it should be feasible.

But again, it this a feature that would be really used? Linux limits the name
to just TASK_COMM_LEN (16), which makes me doubt you can pack really useful
information.

> 
> 
> Third is a way to set the "child" rlimit, like
> posix_spawnattr_setrlimit_np(posix_spawnattr_t *attr, int resource, struct
> rlimit *rlim)
> 
> parent may be running with a high RLIMIT_NOFILE but execute a program that
> uses select() and things will end badly when RLIMIT_NOFILE > FD_SETSIZE.

The rlimit does make sense to me, QNX has posix_spawnattr_setstackmax for
instance.

> 
> I could try to implement this but Im not sure how I should exactly extend
> posix_spawnattr_t in a binary compatible way..

Best option would to use the __pad space member space from posix_spawnattr_t. 
To PR_SET_PDEATHSIG it would be a matter of add another field and decrease
__pad size.

For the new process name and rlimit I think it would be better add extra points
where the posix_spawnattr_* would allocate memory and posix_spawnattr_destroy
would free them.

The rlimit would require list of resources to apply on spawned processed, I
would go allocate a vector of an initial size and reallocate by doubling it
size. 

The only issue would be to add the newer pointers fields without changing
posix_spawnattr_t, since the internal fields alignment might require to use a
different value for the __pad size.

> The other way of course would be to just have a just-before-exec callback,
> clearly documenting what can be done there and everything else is 
>  undefined/evil/"will bring the demise of the human race"

Florian has suggested it some time ago, but I not really sure about it.  Even
now users still use non-async-safe functions on signal handler or after fork
(such as malloc) that only work due an implementation detail on GLIBC. I really
like to avoid it, even though it would require to add multiple posix_spawn
non-portable extensions. The posix_spawn helper process at Linux is really
tricky because it uses CLONE_VM, so we will need to evaluate exaclty what is
allowed to be called in such context which I think it not worth the trouble.

-- 
You are receiving this mail because:
You are on the CC list for the bug.