Re: [glibc] posix: Add terminal control setting support for posix_spawn

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 27/01/2022 07:49, Florian Weimer via Libc-alpha wrote:
> * Adhemerval Zanella via Glibc-cvs:
> 
>> commit 342cc934a3bf74ac618e2318d738f22ac93257ba
>> Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> Date:   Mon Jun 14 14:41:31 2021 -0300
>>
>>     posix: Add terminal control setting support for posix_spawn
>>     
>>     Currently there is no proper way to set the controlling terminal through
>>     posix_spawn in race free manner [1].  This forces shell implementations
>>     to keep using fork+exec when launching background process groups,
>>     even when using posix_spawn yields better performance.
>>     
>>     This patch adds a new GNU extension so the creating process can
>>     configure the created process terminal group.  This is done with a new
>>     flag, POSIX_SPAWN_TCSETPGROUP, along with two new attribute functions:
>>     posix_spawnattr_tcsetpgrp_np, and posix_spawnattr_tcgetpgrp_np.
>>     The function sets a new attribute, spawn-tcgroupfd, that references to
>>     the controlling terminal.
>>     
>>     The controlling terminal is set after the spawn-pgroup attribute, and
>>     uses the spawn-tcgroupfd along with current creating process group
>>     (so it is composable with POSIX_SPAWN_SETPGROUP).
>>     
>>     To create a process and set the controlling terminal, one can use the
>>     following sequence:
>>     
>>         posix_spawnattr_t attr;
>>         posix_spawnattr_init (&attr);
>>         posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP);
>>         posix_spawnattr_tcsetpgrp_np (&attr, tcfd);
>>     
>>     If the idea is also to create a new process groups:
>>     
>>         posix_spawnattr_t attr;
>>         posix_spawnattr_init (&attr);
>>         posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP
>>                                          | POSIX_SPAWN_SETPGROUP);
>>         posix_spawnattr_tcsetpgrp_np (&attr, tcfd);
>>         posix_spawnattr_setpgroup (&attr, 0);
> 
> Sorry, I did not look at this in detail so far.  A file descriptor
> outside the file actions API makes me quite nervous.
> 
> Advanced Programming in the UNIX Environment says (like POSIX), “If the
> process has a controlling terminal, the process can call tcsetpgrp to
> set the foreground process group ID […]”.  This suggests to me that a
> file action may be required to establish a controlling terminal.
> Callers of posix_spawn might restrict that to the spawned process.  As
> far as I know, Linux follows SVR4, and opening the terminal device under
> /dev/pts/ in a session leader that has no controlling terminal
> establishes one.  This translate to a posix_spawn file action.  So the
> action established by posix_spawnattr_tcsetpgrp_np must come after the
> file actions.
> 
> But the descriptor used by posix_spawnattr_tcsetpgrp_np must still be
> open.  So it would have to come before a closefrom file action (or a
> regular close).
> 
> I think this means that the action established by
> posix_spawnattr_tcsetpgrp_np needs to be sequenced among the file
> actions, and needs to be a file action itself.
> 
> Thanks,
> Florian
> 


I did not considered the closefrom in conjunction with closefrom file
action and making a file action does make sense indeed.  Maybe a better
approach would be to remove posix_spawnattr_tcsetpgrp_np and
posix_spawnattr_tcsetpgrp_np and add:

  int posix_spawn_file_actions_tcsetgrp_np (posix_spawn_file_actions_t *,
					    int fd);

Similar to posix_spawn_file_actions_addfchdir_np.

So to create a process and set the controlling terminal, one can use the
following sequence:

  int tcfd = open (_PATH_TTY, O_RDONLY, 0600);

  posix_spawnattr_t attr;
  posix_spawnattr_init (&attr);
  posix_spawnattr_setflags (&attr, POSIX_SPAWN_TCSETPGROUP);
  
  posix_spawn_file_actions_t actions;
  posix_spawn_file_actions_init (&actions);
  posix_spawn_file_actions_tcsetgrp_np (&actions, tcfd);

So users would be able to add a posix_spawn_file_actions_addclose or
posix_spawn_file_actions_addclosefrom_np.