From 2e529e637cef39057d9cf199a1ecb915d97ffcd9 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Sun, 13 Oct 2024 22:16:58 +0200 Subject: posix-timers: Replace call_rcu() by kfree_rcu() for simple kmem_cache_free() callback Since SLOB was removed and since commit 6c6c47b063b5 ("mm, slab: call kvfree_rcu_barrier() from kmem_cache_destroy()"), it is not longer necessary to use call_rcu() when the callback only performs kmem_cache_free(). Use kfree_rcu() directly. The changes were made using Coccinelle. Signed-off-by: Julia Lawall Signed-off-by: Thomas Gleixner Reviewed-by: Uladzislau Rezki (Sony) Link: https://lore.kernel.org/all/20241013201704.49576-12-Julia.Lawall@inria.fr --- kernel/time/posix-timers.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 4576aaed13b2..fc40dacabe78 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -413,18 +413,11 @@ static struct k_itimer * alloc_posix_timer(void) return tmr; } -static void k_itimer_rcu_free(struct rcu_head *head) -{ - struct k_itimer *tmr = container_of(head, struct k_itimer, rcu); - - kmem_cache_free(posix_timers_cache, tmr); -} - static void posix_timer_free(struct k_itimer *tmr) { put_pid(tmr->it_pid); sigqueue_free(tmr->sigq); - call_rcu(&tmr->rcu, k_itimer_rcu_free); + kfree_rcu(tmr, rcu); } static void posix_timer_unhash_and_free(struct k_itimer *tmr) -- cgit v1.2.3 From 68f99be287a59d50a9ad231d523f7e578f8bd28a Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 1 Oct 2024 10:42:00 +0200 Subject: signal: Confine POSIX_TIMERS properly Move the itimer rearming out of the signal code and consolidate all posix timer related functions in the signal code under one ifdef. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241001083835.314100569@linutronix.de --- include/linux/posix-timers.h | 5 +- kernel/signal.c | 125 +++++++++++++++---------------------------- kernel/time/itimer.c | 22 +++++++- kernel/time/posix-timers.c | 15 +++++- 4 files changed, 81 insertions(+), 86 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 453691710839..670bf03a56ef 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -100,6 +100,8 @@ static inline void posix_cputimers_rt_watchdog(struct posix_cputimers *pct, { pct->bases[CPUCLOCK_SCHED].nextevt = runtime; } +void posixtimer_rearm_itimer(struct task_struct *p); +void posixtimer_rearm(struct kernel_siginfo *info); /* Init task static initializer */ #define INIT_CPU_TIMERBASE(b) { \ @@ -122,6 +124,8 @@ struct cpu_timer { }; static inline void posix_cputimers_init(struct posix_cputimers *pct) { } static inline void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit) { } +static inline void posixtimer_rearm_itimer(struct task_struct *p) { } +static inline void posixtimer_rearm(struct kernel_siginfo *info) { } #endif #ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK @@ -196,5 +200,4 @@ void set_process_cpu_timer(struct task_struct *task, unsigned int clock_idx, int update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new); -void posixtimer_rearm(struct kernel_siginfo *info); #endif diff --git a/kernel/signal.c b/kernel/signal.c index 4344860ffcac..b65cc1853a09 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -478,42 +478,6 @@ void flush_signals(struct task_struct *t) } EXPORT_SYMBOL(flush_signals); -#ifdef CONFIG_POSIX_TIMERS -static void __flush_itimer_signals(struct sigpending *pending) -{ - sigset_t signal, retain; - struct sigqueue *q, *n; - - signal = pending->signal; - sigemptyset(&retain); - - list_for_each_entry_safe(q, n, &pending->list, list) { - int sig = q->info.si_signo; - - if (likely(q->info.si_code != SI_TIMER)) { - sigaddset(&retain, sig); - } else { - sigdelset(&signal, sig); - list_del_init(&q->list); - __sigqueue_free(q); - } - } - - sigorsets(&pending->signal, &signal, &retain); -} - -void flush_itimer_signals(void) -{ - struct task_struct *tsk = current; - unsigned long flags; - - spin_lock_irqsave(&tsk->sighand->siglock, flags); - __flush_itimer_signals(&tsk->pending); - __flush_itimer_signals(&tsk->signal->shared_pending); - spin_unlock_irqrestore(&tsk->sighand->siglock, flags); -} -#endif - void ignore_signals(struct task_struct *t) { int i; @@ -636,31 +600,9 @@ int dequeue_signal(sigset_t *mask, kernel_siginfo_t *info, enum pid_type *type) *type = PIDTYPE_TGID; signr = __dequeue_signal(&tsk->signal->shared_pending, mask, info, &resched_timer); -#ifdef CONFIG_POSIX_TIMERS - /* - * itimer signal ? - * - * itimers are process shared and we restart periodic - * itimers in the signal delivery path to prevent DoS - * attacks in the high resolution timer case. This is - * compliant with the old way of self-restarting - * itimers, as the SIGALRM is a legacy signal and only - * queued once. Changing the restart behaviour to - * restart the timer in the signal dequeue path is - * reducing the timer noise on heavy loaded !highres - * systems too. - */ - if (unlikely(signr == SIGALRM)) { - struct hrtimer *tmr = &tsk->signal->real_timer; - - if (!hrtimer_is_queued(tmr) && - tsk->signal->it_real_incr != 0) { - hrtimer_forward(tmr, tmr->base->get_time(), - tsk->signal->it_real_incr); - hrtimer_restart(tmr); - } - } -#endif + + if (unlikely(signr == SIGALRM)) + posixtimer_rearm_itimer(tsk); } recalc_sigpending(); @@ -682,22 +624,12 @@ int dequeue_signal(sigset_t *mask, kernel_siginfo_t *info, enum pid_type *type) */ current->jobctl |= JOBCTL_STOP_DEQUEUED; } -#ifdef CONFIG_POSIX_TIMERS - if (resched_timer) { - /* - * Release the siglock to ensure proper locking order - * of timer locks outside of siglocks. Note, we leave - * irqs disabled here, since the posix-timers code is - * about to disable them again anyway. - */ - spin_unlock(&tsk->sighand->siglock); - posixtimer_rearm(info); - spin_lock(&tsk->sighand->siglock); - /* Don't expose the si_sys_private value to userspace */ - info->si_sys_private = 0; + if (IS_ENABLED(CONFIG_POSIX_TIMERS)) { + if (unlikely(resched_timer)) + posixtimer_rearm(info); } -#endif + return signr; } EXPORT_SYMBOL_GPL(dequeue_signal); @@ -1922,15 +1854,43 @@ int kill_pid(struct pid *pid, int sig, int priv) } EXPORT_SYMBOL(kill_pid); +#ifdef CONFIG_POSIX_TIMERS /* - * These functions support sending signals using preallocated sigqueue - * structures. This is needed "because realtime applications cannot - * afford to lose notifications of asynchronous events, like timer - * expirations or I/O completions". In the case of POSIX Timers - * we allocate the sigqueue structure from the timer_create. If this - * allocation fails we are able to report the failure to the application - * with an EAGAIN error. + * These functions handle POSIX timer signals. POSIX timers use + * preallocated sigqueue structs for sending signals. */ +static void __flush_itimer_signals(struct sigpending *pending) +{ + sigset_t signal, retain; + struct sigqueue *q, *n; + + signal = pending->signal; + sigemptyset(&retain); + + list_for_each_entry_safe(q, n, &pending->list, list) { + int sig = q->info.si_signo; + + if (likely(q->info.si_code != SI_TIMER)) { + sigaddset(&retain, sig); + } else { + sigdelset(&signal, sig); + list_del_init(&q->list); + __sigqueue_free(q); + } + } + + sigorsets(&pending->signal, &signal, &retain); +} + +void flush_itimer_signals(void) +{ + struct task_struct *tsk = current; + + guard(spinlock_irqsave)(&tsk->sighand->siglock); + __flush_itimer_signals(&tsk->pending); + __flush_itimer_signals(&tsk->signal->shared_pending); +} + struct sigqueue *sigqueue_alloc(void) { return __sigqueue_alloc(-1, current, GFP_KERNEL, 0, SIGQUEUE_PREALLOC); @@ -2027,6 +1987,7 @@ ret: rcu_read_unlock(); return ret; } +#endif /* CONFIG_POSIX_TIMERS */ void do_notify_pidfd(struct task_struct *task) { diff --git a/kernel/time/itimer.c b/kernel/time/itimer.c index 00629e658ca1..876d389b2e21 100644 --- a/kernel/time/itimer.c +++ b/kernel/time/itimer.c @@ -151,7 +151,27 @@ COMPAT_SYSCALL_DEFINE2(getitimer, int, which, #endif /* - * The timer is automagically restarted, when interval != 0 + * Invoked from dequeue_signal() when SIG_ALRM is delivered. + * + * Restart the ITIMER_REAL timer if it is armed as periodic timer. Doing + * this in the signal delivery path instead of self rearming prevents a DoS + * with small increments in the high reolution timer case and reduces timer + * noise in general. + */ +void posixtimer_rearm_itimer(struct task_struct *tsk) +{ + struct hrtimer *tmr = &tsk->signal->real_timer; + + if (!hrtimer_is_queued(tmr) && tsk->signal->it_real_incr != 0) { + hrtimer_forward(tmr, tmr->base->get_time(), + tsk->signal->it_real_incr); + hrtimer_restart(tmr); + } +} + +/* + * Interval timers are restarted in the signal delivery path. See + * posixtimer_rearm_itimer(). */ enum hrtimer_restart it_real_fn(struct hrtimer *timer) { diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index fc40dacabe78..d461a32b7260 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -251,7 +251,7 @@ static void common_hrtimer_rearm(struct k_itimer *timr) /* * This function is called from the signal delivery code if - * info->si_sys_private is not zero, which indicates that the timer has to + * info::si_sys_private is not zero, which indicates that the timer has to * be rearmed. Restart the timer and update info::si_overrun. */ void posixtimer_rearm(struct kernel_siginfo *info) @@ -259,9 +259,15 @@ void posixtimer_rearm(struct kernel_siginfo *info) struct k_itimer *timr; unsigned long flags; + /* + * Release siglock to ensure proper locking order versus + * timr::it_lock. Keep interrupts disabled. + */ + spin_unlock(¤t->sighand->siglock); + timr = lock_timer(info->si_tid, &flags); if (!timr) - return; + goto out; if (timr->it_interval && timr->it_requeue_pending == info->si_sys_private) { timr->kclock->timer_rearm(timr); @@ -275,6 +281,11 @@ void posixtimer_rearm(struct kernel_siginfo *info) } unlock_timer(timr, flags); +out: + spin_lock(¤t->sighand->siglock); + + /* Don't expose the si_sys_private value to userspace */ + info->si_sys_private = 0; } int posix_timer_queue_signal(struct k_itimer *timr) -- cgit v1.2.3 From 4febce44cfebcb490b196d5d10ae9f403ca4c956 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 1 Oct 2024 10:42:03 +0200 Subject: posix-timers: Cure si_sys_private race The si_sys_private member of the siginfo which is embedded in the preallocated sigqueue is used by the posix timer code to decide whether a timer must be reprogrammed on signal delivery. The handling of this is racy as a long standing comment in that code documents. It is modified with the timer lock held, but without sighand lock being held. The actual signal delivery code checks for it under sighand lock without holding the timer lock. Hand the new value to send_sigqueue() as argument and store it with sighand lock held. This is an intermediate change to address this issue. The arguments to this function will be cleanup in subsequent changes. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241001083835.434338954@linutronix.de --- include/linux/sched/signal.h | 2 +- kernel/signal.c | 10 +++++++++- kernel/time/posix-timers.c | 15 +-------------- 3 files changed, 11 insertions(+), 16 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h index c8ed09ac29ac..bd9f569231d9 100644 --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -340,7 +340,7 @@ extern int send_sig(int, struct task_struct *, int); extern int zap_other_threads(struct task_struct *p); extern struct sigqueue *sigqueue_alloc(void); extern void sigqueue_free(struct sigqueue *); -extern int send_sigqueue(struct sigqueue *, struct pid *, enum pid_type); +extern int send_sigqueue(struct sigqueue *, struct pid *, enum pid_type, int si_private); extern int do_sigaction(int, struct k_sigaction *, struct k_sigaction *); static inline void clear_notify_signal(void) diff --git a/kernel/signal.c b/kernel/signal.c index f420c430b24a..1563c83ff224 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1919,7 +1919,7 @@ void sigqueue_free(struct sigqueue *q) __sigqueue_free(q); } -int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type) +int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type, int si_private) { int sig = q->info.si_signo; struct sigpending *pending; @@ -1954,6 +1954,14 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type) if (!likely(lock_task_sighand(t, &flags))) goto ret; + /* + * Update @q::info::si_sys_private for posix timer signals with + * sighand locked to prevent a race against dequeue_signal() which + * decides based on si_sys_private whether to invoke + * posixtimer_rearm() or not. + */ + q->info.si_sys_private = si_private; + ret = 1; /* the signal is ignored */ result = TRACE_SIGNAL_IGNORED; if (!prepare_signal(sig, t, false)) diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index d461a32b7260..05af074285fa 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -299,21 +299,8 @@ int posix_timer_queue_signal(struct k_itimer *timr) if (timr->it_interval) si_private = ++timr->it_requeue_pending; - /* - * FIXME: if ->sigq is queued we can race with - * dequeue_signal()->posixtimer_rearm(). - * - * If dequeue_signal() sees the "right" value of - * si_sys_private it calls posixtimer_rearm(). - * We re-queue ->sigq and drop ->it_lock(). - * posixtimer_rearm() locks the timer - * and re-schedules it while ->sigq is pending. - * Not really bad, but not that we want. - */ - timr->sigq->info.si_sys_private = si_private; - type = !(timr->it_sigev_notify & SIGEV_THREAD_ID) ? PIDTYPE_TGID : PIDTYPE_PID; - ret = send_sigqueue(timr->sigq, timr->it_pid, type); + ret = send_sigqueue(timr->sigq, timr->it_pid, type, si_private); /* If we failed to send the signal the timer stops. */ return ret > 0; } -- cgit v1.2.3 From c775ea28d4e23f5e58b6953645ef90c1b27a8e83 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 1 Oct 2024 10:42:04 +0200 Subject: signal: Allow POSIX timer signals to be dropped In case that a timer was reprogrammed or deleted an already pending signal is obsolete. Right now such signals are kept around and eventually delivered. While POSIX is blury about this: - "The effect of disarming or resetting a timer with pending expiration notifications is unspecified." - "The disposition of pending signals for the deleted timer is unspecified." it is reasonable in both cases to expect that pending signals are discarded as they have no meaning anymore. Prepare the signal code to allow dropping posix timer signals. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241001083835.494416923@linutronix.de --- include/linux/posix-timers.h | 5 +++-- kernel/signal.c | 7 ++++--- kernel/time/posix-timers.c | 3 ++- 3 files changed, 9 insertions(+), 6 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 670bf03a56ef..4ab49e5c42af 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -100,8 +100,9 @@ static inline void posix_cputimers_rt_watchdog(struct posix_cputimers *pct, { pct->bases[CPUCLOCK_SCHED].nextevt = runtime; } + void posixtimer_rearm_itimer(struct task_struct *p); -void posixtimer_rearm(struct kernel_siginfo *info); +bool posixtimer_deliver_signal(struct kernel_siginfo *info); /* Init task static initializer */ #define INIT_CPU_TIMERBASE(b) { \ @@ -125,7 +126,7 @@ static inline void posix_cputimers_init(struct posix_cputimers *pct) { } static inline void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit) { } static inline void posixtimer_rearm_itimer(struct task_struct *p) { } -static inline void posixtimer_rearm(struct kernel_siginfo *info) { } +static inline bool posixtimer_deliver_signal(struct kernel_siginfo *info) { return false; } #endif #ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK diff --git a/kernel/signal.c b/kernel/signal.c index 1563c83ff224..df34aa47181e 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -594,6 +594,7 @@ int dequeue_signal(sigset_t *mask, kernel_siginfo_t *info, enum pid_type *type) lockdep_assert_held(&tsk->sighand->siglock); +again: *type = PIDTYPE_PID; signr = __dequeue_signal(&tsk->pending, mask, info, &resched_timer); if (!signr) { @@ -625,9 +626,9 @@ int dequeue_signal(sigset_t *mask, kernel_siginfo_t *info, enum pid_type *type) current->jobctl |= JOBCTL_STOP_DEQUEUED; } - if (IS_ENABLED(CONFIG_POSIX_TIMERS)) { - if (unlikely(resched_timer)) - posixtimer_rearm(info); + if (IS_ENABLED(CONFIG_POSIX_TIMERS) && unlikely(resched_timer)) { + if (!posixtimer_deliver_signal(info)) + goto again; } return signr; diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 05af074285fa..dd0b1dff54d9 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -254,7 +254,7 @@ static void common_hrtimer_rearm(struct k_itimer *timr) * info::si_sys_private is not zero, which indicates that the timer has to * be rearmed. Restart the timer and update info::si_overrun. */ -void posixtimer_rearm(struct kernel_siginfo *info) +bool posixtimer_deliver_signal(struct kernel_siginfo *info) { struct k_itimer *timr; unsigned long flags; @@ -286,6 +286,7 @@ out: /* Don't expose the si_sys_private value to userspace */ info->si_sys_private = 0; + return true; } int posix_timer_queue_signal(struct k_itimer *timr) -- cgit v1.2.3 From 2860d4d315dc01f001dfd328adaf2ab440c47dd3 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 1 Oct 2024 10:42:06 +0200 Subject: posix-timers: Drop signal if timer has been deleted or reprogrammed No point in delivering a signal from the past. POSIX does not specify the behaviour here: - "The effect of disarming or resetting a timer with pending expiration notifications is unspecified." - "The disposition of pending signals for the deleted timer is unspecified." In both cases it is reasonable to expect that pending signals are discarded. Especially in the reprogramming case it does not make sense to account for previous overruns or to deliver a signal for a timer which has been disarmed. Drop the signal as that is conistent and understandable behaviour. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241001083835.553646280@linutronix.de --- kernel/time/posix-timers.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index dd0b1dff54d9..22e1d6bf349b 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -250,14 +250,14 @@ static void common_hrtimer_rearm(struct k_itimer *timr) } /* - * This function is called from the signal delivery code if - * info::si_sys_private is not zero, which indicates that the timer has to - * be rearmed. Restart the timer and update info::si_overrun. + * This function is called from the signal delivery code. It decides + * whether the signal should be dropped and rearms interval timers. */ bool posixtimer_deliver_signal(struct kernel_siginfo *info) { struct k_itimer *timr; unsigned long flags; + bool ret = false; /* * Release siglock to ensure proper locking order versus @@ -279,6 +279,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info) info->si_overrun = timer_overrun_to_int(timr, info->si_overrun); } + ret = true; unlock_timer(timr, flags); out: @@ -286,7 +287,7 @@ out: /* Don't expose the si_sys_private value to userspace */ info->si_sys_private = 0; - return true; + return ret; } int posix_timer_queue_signal(struct k_itimer *timr) -- cgit v1.2.3 From cd1e93aedab7f749760a33e9e094381973b1120e Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 1 Oct 2024 10:42:07 +0200 Subject: posix-timers: Rename k_itimer:: It_requeue_pending Prepare for using this struct member to do a proper reprogramming and deletion accounting so that stale signals can be dropped. No functional change. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241001083835.611997737@linutronix.de --- include/linux/posix-timers.h | 5 ++--- kernel/time/alarmtimer.c | 2 +- kernel/time/posix-cpu-timers.c | 4 ++-- kernel/time/posix-timers.c | 12 ++++++------ 4 files changed, 11 insertions(+), 12 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 4ab49e5c42af..253d106fac2c 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -150,8 +150,7 @@ static inline void posix_cputimers_init_work(void) { } * @it_active: Marker that timer is active * @it_overrun: The overrun counter for pending signals * @it_overrun_last: The overrun at the time of the last delivered signal - * @it_requeue_pending: Indicator that timer waits for being requeued on - * signal delivery + * @it_signal_seq: Sequence count to control signal delivery * @it_sigev_notify: The notify word of sigevent struct for signal delivery * @it_interval: The interval for periodic timers * @it_signal: Pointer to the creators signal struct @@ -172,7 +171,7 @@ struct k_itimer { int it_active; s64 it_overrun; s64 it_overrun_last; - int it_requeue_pending; + unsigned int it_signal_seq; int it_sigev_notify; ktime_t it_interval; struct signal_struct *it_signal; diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index 8bf888641694..75f844385070 100644 --- a/kernel/time/alarmtimer.c +++ b/kernel/time/alarmtimer.c @@ -584,7 +584,7 @@ static enum alarmtimer_restart alarm_handle_timer(struct alarm *alarm, * small intervals cannot starve the system. */ ptr->it_overrun += __alarm_forward_now(alarm, ptr->it_interval, true); - ++ptr->it_requeue_pending; + ++ptr->it_signal_seq; ptr->it_active = 1; result = ALARMTIMER_RESTART; } diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 6bcee4704059..993243b5be98 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -608,7 +608,7 @@ static void cpu_timer_fire(struct k_itimer *timer) * ticking in case the signal is deliverable next time. */ posix_cpu_timer_rearm(timer); - ++timer->it_requeue_pending; + ++timer->it_signal_seq; } } @@ -745,7 +745,7 @@ static void __posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *i * - Timers which expired, but the signal has not yet been * delivered */ - if (iv && ((timer->it_requeue_pending & REQUEUE_PENDING) || sigev_none)) + if (iv && ((timer->it_signal_seq & REQUEUE_PENDING) || sigev_none)) expires = bump_cpu_timer(timer, now); else expires = cpu_timer_getexpires(&timer->it.cpu); diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 22e1d6bf349b..fd321fcc3f6c 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -269,13 +269,13 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info) if (!timr) goto out; - if (timr->it_interval && timr->it_requeue_pending == info->si_sys_private) { + if (timr->it_interval && timr->it_signal_seq == info->si_sys_private) { timr->kclock->timer_rearm(timr); timr->it_active = 1; timr->it_overrun_last = timr->it_overrun; timr->it_overrun = -1LL; - ++timr->it_requeue_pending; + ++timr->it_signal_seq; info->si_overrun = timer_overrun_to_int(timr, info->si_overrun); } @@ -299,7 +299,7 @@ int posix_timer_queue_signal(struct k_itimer *timr) timr->it_active = 0; if (timr->it_interval) - si_private = ++timr->it_requeue_pending; + si_private = ++timr->it_signal_seq; type = !(timr->it_sigev_notify & SIGEV_THREAD_ID) ? PIDTYPE_TGID : PIDTYPE_PID; ret = send_sigqueue(timr->sigq, timr->it_pid, type, si_private); @@ -366,7 +366,7 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer) timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval); ret = HRTIMER_RESTART; - ++timr->it_requeue_pending; + ++timr->it_signal_seq; timr->it_active = 1; } } @@ -660,7 +660,7 @@ void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting) * is a SIGEV_NONE timer move the expiry time forward by intervals, * so expiry is > now. */ - if (iv && (timr->it_requeue_pending & REQUEUE_PENDING || sig_none)) + if (iv && (timr->it_signal_seq & REQUEUE_PENDING || sig_none)) timr->it_overrun += kc->timer_forward(timr, now); remaining = kc->timer_remaining(timr, now); @@ -861,7 +861,7 @@ void posix_timer_set_common(struct k_itimer *timer, struct itimerspec64 *new_set timer->it_interval = 0; /* Prevent reloading in case there is a signal pending */ - timer->it_requeue_pending = (timer->it_requeue_pending + 2) & ~REQUEUE_PENDING; + timer->it_signal_seq = (timer->it_signal_seq + 2) & ~REQUEUE_PENDING; /* Reset overrun accounting */ timer->it_overrun_last = 0; timer->it_overrun = -1LL; -- cgit v1.2.3 From 1550dde8a537b35dbf066c7f9cfe5f9b360bce0d Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 1 Oct 2024 10:42:09 +0200 Subject: posix-timers: Add proper state tracking Right now the state tracking is done by two struct members: - it_active: A boolean which tracks armed/disarmed state - it_signal_seq: A sequence counter which is used to invalidate settings and prevent rearming Replace it_active with it_status and keep properly track about the states in one place. This allows to reuse it_signal_seq to track reprogramming, disarm and delete operations in order to drop signals which are related to the state previous of those operations. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241001083835.670337048@linutronix.de --- include/linux/posix-timers.h | 4 ++-- kernel/time/alarmtimer.c | 2 +- kernel/time/posix-cpu-timers.c | 15 ++++++++------- kernel/time/posix-timers.c | 22 +++++++++++++--------- kernel/time/posix-timers.h | 6 ++++++ 5 files changed, 30 insertions(+), 19 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 253d106fac2c..02afbb4da7f7 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -147,7 +147,7 @@ static inline void posix_cputimers_init_work(void) { } * @kclock: Pointer to the k_clock struct handling this timer * @it_clock: The posix timer clock id * @it_id: The posix timer id for identifying the timer - * @it_active: Marker that timer is active + * @it_status: The status of the timer * @it_overrun: The overrun counter for pending signals * @it_overrun_last: The overrun at the time of the last delivered signal * @it_signal_seq: Sequence count to control signal delivery @@ -168,7 +168,7 @@ struct k_itimer { const struct k_clock *kclock; clockid_t it_clock; timer_t it_id; - int it_active; + int it_status; s64 it_overrun; s64 it_overrun_last; unsigned int it_signal_seq; diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index 75f844385070..452d8aa2f6e0 100644 --- a/kernel/time/alarmtimer.c +++ b/kernel/time/alarmtimer.c @@ -585,7 +585,7 @@ static enum alarmtimer_restart alarm_handle_timer(struct alarm *alarm, */ ptr->it_overrun += __alarm_forward_now(alarm, ptr->it_interval, true); ++ptr->it_signal_seq; - ptr->it_active = 1; + ptr->it_status = POSIX_TIMER_ARMED; result = ALARMTIMER_RESTART; } spin_unlock_irqrestore(&ptr->it_lock, flags); diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 993243b5be98..12f828d704b1 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -453,7 +453,6 @@ static void disarm_timer(struct k_itimer *timer, struct task_struct *p) struct cpu_timer *ctmr = &timer->it.cpu; struct posix_cputimer_base *base; - timer->it_active = 0; if (!cpu_timer_dequeue(ctmr)) return; @@ -494,11 +493,12 @@ static int posix_cpu_timer_del(struct k_itimer *timer) */ WARN_ON_ONCE(ctmr->head || timerqueue_node_queued(&ctmr->node)); } else { - if (timer->it.cpu.firing) + if (timer->it.cpu.firing) { ret = TIMER_RETRY; - else + } else { disarm_timer(timer, p); - + timer->it_status = POSIX_TIMER_DISARMED; + } unlock_task_sighand(p, &flags); } @@ -560,7 +560,7 @@ static void arm_timer(struct k_itimer *timer, struct task_struct *p) struct cpu_timer *ctmr = &timer->it.cpu; u64 newexp = cpu_timer_getexpires(ctmr); - timer->it_active = 1; + timer->it_status = POSIX_TIMER_ARMED; if (!cpu_timer_enqueue(&base->tqhead, ctmr)) return; @@ -586,7 +586,8 @@ static void cpu_timer_fire(struct k_itimer *timer) { struct cpu_timer *ctmr = &timer->it.cpu; - timer->it_active = 0; + timer->it_status = POSIX_TIMER_DISARMED; + if (unlikely(timer->sigq == NULL)) { /* * This a special case for clock_nanosleep, @@ -671,7 +672,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags, ret = TIMER_RETRY; } else { cpu_timer_dequeue(ctmr); - timer->it_active = 0; + timer->it_status = POSIX_TIMER_DISARMED; } /* diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index fd321fcc3f6c..dd72b8e72697 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -272,7 +272,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info) if (timr->it_interval && timr->it_signal_seq == info->si_sys_private) { timr->kclock->timer_rearm(timr); - timr->it_active = 1; + timr->it_status = POSIX_TIMER_ARMED; timr->it_overrun_last = timr->it_overrun; timr->it_overrun = -1LL; ++timr->it_signal_seq; @@ -292,14 +292,17 @@ out: int posix_timer_queue_signal(struct k_itimer *timr) { + enum posix_timer_state state = POSIX_TIMER_DISARMED; int ret, si_private = 0; enum pid_type type; lockdep_assert_held(&timr->it_lock); - timr->it_active = 0; - if (timr->it_interval) + if (timr->it_interval) { + state = POSIX_TIMER_REQUEUE_PENDING; si_private = ++timr->it_signal_seq; + } + timr->it_status = state; type = !(timr->it_sigev_notify & SIGEV_THREAD_ID) ? PIDTYPE_TGID : PIDTYPE_PID; ret = send_sigqueue(timr->sigq, timr->it_pid, type, si_private); @@ -367,7 +370,7 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer) timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval); ret = HRTIMER_RESTART; ++timr->it_signal_seq; - timr->it_active = 1; + timr->it_status = POSIX_TIMER_ARMED; } } @@ -640,10 +643,10 @@ void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting) /* interval timer ? */ if (iv) { cur_setting->it_interval = ktime_to_timespec64(iv); - } else if (!timr->it_active) { + } else if (timr->it_status == POSIX_TIMER_DISARMED) { /* * SIGEV_NONE oneshot timers are never queued and therefore - * timr->it_active is always false. The check below + * timr->it_status is always DISARMED. The check below * vs. remaining time will handle this case. * * For all other timers there is nothing to update here, so @@ -888,7 +891,7 @@ int common_timer_set(struct k_itimer *timr, int flags, if (kc->timer_try_to_cancel(timr) < 0) return TIMER_RETRY; - timr->it_active = 0; + timr->it_status = POSIX_TIMER_DISARMED; posix_timer_set_common(timr, new_setting); /* Keep timer disarmed when it_value is zero */ @@ -901,7 +904,8 @@ int common_timer_set(struct k_itimer *timr, int flags, sigev_none = timr->it_sigev_notify == SIGEV_NONE; kc->timer_arm(timr, expires, flags & TIMER_ABSTIME, sigev_none); - timr->it_active = !sigev_none; + if (!sigev_none) + timr->it_status = POSIX_TIMER_ARMED; return 0; } @@ -1000,7 +1004,7 @@ int common_timer_del(struct k_itimer *timer) timer->it_interval = 0; if (kc->timer_try_to_cancel(timer) < 0) return TIMER_RETRY; - timer->it_active = 0; + timer->it_status = POSIX_TIMER_DISARMED; return 0; } diff --git a/kernel/time/posix-timers.h b/kernel/time/posix-timers.h index 4784ea65f685..4d09677e584e 100644 --- a/kernel/time/posix-timers.h +++ b/kernel/time/posix-timers.h @@ -1,6 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0 */ #define TIMER_RETRY 1 +enum posix_timer_state { + POSIX_TIMER_DISARMED, + POSIX_TIMER_ARMED, + POSIX_TIMER_REQUEUE_PENDING, +}; + struct k_clock { int (*clock_getres)(const clockid_t which_clock, struct timespec64 *tp); -- cgit v1.2.3 From 513793bc6ab331b947111e8efaf8fcef33fb83e5 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:31 +0100 Subject: posix-timers: Make signal delivery consistent Signals of timers which are reprogammed, disarmed or deleted can deliver signals related to the past. The POSIX spec is blury about this: - "The effect of disarming or resetting a timer with pending expiration notifications is unspecified." - "The disposition of pending signals for the deleted timer is unspecified." In both cases it is reasonable to expect that pending signals are discarded. Especially in the reprogramming case it does not make sense to account for previous overruns or to deliver a signal for a timer which has been disarmed. This makes the behaviour consistent and understandable. Remove the si_sys_private check from the signal delivery code and invoke posix_timer_deliver_signal() unconditionally for posix timer related signals. Change posix_timer_deliver_signal() so it controls the actual signal delivery via the return value. It now instructs the signal code to drop the signal when: 1) The timer does not longer exist in the hash table 2) The timer signal_seq value is not the same as the si_sys_private value which was set when the signal was queued. This is also a preparatory change to embed the sigqueue into the k_itimer structure, which in turn allows to remove the si_sys_private magic. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Link: https://lore.kernel.org/all/20241105064213.040348644@linutronix.de --- include/linux/posix-timers.h | 2 -- kernel/signal.c | 6 ++---- kernel/time/posix-cpu-timers.c | 2 +- kernel/time/posix-timers.c | 28 ++++++++++++++++------------ 4 files changed, 19 insertions(+), 19 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 02afbb4da7f7..8c6d97412526 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -137,8 +137,6 @@ static inline void clear_posix_cputimers_work(struct task_struct *p) { } static inline void posix_cputimers_init_work(void) { } #endif -#define REQUEUE_PENDING 1 - /** * struct k_itimer - POSIX.1b interval timer structure. * @list: List head for binding the timer to signals->posix_timers diff --git a/kernel/signal.c b/kernel/signal.c index df34aa47181e..68e6bc70ccf2 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -550,10 +550,8 @@ still_pending: list_del_init(&first->list); copy_siginfo(info, &first->info); - *resched_timer = - (first->flags & SIGQUEUE_PREALLOC) && - (info->si_code == SI_TIMER) && - (info->si_sys_private); + *resched_timer = (first->flags & SIGQUEUE_PREALLOC) && + (info->si_code == SI_TIMER); __sigqueue_free(first); } else { diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 5f444e372464..4305c003c8d4 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -746,7 +746,7 @@ static void __posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *i * - Timers which expired, but the signal has not yet been * delivered */ - if (iv && ((timer->it_signal_seq & REQUEUE_PENDING) || sigev_none)) + if (iv && timer->it_status != POSIX_TIMER_ARMED) expires = bump_cpu_timer(timer, now); else expires = cpu_timer_getexpires(&timer->it.cpu); diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index dd72b8e72697..b380e25d4947 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -269,7 +269,10 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info) if (!timr) goto out; - if (timr->it_interval && timr->it_signal_seq == info->si_sys_private) { + if (timr->it_signal_seq != info->si_sys_private) + goto out_unlock; + + if (timr->it_interval && !WARN_ON_ONCE(timr->it_status != POSIX_TIMER_REQUEUE_PENDING)) { timr->kclock->timer_rearm(timr); timr->it_status = POSIX_TIMER_ARMED; @@ -281,6 +284,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info) } ret = true; +out_unlock: unlock_timer(timr, flags); out: spin_lock(¤t->sighand->siglock); @@ -293,19 +297,18 @@ out: int posix_timer_queue_signal(struct k_itimer *timr) { enum posix_timer_state state = POSIX_TIMER_DISARMED; - int ret, si_private = 0; enum pid_type type; + int ret; lockdep_assert_held(&timr->it_lock); - if (timr->it_interval) { + if (timr->it_interval) state = POSIX_TIMER_REQUEUE_PENDING; - si_private = ++timr->it_signal_seq; - } + timr->it_status = state; type = !(timr->it_sigev_notify & SIGEV_THREAD_ID) ? PIDTYPE_TGID : PIDTYPE_PID; - ret = send_sigqueue(timr->sigq, timr->it_pid, type, si_private); + ret = send_sigqueue(timr->sigq, timr->it_pid, type, timr->it_signal_seq); /* If we failed to send the signal the timer stops. */ return ret > 0; } @@ -663,7 +666,7 @@ void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting) * is a SIGEV_NONE timer move the expiry time forward by intervals, * so expiry is > now. */ - if (iv && (timr->it_signal_seq & REQUEUE_PENDING || sig_none)) + if (iv && timr->it_status != POSIX_TIMER_ARMED) timr->it_overrun += kc->timer_forward(timr, now); remaining = kc->timer_remaining(timr, now); @@ -863,8 +866,6 @@ void posix_timer_set_common(struct k_itimer *timer, struct itimerspec64 *new_set else timer->it_interval = 0; - /* Prevent reloading in case there is a signal pending */ - timer->it_signal_seq = (timer->it_signal_seq + 2) & ~REQUEUE_PENDING; /* Reset overrun accounting */ timer->it_overrun_last = 0; timer->it_overrun = -1LL; @@ -882,8 +883,6 @@ int common_timer_set(struct k_itimer *timr, int flags, if (old_setting) common_timer_get(timr, old_setting); - /* Prevent rearming by clearing the interval */ - timr->it_interval = 0; /* * Careful here. On SMP systems the timer expiry function could be * active and spinning on timr->it_lock. @@ -933,6 +932,9 @@ retry: if (old_spec64) old_spec64->it_interval = ktime_to_timespec64(timr->it_interval); + /* Prevent signal delivery and rearming. */ + timr->it_signal_seq++; + kc = timr->kclock; if (WARN_ON_ONCE(!kc || !kc->timer_set)) error = -EINVAL; @@ -1001,7 +1003,6 @@ int common_timer_del(struct k_itimer *timer) { const struct k_clock *kc = timer->kclock; - timer->it_interval = 0; if (kc->timer_try_to_cancel(timer) < 0) return TIMER_RETRY; timer->it_status = POSIX_TIMER_DISARMED; @@ -1012,6 +1013,9 @@ static inline int timer_delete_hook(struct k_itimer *timer) { const struct k_clock *kc = timer->kclock; + /* Prevent signal delivery and rearming. */ + timer->it_signal_seq++; + if (WARN_ON_ONCE(!kc || !kc->timer_del)) return -EINVAL; return kc->timer_del(timer); -- cgit v1.2.3 From b06b0345fff3678517acd0f1837d52477ba30944 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:32 +0100 Subject: posix-timers: Make signal overrun accounting sensible The handling of the timer overrun in the signal code is inconsistent as it takes previous overruns into account. This is just wrong as after the reprogramming of a timer the overrun count starts over from a clean state, i.e. 0. Don't touch info::si_overrun in send_sigqueue() and only store the overrun value at signal delivery time, which is computed from the timer itself relative to the expiry time. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064213.106738193@linutronix.de --- kernel/signal.c | 6 ------ kernel/time/posix-timers.c | 11 ++++++----- 2 files changed, 6 insertions(+), 11 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/kernel/signal.c b/kernel/signal.c index 68e6bc70ccf2..ba7159b25d51 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1968,15 +1968,9 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type, int s ret = 0; if (unlikely(!list_empty(&q->list))) { - /* - * If an SI_TIMER entry is already queue just increment - * the overrun count. - */ - q->info.si_overrun++; result = TRACE_SIGNAL_ALREADY_PENDING; goto out; } - q->info.si_overrun = 0; signalfd_notify(t, sig); pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending; diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index b380e25d4947..66ed49efc02f 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -233,11 +233,12 @@ __initcall(init_posix_timers); * The siginfo si_overrun field and the return value of timer_getoverrun(2) * are of type int. Clamp the overrun value to INT_MAX */ -static inline int timer_overrun_to_int(struct k_itimer *timr, int baseval) +static inline int timer_overrun_to_int(struct k_itimer *timr) { - s64 sum = timr->it_overrun_last + (s64)baseval; + if (timr->it_overrun_last > (s64)INT_MAX) + return INT_MAX; - return sum > (s64)INT_MAX ? INT_MAX : (int)sum; + return (int)timr->it_overrun_last; } static void common_hrtimer_rearm(struct k_itimer *timr) @@ -280,7 +281,7 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info) timr->it_overrun = -1LL; ++timr->it_signal_seq; - info->si_overrun = timer_overrun_to_int(timr, info->si_overrun); + info->si_overrun = timer_overrun_to_int(timr); } ret = true; @@ -774,7 +775,7 @@ SYSCALL_DEFINE1(timer_getoverrun, timer_t, timer_id) if (!timr) return -EINVAL; - overrun = timer_overrun_to_int(timr, 0); + overrun = timer_overrun_to_int(timr); unlock_timer(timr, flags); return overrun; -- cgit v1.2.3 From 5d916a0988eed5217c103932ff4887c9ae83c89c Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:36 +0100 Subject: posix-timers: Add a refcount to struct k_itimer 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. To make that work correctly it needs reference counting so that timer deletion does not free the timer prematuraly when there is a signal queued or delivered concurrently. Add a rcuref to the posix timer part. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064213.304756440@linutronix.de --- include/linux/posix-timers.h | 14 ++++++++++++++ kernel/time/posix-timers.c | 7 ++++--- 2 files changed, 18 insertions(+), 3 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index bcd01208d795..9740fd0c2933 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -6,11 +6,13 @@ #include #include #include +#include #include #include struct kernel_siginfo; struct task_struct; +struct k_itimer; static inline clockid_t make_process_cpuclock(const unsigned int pid, const clockid_t clock) @@ -105,6 +107,7 @@ static inline void posix_cputimers_rt_watchdog(struct posix_cputimers *pct, void posixtimer_rearm_itimer(struct task_struct *p); bool posixtimer_deliver_signal(struct kernel_siginfo *info); +void posixtimer_free_timer(struct k_itimer *timer); /* Init task static initializer */ #define INIT_CPU_TIMERBASE(b) { \ @@ -129,6 +132,7 @@ static inline void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit) { } static inline void posixtimer_rearm_itimer(struct task_struct *p) { } static inline bool posixtimer_deliver_signal(struct kernel_siginfo *info) { return false; } +static inline void posixtimer_free_timer(struct k_itimer *timer) { } #endif #ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK @@ -156,6 +160,7 @@ static inline void posix_cputimers_init_work(void) { } * @it_signal: Pointer to the creators signal struct * @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 * @it: Union representing the various posix timer type * internals. @@ -180,6 +185,7 @@ struct k_itimer { struct task_struct *it_process; }; struct sigqueue *sigq; + rcuref_t rcuref; union { struct { struct hrtimer timer; @@ -200,4 +206,12 @@ void set_process_cpu_timer(struct task_struct *task, unsigned int clock_idx, int update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new); +#ifdef CONFIG_POSIX_TIMERS +static inline void posixtimer_putref(struct k_itimer *tmr) +{ + if (rcuref_put(&tmr->rcuref)) + posixtimer_free_timer(tmr); +} +#endif /* !CONFIG_POSIX_TIMERS */ + #endif diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 66ed49efc02f..53bd3c4de92c 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -417,10 +417,11 @@ static struct k_itimer * alloc_posix_timer(void) return NULL; } clear_siginfo(&tmr->sigq->info); + rcuref_init(&tmr->rcuref, 1); return tmr; } -static void posix_timer_free(struct k_itimer *tmr) +void posixtimer_free_timer(struct k_itimer *tmr) { put_pid(tmr->it_pid); sigqueue_free(tmr->sigq); @@ -432,7 +433,7 @@ static void posix_timer_unhash_and_free(struct k_itimer *tmr) spin_lock(&hash_lock); hlist_del_rcu(&tmr->t_hash); spin_unlock(&hash_lock); - posix_timer_free(tmr); + posixtimer_putref(tmr); } static int common_timer_create(struct k_itimer *new_timer) @@ -467,7 +468,7 @@ static int do_timer_create(clockid_t which_clock, struct sigevent *event, */ new_timer_id = posix_timer_add(new_timer); if (new_timer_id < 0) { - posix_timer_free(new_timer); + posixtimer_free_timer(new_timer); return new_timer_id; } -- cgit v1.2.3 From ef1c5bcd6daa674392bdf89b8ae889aafd73f956 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:41 +0100 Subject: posix-timers: Store PID type in the timer instead of re-evaluating the signal delivery mode everywhere. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064213.519086500@linutronix.de --- include/linux/posix-timers.h | 2 ++ kernel/time/posix-timers.c | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 200098d27cc0..947176582de9 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -180,6 +181,7 @@ struct k_itimer { s64 it_overrun_last; unsigned int it_signal_seq; int it_sigev_notify; + enum pid_type it_pid_type; ktime_t it_interval; struct signal_struct *it_signal; union { diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 53bd3c4de92c..f18d64c7cd3b 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -298,7 +298,6 @@ out: int posix_timer_queue_signal(struct k_itimer *timr) { enum posix_timer_state state = POSIX_TIMER_DISARMED; - enum pid_type type; int ret; lockdep_assert_held(&timr->it_lock); @@ -308,8 +307,7 @@ int posix_timer_queue_signal(struct k_itimer *timr) timr->it_status = state; - type = !(timr->it_sigev_notify & SIGEV_THREAD_ID) ? PIDTYPE_TGID : PIDTYPE_PID; - ret = send_sigqueue(timr->sigq, timr->it_pid, type, timr->it_signal_seq); + ret = send_sigqueue(timr->sigq, timr->it_pid, timr->it_pid_type, timr->it_signal_seq); /* If we failed to send the signal the timer stops. */ return ret > 0; } @@ -496,6 +494,11 @@ static int do_timer_create(clockid_t which_clock, struct sigevent *event, new_timer->it_pid = get_pid(task_tgid(current)); } + if (new_timer->it_sigev_notify & SIGEV_THREAD_ID) + new_timer->it_pid_type = PIDTYPE_PID; + 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; -- cgit v1.2.3 From 0360ed14d9826678a50fa2b873e522a24cd3c018 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:42 +0100 Subject: signal: Refactor send_sigqueue() To handle posix timers which have their signal ignored via SIG_IGN properly it is required to requeue a ignored signal for delivery when SIG_IGN is lifted so the timer gets rearmed. Split the required code out of send_sigqueue() so it can be reused in context of sigaction(). While at it rename send_sigqueue() to posixtimer_send_sigqueue() so its clear what this is about. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064213.586453412@linutronix.de --- include/linux/posix-timers.h | 1 + include/linux/sched/signal.h | 1 - kernel/signal.c | 82 ++++++++++++++++++++++++-------------------- kernel/time/posix-timers.c | 2 +- 4 files changed, 47 insertions(+), 39 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 947176582de9..52611ea923b2 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -109,6 +109,7 @@ static inline void posix_cputimers_rt_watchdog(struct posix_cputimers *pct, void posixtimer_rearm_itimer(struct task_struct *p); bool posixtimer_init_sigqueue(struct sigqueue *q); +int posixtimer_send_sigqueue(struct k_itimer *tmr); bool posixtimer_deliver_signal(struct kernel_siginfo *info); void posixtimer_free_timer(struct k_itimer *timer); diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h index bd9f569231d9..36283c1c55e9 100644 --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -340,7 +340,6 @@ extern int send_sig(int, struct task_struct *, int); extern int zap_other_threads(struct task_struct *p); extern struct sigqueue *sigqueue_alloc(void); extern void sigqueue_free(struct sigqueue *); -extern int send_sigqueue(struct sigqueue *, struct pid *, enum pid_type, int si_private); extern int do_sigaction(int, struct k_sigaction *, struct k_sigaction *); static inline void clear_notify_signal(void) diff --git a/kernel/signal.c b/kernel/signal.c index 911ed3ab479e..5b71e26abb0e 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1947,40 +1947,54 @@ void sigqueue_free(struct sigqueue *q) __sigqueue_free(q); } -int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type, int si_private) +static void posixtimer_queue_sigqueue(struct sigqueue *q, struct task_struct *t, enum pid_type type) { - int sig = q->info.si_signo; struct sigpending *pending; + int sig = q->info.si_signo; + + signalfd_notify(t, sig); + pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending; + list_add_tail(&q->list, &pending->list); + sigaddset(&pending->signal, sig); + complete_signal(sig, t, type); +} + +/* + * This function is used by POSIX timers to deliver a timer signal. + * Where type is PIDTYPE_PID (such as for timers with SIGEV_THREAD_ID + * set), the signal must be delivered to the specific thread (queues + * into t->pending). + * + * Where type is not PIDTYPE_PID, signals must be delivered to the + * process. In this case, prefer to deliver to current if it is in + * the same thread group as the target process, which avoids + * unnecessarily waking up a potentially idle task. + */ +static inline struct task_struct *posixtimer_get_target(struct k_itimer *tmr) +{ + struct task_struct *t = pid_task(tmr->it_pid, tmr->it_pid_type); + + if (t && tmr->it_pid_type != PIDTYPE_PID && same_thread_group(t, current)) + t = current; + return t; +} + +int posixtimer_send_sigqueue(struct k_itimer *tmr) +{ + struct sigqueue *q = tmr->sigq; + int sig = q->info.si_signo; struct task_struct *t; unsigned long flags; int ret, result; - if (WARN_ON_ONCE(!(q->flags & SIGQUEUE_PREALLOC))) - return 0; - if (WARN_ON_ONCE(q->info.si_code != SI_TIMER)) - return 0; - - ret = -1; - rcu_read_lock(); + guard(rcu)(); - /* - * This function is used by POSIX timers to deliver a timer signal. - * Where type is PIDTYPE_PID (such as for timers with SIGEV_THREAD_ID - * set), the signal must be delivered to the specific thread (queues - * into t->pending). - * - * Where type is not PIDTYPE_PID, signals must be delivered to the - * process. In this case, prefer to deliver to current if it is in - * the same thread group as the target process, which avoids - * unnecessarily waking up a potentially idle task. - */ - t = pid_task(pid, type); + t = posixtimer_get_target(tmr); if (!t) - goto ret; - if (type != PIDTYPE_PID && same_thread_group(t, current)) - t = current; + return -1; + if (!likely(lock_task_sighand(t, &flags))) - goto ret; + return -1; /* * Update @q::info::si_sys_private for posix timer signals with @@ -1988,30 +2002,24 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type, int s * decides based on si_sys_private whether to invoke * posixtimer_rearm() or not. */ - q->info.si_sys_private = si_private; + q->info.si_sys_private = tmr->it_signal_seq; ret = 1; /* the signal is ignored */ - result = TRACE_SIGNAL_IGNORED; - if (!prepare_signal(sig, t, false)) + if (!prepare_signal(sig, t, false)) { + result = TRACE_SIGNAL_IGNORED; goto out; + } ret = 0; if (unlikely(!list_empty(&q->list))) { result = TRACE_SIGNAL_ALREADY_PENDING; goto out; } - - signalfd_notify(t, sig); - pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending; - list_add_tail(&q->list, &pending->list); - sigaddset(&pending->signal, sig); - complete_signal(sig, t, type); + posixtimer_queue_sigqueue(q, t, tmr->it_pid_type); result = TRACE_SIGNAL_DELIVERED; out: - trace_signal_generate(sig, &q->info, t, type != PIDTYPE_PID, result); + trace_signal_generate(sig, &q->info, t, tmr->it_pid_type != PIDTYPE_PID, result); unlock_task_sighand(t, &flags); -ret: - rcu_read_unlock(); return ret; } #endif /* CONFIG_POSIX_TIMERS */ diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index f18d64c7cd3b..0901ed9ca183 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -307,7 +307,7 @@ int posix_timer_queue_signal(struct k_itimer *timr) timr->it_status = state; - ret = send_sigqueue(timr->sigq, timr->it_pid, timr->it_pid_type, timr->it_signal_seq); + ret = posixtimer_send_sigqueue(timr); /* If we failed to send the signal the timer stops. */ return ret > 0; } -- cgit v1.2.3 From 11629b9808e5900d675fd469d19932ea48060de3 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:43 +0100 Subject: signal: Replace resched_timer logic In preparation for handling ignored posix timer signals correctly and embedding the sigqueue struct into struct k_itimer, hand down a pointer to the sigqueue struct into posix_timer_deliver_signal() instead of just having a boolean flag. No functional change. Suggested-by: Eric W. Biederman Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: "Eric W. Biederman" Link: https://lore.kernel.org/all/20241105064213.652658158@linutronix.de --- include/linux/posix-timers.h | 5 +++-- kernel/signal.c | 32 ++++++++++++++++++++------------ kernel/time/posix-timers.c | 2 +- 3 files changed, 24 insertions(+), 15 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 52611ea923b2..39f1db76833a 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -110,7 +110,7 @@ static inline void posix_cputimers_rt_watchdog(struct posix_cputimers *pct, void posixtimer_rearm_itimer(struct task_struct *p); bool posixtimer_init_sigqueue(struct sigqueue *q); int posixtimer_send_sigqueue(struct k_itimer *tmr); -bool posixtimer_deliver_signal(struct kernel_siginfo *info); +bool posixtimer_deliver_signal(struct kernel_siginfo *info, struct sigqueue *timer_sigq); void posixtimer_free_timer(struct k_itimer *timer); /* Init task static initializer */ @@ -135,7 +135,8 @@ static inline void posix_cputimers_init(struct posix_cputimers *pct) { } static inline void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit) { } static inline void posixtimer_rearm_itimer(struct task_struct *p) { } -static inline bool posixtimer_deliver_signal(struct kernel_siginfo *info) { return false; } +static inline bool posixtimer_deliver_signal(struct kernel_siginfo *info, + struct sigqueue *timer_sigq) { return false; } static inline void posixtimer_free_timer(struct k_itimer *timer) { } #endif diff --git a/kernel/signal.c b/kernel/signal.c index 5b71e26abb0e..0ddb5dd284aa 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -545,7 +545,7 @@ bool unhandled_signal(struct task_struct *tsk, int sig) } static void collect_signal(int sig, struct sigpending *list, kernel_siginfo_t *info, - bool *resched_timer) + struct sigqueue **timer_sigq) { struct sigqueue *q, *first = NULL; @@ -568,10 +568,17 @@ still_pending: list_del_init(&first->list); copy_siginfo(info, &first->info); - *resched_timer = (first->flags & SIGQUEUE_PREALLOC) && - (info->si_code == SI_TIMER); - - __sigqueue_free(first); + /* + * posix-timer signals are preallocated and freed when the + * timer goes away. Either directly or by clearing + * SIGQUEUE_PREALLOC so that the next delivery will free + * them. Spare the extra round through __sigqueue_free() + * which is ignoring preallocated signals. + */ + if (unlikely((first->flags & SIGQUEUE_PREALLOC) && (info->si_code == SI_TIMER))) + *timer_sigq = first; + else + __sigqueue_free(first); } else { /* * Ok, it wasn't in the queue. This must be @@ -588,12 +595,12 @@ still_pending: } static int __dequeue_signal(struct sigpending *pending, sigset_t *mask, - kernel_siginfo_t *info, bool *resched_timer) + kernel_siginfo_t *info, struct sigqueue **timer_sigq) { int sig = next_signal(pending, mask); if (sig) - collect_signal(sig, pending, info, resched_timer); + collect_signal(sig, pending, info, timer_sigq); return sig; } @@ -605,18 +612,19 @@ static int __dequeue_signal(struct sigpending *pending, sigset_t *mask, int dequeue_signal(sigset_t *mask, kernel_siginfo_t *info, enum pid_type *type) { struct task_struct *tsk = current; - bool resched_timer = false; + struct sigqueue *timer_sigq; int signr; lockdep_assert_held(&tsk->sighand->siglock); again: *type = PIDTYPE_PID; - signr = __dequeue_signal(&tsk->pending, mask, info, &resched_timer); + timer_sigq = NULL; + signr = __dequeue_signal(&tsk->pending, mask, info, &timer_sigq); if (!signr) { *type = PIDTYPE_TGID; signr = __dequeue_signal(&tsk->signal->shared_pending, - mask, info, &resched_timer); + mask, info, &timer_sigq); if (unlikely(signr == SIGALRM)) posixtimer_rearm_itimer(tsk); @@ -642,8 +650,8 @@ again: current->jobctl |= JOBCTL_STOP_DEQUEUED; } - if (IS_ENABLED(CONFIG_POSIX_TIMERS) && unlikely(resched_timer)) { - if (!posixtimer_deliver_signal(info)) + if (IS_ENABLED(CONFIG_POSIX_TIMERS) && unlikely(timer_sigq)) { + if (!posixtimer_deliver_signal(info, timer_sigq)) goto again; } diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 0901ed9ca183..d6fef064b357 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -254,7 +254,7 @@ 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. */ -bool posixtimer_deliver_signal(struct kernel_siginfo *info) +bool posixtimer_deliver_signal(struct kernel_siginfo *info, struct sigqueue *timer_sigq) { struct k_itimer *timr; unsigned long flags; -- cgit v1.2.3 From 6017a158beb13b412e55a451379798aae5876514 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:45 +0100 Subject: posix-timers: Embed sigqueue in struct k_itimer 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 Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064213.719695194@linutronix.de --- fs/proc/base.c | 4 +- include/linux/posix-timers.h | 23 +++++++++++- kernel/signal.c | 19 ++++++---- kernel/time/posix-timers.c | 88 ++++++++++++++++++++++++++------------------ 4 files changed, 87 insertions(+), 47 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/fs/proc/base.c b/fs/proc/base.c index b31283d81c52..6a37a43241e4 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2553,8 +2553,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 39f1db76833a..28c0a30e0853 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 + /** * cpu_timer - Posix CPU timer representation for k_itimer * @node: timerqueue node to queue in the task/sig @@ -166,7 +168,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. @@ -190,7 +192,7 @@ struct k_itimer { struct pid *it_pid; struct task_struct *it_process; }; - struct sigqueue *sigq; + struct sigqueue sigq; rcuref_t rcuref; union { struct { @@ -218,6 +220,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 0ddb5dd284aa..2d74cd5841ae 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -460,8 +460,10 @@ static struct sigqueue *__sigqueue_alloc(int sig, struct task_struct *t, gfp_t g static void __sigqueue_free(struct sigqueue *q) { - if (q->flags & SIGQUEUE_PREALLOC) + if (q->flags & SIGQUEUE_PREALLOC) { + posixtimer_sigqueue_putref(q); return; + } if (q->ucounts) { dec_rlimit_put_ucounts(q->ucounts, UCOUNT_RLIMIT_SIGPENDING); q->ucounts = NULL; @@ -569,11 +571,11 @@ still_pending: copy_siginfo(info, &first->info); /* - * posix-timer signals are preallocated and freed when the - * timer goes away. Either directly or by clearing - * SIGQUEUE_PREALLOC so that the next delivery will free - * them. Spare the extra round through __sigqueue_free() - * which is ignoring preallocated signals. + * posix-timer signals are preallocated and freed when the last + * reference count is dropped in posixtimer_deliver_signal() or + * immediately on timer deletion when the signal is not pending. + * Spare the extra round through __sigqueue_free() which is + * ignoring preallocated signals. */ if (unlikely((first->flags & SIGQUEUE_PREALLOC) && (info->si_code == SI_TIMER))) *timer_sigq = first; @@ -1989,7 +1991,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; @@ -2020,9 +2022,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 d6fef064b357..2e2c0edcfa97 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -250,15 +250,40 @@ static void common_hrtimer_rearm(struct k_itimer *timr) hrtimer_restart(timer); } +static bool __posixtimer_deliver_signal(struct kernel_siginfo *info, struct k_itimer *timr) +{ + guard(spinlock)(&timr->it_lock); + + /* + * 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_seq != info->si_sys_private || WARN_ON_ONCE(!timr->it_signal)) + return false; + + if (!timr->it_interval || WARN_ON_ONCE(timr->it_status != POSIX_TIMER_REQUEUE_PENDING)) + return true; + + timr->kclock->timer_rearm(timr); + timr->it_status = POSIX_TIMER_ARMED; + timr->it_overrun_last = timr->it_overrun; + timr->it_overrun = -1LL; + ++timr->it_signal_seq; + info->si_overrun = timer_overrun_to_int(timr); + return true; +} + /* * 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 sigqueue *timer_sigq) { - struct k_itimer *timr; - unsigned long flags; - bool ret = false; + struct k_itimer *timr = container_of(timer_sigq, struct k_itimer, sigq); + bool ret; /* * Release siglock to ensure proper locking order versus @@ -266,28 +291,11 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info, struct sigqueue *tim */ spin_unlock(¤t->sighand->siglock); - timr = lock_timer(info->si_tid, &flags); - if (!timr) - goto out; - - if (timr->it_signal_seq != info->si_sys_private) - goto out_unlock; - - if (timr->it_interval && !WARN_ON_ONCE(timr->it_status != POSIX_TIMER_REQUEUE_PENDING)) { - timr->kclock->timer_rearm(timr); + ret = __posixtimer_deliver_signal(info, timr); - timr->it_status = POSIX_TIMER_ARMED; - timr->it_overrun_last = timr->it_overrun; - timr->it_overrun = -1LL; - ++timr->it_signal_seq; - - info->si_overrun = timer_overrun_to_int(timr); - } - ret = true; + /* Drop the reference which was acquired when the signal was queued */ + posixtimer_putref(timr); -out_unlock: - unlock_timer(timr, flags); -out: spin_lock(¤t->sighand->siglock); /* Don't expose the si_sys_private value to userspace */ @@ -404,17 +412,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; } @@ -422,7 +430,8 @@ static struct k_itimer * alloc_posix_timer(void) 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); kfree_rcu(tmr, rcu); } @@ -484,13 +493,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)); } @@ -499,8 +508,8 @@ 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_tid = new_timer->it_id; + new_timer->sigq.info.si_code = SI_TIMER; if (copy_to_user(created_timer_id, &new_timer_id, sizeof (new_timer_id))) { error = -EFAULT; @@ -584,7 +593,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. -- cgit v1.2.3 From 647da5f709f112319c0d51e06f330d8afecb1940 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:48 +0100 Subject: posix-timers: Move sequence logic into struct k_itimer The posix timer signal handling uses siginfo::si_sys_private for handling the sequence counter check. That indirection is not longer required and the sequence count value at signal queueing time can be stored in struct k_itimer itself. This removes the requirement of treating siginfo::si_sys_private special as it's now always zero as the kernel does not touch it anymore. Suggested-by: Eric W. Biederman Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: "Eric W. Biederman" Link: https://lore.kernel.org/all/20241105064213.852619866@linutronix.de --- include/linux/posix-timers.h | 2 ++ include/uapi/asm-generic/siginfo.h | 2 +- kernel/signal.c | 8 +++----- kernel/time/posix-timers.c | 5 +---- 4 files changed, 7 insertions(+), 10 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 28c0a30e0853..49a89614d900 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -162,6 +162,7 @@ static inline void posix_cputimers_init_work(void) { } * @it_overrun: The overrun counter for pending signals * @it_overrun_last: The overrun at the time of the last delivered signal * @it_signal_seq: Sequence count to control signal delivery + * @it_sigqueue_seq: The sequence count at the point where the signal was queued * @it_sigev_notify: The notify word of sigevent struct for signal delivery * @it_interval: The interval for periodic timers * @it_signal: Pointer to the creators signal struct @@ -184,6 +185,7 @@ struct k_itimer { s64 it_overrun; s64 it_overrun_last; unsigned int it_signal_seq; + unsigned int it_sigqueue_seq; int it_sigev_notify; enum pid_type it_pid_type; ktime_t it_interval; diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index b7bc545ec3b2..5a1ca43b5fc6 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -46,7 +46,7 @@ union __sifields { __kernel_timer_t _tid; /* timer id */ int _overrun; /* overrun count */ sigval_t _sigval; /* same as below */ - int _sys_private; /* not to be passed to user */ + int _sys_private; /* Not used by the kernel. Historic leftover. Always 0. */ } _timer; /* POSIX.1b signals */ diff --git a/kernel/signal.c b/kernel/signal.c index d267a2c5e977..d2734dc4d74f 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1976,12 +1976,10 @@ int posixtimer_send_sigqueue(struct k_itimer *tmr) return -1; /* - * Update @q::info::si_sys_private for posix timer signals with - * sighand locked to prevent a race against dequeue_signal() which - * decides based on si_sys_private whether to invoke - * posixtimer_rearm() or not. + * Update @tmr::sigqueue_seq for posix timer signals with sighand + * locked to prevent a race against dequeue_signal(). */ - q->info.si_sys_private = tmr->it_signal_seq; + tmr->it_sigqueue_seq = tmr->it_signal_seq; ret = 1; /* the signal is ignored */ if (!prepare_signal(sig, t, false)) { diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 2e2c0edcfa97..f20c06d0cf09 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -259,7 +259,7 @@ static bool __posixtimer_deliver_signal(struct kernel_siginfo *info, struct k_it * since the signal was queued. In either case, don't rearm and * drop the signal. */ - if (timr->it_signal_seq != info->si_sys_private || WARN_ON_ONCE(!timr->it_signal)) + if (timr->it_signal_seq != timr->it_sigqueue_seq || WARN_ON_ONCE(!timr->it_signal)) return false; if (!timr->it_interval || WARN_ON_ONCE(timr->it_status != POSIX_TIMER_REQUEUE_PENDING)) @@ -297,9 +297,6 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info, struct sigqueue *tim posixtimer_putref(timr); spin_lock(¤t->sighand->siglock); - - /* Don't expose the si_sys_private value to userspace */ - info->si_sys_private = 0; return ret; } -- cgit v1.2.3 From 0e20cd33acc7a173b23900550331ee82a23e9f00 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:51 +0100 Subject: posix-timers: Handle ignored list on delete and exit To handle posix timer signals on sigaction(SIG_IGN) properly, the timers will be queued on a separate ignored list. Add the necessary cleanup code for timer_delete() and exit_itimers(). Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064213.987530588@linutronix.de --- include/linux/posix-timers.h | 4 +++- kernel/time/posix-timers.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 49a89614d900..1608b52a44d5 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -152,7 +152,8 @@ static inline void posix_cputimers_init_work(void) { } /** * struct k_itimer - POSIX.1b interval timer structure. - * @list: List head for binding the timer to signals->posix_timers + * @list: List node for binding the timer to tsk::signal::posix_timers + * @ignored_list: List node for tracking ignored timers in tsk::signal::ignored_posix_timers * @t_hash: Entry in the posix timer hash table * @it_lock: Lock protecting the timer * @kclock: Pointer to the k_clock struct handling this timer @@ -176,6 +177,7 @@ static inline void posix_cputimers_init_work(void) { } */ struct k_itimer { struct hlist_node list; + struct hlist_node ignored_list; struct hlist_node t_hash; spinlock_t it_lock; const struct k_clock *kclock; diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index f20c06d0cf09..2b88fb4e937e 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -1027,6 +1027,18 @@ int common_timer_del(struct k_itimer *timer) return 0; } +/* + * If the deleted timer is on the ignored list, remove it and + * drop the associated reference. + */ +static inline void posix_timer_cleanup_ignored(struct k_itimer *tmr) +{ + if (!hlist_unhashed(&tmr->ignored_list)) { + hlist_del_init(&tmr->ignored_list); + posixtimer_putref(tmr); + } +} + static inline int timer_delete_hook(struct k_itimer *timer) { const struct k_clock *kc = timer->kclock; @@ -1059,6 +1071,7 @@ retry_delete: spin_lock(¤t->sighand->siglock); hlist_del(&timer->list); + posix_timer_cleanup_ignored(timer); spin_unlock(¤t->sighand->siglock); /* * A concurrent lookup could check timer::it_signal lockless. It @@ -1110,6 +1123,8 @@ retry_delete: } hlist_del(&timer->list); + posix_timer_cleanup_ignored(timer); + /* * Setting timer::it_signal to NULL is technically not required * here as nothing can access the timer anymore legitimately via @@ -1142,6 +1157,19 @@ void exit_itimers(struct task_struct *tsk) /* The timers are not longer accessible via tsk::signal */ while (!hlist_empty(&timers)) itimer_delete(hlist_entry(timers.first, struct k_itimer, list)); + + /* + * There should be no timers on the ignored list. itimer_delete() has + * mopped them up. + */ + if (!WARN_ON_ONCE(!hlist_empty(&tsk->signal->ignored_posix_timers))) + return; + + hlist_move_list(&tsk->signal->ignored_posix_timers, &timers); + while (!hlist_empty(&timers)) { + posix_timer_cleanup_ignored(hlist_entry(timers.first, struct k_itimer, + ignored_list)); + } } SYSCALL_DEFINE2(clock_settime, const clockid_t, which_clock, -- cgit v1.2.3 From df7a996b4dab03c889fa86d849447b716f07b069 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:54 +0100 Subject: signal: Queue ignored posixtimers on ignore list Queue posixtimers which have their signal ignored on the ignored list: 1) When the timer fires and the signal has SIG_IGN set 2) When SIG_IGN is installed via sigaction() and a timer signal is already queued This only happens when the signal is for a valid timer, which delivered the signal in periodic mode. One-shot timer signals are correctly dropped. Due to the lock order constraints (sighand::siglock nests inside timer::lock) the signal code cannot access any of the timer fields which are relevant to make this decision, e.g. timer::it_status. This is addressed by establishing a protection scheme which requires to lock both locks on the timer side for modifying decision fields in the timer struct and therefore makes it possible for the signal delivery to evaluate with only sighand:siglock being held: 1) Move the NULLification of timer->it_signal into the sighand::siglock protected section of timer_delete() and check timer::it_signal in the code path which determines whether the signal is dropped or queued on the ignore list. This ensures that a deleted timer cannot be moved onto the ignore list, which would prevent it from being freed on exit() as it is not longer in the process' posix timer list. If the timer got moved to the ignored list before deletion then it is removed from the ignored list under sighand lock in timer_delete(). 2) Provide a new timer::it_sig_periodic flag, which gets set in the signal queue path with both timer and sighand locks held if the timer is actually in periodic mode at expiry time. The ignore list code checks this flag under sighand::siglock and drops the signal when it is not set. If it is set, then the signal is moved to the ignored list independent of the actual state of the timer. When the signal is un-ignored later then the signal is moved back to the signal queue. On signal delivery the posix timer side decides about dropping the signal if the timer was re-armed, dis-armed or deleted based on the signal sequence counter check. If the thread/process exits then not yet delivered signals are discarded which means the reference of the timer containing the sigqueue is dropped and frees the timer. This is way cheaper than requiring all code paths to lock sighand::siglock of the target thread/process on any modification of timer::it_status or going all the way and removing pending signals from the signal queues on every rearm, disarm or delete operation. So the protection scheme here is that on the timer side both timer::lock and sighand::siglock have to be held for modifying timer::it_signal timer::it_sig_periodic which means that on the signal side holding sighand::siglock is enough to evaluate these fields. In posixtimer_deliver_signal() holding timer::lock is sufficient to do the sequence validation against timer::it_signal_seq because a concurrent expiry is waiting on timer::lock to be released. This completes the SIG_IGN handling and such timers are not longer self rearmed which avoids pointless wakeups. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064214.120756416@linutronix.de --- include/linux/posix-timers.h | 2 ++ kernel/signal.c | 80 +++++++++++++++++++++++++++++++++++++++++--- kernel/time/posix-timers.c | 7 +++- 3 files changed, 83 insertions(+), 6 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 1608b52a44d5..43ea6e784a25 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -160,6 +160,7 @@ static inline void posix_cputimers_init_work(void) { } * @it_clock: The posix timer clock id * @it_id: The posix timer id for identifying the timer * @it_status: The status of the timer + * @it_sig_periodic: The periodic status at signal delivery * @it_overrun: The overrun counter for pending signals * @it_overrun_last: The overrun at the time of the last delivered signal * @it_signal_seq: Sequence count to control signal delivery @@ -184,6 +185,7 @@ struct k_itimer { clockid_t it_clock; timer_t it_id; int it_status; + bool it_sig_periodic; s64 it_overrun; s64 it_overrun_last; unsigned int it_signal_seq; diff --git a/kernel/signal.c b/kernel/signal.c index 908b49c594e4..9b098a7a206f 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -59,6 +59,8 @@ #include #include /* for syscall_get_* */ +#include "time/posix-timers.h" + /* * SLAB caches for signal bits. */ @@ -731,6 +733,16 @@ void signal_wake_up_state(struct task_struct *t, unsigned int state) kick_process(t); } +static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q); + +static void sigqueue_free_ignored(struct task_struct *tsk, struct sigqueue *q) +{ + if (likely(!(q->flags & SIGQUEUE_PREALLOC) || q->info.si_code != SI_TIMER)) + __sigqueue_free(q); + else + posixtimer_sig_ignore(tsk, q); +} + /* Remove signals in mask from the pending set and queue. */ static void flush_sigqueue_mask(struct task_struct *p, sigset_t *mask, struct sigpending *s) { @@ -747,7 +759,7 @@ static void flush_sigqueue_mask(struct task_struct *p, sigset_t *mask, struct si list_for_each_entry_safe(q, n, &s->list, list) { if (sigismember(mask, q->info.si_signo)) { list_del_init(&q->list); - __sigqueue_free(q); + sigqueue_free_ignored(p, q); } } } @@ -1964,7 +1976,7 @@ int posixtimer_send_sigqueue(struct k_itimer *tmr) int sig = q->info.si_signo; struct task_struct *t; unsigned long flags; - int ret, result; + int result; guard(rcu)(); @@ -1981,13 +1993,55 @@ int posixtimer_send_sigqueue(struct k_itimer *tmr) */ tmr->it_sigqueue_seq = tmr->it_signal_seq; - ret = 1; /* the signal is ignored */ + /* + * Set the signal delivery status under sighand lock, so that the + * ignored signal handling can distinguish between a periodic and a + * non-periodic timer. + */ + tmr->it_sig_periodic = tmr->it_status == POSIX_TIMER_REQUEUE_PENDING; + if (!prepare_signal(sig, t, false)) { result = TRACE_SIGNAL_IGNORED; + + /* Paranoia check. Try to survive. */ + if (WARN_ON_ONCE(!list_empty(&q->list))) + goto out; + + /* Periodic timers with SIG_IGN are queued on the ignored list */ + if (tmr->it_sig_periodic) { + /* + * Already queued means the timer was rearmed after + * the previous expiry got it on the ignore list. + * Nothing to do for that case. + */ + if (hlist_unhashed(&tmr->ignored_list)) { + /* + * Take a signal reference and queue it on + * the ignored list. + */ + posixtimer_sigqueue_getref(q); + posixtimer_sig_ignore(t, q); + } + } else if (!hlist_unhashed(&tmr->ignored_list)) { + /* + * Covers the case where a timer was periodic and + * then the signal was ignored. Later it was rearmed + * as oneshot timer. The previous signal is invalid + * now, and this oneshot signal has to be dropped. + * Remove it from the ignored list and drop the + * reference count as the signal is not longer + * queued. + */ + hlist_del_init(&tmr->ignored_list); + posixtimer_putref(tmr); + } goto out; } - ret = 0; + /* This should never happen and leaks a reference count */ + if (WARN_ON_ONCE(!hlist_unhashed(&tmr->ignored_list))) + hlist_del_init(&tmr->ignored_list); + if (unlikely(!list_empty(&q->list))) { /* This holds a reference count already */ result = TRACE_SIGNAL_ALREADY_PENDING; @@ -2000,7 +2054,22 @@ int posixtimer_send_sigqueue(struct k_itimer *tmr) out: trace_signal_generate(sig, &q->info, t, tmr->it_pid_type != PIDTYPE_PID, result); unlock_task_sighand(t, &flags); - return ret; + return 0; +} + +static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q) +{ + struct k_itimer *tmr = container_of(q, struct k_itimer, sigq); + + /* + * If the timer is marked deleted already or the signal originates + * from a non-periodic timer, then just drop the reference + * count. Otherwise queue it on the ignored list. + */ + if (tmr->it_signal && tmr->it_sig_periodic) + hlist_add_head(&tmr->ignored_list, &tsk->signal->ignored_posix_timers); + else + posixtimer_putref(tmr); } static void posixtimer_sig_unignore(struct task_struct *tsk, int sig) @@ -2048,6 +2117,7 @@ static void posixtimer_sig_unignore(struct task_struct *tsk, int sig) } } #else /* CONFIG_POSIX_TIMERS */ +static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q) { } static inline void posixtimer_sig_unignore(struct task_struct *tsk, int sig) { } #endif /* !CONFIG_POSIX_TIMERS */ diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index 2b88fb4e937e..ea72db3c9365 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -1072,12 +1072,17 @@ retry_delete: spin_lock(¤t->sighand->siglock); hlist_del(&timer->list); posix_timer_cleanup_ignored(timer); - spin_unlock(¤t->sighand->siglock); /* * A concurrent lookup could check timer::it_signal lockless. It * will reevaluate with timer::it_lock held and observe the NULL. + * + * It must be written with siglock held so that the signal code + * observes timer->it_signal == NULL in do_sigaction(SIG_IGN), + * which prevents it from moving a pending signal of a deleted + * timer to the ignore list. */ WRITE_ONCE(timer->it_signal, NULL); + spin_unlock(¤t->sighand->siglock); unlock_timer(timer, flags); posix_timer_unhash_and_free(timer); -- cgit v1.2.3 From 7a66f72b09bb0762360274b1fb677b3433dbaa06 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 5 Nov 2024 09:14:55 +0100 Subject: posix-timers: Cleanup SIG_IGN workaround leftovers Now that ignored posix timer signals are requeued and the timers are rearmed on signal delivery the workaround to keep such timers alive and self rearm them is not longer required. Remove the relevant hacks and the not longer required return values from the related functions. The alarm timer workarounds will be cleaned up in a separate step. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20241105064214.187239060@linutronix.de --- include/linux/posix-timers.h | 2 +- kernel/signal.c | 7 ++-- kernel/time/alarmtimer.c | 47 ++++++--------------------- kernel/time/posix-cpu-timers.c | 18 +++-------- kernel/time/posix-timers.c | 73 ++++-------------------------------------- kernel/time/posix-timers.h | 2 +- 6 files changed, 24 insertions(+), 125 deletions(-) (limited to 'kernel/time/posix-timers.c') diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 43ea6e784a25..f11f10c97bd9 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -111,7 +111,7 @@ static inline void posix_cputimers_rt_watchdog(struct posix_cputimers *pct, void posixtimer_rearm_itimer(struct task_struct *p); bool posixtimer_init_sigqueue(struct sigqueue *q); -int posixtimer_send_sigqueue(struct k_itimer *tmr); +void posixtimer_send_sigqueue(struct k_itimer *tmr); bool posixtimer_deliver_signal(struct kernel_siginfo *info, struct sigqueue *timer_sigq); void posixtimer_free_timer(struct k_itimer *timer); diff --git a/kernel/signal.c b/kernel/signal.c index 9b098a7a206f..cbf70c808969 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1970,7 +1970,7 @@ static inline struct task_struct *posixtimer_get_target(struct k_itimer *tmr) return t; } -int posixtimer_send_sigqueue(struct k_itimer *tmr) +void posixtimer_send_sigqueue(struct k_itimer *tmr) { struct sigqueue *q = &tmr->sigq; int sig = q->info.si_signo; @@ -1982,10 +1982,10 @@ int posixtimer_send_sigqueue(struct k_itimer *tmr) t = posixtimer_get_target(tmr); if (!t) - return -1; + return; if (!likely(lock_task_sighand(t, &flags))) - return -1; + return; /* * Update @tmr::sigqueue_seq for posix timer signals with sighand @@ -2054,7 +2054,6 @@ int posixtimer_send_sigqueue(struct k_itimer *tmr) out: trace_signal_generate(sig, &q->info, t, tmr->it_pid_type != PIDTYPE_PID, result); unlock_task_sighand(t, &flags); - return 0; } static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q) diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index 452d8aa2f6e0..8543d7f1cdb4 100644 --- a/kernel/time/alarmtimer.c +++ b/kernel/time/alarmtimer.c @@ -197,28 +197,15 @@ static enum hrtimer_restart alarmtimer_fired(struct hrtimer *timer) { struct alarm *alarm = container_of(timer, struct alarm, timer); struct alarm_base *base = &alarm_bases[alarm->type]; - unsigned long flags; - int ret = HRTIMER_NORESTART; - int restart = ALARMTIMER_NORESTART; - spin_lock_irqsave(&base->lock, flags); - alarmtimer_dequeue(base, alarm); - spin_unlock_irqrestore(&base->lock, flags); + scoped_guard (spinlock_irqsave, &base->lock) + alarmtimer_dequeue(base, alarm); if (alarm->function) - restart = alarm->function(alarm, base->get_ktime()); - - spin_lock_irqsave(&base->lock, flags); - if (restart != ALARMTIMER_NORESTART) { - hrtimer_set_expires(&alarm->timer, alarm->node.expires); - alarmtimer_enqueue(base, alarm); - ret = HRTIMER_RESTART; - } - spin_unlock_irqrestore(&base->lock, flags); + alarm->function(alarm, base->get_ktime()); trace_alarmtimer_fired(alarm, base->get_ktime()); - return ret; - + return HRTIMER_NORESTART; } ktime_t alarm_expires_remaining(const struct alarm *alarm) @@ -567,30 +554,14 @@ static enum alarmtimer_type clock2alarm(clockid_t clockid) * * Return: whether the timer is to be restarted */ -static enum alarmtimer_restart alarm_handle_timer(struct alarm *alarm, - ktime_t now) +static enum alarmtimer_restart alarm_handle_timer(struct alarm *alarm, ktime_t now) { - struct k_itimer *ptr = container_of(alarm, struct k_itimer, - it.alarm.alarmtimer); - enum alarmtimer_restart result = ALARMTIMER_NORESTART; - unsigned long flags; - - spin_lock_irqsave(&ptr->it_lock, flags); + struct k_itimer *ptr = container_of(alarm, struct k_itimer, it.alarm.alarmtimer); - if (posix_timer_queue_signal(ptr) && ptr->it_interval) { - /* - * Handle ignored signals and rearm the timer. This will go - * away once we handle ignored signals proper. Ensure that - * small intervals cannot starve the system. - */ - ptr->it_overrun += __alarm_forward_now(alarm, ptr->it_interval, true); - ++ptr->it_signal_seq; - ptr->it_status = POSIX_TIMER_ARMED; - result = ALARMTIMER_RESTART; - } - spin_unlock_irqrestore(&ptr->it_lock, flags); + guard(spinlock_irqsave)(&ptr->it_lock); + posix_timer_queue_signal(ptr); - return result; + return ALARMTIMER_NORESTART; } /** diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 0c441d8c2604..50e8d04ab661 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -603,21 +603,11 @@ static void cpu_timer_fire(struct k_itimer *timer) */ wake_up_process(timer->it_process); cpu_timer_setexpires(ctmr, 0); - } else if (!timer->it_interval) { - /* - * One-shot timer. Clear it as soon as it's fired. - */ + } else { posix_timer_queue_signal(timer); - cpu_timer_setexpires(ctmr, 0); - } else if (posix_timer_queue_signal(timer)) { - /* - * The signal did not get queued because the signal - * was ignored, so we won't get any callback to - * reload the timer. But we need to keep it - * ticking in case the signal is deliverable next time. - */ - posix_cpu_timer_rearm(timer); - ++timer->it_signal_seq; + /* Disable oneshot timers */ + if (!timer->it_interval) + cpu_timer_setexpires(ctmr, 0); } } diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index ea72db3c9365..881a9ce96af7 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -300,21 +300,12 @@ bool posixtimer_deliver_signal(struct kernel_siginfo *info, struct sigqueue *tim return ret; } -int posix_timer_queue_signal(struct k_itimer *timr) +void posix_timer_queue_signal(struct k_itimer *timr) { - enum posix_timer_state state = POSIX_TIMER_DISARMED; - int ret; - lockdep_assert_held(&timr->it_lock); - if (timr->it_interval) - state = POSIX_TIMER_REQUEUE_PENDING; - - timr->it_status = state; - - ret = posixtimer_send_sigqueue(timr); - /* If we failed to send the signal the timer stops. */ - return ret > 0; + timr->it_status = timr->it_interval ? POSIX_TIMER_REQUEUE_PENDING : POSIX_TIMER_DISARMED; + posixtimer_send_sigqueue(timr); } /* @@ -327,62 +318,10 @@ int posix_timer_queue_signal(struct k_itimer *timr) static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer) { struct k_itimer *timr = container_of(timer, struct k_itimer, it.real.timer); - enum hrtimer_restart ret = HRTIMER_NORESTART; - unsigned long flags; - - spin_lock_irqsave(&timr->it_lock, flags); - - if (posix_timer_queue_signal(timr)) { - /* - * The signal was not queued due to SIG_IGN. As a - * consequence the timer is not going to be rearmed from - * the signal delivery path. But as a real signal handler - * can be installed later the timer must be rearmed here. - */ - if (timr->it_interval != 0) { - ktime_t now = hrtimer_cb_get_time(timer); - - /* - * FIXME: What we really want, is to stop this - * timer completely and restart it in case the - * SIG_IGN is removed. This is a non trivial - * change to the signal handling code. - * - * For now let timers with an interval less than a - * jiffy expire every jiffy and recheck for a - * valid signal handler. - * - * This avoids interrupt starvation in case of a - * very small interval, which would expire the - * timer immediately again. - * - * Moving now ahead of time by one jiffy tricks - * hrtimer_forward() to expire the timer later, - * while it still maintains the overrun accuracy - * for the price of a slight inconsistency in the - * timer_gettime() case. This is at least better - * than a timer storm. - * - * Only required when high resolution timers are - * enabled as the periodic tick based timers are - * automatically aligned to the next tick. - */ - if (IS_ENABLED(CONFIG_HIGH_RES_TIMERS)) { - ktime_t kj = TICK_NSEC; - - if (timr->it_interval < kj) - now = ktime_add(now, kj); - } - - timr->it_overrun += hrtimer_forward(timer, now, timr->it_interval); - ret = HRTIMER_RESTART; - ++timr->it_signal_seq; - timr->it_status = POSIX_TIMER_ARMED; - } - } - unlock_timer(timr, flags); - return ret; + guard(spinlock_irqsave)(&timr->it_lock); + posix_timer_queue_signal(timr); + return HRTIMER_NORESTART; } static struct pid *good_sigevent(sigevent_t * event) diff --git a/kernel/time/posix-timers.h b/kernel/time/posix-timers.h index 4d09677e584e..61906f0688c1 100644 --- a/kernel/time/posix-timers.h +++ b/kernel/time/posix-timers.h @@ -42,7 +42,7 @@ extern const struct k_clock clock_process; extern const struct k_clock clock_thread; extern const struct k_clock alarm_clock; -int posix_timer_queue_signal(struct k_itimer *timr); +void posix_timer_queue_signal(struct k_itimer *timr); void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting); int common_timer_set(struct k_itimer *timr, int flags, -- cgit v1.2.3