| Message ID | 20240627035215.1527279-1-dev.jain@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add test to distinguish between thread's signal mask and ucontext_t | expand |
On 6/27/24 09:22, Dev Jain wrote: > This patch series is motivated by the following observation: > > Raise a signal, jump to signal handler. The ucontext_t structure dumped > by kernel to userspace has a uc_sigmask field having the mask of blocked > signals. If you run a fresh minimalistic program doing this, this field > is empty, even if you block some signals while registering the handler > with sigaction(). > > Here is what the man-pages have to say: > > sigaction(2): "sa_mask specifies a mask of signals which should be blocked > (i.e., added to the signal mask of the thread in which the signal handler > is invoked) during execution of the signal handler. In addition, the > signal which triggered the handler will be blocked, unless the SA_NODEFER > flag is used." > > signal(7): Under "Execution of signal handlers", (1.3) implies: > > "The thread's current signal mask is accessible via the ucontext_t > object that is pointed to by the third argument of the signal handler." > > But, (1.4) states: > > "Any signals specified in act->sa_mask when registering the handler with > sigprocmask(2) are added to the thread's signal mask. The signal being > delivered is also added to the signal mask, unless SA_NODEFER was > specified when registering the handler. These signals are thus blocked > while the handler executes." > > There clearly is no distinction being made in the man pages between > "Thread's signal mask" and ucontext_t; this logically should imply > that a signal blocked by populating struct sigaction should be visible > in ucontext_t. > > Here is what the kernel code does (for Aarch64): > > do_signal() -> handle_signal() -> sigmask_to_save(), which returns > ¤t->blocked, is passed to setup_rt_frame() -> setup_sigframe() -> > __copy_to_user(). Hence, ¤t->blocked is copied to ucontext_t > exposed to userspace. Returning back to handle_signal(), > signal_setup_done() -> signal_delivered() -> sigorsets() and > set_current_blocked() are responsible for using information from > struct ksignal ksig, which was populated through the sigaction() > system call in kernel/signal.c: > copy_from_user(&new_sa.sa, act, sizeof(new_sa.sa)), > to update ¤t->blocked; hence, the set of blocked signals for the > current thread is updated AFTER the kernel dumps ucontext_t to > userspace. > > Assuming that the above is indeed the intended behaviour, because it > semantically makes sense, since the signals blocked using sigaction() > remain blocked only till the execution of the handler, and not in the > context present before jumping to the handler (but nothing can be > confirmed from the man-pages), the series introduces a test for > mangling with uc_sigmask. I will send a separate series to fix the > man-pages. > > The proposed selftest has been tested out on Aarch32, Aarch64 and x86_64. > > v3->v4: > - Allocate sigsets as automatic variables to avoid malloc() > > v2->v3: > - ucontext describes current state -> ucontext describes interrupted context > - Add a comment for blockage of USR2 even after return from handler > - Describe blockage of signals in a better way > > v1->v2: > - Replace all occurrences of SIGPIPE with SIGSEGV > - Fixed a mismatch between code comment and ksft log > - Add a testcase: Raise the same signal again; it must not be queued > - Remove unneeded <assert.h>, <unistd.h> > - Give a detailed test description in the comments; also describe the > exact meaning of delivered and blocked > - Handle errors for all libc functions/syscalls > - Mention tests in Makefile and .gitignore in alphabetical order > > v1: > - https://lore.kernel.org/all/20240607122319.768640-1-dev.jain@arm.com/ > > Dev Jain (2): > selftests: Rename sigaltstack to generic signal > selftests: Add a test mangling with uc_sigmask > > tools/testing/selftests/Makefile | 2 +- > .../{sigaltstack => signal}/.gitignore | 3 +- > .../{sigaltstack => signal}/Makefile | 3 +- > .../current_stack_pointer.h | 0 > .../selftests/signal/mangle_uc_sigmask.c | 186 ++++++++++++++++++ > .../sas.c => signal/sigaltstack.c} | 0 > 6 files changed, 191 insertions(+), 3 deletions(-) > rename tools/testing/selftests/{sigaltstack => signal}/.gitignore (57%) > rename tools/testing/selftests/{sigaltstack => signal}/Makefile (53%) > rename tools/testing/selftests/{sigaltstack => signal}/current_stack_pointer.h (100%) > create mode 100644 tools/testing/selftests/signal/mangle_uc_sigmask.c > rename tools/testing/selftests/{sigaltstack/sas.c => signal/sigaltstack.c} (100%) If everything is fine, can this patchset be pulled? Thanks. >
This patch series is motivated by the following observation: Raise a signal, jump to signal handler. The ucontext_t structure dumped by kernel to userspace has a uc_sigmask field having the mask of blocked signals. If you run a fresh minimalistic program doing this, this field is empty, even if you block some signals while registering the handler with sigaction(). Here is what the man-pages have to say: sigaction(2): "sa_mask specifies a mask of signals which should be blocked (i.e., added to the signal mask of the thread in which the signal handler is invoked) during execution of the signal handler. In addition, the signal which triggered the handler will be blocked, unless the SA_NODEFER flag is used." signal(7): Under "Execution of signal handlers", (1.3) implies: "The thread's current signal mask is accessible via the ucontext_t object that is pointed to by the third argument of the signal handler." But, (1.4) states: "Any signals specified in act->sa_mask when registering the handler with sigprocmask(2) are added to the thread's signal mask. The signal being delivered is also added to the signal mask, unless SA_NODEFER was specified when registering the handler. These signals are thus blocked while the handler executes." There clearly is no distinction being made in the man pages between "Thread's signal mask" and ucontext_t; this logically should imply that a signal blocked by populating struct sigaction should be visible in ucontext_t. Here is what the kernel code does (for Aarch64): do_signal() -> handle_signal() -> sigmask_to_save(), which returns ¤t->blocked, is passed to setup_rt_frame() -> setup_sigframe() -> __copy_to_user(). Hence, ¤t->blocked is copied to ucontext_t exposed to userspace. Returning back to handle_signal(), signal_setup_done() -> signal_delivered() -> sigorsets() and set_current_blocked() are responsible for using information from struct ksignal ksig, which was populated through the sigaction() system call in kernel/signal.c: copy_from_user(&new_sa.sa, act, sizeof(new_sa.sa)), to update ¤t->blocked; hence, the set of blocked signals for the current thread is updated AFTER the kernel dumps ucontext_t to userspace. Assuming that the above is indeed the intended behaviour, because it semantically makes sense, since the signals blocked using sigaction() remain blocked only till the execution of the handler, and not in the context present before jumping to the handler (but nothing can be confirmed from the man-pages), the series introduces a test for mangling with uc_sigmask. I will send a separate series to fix the man-pages. The proposed selftest has been tested out on Aarch32, Aarch64 and x86_64. v3->v4: - Allocate sigsets as automatic variables to avoid malloc() v2->v3: - ucontext describes current state -> ucontext describes interrupted context - Add a comment for blockage of USR2 even after return from handler - Describe blockage of signals in a better way v1->v2: - Replace all occurrences of SIGPIPE with SIGSEGV - Fixed a mismatch between code comment and ksft log - Add a testcase: Raise the same signal again; it must not be queued - Remove unneeded <assert.h>, <unistd.h> - Give a detailed test description in the comments; also describe the exact meaning of delivered and blocked - Handle errors for all libc functions/syscalls - Mention tests in Makefile and .gitignore in alphabetical order v1: - https://lore.kernel.org/all/20240607122319.768640-1-dev.jain@arm.com/ Dev Jain (2): selftests: Rename sigaltstack to generic signal selftests: Add a test mangling with uc_sigmask tools/testing/selftests/Makefile | 2 +- .../{sigaltstack => signal}/.gitignore | 3 +- .../{sigaltstack => signal}/Makefile | 3 +- .../current_stack_pointer.h | 0 .../selftests/signal/mangle_uc_sigmask.c | 186 ++++++++++++++++++ .../sas.c => signal/sigaltstack.c} | 0 6 files changed, 191 insertions(+), 3 deletions(-) rename tools/testing/selftests/{sigaltstack => signal}/.gitignore (57%) rename tools/testing/selftests/{sigaltstack => signal}/Makefile (53%) rename tools/testing/selftests/{sigaltstack => signal}/current_stack_pointer.h (100%) create mode 100644 tools/testing/selftests/signal/mangle_uc_sigmask.c rename tools/testing/selftests/{sigaltstack/sas.c => signal/sigaltstack.c} (100%)