give_terminal_to() / maybe_give_terminal_to() race

[Earl Chew]

A newly created process of a foreground job races the
parent shell to configure the controlling terminal. This
can result in the parent shell stealing the controlling
terminal from a grandchild, and giving it to the child.

I think stop_pipeline() needs to be fixed to be more
cautious. What might be the right approach?

When bash starts a foreground job, it places the child
processes in a new process group, and hands over the
controlling terminal. As the new foreground job, the child
might itself be a job control shell (or similar) and create
its own children in a similar way.

Inspection of the source and a runtime trace (see below)
confirm that the newly forked child and the parent race each other
to set the foreground process group of the controlling terminal.
The race sometimes results in the following order where
the parent loses the race:

Time  Parent     Child
  1   TIOCGPGRP
  2              TIOCSPGRP
  ... ...        execve()
  N   TIOCSPGRP

Although TIOCGPGRP captures the parent as the foreground
process group at Time 1, by Time N the child has configured
itself as the foreground process group. The child might even
have successfully started the new executable which might
have created its own foreground child processes.

So when the parent issues TIOCSPGRP at Time N, the outcome
is to wrench the controlling terminal away from any subsidiary
foreground process group established by the child. It is
possible to demonstrate this with some effort using a small
C program.

In make_child(), after the fork():

      /* In the child.  Give this child the right process group, set the
         signals to the default state for a new process. */


          if ((flags & FORK_NOTERM) == 0 && async_p == 0 && 
pipeline_pgrp != shell_pgrp && 
((subshell_environment&(SUBSHELL_ASYNC|SUBSHELL_PIPE)) == 0) && 
running_in_background == 0)
            give_terminal_to (pipeline_pgrp, 0);


In stop_pipeline():

          if (job_control && newjob->pgrp && 
(subshell_environment&SUBSHELL_ASYNC) == 0 && running_in_background == 0)
            maybe_give_terminal_to (shell_pgrp, newjob->pgrp, 0);


To confirm, here is the strace(1) log of the child which contains a
call to TIOCSPGRP:

  getpid()                                = 3952526
  rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
  rt_sigaction(SIGTSTP, {sa_handler=SIG_DFL, sa_mask=[], 
sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, {sa_handler=SIG_IGN, 
sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, 8) = 0
  rt_sigaction(SIGTTIN, {sa_handler=SIG_DFL, sa_mask=[], 
sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, {sa_handler=SIG_IGN, 
sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, 8) = 0
  rt_sigaction(SIGTTOU, {sa_handler=SIG_DFL, sa_mask=[], 
sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, {sa_handler=SIG_IGN, 
sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, 8) = 0
  setpgid(3952526, 3952526)               = 0
  rt_sigprocmask(SIG_BLOCK, [CHLD TSTP TTIN TTOU], [], 8) = 0
  ioctl(255, TIOCSPGRP, [3952526])        = 0
  rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0


Here is the strace(1) log of the parent which contains a call to
both TIOCGPGRP and TIOCSPGRP:

  clone(child_stack=NULL, 
flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, 
child_tidptr=0x7fca461aea10) = 3952526
  rt_sigaction(SIGTERM, {sa_handler=SIG_IGN, sa_mask=[], 
sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, {sa_handler=SIG_DFL, 
sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7fca461e9d60}, 8) = 0
  setpgid(3952526, 3952526)               = 0
  rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
  rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
  close(3)                                = 0
  close(4)                                = 0
  ioctl(255, TIOCGPGRP, [3952510])        = 0
  rt_sigprocmask(SIG_BLOCK, [CHLD TSTP TTIN TTOU], [CHLD], 8) = 0
  ioctl(255, TIOCSPGRP, [3952526])        = 0
  rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
  rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
  rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
  wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 
0}],WSTOPPED|WCONTINUED, NULL) = 3952526

Earl