Re: [PATCH v2] signal: respect the blocked mask in dequeue_synchronous_signal()

[Eric W. Biederman]

Avi Weiss <thnkslprpt@xxxxxxxxx> writes:

> dequeue_synchronous_signal() uses the signal number and a positive
> si_code to identify synchronous signals. An ordinarily queued
> signal can also match this heuristic.
>
> The initial pending-mask check only establishes that an unblocked
> signal in SYNCHRONOUS_MASK is pending. The queue scan can select a
> different blocked signal matching the heuristic and deliver it
> despite its mask.
>
> For example, a thread can block SIGSEGV and queue it to itself with
> a positive si_code using rt_tgsigqueueinfo(). An unblocked SIGTRAP
> then allows the scan to select the blocked SIGSEGV.
>
> Check each candidate against the blocked mask. Genuine forced faults
> are unblocked by force_sig_info_to_task() when necessary.
>
> Fixes: 7146db3317c6 ("signal: Better detection of synchronous signals")
> Signed-off-by: Avi Weiss <thnkslprpt@xxxxxxxxx>
> ---

Acked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>

>
> Reproduced by blocking SIGSEGV, queuing a positive-code SIGSEGV
> with rt_tgsigqueueinfo(), then triggering an unblocked SIGTRAP.
> Without the fix, SIGSEGV was delivered while still blocked.
>
> Tested unpatched and patched kernels under QEMU on UP x86-64, with
> test processes running as UID/GID 1000. The unpatched kernel passed
> 21/38 checks and the patched kernel passed 38/38.
>
> These results are from the original patch base. The rebased v2 has
> not been runtime-tested separately.
>
> Changes in v2:
> - Restore the original queue-scan comment following Eric's feedback.
> - Clarify that ordinarily queued signals can match the
>   synchronous-signal heuristic.
> - Rebase onto current mainline.
>
>  kernel/signal.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index cb6ec7054eee..7e690f4fb5bc 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -689,7 +689,8 @@ static int dequeue_synchronous_signal(kernel_siginfo_t *info)
>  	list_for_each_entry(q, &pending->list, list) {
>  		/* Synchronous signals have a positive si_code */
>  		if ((q->info.si_code > SI_USER) &&
> -		    (sigmask(q->info.si_signo) & SYNCHRONOUS_MASK)) {
> +		    (sigmask(q->info.si_signo) & SYNCHRONOUS_MASK) &&
> +		    !sigismember(&tsk->blocked, q->info.si_signo)) {
>  			sync = q;
>  			goto next;
>  		}
>
> base-commit: 4403278e4ea2f654f8130a5c03eb8b4516a0f201