From ed179fd71030ddd657500591dac37e7499fc7b2c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Apr 2019 10:04:16 +0200 Subject: [PATCH 1/5] tty-ask-password: copy argv[] before forking child Another fix in style of bd169c2be0fbdaf6eb2ea7951e650d5e5983fbf6. Let's also avoid strjoina() in a loop (i.e. stack allocation). While in this specific caseone could get away with it (since we'd immediately afterwards leave the loop) it's still ugly, and every static checker would be totally within its rights to complain. Also, let's simplify things by not relying on argc, since it's redundant anyway, and it's nicer to just treat things as NULL terminated strv array. Fixes: #12180 --- .../tty-ask-password-agent.c | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 790177d6810..d4ce904f3fb 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -695,7 +695,8 @@ static int parse_argv(int argc, char *argv[]) { * If one of the tasks does handle a password, the remaining tasks * will be terminated. */ -static int ask_on_this_console(const char *tty, pid_t *ret_pid, int argc, char *argv[]) { +static int ask_on_this_console(const char *tty, pid_t *ret_pid, char *argv[]) { + _cleanup_strv_free_ char **arguments = NULL; struct sigaction sig = { .sa_handler = nop_signal_handler, .sa_flags = SA_NOCLDSTOP | SA_RESTART, @@ -703,6 +704,10 @@ static int ask_on_this_console(const char *tty, pid_t *ret_pid, int argc, char * pid_t pid; int r; + arguments = strv_copy(argv); + if (!arguments) + return log_oom(); + assert_se(sigprocmask_many(SIG_UNBLOCK, NULL, SIGHUP, SIGCHLD, -1) >= 0); assert_se(sigemptyset(&sig.sa_mask) >= 0); @@ -715,18 +720,24 @@ static int ask_on_this_console(const char *tty, pid_t *ret_pid, int argc, char * if (r < 0) return r; if (r == 0) { - int ac; + char **i; assert_se(prctl(PR_SET_PDEATHSIG, SIGHUP) >= 0); - for (ac = 0; ac < argc; ac++) { - if (streq(argv[ac], "--console")) { - argv[ac] = strjoina("--console=", tty); - break; - } - } + STRV_FOREACH(i, arguments) { + char *k; - assert(ac < argc); + if (!streq(*i, "--console")) + continue; + + k = strjoin("--console=", tty); + if (!k) { + log_oom(); + _exit(EXIT_FAILURE); + } + + free_and_replace(*i, k); + } execv(SYSTEMD_TTY_ASK_PASSWORD_AGENT_BINARY_PATH, argv); _exit(EXIT_FAILURE); @@ -788,7 +799,7 @@ static void terminate_agents(Set *pids) { } } -static int ask_on_consoles(int argc, char *argv[]) { +static int ask_on_consoles(char *argv[]) { _cleanup_set_free_ Set *pids = NULL; _cleanup_strv_free_ char **consoles = NULL; siginfo_t status = {}; @@ -806,7 +817,7 @@ static int ask_on_consoles(int argc, char *argv[]) { /* Start an agent on each console. */ STRV_FOREACH(tty, consoles) { - r = ask_on_this_console(*tty, &pid, argc, argv); + r = ask_on_this_console(*tty, &pid, argv); if (r < 0) return r; @@ -851,7 +862,7 @@ static int run(int argc, char *argv[]) { /* * Spawn a separate process for each console device. */ - return ask_on_consoles(argc, argv); + return ask_on_consoles(argv); if (arg_device) { /* From 4bec7f09f8b1facd125e4897bb80bcf931adbe79 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Apr 2019 10:07:58 +0200 Subject: [PATCH 2/5] tty-ask-password: drop redundant local variable --- src/tty-ask-password-agent/tty-ask-password-agent.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index d4ce904f3fb..4e31eb8b6cf 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -701,7 +701,6 @@ static int ask_on_this_console(const char *tty, pid_t *ret_pid, char *argv[]) { .sa_handler = nop_signal_handler, .sa_flags = SA_NOCLDSTOP | SA_RESTART, }; - pid_t pid; int r; arguments = strv_copy(argv); @@ -716,7 +715,7 @@ static int ask_on_this_console(const char *tty, pid_t *ret_pid, char *argv[]) { sig.sa_handler = SIG_DFL; assert_se(sigaction(SIGHUP, &sig, NULL) >= 0); - r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS|FORK_LOG, &pid); + r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS|FORK_LOG, ret_pid); if (r < 0) return r; if (r == 0) { @@ -743,7 +742,6 @@ static int ask_on_this_console(const char *tty, pid_t *ret_pid, char *argv[]) { _exit(EXIT_FAILURE); } - *ret_pid = pid; return 0; } From 7452917740bc005cd4605b7be313238c0e9c5099 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Apr 2019 10:08:29 +0200 Subject: [PATCH 3/5] tty-ask-password: no need to initialize something already NUL initialized to NUL --- src/tty-ask-password-agent/tty-ask-password-agent.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 4e31eb8b6cf..4633779837a 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -709,7 +709,6 @@ static int ask_on_this_console(const char *tty, pid_t *ret_pid, char *argv[]) { assert_se(sigprocmask_many(SIG_UNBLOCK, NULL, SIGHUP, SIGCHLD, -1) >= 0); - assert_se(sigemptyset(&sig.sa_mask) >= 0); assert_se(sigaction(SIGCHLD, &sig, NULL) >= 0); sig.sa_handler = SIG_DFL; From d850296466c502106e680a2a5c07d3aa3213b97f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Apr 2019 10:10:02 +0200 Subject: [PATCH 4/5] tty-ask-password: simplify signal handler installation --- src/tty-ask-password-agent/tty-ask-password-agent.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 4633779837a..17cf35126fb 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -697,23 +697,24 @@ static int parse_argv(int argc, char *argv[]) { */ static int ask_on_this_console(const char *tty, pid_t *ret_pid, char *argv[]) { _cleanup_strv_free_ char **arguments = NULL; - struct sigaction sig = { + static const struct sigaction sigchld = { .sa_handler = nop_signal_handler, .sa_flags = SA_NOCLDSTOP | SA_RESTART, }; + static const struct sigaction sighup = { + .sa_handler = SIG_DFL, + .sa_flags = SA_RESTART, + }; int r; arguments = strv_copy(argv); if (!arguments) return log_oom(); + assert_se(sigaction(SIGCHLD, &sigchld, NULL) >= 0); + assert_se(sigaction(SIGHUP, &sighup, NULL) >= 0); assert_se(sigprocmask_many(SIG_UNBLOCK, NULL, SIGHUP, SIGCHLD, -1) >= 0); - assert_se(sigaction(SIGCHLD, &sig, NULL) >= 0); - - sig.sa_handler = SIG_DFL; - assert_se(sigaction(SIGHUP, &sig, NULL) >= 0); - r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS|FORK_LOG, ret_pid); if (r < 0) return r; From 189b03779e24d17212a3c812b20117a917f2c696 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Apr 2019 10:10:30 +0200 Subject: [PATCH 5/5] tty-ask-password: re-break comment --- src/tty-ask-password-agent/tty-ask-password-agent.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 17cf35126fb..b5604a745a2 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -688,12 +688,10 @@ static int parse_argv(int argc, char *argv[]) { } /* - * To be able to ask on all terminal devices of /dev/console - * the devices are collected. If more than one device is found, - * then on each of the terminals a inquiring task is forked. - * Every task has its own session and its own controlling terminal. - * If one of the tasks does handle a password, the remaining tasks - * will be terminated. + * To be able to ask on all terminal devices of /dev/console the devices are collected. If more than one + * device is found, then on each of the terminals a inquiring task is forked. Every task has its own session + * and its own controlling terminal. If one of the tasks does handle a password, the remaining tasks will be + * terminated. */ static int ask_on_this_console(const char *tty, pid_t *ret_pid, char *argv[]) { _cleanup_strv_free_ char **arguments = NULL;