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>

9 files changed, 122 insertions, 41 deletions

diff --git a/editors/vi.c b/editors/vi.c
index 3dbe5b471..d37cd48a3 100644
--- a/editors/vi.c
+++ b/editors/vi.c
@@ -1122,7 +1122,7 @@ static int readit(void) // read (maybe cursor) key from stdin
         // on nonblocking stdin.
         // Note: read_key sets errno to 0 on success.
  again:
-        c = read_key(STDIN_FILENO, readbuffer, /*timeout:*/ -1);
+        c = safe_read_key(STDIN_FILENO, readbuffer, /*timeout:*/ -1);
         if (c == -1) { // EOF/error
                 if (errno == EAGAIN) // paranoia
                         goto again;
@@ -4770,7 +4770,7 @@ static void edit_file(char *fn)
                 uint64_t k;
                 write1(ESC"[999;999H" ESC"[6n");
                 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) {
                         uint32_t rc = (k >> 32);
                         columns = (rc & 0x7fff);
diff --git a/include/libbb.h b/include/libbb.h
index 91b456915..b45ce91c5 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1908,6 +1908,8 @@ enum {
  * >=0: poll() for TIMEOUT milliseconds, return -1/EAGAIN on timeout
  */
 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;
 
 
@@ -1961,7 +1963,8 @@ enum {
         USERNAME_COMPLETION = 4 * ENABLE_FEATURE_USERNAME_COMPLETION,
         VI_MODE          = 8 * ENABLE_FEATURE_EDITING_VI,
         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;
 #if ENABLE_FEATURE_EDITING_SAVEHISTORY
diff --git a/libbb/lineedit.c b/libbb/lineedit.c
index e14c78707..f76afd37d 100644
--- a/libbb/lineedit.c
+++ b/libbb/lineedit.c
@@ -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
                  * "\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.
                  */
-                IF_FEATURE_EDITING_WINCH(S.ok_to_redraw = 1;)
-                ic = read_key(STDIN_FILENO, read_key_buffer, timeout);
-                IF_FEATURE_EDITING_WINCH(S.ok_to_redraw = 0;)
+                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;)
+                        ic = read_key(STDIN_FILENO, read_key_buffer, timeout);
+                        IF_FEATURE_EDITING_WINCH(S.ok_to_redraw = 0;)
+                } while (!(state->flags & LI_INTERRUPTIBLE) && errno == EINTR);
+
                 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 (errno == EAGAIN && unicode_idx != 0)
                                 goto pushback;
diff --git a/libbb/read_key.c b/libbb/read_key.c
index 03b7da656..829ae215c 100644
--- a/libbb/read_key.c
+++ b/libbb/read_key.c
@@ -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 (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 */
                                 errno = EAGAIN;
                                 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
                  * "li" too, and cat was getting wrong input.
                  */
-                n = safe_read(fd, buffer, 1);
+                n = read(fd, buffer, 1);
                 if (n <= 0)
                         return -1;
         }
@@ -284,6 +287,15 @@ int64_t FAST_FUNC read_key(int fd, char *buffer, int timeout)
         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)
 {
         unsigned cur_len = (unsigned char)buffer[0];
diff --git a/miscutils/hexedit.c b/miscutils/hexedit.c
index f8ff9b62b..15ad78377 100644
--- a/miscutils/hexedit.c
+++ b/miscutils/hexedit.c
@@ -292,7 +292,7 @@ int hexedit_main(int argc UNUSED_PARAM, char **argv)
                 fflush_all();
                 G.in_read_key = 1;
                 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;
                 if (bb_got_signal)
                         key = CTRL('X');
diff --git a/miscutils/less.c b/miscutils/less.c
index 82c4b21f0..8a0525cb7 100644
--- a/miscutils/less.c
+++ b/miscutils/less.c
@@ -1137,9 +1137,9 @@ static int64_t getch_nowait(void)
 #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 */
-        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 (errno == EAGAIN) {
                         /* No keyboard input available. Since poll() did return,
diff --git a/procps/top.c b/procps/top.c
index 4cd545c69..804d6f258 100644
--- a/procps/top.c
+++ b/procps/top.c
@@ -913,7 +913,7 @@ static unsigned handle_input(unsigned scan_mask, duration_t interval)
         while (1) {
                 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) {
                         /* error/EOF */
                         option_mask32 |= OPT_EOF;
diff --git a/shell/ash.c b/shell/ash.c
index 086773dd7..55df54bd0 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -3679,7 +3679,9 @@ signal_handler(int signo)
                 if (!trap[SIGCHLD])
                         return;
         }
-
+#if ENABLE_FEATURE_EDITING
+        bb_got_signal = signo; /* for read_line_input: "we got a signal" */
+#endif
         gotsig[signo - 1] = 1;
         pending_sig = signo;
 
@@ -10784,33 +10786,52 @@ preadfd(void)
 # endif
                 reinit_unicode_for_ash();
  again:
-//BUG: not in INT_OFF/INT_ON section - SIGINT et al would longjmp out of read_line_input()!
-//This would cause a memory leak in interactive shell
-//(repeated internal allocations in read_line_input):
-// (while kill -INT $$; do :; done) &
+                /* 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)
+                 */
+                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);
+                if (bb_got_signal == SIGINT)
+                        write(STDOUT_FILENO, "^C\n", 3);
+                INT_ON; /* here non-blocked SIGINT will longjmp */
                 if (nr == 0) {
                         /* ^C pressed, "convert" to SIGINT */
-                        write(STDOUT_FILENO, "^C", 2);
-                        raise(SIGINT);
+                        write(STDOUT_FILENO, "^C\n", 3);
+                        raise(SIGINT); /* here non-blocked SIGINT will longjmp */
                         /* raise(SIGINT) did not work! (e.g. if SIGINT
                          * is SIG_IGNed on startup, it stays SIG_IGNed)
                          */
                         if (trap[SIGINT]) {
+ empty_line_input:
                                 buf[0] = '\n';
                                 buf[1] = '\0';
                                 return 1;
                         }
                         exitstatus = 128 + SIGINT;
                         /* bash behavior on ^C + ignored SIGINT: */
-                        write(STDOUT_FILENO, "\n", 1);
                         goto again;
                 }
                 if (nr < 0) {
                         if (errno == 0) {
-                                /* Ctrl+D pressed */
+                                /* ^D pressed */
                                 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
                         else if (errno == EAGAIN && timeout > 0) {
                                 puts("\007timed out waiting for input: auto-logout");
diff --git a/shell/hush.c b/shell/hush.c
index 7d0dc67e4..6dc2ecaac 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -918,6 +918,7 @@ struct globals {
 #if ENABLE_HUSH_INTERACTIVE
         smallint promptmode; /* 0: PS1, 1: PS2 */
 #endif
+        /* set by signal handler if SIGINT is received _and_ its trap is not set */
         smallint flag_SIGINT;
 #if ENABLE_HUSH_LOOPS
         smallint flag_break_continue;
@@ -1944,6 +1945,9 @@ enum {
 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" */
+#endif
 #if ENABLE_HUSH_FAST
         if (sig == SIGCHLD) {
                 G.count_SIGCHLD++;
@@ -2652,30 +2656,53 @@ static int get_user_input(struct in_str *i)
         for (;;) {
                 reinit_unicode_for_hush();
                 G.flag_SIGINT = 0;
-                /* buglet: SIGINT will not make new prompt to appear _at once_,
-                 * only after <Enter>. (^C works immediately) */
-                r = read_line_input(G.line_input_state, prompt_str,
+
+                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,
                                 G.user_input_buf, CONFIG_FEATURE_EDITING_MAX_LEN-1
-                );
-                /* read_line_input intercepts ^C, "convert" it to SIGINT */
-                if (r == 0) {
-                        raise(SIGINT);
+                        );
+                        /* read_line_input intercepts ^C, "convert" it to SIGINT */
+                        if (r == 0)
+                                raise(SIGINT);
+                }
+                /* bash prints ^C (before running a trap, if any)
+                 * both on keyboard ^C and on real SIGINT (non-kbd generated).
+                 */
+                if (sigismember(&G.pending_set, SIGINT)) {
+                        write(STDOUT_FILENO, "^C\n", 3);
+                        G.last_exitcode = 128 | SIGINT;
                 }
                 check_and_run_traps();
-                if (r != 0 && !G.flag_SIGINT)
+                if (r == 0) /* keyboard ^C? */
+                        continue; /* go back, read another input line */
+                if (r > 0) /* normal input? (no ^C, no ^D, no signals) */
                         break;
-                /* ^C or SIGINT: repeat */
-                /* bash prints ^C even on real SIGINT (non-kbd generated) */
-                write(STDOUT_FILENO, "^C\n", 3);
-                G.last_exitcode = 128 | SIGINT;
-        }
-        if (r < 0) {
-                /* EOF/error detected */
-                /* ^D on interactive input goes to next line before exiting: */
-                write(STDOUT_FILENO, "\n", 1);
-                i->p = NULL;
-                i->peek_buf[0] = r = EOF;
-                return r;
+                if (!bb_got_signal) {
+                        /* r < 0: ^D/EOF/error detected (but not signal) */
+                        /* ^D on interactive input goes to next line before exiting: */
+                        write(STDOUT_FILENO, "\n", 1);
+                        i->p = NULL;
+                        i->peek_buf[0] = r = EOF;
+                        return r;
+                }
+                /* it was a signal: go back, read another input line */
         }
         i->p = G.user_input_buf;
         return (unsigned char)*i->p++;