36ac28368

Denys Vlasenko

2025-07-02

SUCCESS Duration: 157s

Subject

shell: fix race between signal handlers setting bb_got_signal and poll()

No binary (build failed)

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

—

(log yok)

include/libbb.h+1 -0

@@ -1951,6 +1951,7 @@ int64_t read_key(int fd, char *buffer, int timeout) FAST_FUNC;
	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;

+	int check_got_signal_and_poll(struct pollfd pfd[1], int timeout) FAST_FUNC;

	#if ENABLE_FEATURE_EDITING
	/* It's NOT just ENABLEd or disabled. It's a number: */

libbb/lineedit.c+0 -1

@@ -2195,7 +2195,6 @@ static int lineedit_read_key(char *read_key_buffer, int timeout)
	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;)
	/* errno = 0; - read_key does this itself */
	ic = read_key(STDIN_FILENO, read_key_buffer, timeout);

libbb/poll_with_signals.c+48 -0

@@ -0,0 +1,48 @@
+	/* vi: set sw=4 ts=4: */
+	/*
+	* Utility routines.
+	*
+	* Copyright (C) 2025 Denys Vlasenko <[email protected]>
+	*
+	* Licensed under GPLv2, see file LICENSE in this source tree.
+	*/
+	//kbuild:lib-y += poll_with_signals.o
+
+	#include "libbb.h"
+
+	/* Shells, for example, need their line input and "read" builtin
+	* to be interruptible, and the naive handling of it a-la:
+	* if (bb_got_signal) {
+	* errno = EINTR;
+	* return -1;
+	* }
+	* poll(pfd, 1, -1); // signal here would set EINTR
+	* is racy.
+	* This is a bit heavy-handed, but safe wrt races:
+	*/
+	int FAST_FUNC check_got_signal_and_poll(struct pollfd pfd[1], int timeout)
+	{
+	int n;
+	struct timespec tv;
+	sigset_t orig_mask;
+
+	if (bb_got_signal) /* optimization */
+	goto eintr;
+
+	if (timeout >= 0) {
+	tv.tv_sec = timeout / 1000;
+	tv.tv_nsec = (timeout % 1000) * 1000000;
+	}
+	/* test bb_got_signal, then poll(), atomically wrt signals */
+	sigfillset(&orig_mask);
+	sigprocmask2(SIG_BLOCK, &orig_mask);
+	if (bb_got_signal) {
+	sigprocmask2(SIG_SETMASK, &orig_mask);
+	eintr:
+	errno = EINTR; /* inform the caller that we got a signal */
+	return -1;
+	}
+	n = ppoll(pfd, 1, timeout >= 0 ? &tv : NULL, &orig_mask);
+	sigprocmask2(SIG_SETMASK, &orig_mask);
+	return n;
+	}

libbb/read_key.c+9 -5

@@ -121,12 +121,16 @@ int64_t FAST_FUNC read_key(int fd, char *buffer, int timeout)
	errno = 0;
	n = (unsigned char)buffer[-1];
	if (n == 0) {
-	/* If no data, wait for input.
-	* If requested, wait TIMEOUT ms. TIMEOUT = -1 is useful
-	* if fd can be in non-blocking mode.
-	*/
+	/* No data. Wait for input. */
+
+	/* timeout == -2 means "do not poll". Else: */
	if (timeout >= -1) {
-	n = poll(&pfd, 1, timeout);
+	/* We must poll even if timeout == -1:
+	* we want to be interrupted if signal arrives,
+	* regardless of SA_RESTART-ness of that signal!
+	*/
+	/* test bb_got_signal, then poll(), atomically wrt signals */
+	n = check_got_signal_and_poll(pfd, timeout);
	if (n < 0 && errno == EINTR)
	return n;
	if (n == 0) {

miscutils/less.c+1 -1

@@ -1139,7 +1139,7 @@ static int64_t getch_nowait(void)

	/* We have kbd_fd in O_NONBLOCK mode, read inside safe_read_key()
	* would not block even if there is no input available */
-	key64 = safe_read_key(kbd_fd, kbd_input, /timeout off:/ -2);
+	key64 = safe_read_key(kbd_fd, kbd_input, /do not poll:/ -2);
	if ((int)key64 == -1) {
	if (errno == EAGAIN) {
	/* No keyboard input available. Since poll() did return,

shell/ash.c+1 -1

@@ -3687,7 +3687,7 @@ signal_handler(int signo)
	return;
	}
	#if ENABLE_FEATURE_EDITING
-	bb_got_signal = signo; /* for read_line_input: "we got a signal" */
+	bb_got_signal = signo; /* for read_line_input / read builtin: "we got a signal" */
	#endif
	gotsig[signo - 1] = 1;
	pending_sig = signo;

shell/hush.c+1 -1

@@ -1973,7 +1973,7 @@ static void record_pending_signo(int sig)
	\|\| (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" */
+	bb_got_signal = sig; /* for read_line_input / read builtin: "we got a signal" */
	}
	#endif
	#if ENABLE_HUSH_FAST

shell/shell_common.c+5 -4

@@ -214,12 +214,13 @@ shell_builtin_read(struct builtin_read_params *params)
	* regardless of SA_RESTART-ness of that signal!
	*/
	errno = 0;
-	pfd[0].events = POLLIN;
-	//TODO race with a signal arriving just before the poll!
-	if (poll(pfd, 1, timeout) <= 0) {
+	pfd->events = POLLIN;
+
+	/* test bb_got_signal, then poll(), atomically wrt signals */
+	if (check_got_signal_and_poll(pfd, timeout) <= 0) {
	/* timed out, or some error */
	err = errno;
-	if (!err) {
+	if (!err) { /* timed out */
	retval = (const char *)(uintptr_t)2;
	break;
	}