Re: [RFC PATCH v2 2/7] misc: Ignore SIGHUP in daemon () while forking

[Adhemerval Zanella Netto]

On 19/04/23 13:02, Sergey Bugaev wrote:
> Under certain conditions, SIGHUP will be sent to the child process when
> the parent process (the one calling daemon ()) terminates -- namely, if
> the parent process was the session leader and the child process was in
> the same session (which will be the case after fork and until setsid)
> and in the foreground process group. To prevent this SIGHUP from killing
> the child, temporarily ignore it. Once we leave the parent's session, we
> can restore the original SIGHUP sigaction.
> 
> Or that's what you'd hope would happen. Unfortunately for us, nothing
> guarantess that signal delivery is synchronous. This means that a SIGHUP
> may be generated and enqueued for the child process while it's still a
> member of the original session, but only delivered to it some time
> later, once it has already left the session and restored the original
> sigaction.
> 
> Still, on many systems signal delivery is synchronous enough, and all
> pending signals will get reliably delivered upon performing a syscall,
> specifically setsid () in this case, so this change is still worth it.
> 
> Also, do not ignore erros from chdir ("/").
> 
> Suggested-by: Cristian Rodríguez <crrodriguez@opensuse.org>
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>

Wouldn't a double fork avoid binding the daemon process to terminal to
reacquire a controlling terminal, thus avoid SIGHUP? This is BZ#19144 [1],
and this is being used by various libc implementations.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=19144

> ---
>  misc/daemon.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/misc/daemon.c b/misc/daemon.c
> index 14577e40..58dde4f0 100644
> --- a/misc/daemon.c
> +++ b/misc/daemon.c
> @@ -35,6 +35,7 @@ static char sccsid[] = "@(#)daemon.c        8.1 (Berkeley) 
> 6/4/93";
>  #include <fcntl.h>
>  #include <paths.h>
>  #include <unistd.h>
> +#include <signal.h>
>  #include <sys/stat.h>
>  
>  #include <device-nrs.h>
> @@ -44,6 +45,14 @@ int
>  daemon (int nochdir, int noclose)
>  {
>    int fd;
> +  int set_sigaction;
> +  struct sigaction act, oact;
> +
> +  /* When the parent process exits, the child might get a SIGHUP if the 
> parent
> +     was a session leader.  Arrange things so that it doesn't terminate us.  
> */
> +  memset (&act, 0, sizeof (act));
> +  act.sa_handler = SIG_IGN;
> +  set_sigaction = __libc_sigaction (SIGHUP, &act, &oact) == 0;
>  
>    switch (__fork ())
>      {
> @@ -60,8 +69,14 @@ daemon (int nochdir, int noclose)
>    if (__setsid () == -1)
>      return -1;
>  
> +  /* Now that we have left the parent's session, we should no longer be at
> +     risk of receiving SIGHUP because of the parent process exiting.  */
> +  if (__glibc_likely (set_sigaction))
> +    __libc_sigaction (SIGHUP, &oact, NULL);
> +
>    if (!nochdir)
> -    (void) __chdir ("/");
> +    if (__glibc_unlikely (__chdir ("/") == -1))
> +      return -1;
>  
>    if (!noclose)
>      {