All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: Thomas Gleixner <tglx@kernel.org>
Cc: Hyunwoo Kim <imv4bel@gmail.com>,
	oleg@redhat.com, brauner@kernel.org, peterz@infradead.org,
	anna-maria@linutronix.de, ebiederm@xmission.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] signal: Use list_del_init_careful() in flush_sigqueue()
Date: Mon, 24 Aug 2026 18:31:44 +0200	[thread overview]
Message-ID: <aoxx8ApYkXzMv_Vb@localhost.localdomain> (raw)
In-Reply-To: <8733w3j1i1.ffs@fw13>

Le Mon, Aug 24, 2026 at 11:45:10AM +0200, Thomas Gleixner a écrit :
> On Mon, Aug 24 2026 at 10:04, Thomas Gleixner wrote:
> 
> > On Sat, Aug 22 2026 at 14:37, Hyunwoo Kim wrote:
> >> diff --git a/kernel/signal.c b/kernel/signal.c
> >> index bbc0fd4cc4d7c1..ec9a0a0490d19f 100644
> >> --- a/kernel/signal.c
> >> +++ b/kernel/signal.c
> >> @@ -482,7 +482,11 @@ void flush_sigqueue(struct sigpending *queue)
> >>  	sigemptyset(&queue->signal);
> >>  	while (!list_empty(&queue->list)) {
> >>  		q = list_entry(queue->list.next, struct sigqueue , list);
> >> -		list_del_init(&q->list);
> >> +		/*
> >> +		 * Pairs with the list_empty() in posixtimer_send_sigqueue().
> >
> > No. That list_empty() would need to be changed to list_empty_careful()
> > to be correct on weakly ordered architectures.
> >
> > Aside of that I'm not convinced that this is the right way to handle
> > this as it cures the symptom and not the underlying problem. Let me
> > stare at this some more.
> 
> Something like the untested below.
> 
> Thanks,
> 
>         tglx
> ---
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -983,6 +983,18 @@ static int de_thread(struct task_struct
>  		}
>  
>  		/*
> +		 * Ensure that POSIX timer SIGEV_THREAD_ID signals pending for
> +		 * the former leader are removed under sighand::siglock _before_
> +		 * taking over the leader's TID. Otherwise the lockless cleanup
> +		 * in release_task() can race against a concurrent signal
> +		 * delivery to the new leader. The former leader has PF_EXITING
> +		 * set which prevents queueing of SIGEV_THREAD_ID signals up to
> +		 * the point where it's sighand gets cleared.
> +		 */
> +		scoped_guard(spinlock_irq, lock)
> +			flush_sigqueue(&leader->pending);
> +
> +		/*
>  		 * The only record we have of the real-time age of a
>  		 * process, regardless of execs it's done, is start_time.
>  		 * All the past CPU time is accumulated in signal_struct
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1998,6 +1998,13 @@ void posixtimer_send_sigqueue(struct k_i
>  		return;
>  
>  	/*
> +	 * If the signal is targeted at a specific thread, validate with sighand
> +	 * lock held that the thread is not exiting.
> +	 */
> +	if (unlikely(tmr->it_pid_type == PIDTYPE_PID  && t->flags & PF_EXITING))
> +		goto unlock;
> +
> +	/*
>  	 * Update @tmr::sigqueue_seq for posix timer signals with sighand
>  	 * locked to prevent a race against dequeue_signal().
>  	 */
> @@ -2088,6 +2095,7 @@ void posixtimer_send_sigqueue(struct k_i
>  	result = TRACE_SIGNAL_DELIVERED;
>  out:
>  	trace_signal_generate(sig, &q->info, t, tmr->it_pid_type != PIDTYPE_PID, result);
> +unlock:
>  	unlock_task_sighand(t, &flags);
>  }
>

This one looks good, FWIW.

Thanks.

-- 
Frederic Weisbecker
SUSE Labs

      parent reply	other threads:[~2026-08-24 16:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-22  5:37 [PATCH] signal: Use list_del_init_careful() in flush_sigqueue() Hyunwoo Kim
2026-08-22 10:27 ` Bradley Morgan
2026-08-23 12:47 ` Oleg Nesterov
2026-08-24  2:53   ` Hyunwoo Kim
2026-08-24  8:28     ` Oleg Nesterov
2026-08-24  8:04 ` Thomas Gleixner
2026-08-24  9:45   ` Thomas Gleixner
2026-08-24 11:02     ` Oleg Nesterov
2026-08-24 11:54       ` Oleg Nesterov
2026-08-24 13:59         ` Frederic Weisbecker
2026-08-24 14:29           ` Oleg Nesterov
2026-08-25 16:58           ` Thomas Gleixner
2026-08-25 18:53             ` Oleg Nesterov
2026-08-25 19:58               ` Thomas Gleixner
2026-08-26  9:36                 ` Oleg Nesterov
2026-08-24 12:11       ` Thomas Gleixner
2026-08-24 16:31     ` Frederic Weisbecker [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoxx8ApYkXzMv_Vb@localhost.localdomain \
    --to=frederic@kernel.org \
    --cc=anna-maria@linutronix.de \
    --cc=brauner@kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=imv4bel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.