Message-ID: <cfa882bf-a8dd-4b0a-98ea-4551a10df466@linuxfoundation.org>
Date: Tue, 29 Oct 2024 01:07:37 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Dev Jain <dev.jain@....com>, shuah@...nel.org, oleg@...hat.com
Cc: mingo@...nel.org, tglx@...utronix.de, bp@...en8.de,
 dave.hansen@...ux.intel.com, 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,
 sj@...nel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [RESEND] [PATCH v6 0/2] Add test to distinguish between thread's
 signal mask and ucontext_t

On 10/8/24 23:14, 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.
> 
> v5->v6:
>   - Drop renaming of sas.c
>   - Include the explanation from the cover letter in the changelog
>     for the second patch

These two patches have been languishing for sometime - hence I applied these
two patches to linux-kselftest next for Linux 6.13-rc1

thanks,
-- Shuah

Powered by blists - more mailing lists