Received-SPF: pass (google.com: domain of linux-kernel+bounces-145069-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Anna-Maria Behnsen <anna-maria@linutronix.de>
To: Thomas Gleixner <tglx@linutronix.de>, LKML <linux-kernel@vger.kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>, John Stultz
 <jstultz@google.com>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar
 <mingo@kernel.org>, Stephen Boyd <sboyd@kernel.org>, Eric Biederman
 <ebiederm@xmission.com>, Oleg Nesterov <oleg@redhat.com>
Subject: [PATCH] posix-timers: Handle returned errors poperly in
 [i]timer_delete()
In-Reply-To: <20240410164558.316665885@linutronix.de>
References: <20240410164558.316665885@linutronix.de>
Date: Mon, 15 Apr 2024 14:30:46 +0200
Message-ID: <8734rm95ah.fsf@somnus>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

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@linutronix.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.