From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AB3C137104C for ; Tue, 25 Aug 2026 19:58:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787687884; cv=none; b=YoOt0aaxcsRYkEaaXjG+3GfBVZwXvElYhJDZSEAYrrXcbdtG2YGARhxLOJetwCYmNvJy5MGCR8fE/ckhh9atO9FNY/mzhrZQO5AjPlVusfiPGmcP9/IKU2wMa/GlTTgyy0per3trAdCs7Z5Xj8QemBJyJrIiGjrm509+z3mwTZI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787687884; c=relaxed/simple; bh=3dj6NXN/etJigAoyx0ZwLL6CWVbI3oD6/YO69A+e/nc=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=ot7zhgYY0y8ZPHt5+pdteIXfYDG+8yPlOTA00gYu2mBLv5zbVzj0poXckiQWGfGey/rGZ5HC00wTLU25OrEnoLu/RLA7LjKMBcZuiOsnQgwj6BqOYOQRs9+W+Mia5lZyfhux5+6axYyf8cgrBpFHYUIpN2o83SnHiT0ibTVmQ2U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hAhcxkx9; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hAhcxkx9" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8A2EB1F000E9; Tue, 25 Aug 2026 19:58:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787687883; bh=1kwxCVzqi5d5GHmAKCeHv6nOj0cQFTRmugdgQuT/i3U=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=hAhcxkx9Dd/nMd8rIjl1PSrTtKEFZAIvR19tZ45+qp4rn3dnGX0mw6tDnqwdK0U/Y WBVIdNjQKkK6SlpG/TBNAGjXxsBkF9Qdb1jWciE+432OaDLCjW2jSlM2pVZEelIT6+ attVSKPSoyUEqc1nGxc9SQJzs7NV89i+c7uavsnQ+cZgqnxBcin2iPL1mG//KqraAW +kJjGDt3AHY8WOFVgtjqQWGoJSjf0ql7R2YNohxcw/oquXyiO6pSKwa+6ZP8p65Ztk lpQ7cAODKgZMJhVdTeIx4XcF2XwsMEgqSGC0zR2ws41fcJyMG9oCKvo9jGQBCzHNO+ LygWWNlH1I2BA== From: Thomas Gleixner To: Oleg Nesterov Cc: Frederic Weisbecker , Hyunwoo Kim , 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() In-Reply-To: References: <875x10hrkt.ffs@fw13> <8733w3j1i1.ffs@fw13> <87pkz6gms6.ffs@fw13> Date: Tue, 25 Aug 2026 21:58:00 +0200 Message-ID: <87fr02gegn.ffs@fw13> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Tue, Aug 25 2026 at 20:53, Oleg Nesterov wrote: > On 08/25, Thomas Gleixner wrote: >> > Is there something to prevent the timer from firing on another CPU, >> > racing with this tiny window and queue the signal to the old leader? After >> > all exchange_tids() is just some RCU pointers changed but there is nothing >> > to synchronize the readers before the flush_sigqueue(). So pid_task() may >> > still return the old leader after it? >> >> You beat me to it. >> >> That's what I initialy thought when I added that exiting check into >> posixtimer_send_queue(), but then the trivial variant lured me away. :) > > I'm afraid I am wrong again... but if change posixtimer_send_sigqueue() > to check !PF_EXITING, de_thread() still can do flush_sigqueue() after > exchange_tids() outside of tasklist_lock? > > And I'd suggest to check t->exit_state instead of PF_EXITING, > posixtimer_send_sigqueue() can't miss it if it is called after > scoped_guard(spinlock_irq, lock). It neither can miss PF_EXITING which is also set under sighand lock. But I think we all looked at it way too narrowly focussed on that specific non-leader exec() scenario. Let's take a step back and look at the larger picture. Once begin_new_exec() sets bprm->point_of_no_return = true there is _ZERO_ reason to queue any posix timer signal anymore. Any failure after that point will be fatal and shut the whole process down. So why worrying about the non-leader exec() oddity? begin_new_exex() { ... bprm->point_of_no_return = true; scoped_guard(spinlock_irq, &me->sighand->siglock) me->signal->flags |= SIGNAL_EXEC; de_thread(me) ... // FIXME: This sequence should be cleaned up with a // posix_timer_exec() function with a proper stub // for CONFIG_POSIX_TIMERS=n. #ifdef CONFIG_POSIX_TIMERS spin_lock_irq(&me->sighand->siglock); posix_cpu_timers_exit(me); spin_unlock_irq(&me->sighand->siglock); exit_itimers(me); flush_itimer_signals(); #endif ... scoped_guard(spinlock_irq, &me->sighand->siglock) me->signal->flags &= ~SIGNAL_EXEC; // SUCCESS return 0; and in posixtimer_send_sigqueue() if (!likely(lock_task_sighand(t, &flags))) return; if (unlikely(t->signal->flags & (SIGNAL_EXEC))) goto unlock; and as we need that check anyway we can just make it: if (unlikely(t->signal->flags & (SIGNAL_EXEC | SIGNAL_GROUP_EXIT))) goto unlock; because there is no point either to queue posix timer signals when SIGNAL_GROUP_EXIT is set, right? Something like the untested below. At least I'm sure that I got the scoped_guard() types right this time. FWIW, I briefly pondered to hide the first part in de_thread(), but that just made my tired brain fail to reason about it. Thanks, tglx --- --- a/fs/exec.c +++ b/fs/exec.c @@ -1148,6 +1148,9 @@ int begin_new_exec(struct linux_binprm * */ bprm->point_of_no_return = true; + scoped_guard(spinlock_irq, &me->sighand->siglock) + me->signal->flags |= SIGNAL_EXEC; + /* Make this the only thread in the thread group */ retval = de_thread(me); if (retval) @@ -1324,6 +1327,10 @@ int begin_new_exec(struct linux_binprm * } bprm->execfd = retval; } + + scoped_guard(spinlock_irq, &me->sighand->siglock) + me->signal->flags &= ~SIGNAL_EXEC; + return 0; out_unlock: --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -261,6 +261,8 @@ struct signal_struct { #define SIGNAL_STOP_STOPPED 0x00000001 /* job control stop in effect */ #define SIGNAL_STOP_CONTINUED 0x00000002 /* SIGCONT since WCONTINUED reap */ #define SIGNAL_GROUP_EXIT 0x00000004 /* group exit in progress */ +#define SIGNAL_EXEC 0x00000008 /* exec in progress */ + /* * Pending notifications to parent. */ --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1991,6 +1991,20 @@ void posixtimer_send_sigqueue(struct k_i return; /* + * If the process is in the middle of exec(), don't queue signals as the + * posix timers of this process are not longer accessible and about to + * be removed. This prevents a race between queueing the signal on a + * exiting former thread group leader in case of an non-leader exec. + * Aside of that it makes no sense to queue anything now when it has to + * be flushed a split second later anyway. + * + * As this conditional is required just use the opportunity and check + * for a group exit too, where queueing signals is equally pointless. + */ + if (unlikely(t->signal->flags & (SIGNAL_EXEC | SIGNAL_GROUP_EXIT))) + goto unlock; + + /* * Update @tmr::sigqueue_seq for posix timer signals with sighand * locked to prevent a race against dequeue_signal(). */ @@ -2081,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); }