Skip to contentSkip to conversation

RFC: How to backport 920f893f735e ("posix-cpu-timers: Prevent UAF caused by non-leader exec() race") down to 5.15.y?

PATCH Applied Salvatore Bonaccorso 3 participants 4 messages Jul 13 – Jul 22

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hi,

920f893f735e ("posix-cpu-timers: Prevent UAF caused by non-leader exec() race") talks about a UAF, and the commit was CC'ed for stable. While checking it I noticed the commit though won't apply cleanly. I was thinking of needing to pick 6fdb2677a594 ("posix-timers: Expand timer_[re]arm() callbacks with a boolean return value") beforehand as preparatory change, and then 920f893f735e ("posix-cpu-timers: Prevent UAF caused by non-leader exec() race"). Would that be sound/sensible?

At least that would apply cleanly (though not yet even compile tested) to 7.1.y and 6.18.y but then already not anymore to 6.12.y, which will need more adjustments.

Thomas, would you intend to provide backports from you directly?

The two patches reflect this RFC.

Regards,
Salvatore

From 484d452c7a831d9d37dfa7589d203e05a835e369 Mon Sep 17 00:00:00 2001
From: Thomas Gleixner <[email protected]>
Date: Wed, 8 Apr 2026 13:53:56 +0200
Subject: [PATCH 1/2] posix-timers: Expand timer_[re]arm() callbacks with a
boolean return value

commit 6fdb2677a594ab38eade927919bbd4d9688bfa1c upstream.

In order to catch expiry times which are already in the past the timer_arm() and timer_rearm() callbacks need to be able to report back to the caller whether the timer has been queued or not.

Change the function signature and let all implementations return true for now. While at it simplify posix_cpu_timer_rearm().

No functional change intended.

---
 kernel/time/alarmtimer.c       |  6 ++++--
 kernel/time/posix-cpu-timers.c | 18 ++++++++++--------
 kernel/time/posix-timers.c     |  6 ++++--
 kernel/time/posix-timers.h     |  4 ++--
 4 files changed, 20 insertions(+), 14 deletions(-)
kernel/time/alarmtimer.c +4 -2
index 6e173d70d825..e10021be190f 100644
@@ -527,12 +527,13 @@ static void alarm_handle_timer(struct alarm *alarm, ktime_t now)
  * alarm_timer_rearm - Posix timer callback for rearming timer
  * @timr:	Pointer to the posixtimer data struct
  */
-static void alarm_timer_rearm(struct k_itimer *timr)
+static bool alarm_timer_rearm(struct k_itimer *timr)
 {
 	struct alarm *alarm = &timr->it.alarm.alarmtimer;
 
 	timr->it_overrun += alarm_forward_now(alarm, timr->it_interval);
 	alarm_start(alarm, alarm->node.expires);
+	return true;
 }
 
 /**
@@ -588,7 +589,7 @@ static void alarm_timer_wait_running(struct k_itimer *timr)
  * @absolute:	Expiry value is absolute time
  * @sigev_none:	Posix timer does not deliver signals
  */
-static void alarm_timer_arm(struct k_itimer *timr, ktime_t expires,
+static bool alarm_timer_arm(struct k_itimer *timr, ktime_t expires,
 			    bool absolute, bool sigev_none)
 {
 	struct alarm *alarm = &timr->it.alarm.alarmtimer;
@@ -600,6 +601,7 @@ static void alarm_timer_arm(struct k_itimer *timr, ktime_t expires,
 		alarm->node.expires = expires;
 	else
 		alarm_start(&timr->it.alarm.alarmtimer, expires);
+	return true;
 }
 
 /**
kernel/time/posix-cpu-timers.c +10 -8
index 0de2bb7cbec0..395e297093f8 100644
@@ -19,7 +19,7 @@
 
 #include "posix-timers.h"
 
-static void posix_cpu_timer_rearm(struct k_itimer *timer);
+static bool posix_cpu_timer_rearm(struct k_itimer *timer);
 
 void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit)
 {
@@ -1011,24 +1011,27 @@ static void check_process_timers(struct task_struct *tsk,
 /*
  * This is called from the signal code (via posixtimer_rearm)
  * when the last timer signal was delivered and we have to reload the timer.
+ *
+ * Return true unconditionally so the core code assumes the timer to be
+ * armed. Otherwise it would requeue the signal.
  */
-static void posix_cpu_timer_rearm(struct k_itimer *timer)
+static bool posix_cpu_timer_rearm(struct k_itimer *timer)
 {
 	clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock);
-	struct task_struct *p;
 	struct sighand_struct *sighand;
+	struct task_struct *p;
 	unsigned long flags;
 	u64 now;
 
-	rcu_read_lock();
+	guard(rcu)();
 	p = cpu_timer_task_rcu(timer);
 	if (!p)
-		goto out;
+		return true;
 
 	/* Protect timer list r/w in arm_timer() */
 	sighand = lock_task_sighand(p, &flags);
 	if (unlikely(sighand == NULL))
-		goto out;
+		return true;
 
 	/*
 	 * Fetch the current sample and update the timer's expiry time.
@@ -1045,8 +1048,7 @@ static void posix_cpu_timer_rearm(struct k_itimer *timer)
 	 */
 	arm_timer(timer, p);
 	unlock_task_sighand(p, &flags);
-out:
-	rcu_read_unlock();
+	return true;
 }
 
 /**
kernel/time/posix-timers.c +4 -2
index 9331e1614124..da04ed42bf82 100644
@@ -288,12 +288,13 @@ static inline int timer_overrun_to_int(struct k_itimer *timr)
 	return (int)timr->it_overrun_last;
 }
 
-static void common_hrtimer_rearm(struct k_itimer *timr)
+static bool common_hrtimer_rearm(struct k_itimer *timr)
 {
 	struct hrtimer *timer = &timr->it.real.timer;
 
 	timr->it_overrun += hrtimer_forward_now(timer, timr->it_interval);
 	hrtimer_restart(timer);
+	return true;
 }
 
 static bool __posixtimer_deliver_signal(struct kernel_siginfo *info, struct k_itimer *timr)
@@ -795,7 +796,7 @@ SYSCALL_DEFINE1(timer_getoverrun, timer_t, timer_id)
 		return timer_overrun_to_int(scoped_timer);
 }
 
-static void common_hrtimer_arm(struct k_itimer *timr, ktime_t expires,
+static bool common_hrtimer_arm(struct k_itimer *timr, ktime_t expires,
 			       bool absolute, bool sigev_none)
 {
 	struct hrtimer *timer = &timr->it.real.timer;
@@ -822,6 +823,7 @@ static void common_hrtimer_arm(struct k_itimer *timr, ktime_t expires,
 
 	if (!sigev_none)
 		hrtimer_start_expires(timer, HRTIMER_MODE_ABS);
+	return true;
 }
 
 static int common_hrtimer_try_to_cancel(struct k_itimer *timr)
kernel/time/posix-timers.h +2 -2
index 7f259e845d24..4ea9611dd716 100644
@@ -27,11 +27,11 @@ struct k_clock {
 	int	(*timer_del)(struct k_itimer *timr);
 	void	(*timer_get)(struct k_itimer *timr,
 			     struct itimerspec64 *cur_setting);
-	void	(*timer_rearm)(struct k_itimer *timr);
+	bool	(*timer_rearm)(struct k_itimer *timr);
 	s64	(*timer_forward)(struct k_itimer *timr, ktime_t now);
 	ktime_t	(*timer_remaining)(struct k_itimer *timr, ktime_t now);
 	int	(*timer_try_to_cancel)(struct k_itimer *timr);
-	void	(*timer_arm)(struct k_itimer *timr, ktime_t expires,
+	bool	(*timer_arm)(struct k_itimer *timr, ktime_t expires,
 			     bool absolute, bool sigev_none);
 	void	(*timer_wait_running)(struct k_itimer *timr);
 };
Signature · 1 line
2.53.0

From 798a9d582522eb528daf68b629de8707482e563b Mon Sep 17 00:00:00 2001
From: Thomas Gleixner <[email protected]>
Date: Fri, 3 Jul 2026 12:02:38 +0200
Subject: [PATCH 2/2] posix-cpu-timers: Prevent UAF caused by non-leader exec()
race

commit 920f893f735e92ba3a1cd9256899a186b161928d upstream.

Wongi and Jungwoo decoded and reported a non-leader exec() related race which can result in an UAF:

sys_timer_delete() exec()
posix_cpu_timer_del()
// Observes old leader
p = pid_task(pid, pid_type); de_thread()

   					  switch_leader();
					  release_task(old_leader)
					    __exit_signal(old_leader)
					      sighand = lock(old_leader, sighand);
					      posix_cpu_timers*_exit();
   sighand = lock_task_sighand(p)	      unhash_task(old_leader);
     sh = lock(p, sighand)	    	      old_leader->sighand = NULL;
					      unlock(sighand);
     (p->sighand == NULL)
	unlock(sh)
	return NULL;
   // Returns without action
   if(!sighand)
      return 0;
   free_posix_timer();

This is "harmless" unless the deleted timer was armed and enqueued in p->signal because on exec() a TGID targeted timer is inherited.

As sys_timer_delete() freed the underlying posix timer object run_posix_cpu_timers() or any timerqueue related add/delete operations on other timers will access the freed object's timerqueue node, which results in an UAF.

There is a similar problem vs. posix_cpu_timer_set(). For regular posix timers it just transiently returns -ESRCH to user space, but for the use case in do_cpu_nanosleep() it's the same UAF just that the k_itimer is allocated on the stack.

Also posix_cpu_timer_rearm() fails to rearm the timer, which means it stops to expire.

While debating solutions Frederic pointed out another problem:

posix_cpu_timer_del(tmr)

					__exit_signal(p)
					  posix_cpu_timers*_exit(p);
					  unhash_task(p);
					  p->sighand = NULL;
     sh = lock_task_sighand(p)
        sighand = p->sighand;
	if (!sighand)
	    return NULL;
	lock(sighand);

     if (!sh)
	WARN_ON_ONCE(timer_queued(tmr));

On weakly ordered architectures it is not guaranteed that posix_cpu_timer_del() will observe the stores in posix_cpu_timers*_exit() when p->sighand is observed as NULL, which means the WARN() can be a false positive.

Solve these issues by:

1) Changing the store in __exit_signal() to smp_store_release().

2) Adding a smp_acquire__after_ctrl_dep() into the !sighand path of lock_task_sighand().

3) Creating a helper function for looking up the task and locking sighand which does not return when sighand == NULL. Instead it retries the task lookup and only if that fails it gives up.

4) Using that helper in the three affected functions.

observes all preceeding stores, i.e. the stores in posix_cpu_timers*_exit() and the ones in unhash_task().

gracefully. When the task lookup returns the old leader, but sighand == NULL then it retries. In the non-leader exec() case the subsequent task lookup will observe the new leader due to #1/#2. In normal exit() scenarios the subsequent lookup fails.

When the task lookup fails, the function also checks whether the timer is still enqueued and issues a warning if that's the case. Unfortunately there is nothing which can be done about it, but as the task is already not longer visible the timer should not be accessed anymore. This check also requires memory ordering, which is not provided when the first lookup fails. To achieve that the check is preceeded by a smp_rmb() which pairs with the smp_wmb() in write_seqlock() in __exit_signal(). That ensures that the stores in posix_cpu_timers*_exit() are visible.

The history of the non-leader exec() issue goes back to the early days of posix CPU timers, which stored a pointer to the group leader task in the timer. That obviously fails when a non-leader exec() switches the leader. commit e0a70217107e ("posix-cpu-timers: workaround to suppress the problems with mt exec") added a temporary workaround for that in 2010 which survived about 10 years. The fix for the workaround changed the task pointer to a pid pointer, but failed to see the subtle race described above. So the Fixes tag picks that commit, which seems to be halfways accurate.

Thanks to Frederic Weissbecker, Oleg Nesterov and Peter Zijlstra for review, feedback and suggestions and to Wongi and Jungwoo for the excellent bug report and analysis!

Fixes 55e8c8eb2c7b ("posix-cpu-timers: Store a reference to a pid not a task")
---
 kernel/exit.c                  |   7 +-
 kernel/signal.c                |  10 +-
 kernel/time/posix-cpu-timers.c | 173 ++++++++++++++++++++++-----------
 3 files changed, 130 insertions(+), 60 deletions(-)
kernel/exit.c +6 -1
index f50d73c272d6..7ba7966a23b9 100644
@@ -212,7 +212,12 @@ static void __exit_signal(struct release_task_post *post, struct task_struct *ts
 	__unhash_process(post, tsk, group_dead);
 	write_sequnlock(&sig->stats_lock);
 
-	tsk->sighand = NULL;
+	/*
+	 * Ensure that all preceeding state is visible. Pairs with
+	 * the smp_acquire__after_ctrl_dep() in the sighand == NULL
+	 * path of lock_task_sighand().
+	 */
+	smp_store_release(&tsk->sighand, NULL);
 	spin_unlock(&sighand->siglock);
 
 	__cleanup_sighand(sighand);
kernel/signal.c +9 -1
index 9c2b32c4d755..bbc0fd4cc4d7 100644
@@ -1362,8 +1362,16 @@ struct sighand_struct *lock_task_sighand(struct task_struct *tsk,
 	rcu_read_lock();
 	for (;;) {
 		sighand = rcu_dereference(tsk->sighand);
-		if (unlikely(sighand == NULL))
+		if (unlikely(sighand == NULL)) {
+			/*
+			 * Pairs with the smp_store_release() in
+			 * __exit_signal().  It ensures that all state
+			 * modifications to the task preceeding the store are
+			 * visible to the callers of lock_task_sighand().
+			 */
+			smp_acquire__after_ctrl_dep();
 			break;
+		}
 
 		/*
 		 * This sighand can be already freed and even reused, but
kernel/time/posix-cpu-timers.c +115 -58

236 lines not on this page · raw message

Signature · 1 line
2.53.0
#1 lore raw
Quoted 10 lines · Salvatore Bonaccorso

Hi,

920f893f735e ("posix-cpu-timers: Prevent UAF caused by non-leader exec() race") talks about a UAF, and the commit was CC'ed for stable. While checking it I noticed the commit though won't apply cleanly. I was thinking of needing to pick 6fdb2677a594 ("posix-timers: Expand timer_[re]arm() callbacks with a boolean return value") beforehand as preparatory change, and then 920f893f735e ("posix-cpu-timers: Prevent UAF caused by non-leader exec() race"). Would that be sound/sensible?

Quoted 7 lines

At least that would apply cleanly (though not yet even compile tested) to 7.1.y and 6.18.y but then already not anymore to 6.12.y, which will need more adjustments.

Thomas, would you intend to provide backports from you directly?

The two patches reflect this RFC.

These look sane, I've queued them up now, thanks!

greg k-h

maintainer Applied
#2 lore raw
Quoted 5 lines
Quoted 3 lines

At least that would apply cleanly (though not yet even compile tested) to 7.1.y and 6.18.y but then already not anymore to 6.12.y, which will need more adjustments.

These look sane, I've queued them up now, thanks!

Queued the pair for 6.18 as well, thanks.

Signature · 2 lines
Thanks,
Sasha
#3 lore raw

Hi Sasha,

Quoted 7 lines · Sasha Levin
Quoted 5 lines
Quoted 3 lines

At least that would apply cleanly (though not yet even compile tested) to 7.1.y and 6.18.y but then already not anymore to 6.12.y, which will need more adjustments.

These look sane, I've queued them up now, thanks!

Queued the pair for 6.18 as well, thanks.

Thanks Sasha and Greg. So this means now we can have a look how this should be backported as well to things down the other stable series.

Regards,
Salvatore