Received-SPF: pass (google.com: domain of linux-kernel+bounces-209415-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <8918a555-1ed4-46da-bd63-4c5e324a9284@arm.com>
Date: Tue, 11 Jun 2024 13:19:52 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 0/2] Add test to distinguish between thread's signal
 mask and ucontext_t
To: shuah@kernel.org, oleg@redhat.com, stsp2@yandex.ru
Cc: mingo@kernel.org, tglx@linutronix.de, mark.rutland@arm.com,
 ryan.roberts@arm.com, broonie@kernel.org, suzuki.poulose@arm.com,
 Anshuman.Khandual@arm.com, DeepakKumar.Mishra@arm.com,
 AneeshKumar.KizhakeVeetil@arm.com, linux-kselftest@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240611074307.812939-1-dev.jain@arm.com>
Content-Language: en-US
From: Dev Jain <dev.jain@arm.com>
In-Reply-To: <20240611074307.812939-1-dev.jain@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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%)
>