Re: [PATCH] signal: fix lockup in complete_signal() racing with mt exec

[Nickolai Zeldovich]

Thanks for taking this over, and for the better fix!

On Sat, Oct 31, 2026 at 9:47 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> Nickolai Zeldovich says:
>
>     kill(2) of a process while a non-leader thread of it is in execve()
>     can lock up the machine: complete_signal() loops forever with siglock
>     held and interrupts off, and de_thread() then spins on siglock in
>     release_task() with tasklist_lock held.
>
>     The curr_target walk uses next_thread(), which wraps from the last
>     thread to ->group_leader on the assumption that the leader is first
>     on signal->thread_head. de_thread() makes the exec'ing thread, the
>     last one, the leader before release_task() unlinks the old leader,
>     so a walk that starts at the old leader stays on the new one if
>     that thread blocks the signal, and never gets back to curr_target.
>
> Change complete_signal() to use list_next_entry_circular() instead
> of next_thread().
>
> Other users of next_thread/while_each_thread/for_other_threads look safe,
> so this patch only changes complete_signal() to make a simple fix for
> -stable. However, in the longer term we should update next_thread()
> or add another helper, but this needs more work; next_thread() should
> work even when signal->thread_head is empty.
>
> Fixes: d639cf4abb4d ("change next_thread() to use __next_thread() ?: group_leader")
> Reported-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
> Link: https://lore.kernel.org/all/CADGQE7EusFrrSoBnhXhdE20tZJrp1EDe=2623mS8vEMY9hn53A@xxxxxxxxxxxxxx/
> Originally-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> ---
>  kernel/signal.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 2474507ac92b..bf5518be67a2 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -987,7 +987,8 @@ static void complete_signal(int sig, struct task_struct *p, enum pid_type type)
>                  */
>                 t = signal->curr_target;
>                 while (!wants_signal(sig, t)) {
> -                       t = next_thread(t);
> +                       t = list_next_entry_circular(t, &signal->thread_head,
> +                                                       thread_node);
>                         if (t == signal->curr_target)
>                                 /*
>                                  * No thread needs to be woken.
> --
> 2.55.0
>
>

Reviewed-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
Tested-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>

Nickolai.