[PATCH] signal: skip blocked signals in dequeue_synchronous_signal()

[PATCH] signal: skip blocked signals in dequeue_synchronous_signal()

[Avi Weiss]

dequeue_synchronous_signal() checks whether an unblocked
synchronous signal is pending, but its queue scan can select a
different blocked entry with a positive si_code. This can deliver
a signal while it is still blocked.

Check the blocked mask for each queued entry. Forced faults
are unblocked when necessary by force_sig_info_to_task(), and the
existing pending bookkeeping is unchanged.

Fixes: 7146db3317c6 ("signal: Better detection of synchronous signals")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---

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.

Patched version behaved as expected.

 kernel/signal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index e433de93b430..4dcc47a7d30d 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -684,12 +684,13 @@ static int dequeue_synchronous_signal(kernel_siginfo_t *info)
 		return 0;
 
 	/*
-	 * Return the first synchronous signal in the queue.
+	 * Return the first unblocked synchronous signal in the queue.
 	 */
 	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: f47d223627f80cb7a1eb14c585fe6cb35de5e984
-- 
2.43.0

Re: [PATCH] signal: skip blocked signals in dequeue_synchronous_signal()

[Oleg Nesterov]

On 10/20, Avi Weiss wrote:
>
> dequeue_synchronous_signal() checks whether an unblocked
> synchronous signal is pending, but its queue scan can select a
> different blocked entry with a positive si_code. This can deliver
> a signal while it is still blocked.

Heh ;) I can't understand how no one noticed this before.

>  	/*
> -	 * Return the first synchronous signal in the queue.
> +	 * Return the first unblocked synchronous signal in the queue.
>  	 */
>  	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;
>  		}

Thanks! I'll send a minor cleanup on top of this patch later.

CC: stable@vger.kernel.org

Acked-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH] signal: skip blocked signals in dequeue_synchronous_signal()

[Christian Brauner]

On Tue, 20 Oct 2026 21:11:01 +0300, Avi Weiss wrote:
> dequeue_synchronous_signal() checks whether an unblocked
> synchronous signal is pending, but its queue scan can select a
> different blocked entry with a positive si_code. This can deliver
> a signal while it is still blocked.
> 
> Check the blocked mask for each queued entry. Forced faults
> are unblocked when necessary by force_sig_info_to_task(), and the
> existing pending bookkeeping is unchanged.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] signal: skip blocked signals in dequeue_synchronous_signal()
      https://git.kernel.org/vfs/vfs/c/2e835d0af134

Re: [PATCH] signal: skip blocked signals in dequeue_synchronous_signal()

[Eric W. Biederman]

Avi Weiss <thnkslprpt@gmail.com> writes:

> dequeue_synchronous_signal() checks whether an unblocked
> synchronous signal is pending, but its queue scan can select a
> different blocked entry with a positive si_code. This can deliver
> a signal while it is still blocked.
>
> Check the blocked mask for each queued entry. Forced faults
> are unblocked when necessary by force_sig_info_to_task(), and the
> existing pending bookkeeping is unchanged.

I am going to quibble a little with your description.

What you describe above as a forced fault, is what the code calls a
synchronous signal.  As such synchronous signals by definition can not
be blocked.  Unfortunately the code does not currently have a way to
distinguish between such a signal number used synchronously, and such a
signal number used normally.

Given the poor state of delivering these things I think the code change
is technically correct.

The comments change is wrong as there is no such thing as a blocked
synchronous signal.

Eric

> Fixes: 7146db3317c6 ("signal: Better detection of synchronous signals")
> Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
> ---
>
> 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.
>
> Patched version behaved as expected.
>
>  kernel/signal.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index e433de93b430..4dcc47a7d30d 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -684,12 +684,13 @@ static int dequeue_synchronous_signal(kernel_siginfo_t *info)
>  		return 0;
>  
>  	/*
> -	 * Return the first synchronous signal in the queue.
> +	 * Return the first unblocked synchronous signal in the queue.
>  	 */
>  	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: f47d223627f80cb7a1eb14c585fe6cb35de5e984