From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>,
Frederic Weisbecker <frederic@kernel.org>,
Benjamin Segall <bsegall@google.com>,
Eric Dumazet <edumazet@google.com>,
Andrey Vagin <avagin@openvz.org>,
Pavel Tikhomirov <ptikhomirov@virtuozzo.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: [patch 09/11] posix-timers: Dont iterate /proc/$PID/timers with sighand::siglock held
Date: Mon, 24 Feb 2025 11:15:37 +0100 (CET) [thread overview]
Message-ID: <20250224101343.541884406@linutronix.de> (raw)
In-Reply-To: 20250224095736.145530367@linutronix.de
The readout of /proc/$PID/timers holds sighand::siglock with interrupts
disabled. That is required to protect against concurrent modifications of
the task::signal::posix_timers list because the list is not RCU safe.
With the conversion of the timer storage to RCU protected hlist, this is
not longer required.
The only requirement is to protect the returned entry against a concurrent
free, which is trivial as the timers are RCU protected.
Removing the trylock of sighand::siglock is benign because the life time of
task_struct::signal is bound to the life time of the task_struct itself.
There are two scenarios where this matters:
1) The process is life and not about to be checkpointed
2) The process is stopped via ptrace for checkpointing
#1 is a racy snapshot of the armed timers and nothing can rely on it. It's
not more than debug information and it has been that way before because
sighand lock is dropped when the buffer is full and the restart of
the iteration might find a completely different set of timers.
The task and therefore task::signal cannot be freed as timers_start()
acquired a reference count via get_pid_task().
#2 the process is stopped for checkpointing so nothing can delete or create
timers at this point. Neither can the process exit during the traversal.
If CRIU fails to observe an exit in progress prior to the dissimination
of the timers, then there are more severe problems to solve in the CRIU
mechanics as they can't rely on posix timers being enabled in the first
place.
Therefore replace the lock acquisition with rcu_read_lock() and switch the
timer storage traversal over to seq_hlist_*_rcu().
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
fs/proc/base.c | 48 ++++++++++++++++++++----------------------------
1 file changed, 20 insertions(+), 28 deletions(-)
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2497,11 +2497,9 @@ static const struct file_operations proc
#if defined(CONFIG_CHECKPOINT_RESTORE) && defined(CONFIG_POSIX_TIMERS)
struct timers_private {
- struct pid *pid;
- struct task_struct *task;
- struct sighand_struct *sighand;
- struct pid_namespace *ns;
- unsigned long flags;
+ struct pid *pid;
+ struct task_struct *task;
+ struct pid_namespace *ns;
};
static void *timers_start(struct seq_file *m, loff_t *pos)
@@ -2512,54 +2510,48 @@ static void *timers_start(struct seq_fil
if (!tp->task)
return ERR_PTR(-ESRCH);
- tp->sighand = lock_task_sighand(tp->task, &tp->flags);
- if (!tp->sighand)
- return ERR_PTR(-ESRCH);
-
- return seq_hlist_start(&tp->task->signal->posix_timers, *pos);
+ rcu_read_lock();
+ return seq_hlist_start_rcu(&tp->task->signal->posix_timers, *pos);
}
static void *timers_next(struct seq_file *m, void *v, loff_t *pos)
{
struct timers_private *tp = m->private;
- return seq_hlist_next(v, &tp->task->signal->posix_timers, pos);
+
+ return seq_hlist_next_rcu(v, &tp->task->signal->posix_timers, pos);
}
static void timers_stop(struct seq_file *m, void *v)
{
struct timers_private *tp = m->private;
- if (tp->sighand) {
- unlock_task_sighand(tp->task, &tp->flags);
- tp->sighand = NULL;
- }
-
if (tp->task) {
put_task_struct(tp->task);
tp->task = NULL;
+ rcu_read_unlock();
}
}
static int show_timer(struct seq_file *m, void *v)
{
- struct k_itimer *timer;
- struct timers_private *tp = m->private;
- int notify;
static const char * const nstr[] = {
- [SIGEV_SIGNAL] = "signal",
- [SIGEV_NONE] = "none",
- [SIGEV_THREAD] = "thread",
+ [SIGEV_SIGNAL] = "signal",
+ [SIGEV_NONE] = "none",
+ [SIGEV_THREAD] = "thread",
};
- timer = hlist_entry((struct hlist_node *)v, struct k_itimer, list);
- notify = timer->it_sigev_notify;
+ struct k_itimer *timer = hlist_entry((struct hlist_node *)v, struct k_itimer, list);
+ struct timers_private *tp = m->private;
+ int notify = timer->it_sigev_notify;
+
+ guard(spinlock_irq)(&timer->it_lock);
+ if (!posixtimer_valid(timer))
+ return 0;
seq_printf(m, "ID: %d\n", timer->it_id);
- seq_printf(m, "signal: %d/%px\n",
- timer->sigq.info.si_signo,
+ seq_printf(m, "signal: %d/%px\n", timer->sigq.info.si_signo,
timer->sigq.info.si_value.sival_ptr);
- seq_printf(m, "notify: %s/%s.%d\n",
- nstr[notify & ~SIGEV_THREAD_ID],
+ seq_printf(m, "notify: %s/%s.%d\n", nstr[notify & ~SIGEV_THREAD_ID],
(notify & SIGEV_THREAD_ID) ? "tid" : "pid",
pid_nr_ns(timer->it_pid, tp->ns));
seq_printf(m, "ClockID: %d\n", timer->it_clock);
next prev parent reply other threads:[~2025-02-24 10:15 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-24 10:15 [patch 00/11] posix-timers: Rework the global hash table and provide a sane mechanism for CRIU Thomas Gleixner
2025-02-24 10:15 ` [patch 01/11] posix-timers: Initialise timer before adding it to the hash table Thomas Gleixner
2025-02-24 10:15 ` [patch 02/11] posix-timers: Add cond_resched() to posix_timer_add() search loop Thomas Gleixner
2025-02-24 10:15 ` [patch 03/11] posix-timers: Cleanup includes Thomas Gleixner
2025-02-24 10:15 ` [patch 04/11] posix-timers: Remove pointless unlock_timer() wrapper Thomas Gleixner
2025-02-24 16:21 ` Peter Zijlstra
2025-02-24 18:43 ` Thomas Gleixner
2025-02-24 21:55 ` Peter Zijlstra
2025-02-24 10:15 ` [patch 05/11] posix-timers: Rework timer removal Thomas Gleixner
2025-02-24 10:15 ` [patch 06/11] posix-timers: Make signal_struct::next_posix_timer_id an atomic_t Thomas Gleixner
2025-02-24 13:20 ` Peter Zijlstra
2025-02-24 13:34 ` Eric Dumazet
2025-02-24 19:38 ` Thomas Gleixner
2025-02-24 10:15 ` [patch 07/11] posix-timers: Improve hash table performance Thomas Gleixner
2025-02-24 19:45 ` Thomas Gleixner
2025-02-24 10:15 ` [patch 08/11] posix-timers: Make per process list RCU safe Thomas Gleixner
2025-02-24 10:15 ` Thomas Gleixner [this message]
2025-02-24 10:15 ` [patch 10/11] posix-timers: Provide a mechanism to allocate a given timer ID Thomas Gleixner
2025-02-24 10:15 ` [patch 11/11] selftests/timers/posix-timers: Add a test for exact allocation mode Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250224101343.541884406@linutronix.de \
--to=tglx@linutronix.de \
--cc=anna-maria@linutronix.de \
--cc=avagin@openvz.org \
--cc=bsegall@google.com \
--cc=edumazet@google.com \
--cc=frederic@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=ptikhomirov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox