Re: [PATCH V2] signal/posixtimers: Handle ignore/blocked sequences correctly

Frederic Weisbecker
2025-01-15T00:49:55Z

Le Tue, Jan 14, 2025 at 06:28:44PM +0100, Thomas Gleixner a écrit :
> syzbot triggered the warning in posixtimer_send_sigqueue(), which warns
> about a non-ignored signal being already queued on the ignored list.
>
> The warning is actually bogus, as the following sequence causes this:
>
> signal($SIG, SIGIGN);
> timer_settime(...); // arm periodic timer
>
> timer fires, signal is ignored and queued on ignored list
>
> sigprocmask(SIG_BLOCK, ...); // block the signal
> timer_settime(...); // re-arm periodic timer
>
> timer fires, signal is not ignored because it is blocked
> ---> Warning triggers as signal is on the ignored list
>
> Ideally timer_settime() could remove the signal, but that's racy and
> incomplete vs. other scenarios and requires a full reevaluation of the
> pending signal list.
>
> Instead of adding more complexity, handle it gracefully by removing the
> warning and requeueing the signal to the pending list. That's correct
> versus:
>
> 1) sig[timed]wait() as that does not check for SIGIGN and only relies on
> dequeue_signal() -> posixtimers_deliver_signal() to check whether the
> pending signal is still valid.
>
> 2) Unblocking of the signal.
>
> - If the unblocking happens before SIGIGN is replaced by a signal
> handler, then the timer is rearmed in dequeue_signal(), but
> get_signal() will ignore it. The next timer expiry will move it back
> to the ignored list.
>
> - If SIGIGN was replaced before unblocking, then the signal will be
> delivered and a subsequent expiry will queue a signal on the pending
> list again.
>
> There is a related scenario to trigger the complementary warning in the
> signal ignored path, which does not expect the signal to be on the pending
> list when it is ignored. That can be triggered even before the above change
> via:
>
> task1 task2
>
> signal($SIG, SIGIGN);
> sigprocmask(SIG_BLOCK, ...);
>
> timer_create(); // Signal target is task2
> timer_settime(...); // arm periodic timer
>
> timer fires, signal is not ignored because it is blocked
> and queued on the pending list of task2
>
> syscall()
> // Sets the pending flag
> sigprocmask(SIG_UNBLOCK, ...);
>
> -> preemption, task2 cannot dequeue the signal
>
> timer_settime(...); // re-arm periodic timer
>
> timer fires, signal is ignored
> ---> Warning triggers as signal is on task2's pending list
> and the thread group is not exiting
>
> Consequently, remove that warning too and just keep the signal on the
> pending list.
>
> The following attempt to deliver the signal on return to user space of
> task2 will ignore the signal and a subsequent expiry will bring it back to
> the ignored list, if it did not get blocked or un-ignored before that.
>
> Fixes: df7a996b4dab ("signal: Queue ignored posixtimers on ignore list")
> Reported-by: syzbot+3c2e3cc60665d71de2f7@syzkaller.appspotmail.com
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>