mbox series

[v2,0/2] Add test to distinguish between thread's signal mask and ucontext_t

Message ID 20240611074307.812939-1-dev.jain@arm.com (mailing list archive)
Headers show
Series Add test to distinguish between thread's signal mask and ucontext_t | expand

Message

Dev Jain June 11, 2024, 7:43 a.m. UTC
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
&current->blocked, is passed to setup_rt_frame() -> setup_sigframe() ->
__copy_to_user(). Hence, &current->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 &current->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.

Changes in v2:
- Replace all occurrences of SIGPIPE with SIGSEGV
- 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

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      | 194 ++++++++++++++++++
 .../sas.c => signal/sigaltstack.c}            |   0
 6 files changed, 199 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%)

Comments

Dev Jain June 11, 2024, 7:49 a.m. UTC | #1
Sorry, please ignore this email thread; I am sending another one.

On 6/11/24 13:13, 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
> &current->blocked, is passed to setup_rt_frame() -> setup_sigframe() ->
> __copy_to_user(). Hence, &current->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 &current->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.
>
> Changes in v2:
> - Replace all occurrences of SIGPIPE with SIGSEGV
> - 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
>
> 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      | 194 ++++++++++++++++++
>   .../sas.c => signal/sigaltstack.c}            |   0
>   6 files changed, 199 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%)
>