* [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t
@ 2024-06-11 7:43 Dev Jain
2024-06-11 7:43 ` [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Dev Jain @ 2024-06-11 7:43 UTC (permalink / raw)
To: shuah, oleg, stsp2
Cc: mingo, tglx, mark.rutland, ryan.roberts, broonie, suzuki.poulose,
Anshuman.Khandual, DeepakKumar.Mishra, AneeshKumar.KizhakeVeetil,
linux-kselftest, linux-kernel, Dev Jain
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.
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%)
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal
2024-06-11 7:43 [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
@ 2024-06-11 7:43 ` Dev Jain
2024-06-11 7:43 ` [PATCH v2 2/2] selftests: Add a test mangling with uc_sigmask Dev Jain
2024-06-11 7:49 ` [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
2 siblings, 0 replies; 7+ messages in thread
From: Dev Jain @ 2024-06-11 7:43 UTC (permalink / raw)
To: shuah, oleg, stsp2
Cc: mingo, tglx, mark.rutland, ryan.roberts, broonie, suzuki.poulose,
Anshuman.Khandual, DeepakKumar.Mishra, AneeshKumar.KizhakeVeetil,
linux-kselftest, linux-kernel, Dev Jain
Rename sigaltstack to signal, and rename the existing test to
sigaltstack.c.
Signed-off-by: Dev Jain <dev.jain@arm.com>
---
tools/testing/selftests/Makefile | 2 +-
tools/testing/selftests/{sigaltstack => signal}/.gitignore | 2 +-
tools/testing/selftests/{sigaltstack => signal}/Makefile | 2 +-
.../selftests/{sigaltstack => signal}/current_stack_pointer.h | 0
.../selftests/{sigaltstack/sas.c => signal/sigaltstack.c} | 0
5 files changed, 3 insertions(+), 3 deletions(-)
rename tools/testing/selftests/{sigaltstack => signal}/.gitignore (76%)
rename tools/testing/selftests/{sigaltstack => signal}/Makefile (72%)
rename tools/testing/selftests/{sigaltstack => signal}/current_stack_pointer.h (100%)
rename tools/testing/selftests/{sigaltstack/sas.c => signal/sigaltstack.c} (100%)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 9039f3709aff..eee1031dc18f 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -85,7 +85,7 @@ TARGETS += rtc
TARGETS += rust
TARGETS += seccomp
TARGETS += sgx
-TARGETS += sigaltstack
+TARGETS += signal
TARGETS += size
TARGETS += sparc64
TARGETS += splice
diff --git a/tools/testing/selftests/sigaltstack/.gitignore b/tools/testing/selftests/signal/.gitignore
similarity index 76%
rename from tools/testing/selftests/sigaltstack/.gitignore
rename to tools/testing/selftests/signal/.gitignore
index 50a19a8888ce..98a7bbc4f325 100644
--- a/tools/testing/selftests/sigaltstack/.gitignore
+++ b/tools/testing/selftests/signal/.gitignore
@@ -1,2 +1,2 @@
# SPDX-License-Identifier: GPL-2.0-only
-sas
+sigaltstack
diff --git a/tools/testing/selftests/sigaltstack/Makefile b/tools/testing/selftests/signal/Makefile
similarity index 72%
rename from tools/testing/selftests/sigaltstack/Makefile
rename to tools/testing/selftests/signal/Makefile
index 3e96d5d47036..dd6be992fd81 100644
--- a/tools/testing/selftests/sigaltstack/Makefile
+++ b/tools/testing/selftests/signal/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
CFLAGS = -Wall
-TEST_GEN_PROGS = sas
+TEST_GEN_PROGS = sigaltstack
include ../lib.mk
diff --git a/tools/testing/selftests/sigaltstack/current_stack_pointer.h b/tools/testing/selftests/signal/current_stack_pointer.h
similarity index 100%
rename from tools/testing/selftests/sigaltstack/current_stack_pointer.h
rename to tools/testing/selftests/signal/current_stack_pointer.h
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/signal/sigaltstack.c
similarity index 100%
rename from tools/testing/selftests/sigaltstack/sas.c
rename to tools/testing/selftests/signal/sigaltstack.c
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/2] selftests: Add a test mangling with uc_sigmask
2024-06-11 7:43 [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
2024-06-11 7:43 ` [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
@ 2024-06-11 7:43 ` Dev Jain
2024-06-11 7:49 ` [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
2 siblings, 0 replies; 7+ messages in thread
From: Dev Jain @ 2024-06-11 7:43 UTC (permalink / raw)
To: shuah, oleg, stsp2
Cc: mingo, tglx, mark.rutland, ryan.roberts, broonie, suzuki.poulose,
Anshuman.Khandual, DeepakKumar.Mishra, AneeshKumar.KizhakeVeetil,
linux-kselftest, linux-kernel, Dev Jain
This test asserts the relation between blocked signal, delivered signal,
and ucontext. The ucontext is mangled with, by adding a signal mask to
it; on return from the handler, the thread must block the corresponding
signal.
In the test description, I have also described what it exactly means for
a signal to be delivered or blocked, for ease of clarity.
Signed-off-by: Dev Jain <dev.jain@arm.com>
---
tools/testing/selftests/signal/.gitignore | 1 +
tools/testing/selftests/signal/Makefile | 3 +-
.../selftests/signal/mangle_uc_sigmask.c | 194 ++++++++++++++++++
3 files changed, 197 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/signal/mangle_uc_sigmask.c
diff --git a/tools/testing/selftests/signal/.gitignore b/tools/testing/selftests/signal/.gitignore
index 98a7bbc4f325..397fef11c89f 100644
--- a/tools/testing/selftests/signal/.gitignore
+++ b/tools/testing/selftests/signal/.gitignore
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
+mangle_uc_sigmask
sigaltstack
diff --git a/tools/testing/selftests/signal/Makefile b/tools/testing/selftests/signal/Makefile
index dd6be992fd81..735387a53114 100644
--- a/tools/testing/selftests/signal/Makefile
+++ b/tools/testing/selftests/signal/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
CFLAGS = -Wall
-TEST_GEN_PROGS = sigaltstack
+TEST_GEN_PROGS = mangle_uc_sigmask
+TEST_GEN_PROGS += sigaltstack
include ../lib.mk
diff --git a/tools/testing/selftests/signal/mangle_uc_sigmask.c b/tools/testing/selftests/signal/mangle_uc_sigmask.c
new file mode 100644
index 000000000000..9d4644106465
--- /dev/null
+++ b/tools/testing/selftests/signal/mangle_uc_sigmask.c
@@ -0,0 +1,194 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 ARM Ltd.
+ *
+ * Author: Dev Jain <dev.jain@arm.com>
+ *
+ * Test describing a clear distinction between signal states - delivered and
+ * blocked, and their relation with ucontext.
+ *
+ * A signal is said to be delivered, when the program takes action on the
+ * signal: such action may involve termination of the process, ignoring the
+ * signal, terminating with core dump, stopping the process, or continuing the
+ * process if it was currently stopped. A signal is said to be blocked when the
+ * program refuses to take any of the above actions; note that, this is not the
+ * same as ignoring the signal. At a later time, the program may unblock the
+ * signal and then it will have to take one of the five actions
+ * described above.
+ *
+ * We test the following functionalities of the kernel:
+ *
+ * ucontext_t describes the current state of the thread; this implies that, in
+ * case of registering a handler and catching the corresponding signal, that
+ * state is before what was jumping into the handler.
+ *
+ * The thread's mask of blocked signals can be permanently changed, i.e, not
+ * just during the execution of the handler, by mangling with uc_sigmask
+ * from inside the handler.
+ *
+ * Assume that we block the set of signals, S1, by sigaction(), and say, the
+ * signal for which the handler was installed, is S2. When S2 is sent to the
+ * program, it will be considered "delivered", since we will act on the
+ * signal and jump to the handler. Any instances of S1 or S2 raised, while the
+ * program is executing inside the handler, will be blocked; they will be
+ * delivered immediately upon termination of the handler.
+ *
+ * For standard signals (also see real-time signals in the man page), multiple
+ * blocked instances of the same signal are not queued; such a signal will
+ * be delivered just once.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <ucontext.h>
+
+#include "../kselftest.h"
+
+void handler_verify_ucontext(int signo, siginfo_t *info, void *uc)
+{
+ int ret;
+
+ /* Kernel dumps ucontext with USR2 blocked */
+ ret = sigismember(&(((ucontext_t *)uc)->uc_sigmask), SIGUSR2);
+ ksft_test_result(ret == 1, "USR2 blocked in ucontext\n");
+
+ /*
+ * USR2 is blocked; can be delivered neither here, nor after
+ * exit from handler
+ */
+ if (raise(SIGUSR2))
+ ksft_exit_fail_perror("raise");
+}
+
+void handler_segv(int signo, siginfo_t *info, void *uc)
+{
+ /*
+ * Three cases possible:
+ * 1. Program already terminated due to segmentation fault.
+ * 2. SEGV was blocked even after returning from handler_usr.
+ * 3. SEGV was delivered on returning from handler_usr.
+ * The last option must happen.
+ */
+ ksft_test_result_pass("SEGV delivered\n");
+}
+
+static int cnt;
+
+void handler_usr(int signo, siginfo_t *info, void *uc)
+{
+ int ret;
+
+ /*
+ * Break out of infinite recursion caused by raise(SIGUSR1) invoked
+ * from inside the handler
+ */
+ ++cnt;
+ if (cnt > 1)
+ return;
+
+ ksft_print_msg("In handler_usr\n");
+
+ /* SEGV blocked during handler execution, delivered on return */
+ if (raise(SIGSEGV))
+ ksft_exit_fail_perror("raise");
+
+ ksft_print_msg("SEGV bypassed successfully\n");
+
+ /*
+ * Signal responsible for handler invocation is blocked by default;
+ * delivered on return, leading to recursion
+ */
+ if (raise(SIGUSR1))
+ ksft_exit_fail_perror("raise");
+
+ ksft_test_result(cnt == 1,
+ "USR1 is blocked, cannot invoke handler right now\n");
+
+ /* Raise USR1 again; only one instance must be delivered upon exit */
+ if (raise(SIGUSR1))
+ ksft_exit_fail_perror("raise");
+
+ /* SEGV has been blocked in sa_mask, but ucontext is invariant */
+ ret = sigismember(&(((ucontext_t *)uc)->uc_sigmask), SIGSEGV);
+ ksft_test_result(ret == 0, "USR1 not blocked in ucontext\n");
+
+ /* USR1 has been blocked, but ucontext is invariant */
+ ret = sigismember(&(((ucontext_t *)uc)->uc_sigmask), SIGUSR1);
+ ksft_test_result(ret == 0, "SEGV not blocked in ucontext\n");
+
+ /*
+ * Mangle ucontext; this will be copied back into ¤t->blocked
+ * on return from the handler.
+ */
+ if (sigaddset(&((ucontext_t *)uc)->uc_sigmask, SIGUSR2))
+ ksft_exit_fail_perror("sigaddset");
+}
+
+int main(int argc, char *argv[])
+{
+ struct sigaction act, act2;
+ sigset_t *set, *oldset;
+
+ ksft_print_header();
+ ksft_set_plan(7);
+
+ act.sa_flags = SA_SIGINFO;
+ act.sa_sigaction = &handler_usr;
+
+ /* Add SEGV to blocked mask */
+ if (sigemptyset(&act.sa_mask) || sigaddset(&act.sa_mask, SIGSEGV)
+ || (sigismember(&act.sa_mask, SIGSEGV) != 1))
+ ksft_exit_fail_msg("Cannot add SEGV to blocked mask\n");
+
+ if (sigaction(SIGUSR1, &act, NULL))
+ ksft_exit_fail_perror("Cannot install handler");
+
+ act2.sa_flags = SA_SIGINFO;
+ act2.sa_sigaction = &handler_segv;
+
+ if (sigaction(SIGSEGV, &act2, NULL))
+ ksft_exit_fail_perror("Cannot install handler");
+
+ /* Invoke handler */
+ if (raise(SIGUSR1))
+ ksft_exit_fail_perror("raise");
+
+ /* USR1 must not be queued */
+ ksft_test_result(cnt == 2, "handler invoked only twice\n");
+
+ /* Mangled ucontext implies USR2 is blocked for current thread */
+ if (raise(SIGUSR2))
+ ksft_exit_fail_perror("raise");
+
+ ksft_print_msg("USR2 bypassed successfully\n");
+
+ act.sa_sigaction = &handler_verify_ucontext;
+ if (sigaction(SIGUSR1, &act, NULL))
+ ksft_exit_fail_perror("Cannot install handler");
+
+ if (raise(SIGUSR1))
+ ksft_exit_fail_perror("raise");
+
+ ksft_print_msg("USR2 still blocked on return from handler\n");
+
+ /* Confirm USR2 blockage by sigprocmask() too */
+ set = malloc(sizeof(sigset_t *));
+ if (!set)
+ ksft_exit_fail_perror("malloc");
+
+ oldset = malloc(sizeof(sigset_t *));
+ if (!oldset)
+ ksft_exit_fail_perror("malloc");
+
+ if (sigemptyset(set))
+ ksft_exit_fail_perror("sigemptyset");
+
+ if (sigprocmask(SIG_BLOCK, set, oldset))
+ ksft_exit_fail_perror("sigprocmask");
+
+ ksft_test_result(sigismember(oldset, SIGUSR2) == 1,
+ "USR2 present in ¤t->blocked\n");
+
+ ksft_finished();
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t
2024-06-11 7:43 [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
2024-06-11 7:43 ` [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
2024-06-11 7:43 ` [PATCH v2 2/2] selftests: Add a test mangling with uc_sigmask Dev Jain
@ 2024-06-11 7:49 ` Dev Jain
2 siblings, 0 replies; 7+ messages in thread
From: Dev Jain @ 2024-06-11 7:49 UTC (permalink / raw)
To: shuah, oleg, stsp2
Cc: mingo, tglx, mark.rutland, ryan.roberts, broonie, suzuki.poulose,
Anshuman.Khandual, DeepakKumar.Mishra, AneeshKumar.KizhakeVeetil,
linux-kselftest, linux-kernel
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
> ¤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.
>
> 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%)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t
@ 2024-06-11 7:56 Dev Jain
2024-06-11 7:56 ` [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
0 siblings, 1 reply; 7+ messages in thread
From: Dev Jain @ 2024-06-11 7:56 UTC (permalink / raw)
To: shuah, oleg, stsp2
Cc: mingo, tglx, mark.rutland, ryan.roberts, broonie, suzuki.poulose,
Anshuman.Khandual, DeepakKumar.Mishra, AneeshKumar.KizhakeVeetil,
linux-kselftest, linux-kernel, Dev Jain
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.
Changes in 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
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%)
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal
2024-06-11 7:56 Dev Jain
@ 2024-06-11 7:56 ` Dev Jain
2024-06-11 10:54 ` Mark Brown
0 siblings, 1 reply; 7+ messages in thread
From: Dev Jain @ 2024-06-11 7:56 UTC (permalink / raw)
To: shuah, oleg, stsp2
Cc: mingo, tglx, mark.rutland, ryan.roberts, broonie, suzuki.poulose,
Anshuman.Khandual, DeepakKumar.Mishra, AneeshKumar.KizhakeVeetil,
linux-kselftest, linux-kernel, Dev Jain
Rename sigaltstack to signal, and rename the existing test to
sigaltstack.c.
Signed-off-by: Dev Jain <dev.jain@arm.com>
---
tools/testing/selftests/Makefile | 2 +-
tools/testing/selftests/{sigaltstack => signal}/.gitignore | 2 +-
tools/testing/selftests/{sigaltstack => signal}/Makefile | 2 +-
.../selftests/{sigaltstack => signal}/current_stack_pointer.h | 0
.../selftests/{sigaltstack/sas.c => signal/sigaltstack.c} | 0
5 files changed, 3 insertions(+), 3 deletions(-)
rename tools/testing/selftests/{sigaltstack => signal}/.gitignore (76%)
rename tools/testing/selftests/{sigaltstack => signal}/Makefile (72%)
rename tools/testing/selftests/{sigaltstack => signal}/current_stack_pointer.h (100%)
rename tools/testing/selftests/{sigaltstack/sas.c => signal/sigaltstack.c} (100%)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 9039f3709aff..eee1031dc18f 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -85,7 +85,7 @@ TARGETS += rtc
TARGETS += rust
TARGETS += seccomp
TARGETS += sgx
-TARGETS += sigaltstack
+TARGETS += signal
TARGETS += size
TARGETS += sparc64
TARGETS += splice
diff --git a/tools/testing/selftests/sigaltstack/.gitignore b/tools/testing/selftests/signal/.gitignore
similarity index 76%
rename from tools/testing/selftests/sigaltstack/.gitignore
rename to tools/testing/selftests/signal/.gitignore
index 50a19a8888ce..98a7bbc4f325 100644
--- a/tools/testing/selftests/sigaltstack/.gitignore
+++ b/tools/testing/selftests/signal/.gitignore
@@ -1,2 +1,2 @@
# SPDX-License-Identifier: GPL-2.0-only
-sas
+sigaltstack
diff --git a/tools/testing/selftests/sigaltstack/Makefile b/tools/testing/selftests/signal/Makefile
similarity index 72%
rename from tools/testing/selftests/sigaltstack/Makefile
rename to tools/testing/selftests/signal/Makefile
index 3e96d5d47036..dd6be992fd81 100644
--- a/tools/testing/selftests/sigaltstack/Makefile
+++ b/tools/testing/selftests/signal/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
CFLAGS = -Wall
-TEST_GEN_PROGS = sas
+TEST_GEN_PROGS = sigaltstack
include ../lib.mk
diff --git a/tools/testing/selftests/sigaltstack/current_stack_pointer.h b/tools/testing/selftests/signal/current_stack_pointer.h
similarity index 100%
rename from tools/testing/selftests/sigaltstack/current_stack_pointer.h
rename to tools/testing/selftests/signal/current_stack_pointer.h
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/signal/sigaltstack.c
similarity index 100%
rename from tools/testing/selftests/sigaltstack/sas.c
rename to tools/testing/selftests/signal/sigaltstack.c
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal
2024-06-11 7:56 ` [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
@ 2024-06-11 10:54 ` Mark Brown
2024-06-11 11:02 ` Dev Jain
0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2024-06-11 10:54 UTC (permalink / raw)
To: Dev Jain
Cc: shuah, oleg, stsp2, mingo, tglx, mark.rutland, ryan.roberts,
suzuki.poulose, Anshuman.Khandual, DeepakKumar.Mishra,
AneeshKumar.KizhakeVeetil, linux-kselftest, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
On Tue, Jun 11, 2024 at 01:26:49PM +0530, Dev Jain wrote:
> Rename sigaltstack to signal, and rename the existing test to
> sigaltstack.c.
>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
If people review patches and give a tag for them you should carry the
tag forward to avoid having to duplicate the review. From v1:
I think this is reasonable if we're going to add more generic signal
tests - sigaltstack is a fairly small bit of functionality and having it
covered as part of a broader signal suite and the overhead of setting up
the suite separately is probably not worth it.
Reviewed-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal
2024-06-11 10:54 ` Mark Brown
@ 2024-06-11 11:02 ` Dev Jain
0 siblings, 0 replies; 7+ messages in thread
From: Dev Jain @ 2024-06-11 11:02 UTC (permalink / raw)
To: Mark Brown
Cc: shuah, oleg, stsp2, mingo, tglx, mark.rutland, ryan.roberts,
suzuki.poulose, Anshuman.Khandual, DeepakKumar.Mishra,
AneeshKumar.KizhakeVeetil, linux-kselftest, linux-kernel
On 6/11/24 16:24, Mark Brown wrote:
> On Tue, Jun 11, 2024 at 01:26:49PM +0530, Dev Jain wrote:
>> Rename sigaltstack to signal, and rename the existing test to
>> sigaltstack.c.
>>
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>> ---
> If people review patches and give a tag for them you should carry the
> tag forward to avoid having to duplicate the review. From v1:
>
> I think this is reasonable if we're going to add more generic signal
> tests - sigaltstack is a fairly small bit of functionality and having it
> covered as part of a broader signal suite and the overhead of setting up
> the suite separately is probably not worth it.
>
> Reviewed-by: Mark Brown <broonie@kernel.org>
Ah yes, I did know it, I forgot. Shall take care.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-06-11 11:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-11 7:43 [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
2024-06-11 7:43 ` [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
2024-06-11 7:43 ` [PATCH v2 2/2] selftests: Add a test mangling with uc_sigmask Dev Jain
2024-06-11 7:49 ` [PATCH v2 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
-- strict thread matches above, loose matches on Subject: below --
2024-06-11 7:56 Dev Jain
2024-06-11 7:56 ` [PATCH v2 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
2024-06-11 10:54 ` Mark Brown
2024-06-11 11:02 ` Dev Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox