linux-kernel - [PATCH] posix-timers: Handle returned errors poperly in [i]timer

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]

Date: Mon, 15 Apr 2024 14:30:46 +0200
From: Anna-Maria Behnsen <anna-maria@...utronix.de>
To: Thomas Gleixner <tglx@...utronix.de>, LKML <linux-kernel@...r.kernel.org>
Cc: Frederic Weisbecker <frederic@...nel.org>, 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: [PATCH] posix-timers: Handle returned errors poperly in
 [i]timer_delete()

timer_delete_hook() returns -EINVAL when the clock or the timer_del
callback of the clock does not exist. This return value is not handled by
the callsites timer_delete() and itimer_delete().

Therefore add proper error handling.

Signed-off-by: Anna-Maria Behnsen <anna-maria@...utronix.de>
---
When having a look at the posix timer code during reviewing the queue, I
stumbled over this inconsitency. Maybe you want to have it in your
cleanup queue. Patch applies on top of your queue.

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

--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -1009,6 +1009,7 @@ SYSCALL_DEFINE1(timer_delete, timer_t, t
 {
 	struct k_itimer *timer;
 	unsigned long flags;
+	int ret;
 
 	timer = lock_timer(timer_id, &flags);
 
@@ -1019,7 +1020,11 @@ SYSCALL_DEFINE1(timer_delete, timer_t, t
 	/* Prevent signal delivery and rearming. */
 	timer->it_signal_seq++;
 
-	if (unlikely(timer_delete_hook(timer) == TIMER_RETRY)) {
+	ret = timer_delete_hook(timer);
+	if (ret < 0)
+		return ret;
+
+	if (unlikely(ret == TIMER_RETRY)) {
 		/* Unlocks and relocks the timer if it still exists */
 		timer = timer_wait_running(timer, &flags);
 		goto retry_delete;
@@ -1047,6 +1052,7 @@ SYSCALL_DEFINE1(timer_delete, timer_t, t
 static void itimer_delete(struct k_itimer *timer)
 {
 	unsigned long flags;
+	int ret;
 
 	/*
 	 * irqsave is required to make timer_wait_running() work.
@@ -1054,13 +1060,17 @@ static void itimer_delete(struct k_itime
 	spin_lock_irqsave(&timer->it_lock, flags);
 
 retry_delete:
+	ret = timer_delete_hook(timer);
+	if (WARN_ON_ONCE(ret < 0))
+		return;
+
 	/*
 	 * Even if the timer is not longer accessible from other tasks
 	 * it still might be armed and queued in the underlying timer
 	 * mechanism. Worse, that timer mechanism might run the expiry
 	 * function concurrently.
 	 */
-	if (timer_delete_hook(timer) == TIMER_RETRY) {
+	if (ret == TIMER_RETRY) {
 		/*
 		 * Timer is expired concurrently, prevent livelocks
 		 * and pointless spinning on RT.