← Commit list

12566e7f9

Denys Vlasenko
Denys Vlasenko
2022-01-17
SUCCESS Duration: 156s

Subject

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

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

editors/vi.c +2 -2include/libbb.h +4 -1libbb/lineedit.c +21 -3libbb/read_key.c +14 -2miscutils/hexedit.c +1 -1miscutils/less.c +2 -2procps/top.c +1 -1shell/ash.c +30 -9shell/hush.c +47 -20
editors/vi.c+2 -2
@@ -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);
include/libbb.h+4 -1
@@ -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
libbb/lineedit.c+21 -3
@@ -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;
libbb/read_key.c+14 -2
@@ -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];
miscutils/hexedit.c+1 -1
@@ -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');
miscutils/less.c+2 -2
@@ -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,
procps/top.c+1 -1
@@ -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;
shell/ash.c+30 -9
@@ -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");
shell/hush.c+47 -20
@@ -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++;