Re: [PATCH] signal/x86: Don't send SIGSEGV twice on SEGV_PKUERR

[Eric W. Biederman]

Borislav Petkov <bp@xxxxxxxxx> writes:

> On Tue, Jun 01, 2021 at 04:52:03PM +0800, Jiashuo Liang wrote:
>> Before this patch, the __bad_area_nosemaphore function calls both
>> force_sig_pkuerr and force_sig_fault when handling SEGV_PKUERR. This does
>> not cause problems because the second signal is filtered by the
>> legacy_queue check in __send_signal.
>
> I'm likely missing something but the first signal gets filtered by that
> same legacy_queue() check too, no?
>
> Because both calls end up in
>
> 	force_sig_info_to_task(info, current);
>
> with the info struct populated with:
>
> 	info.si_signo = SIGSEGV;
>         info.si_errno = 0;
>         info.si_code  = SEGV_PKUERR;
>         info.si_addr  = addr;
>         info.si_pkey  = pkey;
>
> except the second call - force_sig_fault() - doesn't put pkey in
> ->si_pkey.
>
> So what's up?

There are two ways signals get delivered.  The old fashioned way in the
signal bitmap, and the new fangled way by queuing sigqueue_info.  In the
old fashioned way there is no information except that the signal itself
was delivered, and if the signal is sent twice it is impossible to find
out.  In the new fangled way because the sigqueue_info can vary between
different times a signal is sent you can both see that a signal was
delivered twice (because there are two distinct entries in the queue),
but also possibly tell those two times a signal was sent apart.

The new real time signals can queue as many sigqueue_info's as their
rlimit allows.  The old signals are limited to exactly one sigqueue_info
per signal number.

In this case the legacy_queue check tests to see if the signal is
already pending (present in the signal bitmap) and not a new real time
signal (which means only one sigqueue_info entry is allowed in the
signal queue).

Or in short I think everything turns out ok because the first signal is
delivered, and the second just happens to get dropped as a duplicate by
__send_signal.  That is fragile and confusing to depend on so we should
just fix the code to not send the wrong signal.

> Thx.

I hope that clears things up.

Eric