2024-01-22 16:57:19 +00:00
|
|
|
From 9bd94469a4ea8355cd39603280261eeff2cd0579 Mon Sep 17 00:00:00 2001
|
2024-01-04 12:38:55 +00:00
|
|
|
From: Thomas Gleixner <tglx@linutronix.de>
|
|
|
|
Date: Thu, 1 Jun 2023 20:58:47 +0200
|
|
|
|
Subject: [PATCH 60/62] posix-timers: Ensure timer ID search-loop limit is
|
|
|
|
valid
|
|
|
|
|
|
|
|
posix_timer_add() tries to allocate a posix timer ID by starting from the
|
|
|
|
cached ID which was stored by the last successful allocation.
|
|
|
|
|
|
|
|
This is done in a loop searching the ID space for a free slot one by
|
|
|
|
one. The loop has to terminate when the search wrapped around to the
|
|
|
|
starting point.
|
|
|
|
|
|
|
|
But that's racy vs. establishing the starting point. That is read out
|
|
|
|
lockless, which leads to the following problem:
|
|
|
|
|
|
|
|
CPU0 CPU1
|
|
|
|
posix_timer_add()
|
|
|
|
start = sig->posix_timer_id;
|
|
|
|
lock(hash_lock);
|
|
|
|
... posix_timer_add()
|
|
|
|
if (++sig->posix_timer_id < 0)
|
|
|
|
start = sig->posix_timer_id;
|
|
|
|
sig->posix_timer_id = 0;
|
|
|
|
|
|
|
|
So CPU1 can observe a negative start value, i.e. -1, and the loop break
|
|
|
|
never happens because the condition can never be true:
|
|
|
|
|
|
|
|
if (sig->posix_timer_id == start)
|
|
|
|
break;
|
|
|
|
|
|
|
|
While this is unlikely to ever turn into an endless loop as the ID space is
|
|
|
|
huge (INT_MAX), the racy read of the start value caught the attention of
|
|
|
|
KCSAN and Dmitry unearthed that incorrectness.
|
|
|
|
|
|
|
|
Rewrite it so that all id operations are under the hash lock.
|
|
|
|
|
|
|
|
Reported-by: syzbot+5c54bd3eb218bb595aa9@syzkaller.appspotmail.com
|
|
|
|
Reported-by: Dmitry Vyukov <dvyukov@google.com>
|
|
|
|
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
|
|
|
|
Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
|
|
|
|
Link: https://lore.kernel.org/r/87bkhzdn6g.ffs@tglx
|
|
|
|
|
|
|
|
(cherry picked from commit 8ce8849dd1e78dadcee0ec9acbd259d239b7069f)
|
|
|
|
Signed-off-by: Clark Williams <clark.williams@gmail.com>
|
|
|
|
---
|
|
|
|
include/linux/sched/signal.h | 2 +-
|
|
|
|
kernel/time/posix-timers.c | 31 ++++++++++++++++++-------------
|
|
|
|
2 files changed, 19 insertions(+), 14 deletions(-)
|
|
|
|
|
|
|
|
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
|
|
|
|
index 20099268fa25..669e8cff40c7 100644
|
|
|
|
--- a/include/linux/sched/signal.h
|
|
|
|
+++ b/include/linux/sched/signal.h
|
|
|
|
@@ -135,7 +135,7 @@ struct signal_struct {
|
|
|
|
#ifdef CONFIG_POSIX_TIMERS
|
|
|
|
|
|
|
|
/* POSIX.1b Interval Timers */
|
|
|
|
- int posix_timer_id;
|
|
|
|
+ unsigned int next_posix_timer_id;
|
|
|
|
struct list_head posix_timers;
|
|
|
|
|
|
|
|
/* ITIMER_REAL timer for the process */
|
|
|
|
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
|
|
|
|
index ed3c4a954398..2d6cf93ca370 100644
|
|
|
|
--- a/kernel/time/posix-timers.c
|
|
|
|
+++ b/kernel/time/posix-timers.c
|
|
|
|
@@ -140,25 +140,30 @@ static struct k_itimer *posix_timer_by_id(timer_t id)
|
|
|
|
static int posix_timer_add(struct k_itimer *timer)
|
|
|
|
{
|
|
|
|
struct signal_struct *sig = current->signal;
|
|
|
|
- int first_free_id = sig->posix_timer_id;
|
|
|
|
struct hlist_head *head;
|
|
|
|
- int ret = -ENOENT;
|
|
|
|
+ unsigned int cnt, id;
|
|
|
|
|
|
|
|
- do {
|
|
|
|
+ /*
|
|
|
|
+ * FIXME: Replace this by a per signal struct xarray once there is
|
|
|
|
+ * a plan to handle the resulting CRIU regression gracefully.
|
|
|
|
+ */
|
|
|
|
+ for (cnt = 0; cnt <= INT_MAX; cnt++) {
|
|
|
|
spin_lock(&hash_lock);
|
|
|
|
- head = &posix_timers_hashtable[hash(sig, sig->posix_timer_id)];
|
|
|
|
- if (!__posix_timers_find(head, sig, sig->posix_timer_id)) {
|
|
|
|
+ id = sig->next_posix_timer_id;
|
|
|
|
+
|
|
|
|
+ /* Write the next ID back. Clamp it to the positive space */
|
|
|
|
+ sig->next_posix_timer_id = (id + 1) & INT_MAX;
|
|
|
|
+
|
|
|
|
+ head = &posix_timers_hashtable[hash(sig, id)];
|
|
|
|
+ if (!__posix_timers_find(head, sig, id)) {
|
|
|
|
hlist_add_head_rcu(&timer->t_hash, head);
|
|
|
|
- ret = sig->posix_timer_id;
|
|
|
|
+ spin_unlock(&hash_lock);
|
|
|
|
+ return id;
|
|
|
|
}
|
|
|
|
- if (++sig->posix_timer_id < 0)
|
|
|
|
- sig->posix_timer_id = 0;
|
|
|
|
- if ((sig->posix_timer_id == first_free_id) && (ret == -ENOENT))
|
|
|
|
- /* Loop over all possible ids completed */
|
|
|
|
- ret = -EAGAIN;
|
|
|
|
spin_unlock(&hash_lock);
|
|
|
|
- } while (ret == -ENOENT);
|
|
|
|
- return ret;
|
|
|
|
+ }
|
|
|
|
+ /* POSIX return code when no timer ID could be allocated */
|
|
|
|
+ return -EAGAIN;
|
|
|
|
}
|
|
|
|
|
|
|
|
static inline void unlock_timer(struct k_itimer *timr, unsigned long flags)
|
|
|
|
--
|
|
|
|
2.43.0
|
|
|
|
|