From: Thomas Gleixner <tglx@linutronix.de>
To cure the SIG_IGN handling for posix interval timers, the preallocated
sigqueue needs to be embedded into struct k_itimer to prevent life time
races of all sorts.
Now that the prerequisites are in place, embed the sigqueue into struct
k_itimer and fixup the relevant usage sites.
Aside of preparing for proper SIG_IGN handling, this spares an extra
allocation.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
fs/proc/base.c | 4 +--
include/linux/posix-timers.h | 23 ++++++++++++++++--
kernel/signal.c | 12 +++++++--
kernel/time/posix-timers.c | 59 +++++++++++++++++++++++++++------------------
4 files changed, 69 insertions(+), 29 deletions(-)
---
diff --git a/fs/proc/base.c b/fs/proc/base.c
index dd579332a7f8..f01ea013ff9b 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2496,8 +2496,8 @@ static int show_timer(struct seq_file *m, void *v)
seq_printf(m, "ID: %d\n", timer->it_id);
seq_printf(m, "signal: %d/%px\n",
- timer->sigq->info.si_signo,
- timer->sigq->info.si_value.sival_ptr);
+ 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],
(notify & SIGEV_THREAD_ID) ? "tid" : "pid",
diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 52611ea923b2..ddd7ccd9ba77 100644
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -39,6 +39,8 @@ static inline int clockid_to_fd(const clockid_t clk)
#ifdef CONFIG_POSIX_TIMERS
+#include <linux/signal_types.h>
+
/**
* cpu_timer - Posix CPU timer representation for k_itimer
* @node: timerqueue node to queue in the task/sig
@@ -165,7 +167,7 @@ static inline void posix_cputimers_init_work(void) { }
* @it_pid: The pid of the process/task targeted by the signal
* @it_process: The task to wakeup on clock_nanosleep (CPU timers)
* @rcuref: Reference count for life time management
- * @sigq: Pointer to preallocated sigqueue
+ * @sigq: Embedded sigqueue
* @it: Union representing the various posix timer type
* internals.
* @rcu: RCU head for freeing the timer.
@@ -189,7 +191,7 @@ struct k_itimer {
struct pid *it_pid;
struct task_struct *it_process;
};
- struct sigqueue *sigq;
+ struct sigqueue sigq;
rcuref_t rcuref;
union {
struct {
@@ -217,6 +219,23 @@ static inline void posixtimer_putref(struct k_itimer *tmr)
if (rcuref_put(&tmr->rcuref))
posixtimer_free_timer(tmr);
}
+
+static inline void posixtimer_sigqueue_getref(struct sigqueue *q)
+{
+ struct k_itimer *tmr = container_of(q, struct k_itimer, sigq);
+
+ WARN_ON_ONCE(!rcuref_get(&tmr->rcuref));
+}
+
+static inline void posixtimer_sigqueue_putref(struct sigqueue *q)
+{
+ struct k_itimer *tmr = container_of(q, struct k_itimer, sigq);
+
+ posixtimer_putref(tmr);
+}
+#else /* CONFIG_POSIX_TIMERS */
+static inline void posixtimer_sigqueue_getref(struct sigqueue *q) { }
+static inline void posixtimer_sigqueue_putref(struct sigqueue *q) { }
#endif /* !CONFIG_POSIX_TIMERS */
#endif
diff --git a/kernel/signal.c b/kernel/signal.c
index 01102470e174..7a07f86e2ae6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -566,7 +566,12 @@ static void collect_signal(int sig, struct sigpending *list, kernel_siginfo_t *i
still_pending:
list_del_init(&first->list);
copy_siginfo(info, &first->info);
- __sigqueue_free(first);
+ /*
+ * Do not drop the reference count for posix timer
+ * signals. That's done in posix_timer_deliver_signal().
+ */
+ if (info->si_code != SI_TIMER)
+ __sigqueue_free(first);
} else {
/*
* Ok, it wasn't in the queue. This must be
@@ -1981,7 +1986,7 @@ static inline struct task_struct *posixtimer_get_target(struct k_itimer *tmr)
int posixtimer_send_sigqueue(struct k_itimer *tmr)
{
- struct sigqueue *q = tmr->sigq;
+ struct sigqueue *q = &tmr->sigq;
int sig = q->info.si_signo;
struct task_struct *t;
unsigned long flags;
@@ -2041,9 +2046,12 @@ int posixtimer_send_sigqueue(struct k_itimer *tmr)
ret = 0;
if (unlikely(!list_empty(&q->list))) {
+ /* This holds a reference count already */
result = TRACE_SIGNAL_ALREADY_PENDING;
goto out;
}
+
+ posixtimer_sigqueue_getref(q);
posixtimer_queue_sigqueue(q, t, tmr->it_pid_type);
result = TRACE_SIGNAL_DELIVERED;
out:
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 369c8f1c5e4c..b62e3ccb45ff 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -251,12 +251,13 @@ static void common_hrtimer_rearm(struct k_itimer *timr)
/*
* This function is called from the signal delivery code. It decides
- * whether the signal should be dropped and rearms interval timers.
+ * whether the signal should be dropped and rearms interval timers. The
+ * timer can be unconditionally accessed as there is a reference held on
+ * it.
*/
bool posixtimer_deliver_signal(struct kernel_siginfo *info)
{
- struct k_itimer *timr;
- unsigned long flags;
+ struct k_itimer *timr = info->si_sys_privptr;
bool ret = false;
/*
@@ -264,12 +265,14 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info)
* timr::it_lock. Keep interrupts disabled.
*/
spin_unlock(¤t->sighand->siglock);
+ spin_lock(&timr->it_lock);
- timr = lock_timer(info->si_tid, &flags);
- if (!timr)
- goto out;
-
- if (timr->it_signal_seq != info->si_sys_private)
+ /*
+ * Check if the timer is still alive or whether it got modified
+ * since the signal was queued. In either case, don't rearm and
+ * drop the signal.
+ */
+ if (!timr->it_signal || timr->it_signal_seq != info->si_sys_private)
goto out_unlock;
if (timr->it_interval && timr->it_status == POSIX_TIMER_REQUEUE_PENDING) {
@@ -285,8 +288,10 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info)
ret = true;
out_unlock:
- unlock_timer(timr, flags);
-out:
+ spin_unlock(&timr->it_lock);
+ /* Drop the reference which was acquired when the signal was queued */
+ posixtimer_putref(timr);
+
spin_lock(¤t->sighand->siglock);
/* Don't expose the si_sys_priv* values to userspace */
@@ -405,17 +410,17 @@ static struct pid *good_sigevent(sigevent_t * event)
}
}
-static struct k_itimer * alloc_posix_timer(void)
+static struct k_itimer *alloc_posix_timer(void)
{
struct k_itimer *tmr = kmem_cache_zalloc(posix_timers_cache, GFP_KERNEL);
if (!tmr)
return tmr;
- if (unlikely(!(tmr->sigq = sigqueue_alloc()))) {
+
+ if (unlikely(!posixtimer_init_sigqueue(&tmr->sigq))) {
kmem_cache_free(posix_timers_cache, tmr);
return NULL;
}
- clear_siginfo(&tmr->sigq->info);
rcuref_init(&tmr->rcuref, 1);
return tmr;
}
@@ -430,7 +435,8 @@ static void k_itimer_rcu_free(struct rcu_head *head)
void posixtimer_free_timer(struct k_itimer *tmr)
{
put_pid(tmr->it_pid);
- sigqueue_free(tmr->sigq);
+ if (tmr->sigq.ucounts)
+ dec_rlimit_put_ucounts(tmr->sigq.ucounts, UCOUNT_RLIMIT_SIGPENDING);
call_rcu(&tmr->rcu, k_itimer_rcu_free);
}
@@ -492,13 +498,13 @@ static int do_timer_create(clockid_t which_clock, struct sigevent *event,
goto out;
}
new_timer->it_sigev_notify = event->sigev_notify;
- new_timer->sigq->info.si_signo = event->sigev_signo;
- new_timer->sigq->info.si_value = event->sigev_value;
+ new_timer->sigq.info.si_signo = event->sigev_signo;
+ new_timer->sigq.info.si_value = event->sigev_value;
} else {
new_timer->it_sigev_notify = SIGEV_SIGNAL;
- new_timer->sigq->info.si_signo = SIGALRM;
- memset(&new_timer->sigq->info.si_value, 0, sizeof(sigval_t));
- new_timer->sigq->info.si_value.sival_int = new_timer->it_id;
+ new_timer->sigq.info.si_signo = SIGALRM;
+ memset(&new_timer->sigq.info.si_value, 0, sizeof(sigval_t));
+ new_timer->sigq.info.si_value.sival_int = new_timer->it_id;
new_timer->it_pid = get_pid(task_tgid(current));
}
@@ -507,9 +513,9 @@ static int do_timer_create(clockid_t which_clock, struct sigevent *event,
else
new_timer->it_pid_type = PIDTYPE_TGID;
- new_timer->sigq->info.si_tid = new_timer->it_id;
- new_timer->sigq->info.si_code = SI_TIMER;
- new_timer->sigq->info.si_sys_privptr = new_timer;
+ new_timer->sigq.info.si_tid = new_timer->it_id;
+ new_timer->sigq.info.si_code = SI_TIMER;
+ new_timer->sigq.info.si_sys_privptr = new_timer;
if (copy_to_user(created_timer_id, &new_timer_id, sizeof (new_timer_id))) {
error = -EFAULT;
@@ -593,7 +599,14 @@ static struct k_itimer *__lock_timer(timer_t timer_id, unsigned long *flags)
* 1) Set timr::it_signal to NULL with timr::it_lock held
* 2) Release timr::it_lock
* 3) Remove from the hash under hash_lock
- * 4) Call RCU for removal after the grace period
+ * 4) Put the reference count.
+ *
+ * The reference count might not drop to zero if timr::sigq is
+ * queued. In that case the signal delivery or flush will put the
+ * last reference count.
+ *
+ * When the reference count reaches zero, the timer is scheduled
+ * for RCU removal after the grace period.
*
* Holding rcu_read_lock() accross the lookup ensures that
* the timer cannot be freed.