linux-kernel - Re: [patch v6 17/20] signal: Queue ignored posixtimers on ignore list

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bjyxr8q9.ffs@tglx>
Date: Sat, 02 Nov 2024 15:49:50 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Anna-Maria Behnsen
 <anna-maria@...utronix.de>, John Stultz <jstultz@...gle.com>, Peter
 Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, Stephen
 Boyd <sboyd@...nel.org>, Eric Biederman <ebiederm@...ssion.com>, Oleg
 Nesterov <oleg@...hat.com>
Subject: Re: [patch v6 17/20] signal: Queue ignored posixtimers on ignore list

On Fri, Nov 01 2024 at 21:47, Thomas Gleixner wrote:
> On Fri, Nov 01 2024 at 15:21, Frederic Weisbecker wrote:
>> Le Thu, Oct 31, 2024 at 04:46:43PM +0100, Thomas Gleixner a écrit :
>>> +static void sigqueue_free_ignored(struct task_struct *tsk, struct sigqueue *q)
>>> +{
>>> +	if (likely(!(q->flags & SIGQUEUE_PREALLOC) || q->info.si_code != SI_TIMER))
>>> +		__sigqueue_free(q);
>>> +	else
>>> +		posixtimer_sig_ignore(tsk, q);
>>
>> So this happens when the signal is ignored and delays it to when it will be
>> unignored. But the comment on do_sigaction() says:
>>
>> 		/*
>> 		 * POSIX 3.3.1.3:
>> 		 *  "Setting a signal action to SIG_IGN for a signal that is
>> 		 *   pending shall cause the pending signal to be discarded,
>> 		 *   whether or not it is blocked."
>> 		 *
>> 		 */
>>
>> Are posix timers an exception to that rule?
>>
>> Also I see flush_sigqueue_mask() called on other occasions, for example
>> when a STOP signal happens to remove pending CONT, not sure if posix
>> timers can set SIGCONT...
>
> No. The problem with posix timers is that they are magically different
> from regular signals in the case of periodic timers.
>
> When the signal is ignored at expiry, then the signal is not delivered
> and is 'dropped'. But when SIG_IGN is removed then the following period
> expiry has to deliver the signal.
>
> Right now the kernel ensures that by keeping the timer self rearming and
> rate limiting it for obvious reasons. That's a completely pointless
> exercise up to the point where SIG_IGN is removed.
>
> The only way to avoid the self rearming is to actually stop the timer
> when the signal is ignored and rearm it when SIG_IGN for the specific
> signal is removed.
>
> That's what this magic does...

But it does not exclude oneshot timers from this. Their signal has to be
dropped for real.

Delta patch below.

Thanks,

        tglx
---
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -731,7 +731,7 @@ void signal_wake_up_state(struct task_st
 		kick_process(t);
 }
 
-static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q);
+static inline bool posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q);
 
 static void sigqueue_free_ignored(struct task_struct *tsk, struct sigqueue *q)
 {
@@ -1999,8 +1999,8 @@ int posixtimer_send_sigqueue(struct k_it
 			goto out;
 
 		if (hlist_unhashed(&tmr->ignored_list)) {
-			hlist_add_head(&tmr->ignored_list, &t->signal->ignored_posix_timers);
-			posixtimer_sigqueue_getref(q);
+			if (posixtimer_sig_ignore(t, tmr))
+				posixtimer_sigqueue_getref(q);
 		}
 		goto out;
 	}
@@ -2024,11 +2024,15 @@ int posixtimer_send_sigqueue(struct k_it
 	return 0;
 }
 
-static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q)
+static inline bool posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q)
 {
 	struct k_itimer *tmr = container_of(q, struct k_itimer, sigq);
 
+	/* Only queue periodic timer signals */
+	if (tmr->status != POSIX_TIMER_REQUEUE_PENDING)
+		return false;
 	hlist_add_head(&tmr->ignored_list, &tsk->signal->ignored_posix_timers);
+	return true;
 }
 
 static void posixtimer_sig_unignore(struct task_struct *tsk, int sig)