kernel/thread: restore signal mask just before returning to userland

* otherwise the signal to be handled might be blocked. fixes #15193
* also remove automatic syscall restart on _kern_select, to match Linux and
BSDs behavior: this fixes parallel build with newer gnu make, which happens
to use pselect.
* also remove automatic syscall restart on _kern_poll.

from https://man7.org/linux/man-pages/man7/signal.7.html
"The following interfaces are never restarted after being
       interrupted by a signal handler, regardless of the use of
       SA_RESTART; they always fail with the error EINTR when
       interrupted by a signal handler: ...
	select(2), and pselect(2)."
from https://notes.shichao.io/unp/ch6/
"Berkeley-derived kernels never automatically restart select."

Change-Id: I3e9488f60c966b38d427f992f06e6e2217d4adc5
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3636
Reviewed-by: Jérôme Duval <jerome.duval@gmail.com>
Reviewed-by: Axel Dörfler <axeld@pinc-software.de>
This commit is contained in:
Jérôme Duval 2021-01-10 14:57:51 +01:00
parent 6ff344d7c6
commit 29536a2334
3 changed files with 43 additions and 12 deletions

View File

@ -455,6 +455,10 @@ struct Thread : TeamThreadIteratorEntry<thread_id>, KernelReferenceable {
// non-0 after a return from _user_sigsuspend(), containing the inverted // non-0 after a return from _user_sigsuspend(), containing the inverted
// original signal mask, reset in handle_signals(); only accessed by // original signal mask, reset in handle_signals(); only accessed by
// this thread // this thread
sigset_t old_sig_block_mask;
// the old sig_block_mask to be restored when returning to userland
// when THREAD_FLAGS_OLD_SIGMASK is set
ucontext_t* user_signal_context; // only accessed by this thread ucontext_t* user_signal_context; // only accessed by this thread
addr_t signal_stack_base; // only accessed by this thread addr_t signal_stack_base; // only accessed by this thread
size_t signal_stack_size; // only accessed by this thread size_t signal_stack_size; // only accessed by this thread
@ -844,5 +848,7 @@ using BKernel::ProcessGroupList;
#define THREAD_FLAGS_COMPAT_MODE 0x2000 #define THREAD_FLAGS_COMPAT_MODE 0x2000
// the thread runs a compatibility mode (for instance IA32 on x86_64). // the thread runs a compatibility mode (for instance IA32 on x86_64).
#endif #endif
#define THREAD_FLAGS_OLD_SIGMASK 0x4000
// the thread has an old sigmask to be restored
#endif /* _KERNEL_THREAD_TYPES_H */ #endif /* _KERNEL_THREAD_TYPES_H */

View File

@ -1938,6 +1938,16 @@ dump_thread_list(int argc, char **argv)
} }
static void
update_thread_sigmask_on_exit(Thread* thread)
{
if ((thread->flags & THREAD_FLAGS_OLD_SIGMASK) != 0) {
thread->flags &= ~THREAD_FLAGS_OLD_SIGMASK;
sigprocmask(SIG_SETMASK, &thread->old_sig_block_mask, NULL);
}
}
// #pragma mark - private kernel API // #pragma mark - private kernel API
@ -2318,6 +2328,8 @@ thread_at_kernel_exit(void)
disable_interrupts(); disable_interrupts();
update_thread_sigmask_on_exit(thread);
// track kernel time // track kernel time
bigtime_t now = system_time(); bigtime_t now = system_time();
SpinLocker threadTimeLocker(thread->time_lock); SpinLocker threadTimeLocker(thread->time_lock);
@ -2339,6 +2351,8 @@ thread_at_kernel_exit_no_signals(void)
TRACE(("thread_at_kernel_exit_no_signals: exit thread %" B_PRId32 "\n", TRACE(("thread_at_kernel_exit_no_signals: exit thread %" B_PRId32 "\n",
thread->id)); thread->id));
update_thread_sigmask_on_exit(thread);
// track kernel time // track kernel time
bigtime_t now = system_time(); bigtime_t now = system_time();
SpinLocker threadTimeLocker(thread->time_lock); SpinLocker threadTimeLocker(thread->time_lock);

View File

@ -475,15 +475,21 @@ common_select(int numFDs, fd_set *readSet, fd_set *writeSet, fd_set *errorSet,
// set new signal mask // set new signal mask
sigset_t oldSigMask; sigset_t oldSigMask;
if (sigMask != NULL) if (sigMask != NULL) {
sigprocmask(SIG_SETMASK, sigMask, &oldSigMask); sigprocmask(SIG_SETMASK, sigMask, &oldSigMask);
if (!kernel) {
Thread *thread = thread_get_current_thread();
thread->old_sig_block_mask = oldSigMask;
thread->flags |= THREAD_FLAGS_OLD_SIGMASK;
}
}
// wait for something to happen // wait for something to happen
status = acquire_sem_etc(sync->sem, 1, status = acquire_sem_etc(sync->sem, 1,
B_CAN_INTERRUPT | (timeout >= 0 ? B_ABSOLUTE_TIMEOUT : 0), timeout); B_CAN_INTERRUPT | (timeout >= 0 ? B_ABSOLUTE_TIMEOUT : 0), timeout);
// restore the old signal mask // restore the old signal mask
if (sigMask != NULL) if (sigMask != NULL && kernel)
sigprocmask(SIG_SETMASK, &oldSigMask, NULL); sigprocmask(SIG_SETMASK, &oldSigMask, NULL);
PRINT(("common_select(): acquire_sem_etc() returned: %lx\n", status)); PRINT(("common_select(): acquire_sem_etc() returned: %lx\n", status));
@ -927,7 +933,12 @@ _user_select(int numFDs, fd_set *userReadSet, fd_set *userWriteSet,
sigset_t sigMask; sigset_t sigMask;
int result; int result;
syscall_restart_handle_timeout_pre(timeout); if (timeout >= 0) {
timeout += system_time();
// deal with overflow
if (timeout < 0)
timeout = B_INFINITE_TIMEOUT;
}
if (numFDs < 0 || !check_max_fds(numFDs)) if (numFDs < 0 || !check_max_fds(numFDs))
return B_BAD_VALUE; return B_BAD_VALUE;
@ -994,8 +1005,7 @@ _user_select(int numFDs, fd_set *userReadSet, fd_set *userWriteSet,
|| (errorSet != NULL || (errorSet != NULL
&& user_memcpy(userErrorSet, errorSet, bytes) < B_OK))) { && user_memcpy(userErrorSet, errorSet, bytes) < B_OK))) {
result = B_BAD_ADDRESS; result = B_BAD_ADDRESS;
} else }
syscall_restart_handle_timeout_post(result, timeout);
err: err:
free(readSet); free(readSet);
@ -1013,16 +1023,19 @@ _user_poll(struct pollfd *userfds, int numFDs, bigtime_t timeout)
size_t bytes; size_t bytes;
int result; int result;
syscall_restart_handle_timeout_pre(timeout); if (timeout >= 0) {
timeout += system_time();
// deal with overflow
if (timeout < 0)
timeout = B_INFINITE_TIMEOUT;
}
if (numFDs < 0) if (numFDs < 0)
return B_BAD_VALUE; return B_BAD_VALUE;
if (numFDs == 0) { if (numFDs == 0) {
// special case: no FDs // special case: no FDs
result = common_poll(NULL, 0, timeout, false); return common_poll(NULL, 0, timeout, false);
return result < 0
? syscall_restart_handle_timeout_post(result, timeout) : result;
} }
if (!check_max_fds(numFDs)) if (!check_max_fds(numFDs))
@ -1047,9 +1060,7 @@ _user_poll(struct pollfd *userfds, int numFDs, bigtime_t timeout)
if (numFDs > 0 && user_memcpy(userfds, fds, bytes) != 0) { if (numFDs > 0 && user_memcpy(userfds, fds, bytes) != 0) {
if (result >= 0) if (result >= 0)
result = B_BAD_ADDRESS; result = B_BAD_ADDRESS;
} else }
syscall_restart_handle_timeout_post(result, timeout);
err: err:
free(fds); free(fds);