[systemd-devel] [PATCH] tty-ask-password-agent: reset a signal handler for SIGTERM to the default

[HATAYAMA Daisuke]

Hello,

When trapping SIGTERM in a script and running systemctl from the
script, systemctl sometimes hangs with tty-ask-password agent process.

I have no idea whether systemd developpers think this is a bug or
not. If this is a bug, I have three ideas to fix this. This patch is
based on the 1).

1) Reset a signal handler for SIGTERM to the default before spawning
   an agent process from systemctl.

2) Prepare a timeout limit in systemctl. If it exceeds the limit, kill
   the agent process forsively by SIGKILL.

3) Use another IPC instead signal mechanism to kill the agent process
   such as pipe.

==
Subject: [PATCH] tty-ask-password-agent: reset a signal handler for SIGTERM to the default

If a signal handler for SIGTERM is SIG_IGN, SIGTERM could be
immediately processed before sigprocmask(). Then, as a result,
systemctl hangs at waitid() and agent at poll().

The actual case where a signal handler for SIGTERM becomes SIG_IGN is
when systemctl is used in a service script that traps SIGTERM.

Here is an example.

~]# trap "" 15
~]# strace -ff -F -e trace=clone,execve,rt_sigprocmask,signalfd4,poll,kill,waitid systemctl stop kdump
execve("/usr/bin/systemctl", ["systemctl", "stop", "kdump"], [/* 30 vars */]) = 0
rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
poll([{fd=4, events=POLLOUT}], 1, 90000) = 1 ([{fd=4, revents=POLLOUT}])
poll([{fd=4, events=POLLIN}], 1, 90000) = 1 ([{fd=4, revents=POLLIN}])
poll([{fd=4, events=POLLOUT}], 1, 90000) = 1 ([{fd=4, revents=POLLOUT}])
poll([{fd=4, events=POLLIN}], 1, 90000) = 1 ([{fd=4, revents=POLLIN}])
poll([{fd=4, events=POLLOUT}], 1, 90000) = 1 ([{fd=4, revents=POLLOUT}])
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fe39df60b50) = 19820
poll([{fd=4, events=POLLIN}], 1, 25000) = 1 ([{fd=4, revents=POLLIN}])
poll([{fd=4, events=POLLIN}], 1, 25000) = 1 ([{fd=4, revents=POLLIN}])
poll([{fd=4, events=POLLIN}], 1, 25000) = 1 ([{fd=4, revents=POLLIN}])
poll([{fd=4, events=POLLIN}], 1, 25000) = 1 ([{fd=4, revents=POLLIN}])
poll([{fd=4, events=POLLIN}], 1, 25000) = 1 ([{fd=4, revents=POLLIN}])
kill(19820, SIGTERM)                    = 0
kill(19820, SIGCONT)                    = 0
waitid(P_PID, 19820, Process 19820 attached
 <unfinished ...>
[pid 19820] --- SIGTERM {si_signo=SIGTERM, si_code=SI_USER, si_pid=19819, si_uid=0} ---
[pid 19820] --- SIGCONT {si_signo=SIGCONT, si_code=SI_USER, si_pid=19819, si_uid=0} ---
[pid 19820] execve("/usr/bin/systemd-tty-ask-password-agent", ["/usr/bin/systemd-tty-ask-passwor"..., "--watch"], [/* 30 vars */]) = 0
[pid 19820] rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
[pid 19820] rt_sigprocmask(SIG_SETMASK, [INT TERM], NULL, 8) = 0
[pid 19820] signalfd4(-1, [INT TERM], 8, O_NONBLOCK|O_CLOEXEC) = 5
[pid 19820] poll([{fd=4, events=POLLIN}, {fd=5, events=POLLIN}], 2, 4294967295
---
 src/shared/spawn-ask-password-agent.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/shared/spawn-ask-password-agent.c b/src/shared/spawn-ask-password-agent.c
index c1a9c58..604fad6 100644
--- a/src/shared/spawn-ask-password-agent.c
+++ b/src/shared/spawn-ask-password-agent.c
@@ -34,6 +34,10 @@
 static pid_t agent_pid = 0;
 
 int ask_password_agent_open(void) {
+        struct sigaction sa_old, sa_new = {
+                .sa_handler = SIG_DFL,
+                .sa_flags = SA_RESTART,
+        };
         int r;
 
         if (agent_pid > 0)
@@ -44,6 +48,15 @@ int ask_password_agent_open(void) {
         if (!isatty(STDIN_FILENO))
                 return 0;
 
+        /* If a signal handler for SIGTERM is SIG_IGN, it's
+         * immediately processed by systemd-tty-ask-password process
+         * before sigprocmask() and then systemctl hangs at waitid()
+         * and agent at poll(). To avoid such case, reset a signal
+         * hander for SIGTERM to the default.
+         */
+        if (sigaction(SIGTERM, &sa_new, &sa_old) < 0)
+                return -errno;
+
         r = fork_agent(&agent_pid,
                        NULL, 0,
                        SYSTEMD_TTY_ASK_PASSWORD_AGENT_BINARY_PATH,
@@ -51,6 +64,9 @@ int ask_password_agent_open(void) {
         if (r < 0)
                 log_error("Failed to fork TTY ask password agent: %s", strerror(-r));
 
+        if (sigaction(SIGTERM, &sa_old, NULL) < 0)
+                return -errno;
+
         return r;
 }
 
-- 
1.9.3