[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4d5758b8-b1ef-4c6d-8fda-2a7c2d48d975@arm.com>
Date: Tue, 16 Jul 2024 15:14:16 +0530
From: Dev Jain <dev.jain@....com>
To: shuah@...nel.org, oleg@...hat.com
Cc: mingo@...nel.org, tglx@...utronix.de, mark.rutland@....com,
ryan.roberts@....com, broonie@...nel.org, suzuki.poulose@....com,
Anshuman.Khandual@....com, DeepakKumar.Mishra@....com,
aneesh.kumar@...nel.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 0/2] Add test to distinguish between thread's signal
mask and ucontext_t
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.
>
Powered by blists - more mailing lists