Received-SPF: pass (google.com: domain of linux-kernel+bounces-210919-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <ec1973ee-909d-41a2-8b32-256302d190b4@arm.com>
Date: Wed, 12 Jun 2024 10:14:01 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/2] selftests: Add a test mangling with uc_sigmask
To: Mark Brown <broonie@kernel.org>
Cc: shuah@kernel.org, oleg@redhat.com, stsp2@yandex.ru, mingo@kernel.org,
 tglx@linutronix.de, mark.rutland@arm.com, ryan.roberts@arm.com,
 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: <20240611075650.814397-1-dev.jain@arm.com>
 <20240611075650.814397-3-dev.jain@arm.com>
 <Zmg0GoGnJFbPysfK@finisterre.sirena.org.uk>
Content-Language: en-US
From: Dev Jain <dev.jain@arm.com>
In-Reply-To: <Zmg0GoGnJFbPysfK@finisterre.sirena.org.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 6/11/24 16:55, Mark Brown wrote:
> On Tue, Jun 11, 2024 at 01:26:50PM +0530, Dev Jain wrote:
>
>> + * 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.
> I'm not sure that's what my understanding of a blocked signal is, I
> would interpret "blocked" as a signal being masked (this usage can be
> seen in for example sigaction(2)).  I'd also interpret delivery of the
> signal as happening when the signal handler is invoked rather than
> something that the handler has control over (the comment later on says
> that so I think it's just an issue here).  Perhaps I'm confused about
> terminology though, this is just usage I've picked up and ICBW.

Isn't "signal being masked" equivalent to what I wrote...
man signal(7): Under "Signal mask and pending signals":-
"A signal may be blocked, which means that it will not be delivered
until it is later unblocked."
Under "Signal dispositions":-
"Each signal has a current disposition, which determines how the
process behaves when it is delivered the signal."

The above must imply that, the delivery of a signal implies a signal
disposition coming into picture; so in case of blocked signal, the
following should happen:
Set disposition (default, ignore, or jump to handler) -> block SIG_x using,
say, sigprocmask() -> raise(SIG_x) -> nothing happens, do normal work ->
unblock SIG_x by sigprocmask() -> immediately act on disposition, since the
signal will be delivered.
When I wrote "such action may involve termination of the process..." I should
have also included "or jump to a signal handler".

"The comment later on says that", which comment and what does it say,
sorry didn't get you.

>
>> + * 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.
> See also SA_NODEFER.

Yes, thanks for the note, but do  need to include it in the
comments? This is a specific setting...

>
>> +	/* SEGV has been blocked in sa_mask, but ucontext is invariant */
>> +	ret = sigismember(&(((ucontext_t *)uc)->uc_sigmask), SIGSEGV);
>> +	ksft_test_result(ret == 0, "SEGV 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, "USR1 not blocked in ucontext\n");
> We're not manipulating the masks outside of main() so it's a bit unclear
> what the mention of ucontext being invariant is all about here?

This is the point I raised in the cover letter and in this program:  the mask
stores the set of blocked signals. What should happen when I block signals
using sigaction()? According to the man pages, one could easily come to
an erroneous conclusion that these signals will also be present as blocked
in ucontext. I am making a point that, SEGV and USR1 have been blocked,
but they have not been added into ucontext, i.e ucontext is invariant w.r.t
to before and in the handler.

>
>> +	/* 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");
> But we just raised SIGUSR1 rather than SIGUSR2?  If nothing else this
> bit is a little unclear.

Before raise(SIGUSR1), we register a handler for it: handler_verify_ucontext.
So, we jump there, we verify that USR2 is present in ucontext (since we mangled
with ucontext before), then we raise(SIGUSR2): the program must not terminate
since USR2 is blocked in &current->blocked. This is described by ksft_print_msg().