From 93e0898c663a533082b5f3c2e7dcce93ec47076d Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 26 Jan 2023 12:56:33 +0100 Subject: shell: fix SIGWINCH and SIGCHLD (in hush) interrupting line input, closes 15256 function old new delta record_pending_signo 32 63 +31 lineedit_read_key 231 224 -7 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 1/1 up/down: 31/-7) Total: 24 bytes Signed-off-by: Denys Vlasenko --- shell/ash.c | 3 ++- shell/hush.c | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 18ccc1329..5f8c8ea19 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -10821,7 +10821,8 @@ preadfd(void) again: /* For shell, LI_INTERRUPTIBLE is set: * read_line_input will abort on either - * getting EINTR in poll(), or if it sees bb_got_signal != 0 + * getting EINTR in poll() and bb_got_signal became != 0, + * or if it sees bb_got_signal != 0 * (IOW: if signal arrives before poll() is reached). * Interactive testcases: * (while kill -INT $$; do sleep 1; done) & diff --git a/shell/hush.c b/shell/hush.c index d111f0cc5..f064b8fd2 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1946,7 +1946,12 @@ static void record_pending_signo(int sig) { sigaddset(&G.pending_set, sig); #if ENABLE_FEATURE_EDITING - bb_got_signal = sig; /* for read_line_input: "we got a signal" */ + if (sig != SIGCHLD + || (G_traps && G_traps[SIGCHLD] && G_traps[SIGCHLD][0]) + /* ^^^ if SIGCHLD, interrupt line reading only if it has a trap */ + ) { + bb_got_signal = sig; /* for read_line_input: "we got a signal" */ + } #endif #if ENABLE_HUSH_FAST if (sig == SIGCHLD) { @@ -2669,7 +2674,8 @@ static int get_user_input(struct in_str *i) } else { /* For shell, LI_INTERRUPTIBLE is set: * read_line_input will abort on either - * getting EINTR in poll(), or if it sees bb_got_signal != 0 + * getting EINTR in poll() and bb_got_signal became != 0, + * or if it sees bb_got_signal != 0 * (IOW: if signal arrives before poll() is reached). * Interactive testcases: * (while kill -INT $$; do sleep 1; done) & -- cgit v1.2.3-55-g6feb From 6101b6d3eaa0fe5096c43d4fc648d49a532ee9c0 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 30 Jan 2023 15:57:04 +0100 Subject: hush: remove special handling of SIGHUP Kernel should do the right thing. (ash and dash do not have special SIGHUP handling.) function old new delta check_and_run_traps 278 214 -64 Signed-off-by: Denys Vlasenko --- shell/hush.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) (limited to 'shell') diff --git a/shell/hush.c b/shell/hush.c index f064b8fd2..9b87e28cf 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1830,6 +1830,8 @@ static void restore_G_args(save_arg_t *sv, char **argv) * SIGTERM (interactive): ignore * SIGHUP (interactive): * send SIGCONT to stopped jobs, send SIGHUP to all jobs and exit +//HUP: we don't need to do this, kernel does this for us +//HUP: ("orphaned process group" handling according to POSIX) * SIGTTIN, SIGTTOU, SIGTSTP (if job control is on): ignore * Note that ^Z is handled not by trapping SIGTSTP, but by seeing * that all pipe members are stopped. Try this in bash: @@ -1931,7 +1933,7 @@ enum { SPECIAL_INTERACTIVE_SIGS = 0 | (1 << SIGTERM) | (1 << SIGINT) - | (1 << SIGHUP) +//HUP | (1 << SIGHUP) , SPECIAL_JOBSTOP_SIGS = 0 #if ENABLE_HUSH_JOB @@ -2177,23 +2179,23 @@ static int check_and_run_traps(void) last_sig = sig; break; #if ENABLE_HUSH_JOB - case SIGHUP: { -//TODO: why are we doing this? ash and dash don't do this, -//they have no handler for SIGHUP at all, -//they rely on kernel to send SIGHUP+SIGCONT to orphaned process groups - struct pipe *job; - debug_printf_exec("%s: sig:%d default SIGHUP handler\n", __func__, sig); - /* bash is observed to signal whole process groups, - * not individual processes */ - for (job = G.job_list; job; job = job->next) { - if (job->pgrp <= 0) - continue; - debug_printf_exec("HUPing pgrp %d\n", job->pgrp); - if (kill(- job->pgrp, SIGHUP) == 0) - kill(- job->pgrp, SIGCONT); - } - sigexit(SIGHUP); - } +//HUP case SIGHUP: { +//HUP//TODO: why are we doing this? ash and dash don't do this, +//HUP//they have no handler for SIGHUP at all, +//HUP//they rely on kernel to send SIGHUP+SIGCONT to orphaned process groups +//HUP struct pipe *job; +//HUP debug_printf_exec("%s: sig:%d default SIGHUP handler\n", __func__, sig); +//HUP /* bash is observed to signal whole process groups, +//HUP * not individual processes */ +//HUP for (job = G.job_list; job; job = job->next) { +//HUP if (job->pgrp <= 0) +//HUP continue; +//HUP debug_printf_exec("HUPing pgrp %d\n", job->pgrp); +//HUP if (kill(- job->pgrp, SIGHUP) == 0) +//HUP kill(- job->pgrp, SIGCONT); +//HUP } +//HUP sigexit(SIGHUP); +//HUP } #endif #if ENABLE_HUSH_FAST case SIGCHLD: -- cgit v1.2.3-55-g6feb From 1fdb33bd07e52cea832a6584c79e9aa11987da1f Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 30 Jan 2023 16:49:48 +0100 Subject: hush: restore tty pgrp on SIGHUP Found one case where SIGHUP does need some handling. ash does not restore tty pgrp when killed by SIGHUP, and this means process which started ash needs to restore it, or it would get backgrounded when trying to use tty. function old new delta check_and_run_traps 214 229 +15 Signed-off-by: Denys Vlasenko --- shell/hush.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'shell') diff --git a/shell/hush.c b/shell/hush.c index 9b87e28cf..80a39925f 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1831,7 +1831,10 @@ static void restore_G_args(save_arg_t *sv, char **argv) * SIGHUP (interactive): * send SIGCONT to stopped jobs, send SIGHUP to all jobs and exit //HUP: we don't need to do this, kernel does this for us -//HUP: ("orphaned process group" handling according to POSIX) +//HUP: ("orphaned process group" handling according to POSIX). +//HUP: We still have a SIGHUP handler, just to have tty pgrp restored +//HUP: (otherwise e.g. Midnight Commander backgrounds when hush +//HUP: started from it gets killed by SIGHUP). * SIGTTIN, SIGTTOU, SIGTSTP (if job control is on): ignore * Note that ^Z is handled not by trapping SIGTSTP, but by seeing * that all pipe members are stopped. Try this in bash: @@ -1933,7 +1936,7 @@ enum { SPECIAL_INTERACTIVE_SIGS = 0 | (1 << SIGTERM) | (1 << SIGINT) -//HUP | (1 << SIGHUP) + | (1 << SIGHUP) , SPECIAL_JOBSTOP_SIGS = 0 #if ENABLE_HUSH_JOB @@ -2179,7 +2182,7 @@ static int check_and_run_traps(void) last_sig = sig; break; #if ENABLE_HUSH_JOB -//HUP case SIGHUP: { + case SIGHUP: { //HUP//TODO: why are we doing this? ash and dash don't do this, //HUP//they have no handler for SIGHUP at all, //HUP//they rely on kernel to send SIGHUP+SIGCONT to orphaned process groups @@ -2194,8 +2197,8 @@ static int check_and_run_traps(void) //HUP if (kill(- job->pgrp, SIGHUP) == 0) //HUP kill(- job->pgrp, SIGCONT); //HUP } -//HUP sigexit(SIGHUP); -//HUP } + sigexit(SIGHUP); + } #endif #if ENABLE_HUSH_FAST case SIGCHLD: -- cgit v1.2.3-55-g6feb From 93ae7464e6e460f25b73e4ffefd2d9a6499eae30 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 30 Jan 2023 18:47:18 +0100 Subject: hush: restore SIGHUP handling, this time explain why we do what we do function old new delta check_and_run_traps 229 278 +49 Signed-off-by: Denys Vlasenko --- shell/hush.c | 48 ++++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-) (limited to 'shell') diff --git a/shell/hush.c b/shell/hush.c index 80a39925f..e6be70078 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1829,12 +1829,13 @@ static void restore_G_args(save_arg_t *sv, char **argv) * SIGQUIT: ignore * SIGTERM (interactive): ignore * SIGHUP (interactive): - * send SIGCONT to stopped jobs, send SIGHUP to all jobs and exit -//HUP: we don't need to do this, kernel does this for us -//HUP: ("orphaned process group" handling according to POSIX). -//HUP: We still have a SIGHUP handler, just to have tty pgrp restored -//HUP: (otherwise e.g. Midnight Commander backgrounds when hush -//HUP: started from it gets killed by SIGHUP). + * Send SIGCONT to stopped jobs, send SIGHUP to all jobs and exit. + * Kernel would do this for us ("orphaned process group" handling + * according to POSIX) if we are a session leader and thus our death + * frees the controlling tty, but to be bash-compatible, we also do it + * for every interactive shell's death by SIGHUP. + * (Also, we need to restore tty pgrp, otherwise e.g. Midnight Commander + * backgrounds when hush started from it gets killed by SIGHUP). * SIGTTIN, SIGTTOU, SIGTSTP (if job control is on): ignore * Note that ^Z is handled not by trapping SIGTSTP, but by seeing * that all pipe members are stopped. Try this in bash: @@ -2183,20 +2184,27 @@ static int check_and_run_traps(void) break; #if ENABLE_HUSH_JOB case SIGHUP: { -//HUP//TODO: why are we doing this? ash and dash don't do this, -//HUP//they have no handler for SIGHUP at all, -//HUP//they rely on kernel to send SIGHUP+SIGCONT to orphaned process groups -//HUP struct pipe *job; -//HUP debug_printf_exec("%s: sig:%d default SIGHUP handler\n", __func__, sig); -//HUP /* bash is observed to signal whole process groups, -//HUP * not individual processes */ -//HUP for (job = G.job_list; job; job = job->next) { -//HUP if (job->pgrp <= 0) -//HUP continue; -//HUP debug_printf_exec("HUPing pgrp %d\n", job->pgrp); -//HUP if (kill(- job->pgrp, SIGHUP) == 0) -//HUP kill(- job->pgrp, SIGCONT); -//HUP } + /* if (G_interactive_fd) - no need to check, the handler + * is only installed if we *are* interactive */ + { + /* bash compat: "Before exiting, an interactive + * shell resends the SIGHUP to all jobs, running + * or stopped. Stopped jobs are sent SIGCONT + * to ensure that they receive the SIGHUP." + */ + struct pipe *job; + debug_printf_exec("%s: sig:%d default SIGHUP handler\n", __func__, sig); + /* bash is observed to signal whole process groups, + * not individual processes */ + for (job = G.job_list; job; job = job->next) { + if (job->pgrp <= 0) + continue; + debug_printf_exec("HUPing pgrp %d\n", job->pgrp); + if (kill(- job->pgrp, SIGHUP) == 0) + kill(- job->pgrp, SIGCONT); + } + } + /* this restores tty pgrp, then kills us with SIGHUP */ sigexit(SIGHUP); } #endif -- cgit v1.2.3-55-g6feb