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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z4cGMLwLAVDZVFbQ@pavilion.home>
Date: Wed, 15 Jan 2025 01:49:52 +0100
From: Frederic Weisbecker <frederic@...nel.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: syzbot <syzbot+3c2e3cc60665d71de2f7@...kaller.appspotmail.com>,
	anna-maria@...utronix.de, linux-kernel@...r.kernel.org,
	peterz@...radead.org, syzkaller-bugs@...glegroups.com,
	Eric Biederman <ebiederm@...ssion.com>,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH V2] signal/posixtimers: Handle ignore/blocked sequences
 correctly

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@...kaller.appspotmail.com
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>

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