linux-kernel - [patch 09/11] posix-timers: Fixup stale commnt and reduce ifdeffery

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20210923153339.932440049@linutronix.de>
Date:   Thu, 23 Sep 2021 18:04:34 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     LKML <linux-kernel@...r.kernel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>
Subject: [patch 09/11] posix-timers: Fixup stale commnt and reduce ifdeffery

When chasing all invocations of hrtimer_forward() the comment in
posix_timer_fn() turned out to be stale and misleading.

When it was written there was an idea floating around to actually let
periodic posix timers which fail signal delivery due to SIG_IGN not rearm
the underlying hrtimer and restart the timer when SIG_IGN is lifted. In
principle a good idea, but all implementation attempts ended in creating
more problems than they solved. But the comment was left around and
suggests for 14 years now that this is going to be fixed soonish.

Rewrite it to reflect reality and replace the ifdeffery close to it with
IS_ENABLED().

Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
---
 kernel/time/posix-timers.c |   38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -387,35 +387,27 @@ static enum hrtimer_restart posix_timer_
 			ktime_t now = hrtimer_cb_get_time(timer);
 
 			/*
-			 * FIXME: What we really want, is to stop this
-			 * timer completely and restart it in case the
-			 * SIG_IGN is removed. This is a non trivial
-			 * change which involves sighand locking
-			 * (sigh !), which we don't want to do late in
-			 * the release cycle.
-			 *
-			 * For now we just let timers with an interval
-			 * less than a jiffie expire every jiffie to
-			 * avoid softirq starvation in case of SIG_IGN
-			 * and a very small interval, which would put
-			 * the timer right back on the softirq pending
-			 * list. By moving now ahead of time we trick
-			 * hrtimer_forward() to expire the timer
-			 * later, while we still maintain the overrun
-			 * accuracy, but have some inconsistency in
-			 * the timer_gettime() case. This is at least
-			 * better than a starved softirq. A more
-			 * complex fix which solves also another related
-			 * inconsistency is already in the pipeline.
+			 * What we really want, is to stop this timer
+			 * completely and restart it in case the SIG_IGN is
+			 * removed. That's not trivial. For now we just let
+			 * timers with an interval less than a jiffie
+			 * expire every jiffie to avoid softirq starvation
+			 * in case of SIG_IGN and a very small interval,
+			 * which would put the timer right back on the
+			 * softirq pending list. By moving now ahead of
+			 * time we trick hrtimer_forward() to expire the
+			 * timer later, while we still maintain the overrun
+			 * accuracy, but have some inconsistency in the
+			 * timer_gettime() case. This is at least better
+			 * than a starved softirq.
 			 */
-#ifdef CONFIG_HIGH_RES_TIMERS
-			{
+			if (IS_ENABLED(CONFIG_HIGH_RES_TIMERS)) {
 				ktime_t kj = NSEC_PER_SEC / HZ;
 
 				if (timr->it_interval < kj)
 					now = ktime_add(now, kj);
 			}
-#endif
+
 			timr->it_overrun += hrtimer_forward(timer, now,
 							    timr->it_interval);
 			ret = HRTIMER_RESTART;