[PATCH 1/2] posix-timers: Report zero remaining time for disarmed SIGEV_NONE timers

[Hung-Jui Lin <alex.lin.programmer@gmail.com>]

SIGEV_NONE timers are never queued. common_timer_get() emulates their
expiry by computing the remaining time from the stored expiry value.
Because an armed SIGEV_NONE oneshot timer is not queued, its it_status
stays POSIX_TIMER_DISARMED. So common_timer_get() cannot use it_status to
skip a disarmed SIGEV_NONE timer and evaluates the stored expiry value for
all SIGEV_NONE timers.

Disarming a timer with timer_settime() and a zero it_value cancels the
timer and sets it_status to POSIX_TIMER_DISARMED, but leaves the expiry
value unchanged. As a consequence, timer_gettime() on a disarmed SIGEV_NONE
timer reports the remaining time of the previous arming until that time has
passed, instead of zero as POSIX requires for a disarmed timer. Example
with a CLOCK_MONOTONIC timer armed with it_value = 10ms and it_interval =
100ms, then disarmed with an all-zero itimerspec:

  armed:    it_value = 0.009999000  it_interval = 0.100000000
  disarmed: it_value = 0.008700792  it_interval = 0.000000000

SIGEV_SIGNAL timers and SIGEV_NONE CPU timers report zero in the same
situation. All clocks which use common_timer_get() are affected, including
the alarm timer clocks.

Mark SIGEV_NONE timers as POSIX_TIMER_ARMED when they are armed, so that
common_timer_get() returns early for every disarmed timer. Forward
SIGEV_NONE interval timers in common_timer_get() regardless of it_status,
as they are never queued and therefore never forwarded by the expiry code.

Tested on arm64 in QEMU: after the change, the disarmed timer above reports
zero. Armed SIGEV_NONE oneshot and interval timers on CLOCK_MONOTONIC,
CLOCK_REALTIME, CLOCK_BOOTTIME and CLOCK_BOOTTIME_ALARM still report their
remaining time, and the posix_timers selftest passes.

Fixes: becf8b5d00f4 ("[PATCH] hrtimer: convert posix timers completely")
Signed-off-by: Hung-Jui Lin <alex.lin.programmer@gmail.com>
---

Notes:
    Tested on arm64 under QEMU (virt, HVF), base tip timers/core
    348f54c435bf, arm64 defconfig plus kernel/configs/x86_debug.config
    (lockdep, DEBUG_OBJECTS, KMEMLEAK, DEBUG_PAGEALLOC, ...; the x86-only
    options do not exist on arm64), 9p built in:
    
                                            unpatched         patched
      reproducer: disarmed it_value         0.008700792       0
      posix_timers selftest, existing 19    19/19 pass        19/19 pass
      posix_timers selftest with patch 2    20/21, #14 fails  21/21 pass
    
    SIGEV_NONE timers on CLOCK_MONOTONIC, CLOCK_REALTIME, CLOCK_BOOTTIME,
    CLOCK_BOOTTIME_ALARM and CLOCK_PROCESS_CPUTIME_ID: never-armed, armed
    oneshot (5s), expired oneshot, armed interval and forwarded interval
    timers report the same values with and without the patch. Only the
    disarmed case changes: non-zero before (CPU timers were already 0),
    0 after. No lockdep or debugobjects splat after the tests. W=1 build of
    kernel/time/posix-timers.o has no warnings before or after.
    
    Fixes: tag checked by running the reproducer on x86_64 kernels in
    QEMU: 97735f25d2ba (the parent of becf8b5d00f4) reports 0 after
    disarm, becf8b5d00f4 reports the stale value. v2.6.17, v4.15 and v6.12
    also report the stale value.
    
    Not done: this patch was not built or booted on x86 or other
    architectures; no kmemleak scan was run; the series applies cleanly to
    mainline master a804c5742bff, but was not built or run there.

 kernel/time/posix-timers.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 188dbedbf..5eabec168 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -694,16 +694,8 @@ void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting)
 	if (iv) {
 		cur_setting->it_interval = ktime_to_timespec64(iv);
 	} else if (timr->it_status == POSIX_TIMER_DISARMED) {
-		/*
-		 * SIGEV_NONE oneshot timers are never queued and therefore
-		 * timr->it_status is always DISARMED. The check below
-		 * vs. remaining time will handle this case.
-		 *
-		 * For all other timers there is nothing to update here, so
-		 * return.
-		 */
-		if (!sig_none)
-			return;
+		/* Nothing to update for a disarmed timer */
+		return;
 	}
 
 	now = kc->clock_get_ktime(timr->it_clock);
@@ -713,7 +705,7 @@ void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting)
 	 * is a SIGEV_NONE timer move the expiry time forward by intervals,
 	 * so expiry is > now.
 	 */
-	if (iv && timr->it_status != POSIX_TIMER_ARMED)
+	if (iv && (sig_none || timr->it_status != POSIX_TIMER_ARMED))
 		timr->it_overrun += kc->timer_forward(timr, now);
 
 	remaining = kc->timer_remaining(timr, now);
@@ -915,8 +907,15 @@ int common_timer_set(struct k_itimer *timr, int flags,
 	sigev_none = timr->it_sigev_notify == SIGEV_NONE;
 
 	if (kc->timer_arm(timr, expires, flags & TIMER_ABSTIME, sigev_none)) {
-		if (!sigev_none)
-			timr->it_status = POSIX_TIMER_ARMED;
+		/*
+		 * SIGEV_NONE timers are not queued, but mark them armed
+		 * so that common_timer_get() can tell them apart from
+		 * disarmed timers. An expired SIGEV_NONE timer stays
+		 * armed, which is harmless: common_timer_get() reports
+		 * zero remaining time for it and it never reaches the
+		 * signal delivery code.
+		 */
+		timr->it_status = POSIX_TIMER_ARMED;
 	} else {
 		/* Timer was already expired, queue the signal */
 		posix_timer_queue_signal(timr);

base-commit: 348f54c435bf3177baa78201952810b255d44b30
-- 
2.56.0