ash,hush: fix handling of SIGINT while waiting for interactive input

function                                             old     new   delta
lineedit_read_key                                    160     237     +77
__pgetc                                              522     589     +67
fgetc_interactive                                    244     309     +65
safe_read_key                                          -      39     +39
read_key                                             588     607     +19
record_pending_signo                                  23      32      +9
signal_handler                                        75      81      +6
.rodata                                           104312  104309      -3
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 6/1 up/down: 282/-3)            Total: 279 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This commit is contained in:
Denys Vlasenko 2022-01-17 03:02:40 +01:00
parent a277506a64
commit 12566e7f9b
9 changed files with 122 additions and 41 deletions

View File

@ -1122,7 +1122,7 @@ static int readit(void) // read (maybe cursor) key from stdin
// on nonblocking stdin. // on nonblocking stdin.
// Note: read_key sets errno to 0 on success. // Note: read_key sets errno to 0 on success.
again: again:
c = read_key(STDIN_FILENO, readbuffer, /*timeout:*/ -1); c = safe_read_key(STDIN_FILENO, readbuffer, /*timeout:*/ -1);
if (c == -1) { // EOF/error if (c == -1) { // EOF/error
if (errno == EAGAIN) // paranoia if (errno == EAGAIN) // paranoia
goto again; goto again;
@ -4770,7 +4770,7 @@ static void edit_file(char *fn)
uint64_t k; uint64_t k;
write1(ESC"[999;999H" ESC"[6n"); write1(ESC"[999;999H" ESC"[6n");
fflush_all(); fflush_all();
k = read_key(STDIN_FILENO, readbuffer, /*timeout_ms:*/ 100); k = safe_read_key(STDIN_FILENO, readbuffer, /*timeout_ms:*/ 100);
if ((int32_t)k == KEYCODE_CURSOR_POS) { if ((int32_t)k == KEYCODE_CURSOR_POS) {
uint32_t rc = (k >> 32); uint32_t rc = (k >> 32);
columns = (rc & 0x7fff); columns = (rc & 0x7fff);

View File

@ -1908,6 +1908,8 @@ enum {
* >=0: poll() for TIMEOUT milliseconds, return -1/EAGAIN on timeout * >=0: poll() for TIMEOUT milliseconds, return -1/EAGAIN on timeout
*/ */
int64_t read_key(int fd, char *buffer, int timeout) FAST_FUNC; int64_t read_key(int fd, char *buffer, int timeout) FAST_FUNC;
/* This version loops on EINTR: */
int64_t safe_read_key(int fd, char *buffer, int timeout) FAST_FUNC;
void read_key_ungets(char *buffer, const char *str, unsigned len) FAST_FUNC; void read_key_ungets(char *buffer, const char *str, unsigned len) FAST_FUNC;
@ -1961,7 +1963,8 @@ enum {
USERNAME_COMPLETION = 4 * ENABLE_FEATURE_USERNAME_COMPLETION, USERNAME_COMPLETION = 4 * ENABLE_FEATURE_USERNAME_COMPLETION,
VI_MODE = 8 * ENABLE_FEATURE_EDITING_VI, VI_MODE = 8 * ENABLE_FEATURE_EDITING_VI,
WITH_PATH_LOOKUP = 0x10, WITH_PATH_LOOKUP = 0x10,
FOR_SHELL = DO_HISTORY | TAB_COMPLETION | USERNAME_COMPLETION, LI_INTERRUPTIBLE = 0x20,
FOR_SHELL = DO_HISTORY | TAB_COMPLETION | USERNAME_COMPLETION | LI_INTERRUPTIBLE,
}; };
line_input_t *new_line_input_t(int flags) FAST_FUNC; line_input_t *new_line_input_t(int flags) FAST_FUNC;
#if ENABLE_FEATURE_EDITING_SAVEHISTORY #if ENABLE_FEATURE_EDITING_SAVEHISTORY

View File

@ -2161,12 +2161,30 @@ static int lineedit_read_key(char *read_key_buffer, int timeout)
* insist on full MB_CUR_MAX buffer to declare input like * insist on full MB_CUR_MAX buffer to declare input like
* "\xff\n",pause,"ls\n" invalid and thus won't lose "ls". * "\xff\n",pause,"ls\n" invalid and thus won't lose "ls".
* *
* If LI_INTERRUPTIBLE, return -1 if got EINTR in poll()
* inside read_key, or if bb_got_signal != 0 (IOW: if signal
* arrived before poll() is reached).
*
* Note: read_key sets errno to 0 on success. * Note: read_key sets errno to 0 on success.
*/ */
do {
if ((state->flags & LI_INTERRUPTIBLE) && bb_got_signal) {
errno = EINTR;
return -1;
}
//FIXME: still races here with signals, but small window to poll() inside read_key
IF_FEATURE_EDITING_WINCH(S.ok_to_redraw = 1;) IF_FEATURE_EDITING_WINCH(S.ok_to_redraw = 1;)
ic = read_key(STDIN_FILENO, read_key_buffer, timeout); ic = read_key(STDIN_FILENO, read_key_buffer, timeout);
IF_FEATURE_EDITING_WINCH(S.ok_to_redraw = 0;) IF_FEATURE_EDITING_WINCH(S.ok_to_redraw = 0;)
} while (!(state->flags & LI_INTERRUPTIBLE) && errno == EINTR);
if (errno) { if (errno) {
/* LI_INTERRUPTIBLE can bail out with EINTR here,
* but nothing really guarantees that bb_got_signal
* is nonzero. Follow the least surprise principle:
*/
if (errno == EINTR && bb_got_signal == 0)
bb_got_signal = 255; /* something nonzero */
#if ENABLE_UNICODE_SUPPORT #if ENABLE_UNICODE_SUPPORT
if (errno == EAGAIN && unicode_idx != 0) if (errno == EAGAIN && unicode_idx != 0)
goto pushback; goto pushback;

View File

@ -126,7 +126,10 @@ int64_t FAST_FUNC read_key(int fd, char *buffer, int timeout)
* if fd can be in non-blocking mode. * if fd can be in non-blocking mode.
*/ */
if (timeout >= -1) { if (timeout >= -1) {
if (safe_poll(&pfd, 1, timeout) == 0) { n = poll(&pfd, 1, timeout);
if (n < 0 && errno == EINTR)
return n;
if (n == 0) {
/* Timed out */ /* Timed out */
errno = EAGAIN; errno = EAGAIN;
return -1; return -1;
@ -138,7 +141,7 @@ int64_t FAST_FUNC read_key(int fd, char *buffer, int timeout)
* When we were reading 3 bytes here, we were eating * When we were reading 3 bytes here, we were eating
* "li" too, and cat was getting wrong input. * "li" too, and cat was getting wrong input.
*/ */
n = safe_read(fd, buffer, 1); n = read(fd, buffer, 1);
if (n <= 0) if (n <= 0)
return -1; return -1;
} }
@ -284,6 +287,15 @@ int64_t FAST_FUNC read_key(int fd, char *buffer, int timeout)
goto start_over; goto start_over;
} }
int64_t FAST_FUNC safe_read_key(int fd, char *buffer, int timeout)
{
int64_t r;
do {
r = read_key(fd, buffer, timeout);
} while (errno == EINTR);
return r;
}
void FAST_FUNC read_key_ungets(char *buffer, const char *str, unsigned len) void FAST_FUNC read_key_ungets(char *buffer, const char *str, unsigned len)
{ {
unsigned cur_len = (unsigned char)buffer[0]; unsigned cur_len = (unsigned char)buffer[0];

View File

@ -292,7 +292,7 @@ int hexedit_main(int argc UNUSED_PARAM, char **argv)
fflush_all(); fflush_all();
G.in_read_key = 1; G.in_read_key = 1;
if (!bb_got_signal) if (!bb_got_signal)
key = read_key(STDIN_FILENO, G.read_key_buffer, -1); key = safe_read_key(STDIN_FILENO, G.read_key_buffer, -1);
G.in_read_key = 0; G.in_read_key = 0;
if (bb_got_signal) if (bb_got_signal)
key = CTRL('X'); key = CTRL('X');

View File

@ -1137,9 +1137,9 @@ static int64_t getch_nowait(void)
#endif #endif
} }
/* We have kbd_fd in O_NONBLOCK mode, read inside read_key() /* We have kbd_fd in O_NONBLOCK mode, read inside safe_read_key()
* would not block even if there is no input available */ * would not block even if there is no input available */
key64 = read_key(kbd_fd, kbd_input, /*timeout off:*/ -2); key64 = safe_read_key(kbd_fd, kbd_input, /*timeout off:*/ -2);
if ((int)key64 == -1) { if ((int)key64 == -1) {
if (errno == EAGAIN) { if (errno == EAGAIN) {
/* No keyboard input available. Since poll() did return, /* No keyboard input available. Since poll() did return,

View File

@ -913,7 +913,7 @@ static unsigned handle_input(unsigned scan_mask, duration_t interval)
while (1) { while (1) {
int32_t c; int32_t c;
c = read_key(STDIN_FILENO, G.kbd_input, interval * 1000); c = safe_read_key(STDIN_FILENO, G.kbd_input, interval * 1000);
if (c == -1 && errno != EAGAIN) { if (c == -1 && errno != EAGAIN) {
/* error/EOF */ /* error/EOF */
option_mask32 |= OPT_EOF; option_mask32 |= OPT_EOF;

View File

@ -3679,7 +3679,9 @@ signal_handler(int signo)
if (!trap[SIGCHLD]) if (!trap[SIGCHLD])
return; return;
} }
#if ENABLE_FEATURE_EDITING
bb_got_signal = signo; /* for read_line_input: "we got a signal" */
#endif
gotsig[signo - 1] = 1; gotsig[signo - 1] = 1;
pending_sig = signo; pending_sig = signo;
@ -10784,33 +10786,52 @@ preadfd(void)
# endif # endif
reinit_unicode_for_ash(); reinit_unicode_for_ash();
again: again:
//BUG: not in INT_OFF/INT_ON section - SIGINT et al would longjmp out of read_line_input()! /* For shell, LI_INTERRUPTIBLE is set:
//This would cause a memory leak in interactive shell * read_line_input will abort on either
//(repeated internal allocations in read_line_input): * getting EINTR in poll(), or if it sees bb_got_signal != 0
// (while kill -INT $$; do :; done) & * (IOW: if signal arrives before poll() is reached).
* Interactive testcases:
* (while kill -INT $$; do sleep 1; done) &
* #^^^ prints ^C, prints prompt, repeats
* trap 'echo I' int; (while kill -INT $$; do sleep 1; done) &
* #^^^ prints ^C, prints "I", prints prompt, repeats
* trap 'echo T' term; (while kill $$; do sleep 1; done) &
* #^^^ prints "T", prints prompt, repeats
* #(bash 5.0.17 exits after first "T", looks like a bug)
*/
bb_got_signal = 0;
INT_OFF; /* no longjmp'ing out of read_line_input please */
nr = read_line_input(line_input_state, cmdedit_prompt, buf, IBUFSIZ); nr = read_line_input(line_input_state, cmdedit_prompt, buf, IBUFSIZ);
if (bb_got_signal == SIGINT)
write(STDOUT_FILENO, "^C\n", 3);
INT_ON; /* here non-blocked SIGINT will longjmp */
if (nr == 0) { if (nr == 0) {
/* ^C pressed, "convert" to SIGINT */ /* ^C pressed, "convert" to SIGINT */
write(STDOUT_FILENO, "^C", 2); write(STDOUT_FILENO, "^C\n", 3);
raise(SIGINT); raise(SIGINT); /* here non-blocked SIGINT will longjmp */
/* raise(SIGINT) did not work! (e.g. if SIGINT /* raise(SIGINT) did not work! (e.g. if SIGINT
* is SIG_IGNed on startup, it stays SIG_IGNed) * is SIG_IGNed on startup, it stays SIG_IGNed)
*/ */
if (trap[SIGINT]) { if (trap[SIGINT]) {
empty_line_input:
buf[0] = '\n'; buf[0] = '\n';
buf[1] = '\0'; buf[1] = '\0';
return 1; return 1;
} }
exitstatus = 128 + SIGINT; exitstatus = 128 + SIGINT;
/* bash behavior on ^C + ignored SIGINT: */ /* bash behavior on ^C + ignored SIGINT: */
write(STDOUT_FILENO, "\n", 1);
goto again; goto again;
} }
if (nr < 0) { if (nr < 0) {
if (errno == 0) { if (errno == 0) {
/* Ctrl+D pressed */ /* ^D pressed */
nr = 0; nr = 0;
} }
else if (errno == EINTR) { /* got signal? */
if (bb_got_signal != SIGINT)
write(STDOUT_FILENO, "\n", 1);
goto empty_line_input;
}
# if ENABLE_ASH_IDLE_TIMEOUT # if ENABLE_ASH_IDLE_TIMEOUT
else if (errno == EAGAIN && timeout > 0) { else if (errno == EAGAIN && timeout > 0) {
puts("\007timed out waiting for input: auto-logout"); puts("\007timed out waiting for input: auto-logout");

View File

@ -918,6 +918,7 @@ struct globals {
#if ENABLE_HUSH_INTERACTIVE #if ENABLE_HUSH_INTERACTIVE
smallint promptmode; /* 0: PS1, 1: PS2 */ smallint promptmode; /* 0: PS1, 1: PS2 */
#endif #endif
/* set by signal handler if SIGINT is received _and_ its trap is not set */
smallint flag_SIGINT; smallint flag_SIGINT;
#if ENABLE_HUSH_LOOPS #if ENABLE_HUSH_LOOPS
smallint flag_break_continue; smallint flag_break_continue;
@ -1944,6 +1945,9 @@ enum {
static void record_pending_signo(int sig) static void record_pending_signo(int sig)
{ {
sigaddset(&G.pending_set, sig); sigaddset(&G.pending_set, sig);
#if ENABLE_FEATURE_EDITING
bb_got_signal = sig; /* for read_line_input: "we got a signal" */
#endif
#if ENABLE_HUSH_FAST #if ENABLE_HUSH_FAST
if (sig == SIGCHLD) { if (sig == SIGCHLD) {
G.count_SIGCHLD++; G.count_SIGCHLD++;
@ -2652,31 +2656,54 @@ static int get_user_input(struct in_str *i)
for (;;) { for (;;) {
reinit_unicode_for_hush(); reinit_unicode_for_hush();
G.flag_SIGINT = 0; G.flag_SIGINT = 0;
/* buglet: SIGINT will not make new prompt to appear _at once_,
* only after <Enter>. (^C works immediately) */ bb_got_signal = 0;
if (!sigisemptyset(&G.pending_set)) {
/* Whoops, already got a signal, do not call read_line_input */
bb_got_signal = r = -1;
} 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
* (IOW: if signal arrives before poll() is reached).
* Interactive testcases:
* (while kill -INT $$; do sleep 1; done) &
* #^^^ prints ^C, prints prompt, repeats
* trap 'echo I' int; (while kill -INT $$; do sleep 1; done) &
* #^^^ prints ^C, prints "I", prints prompt, repeats
* trap 'echo T' term; (while kill $$; do sleep 1; done) &
* #^^^ prints "T", prints prompt, repeats
* #(bash 5.0.17 exits after first "T", looks like a bug)
*/
r = read_line_input(G.line_input_state, prompt_str, r = read_line_input(G.line_input_state, prompt_str,
G.user_input_buf, CONFIG_FEATURE_EDITING_MAX_LEN-1 G.user_input_buf, CONFIG_FEATURE_EDITING_MAX_LEN-1
); );
/* read_line_input intercepts ^C, "convert" it to SIGINT */ /* read_line_input intercepts ^C, "convert" it to SIGINT */
if (r == 0) { if (r == 0)
raise(SIGINT); raise(SIGINT);
} }
check_and_run_traps(); /* bash prints ^C (before running a trap, if any)
if (r != 0 && !G.flag_SIGINT) * both on keyboard ^C and on real SIGINT (non-kbd generated).
break; */
/* ^C or SIGINT: repeat */ if (sigismember(&G.pending_set, SIGINT)) {
/* bash prints ^C even on real SIGINT (non-kbd generated) */
write(STDOUT_FILENO, "^C\n", 3); write(STDOUT_FILENO, "^C\n", 3);
G.last_exitcode = 128 | SIGINT; G.last_exitcode = 128 | SIGINT;
} }
if (r < 0) { check_and_run_traps();
/* EOF/error detected */ if (r == 0) /* keyboard ^C? */
continue; /* go back, read another input line */
if (r > 0) /* normal input? (no ^C, no ^D, no signals) */
break;
if (!bb_got_signal) {
/* r < 0: ^D/EOF/error detected (but not signal) */
/* ^D on interactive input goes to next line before exiting: */ /* ^D on interactive input goes to next line before exiting: */
write(STDOUT_FILENO, "\n", 1); write(STDOUT_FILENO, "\n", 1);
i->p = NULL; i->p = NULL;
i->peek_buf[0] = r = EOF; i->peek_buf[0] = r = EOF;
return r; return r;
} }
/* it was a signal: go back, read another input line */
}
i->p = G.user_input_buf; i->p = G.user_input_buf;
return (unsigned char)*i->p++; return (unsigned char)*i->p++;
# else # else