From: Peter Zijlstra <peterz@infradead.org> stable inclusion from stable-v5.10.246 commit af53aaf20722d745a69a051114a1ae237f5b922e category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/16862 CVE: CVE-2026-64560 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=... -------------------------------- [ Upstream commit 54da6a0924311c7cf5015533991e44fb8eb12773 ] Use __attribute__((__cleanup__(func))) to build: - simple auto-release pointers using __free() - 'classes' with constructor and destructor semantics for scope-based resource management. - lock guards based on the above classes. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20230612093537.614161713%40infradead.org Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Conflicts: include/linux/device.h include/linux/mutex.h include/linux/preempt.h include/linux/slab.h [ Header context conflict. ] Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/dma/ioat/dma.c | 12 +- include/linux/cleanup.h | 171 ++++++++++++++++++++++++++++ include/linux/compiler-clang.h | 9 ++ include/linux/compiler_attributes.h | 6 + include/linux/device.h | 7 ++ include/linux/file.h | 6 + include/linux/irqflags.h | 7 ++ include/linux/mutex.h | 4 + include/linux/percpu.h | 4 + include/linux/preempt.h | 5 + include/linux/rcupdate.h | 3 + include/linux/rwsem.h | 9 ++ include/linux/sched/task.h | 2 + include/linux/slab.h | 4 +- include/linux/spinlock.h | 32 ++++++ include/linux/srcu.h | 5 + scripts/checkpatch.pl | 2 +- 17 files changed, 280 insertions(+), 8 deletions(-) create mode 100644 include/linux/cleanup.h diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c index e2070df6cad2..0b846c605d4b 100644 --- a/drivers/dma/ioat/dma.c +++ b/drivers/dma/ioat/dma.c @@ -584,11 +584,11 @@ desc_get_errstat(struct ioatdma_chan *ioat_chan, struct ioat_ring_ent *desc) } /** - * __cleanup - reclaim used descriptors + * __ioat_cleanup - reclaim used descriptors * @ioat_chan: channel (ring) to clean * @phys_complete: zeroed (or not) completion address (from status) */ -static void __cleanup(struct ioatdma_chan *ioat_chan, dma_addr_t phys_complete) +static void __ioat_cleanup(struct ioatdma_chan *ioat_chan, dma_addr_t phys_complete) { struct ioatdma_device *ioat_dma = ioat_chan->ioat_dma; struct ioat_ring_ent *desc; @@ -675,7 +675,7 @@ static void ioat_cleanup(struct ioatdma_chan *ioat_chan) spin_lock_bh(&ioat_chan->cleanup_lock); if (ioat_cleanup_preamble(ioat_chan, &phys_complete)) - __cleanup(ioat_chan, phys_complete); + __ioat_cleanup(ioat_chan, phys_complete); if (is_ioat_halted(*ioat_chan->completion)) { u32 chanerr = readl(ioat_chan->reg_base + IOAT_CHANERR_OFFSET); @@ -712,7 +712,7 @@ static void ioat_restart_channel(struct ioatdma_chan *ioat_chan) ioat_quiesce(ioat_chan, 0); if (ioat_cleanup_preamble(ioat_chan, &phys_complete)) - __cleanup(ioat_chan, phys_complete); + __ioat_cleanup(ioat_chan, phys_complete); __ioat_restart_chan(ioat_chan); } @@ -786,7 +786,7 @@ static void ioat_eh(struct ioatdma_chan *ioat_chan) /* cleanup so tail points to descriptor that caused the error */ if (ioat_cleanup_preamble(ioat_chan, &phys_complete)) - __cleanup(ioat_chan, phys_complete); + __ioat_cleanup(ioat_chan, phys_complete); chanerr = readl(ioat_chan->reg_base + IOAT_CHANERR_OFFSET); pci_read_config_dword(pdev, IOAT_PCI_CHANERR_INT_OFFSET, &chanerr_int); @@ -943,7 +943,7 @@ void ioat_timer_event(struct timer_list *t) /* timer restarted in ioat_cleanup_preamble * and IOAT_COMPLETION_ACK cleared */ - __cleanup(ioat_chan, phys_complete); + __ioat_cleanup(ioat_chan, phys_complete); goto unlock_out; } diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h new file mode 100644 index 000000000000..53f1a7a932b0 --- /dev/null +++ b/include/linux/cleanup.h @@ -0,0 +1,171 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __LINUX_GUARDS_H +#define __LINUX_GUARDS_H + +#include <linux/compiler.h> + +/* + * DEFINE_FREE(name, type, free): + * simple helper macro that defines the required wrapper for a __free() + * based cleanup function. @free is an expression using '_T' to access + * the variable. + * + * __free(name): + * variable attribute to add a scoped based cleanup to the variable. + * + * no_free_ptr(var): + * like a non-atomic xchg(var, NULL), such that the cleanup function will + * be inhibited -- provided it sanely deals with a NULL value. + * + * return_ptr(p): + * returns p while inhibiting the __free(). + * + * Ex. + * + * DEFINE_FREE(kfree, void *, if (_T) kfree(_T)) + * + * struct obj *p __free(kfree) = kmalloc(...); + * if (!p) + * return NULL; + * + * if (!init_obj(p)) + * return NULL; + * + * return_ptr(p); + */ + +#define DEFINE_FREE(_name, _type, _free) \ + static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; } + +#define __free(_name) __cleanup(__free_##_name) + +#define no_free_ptr(p) \ + ({ __auto_type __ptr = (p); (p) = NULL; __ptr; }) + +#define return_ptr(p) return no_free_ptr(p) + + +/* + * DEFINE_CLASS(name, type, exit, init, init_args...): + * helper to define the destructor and constructor for a type. + * @exit is an expression using '_T' -- similar to FREE above. + * @init is an expression in @init_args resulting in @type + * + * EXTEND_CLASS(name, ext, init, init_args...): + * extends class @name to @name@ext with the new constructor + * + * CLASS(name, var)(args...): + * declare the variable @var as an instance of the named class + * + * Ex. + * + * DEFINE_CLASS(fdget, struct fd, fdput(_T), fdget(fd), int fd) + * + * CLASS(fdget, f)(fd); + * if (!f.file) + * return -EBADF; + * + * // use 'f' without concern + */ + +#define DEFINE_CLASS(_name, _type, _exit, _init, _init_args...) \ +typedef _type class_##_name##_t; \ +static inline void class_##_name##_destructor(_type *p) \ +{ _type _T = *p; _exit; } \ +static inline _type class_##_name##_constructor(_init_args) \ +{ _type t = _init; return t; } + +#define EXTEND_CLASS(_name, ext, _init, _init_args...) \ +typedef class_##_name##_t class_##_name##ext##_t; \ +static inline void class_##_name##ext##_destructor(class_##_name##_t *p)\ +{ class_##_name##_destructor(p); } \ +static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \ +{ class_##_name##_t t = _init; return t; } + +#define CLASS(_name, var) \ + class_##_name##_t var __cleanup(class_##_name##_destructor) = \ + class_##_name##_constructor + + +/* + * DEFINE_GUARD(name, type, lock, unlock): + * trivial wrapper around DEFINE_CLASS() above specifically + * for locks. + * + * guard(name): + * an anonymous instance of the (guard) class + * + * scoped_guard (name, args...) { }: + * similar to CLASS(name, scope)(args), except the variable (with the + * explicit name 'scope') is declard in a for-loop such that its scope is + * bound to the next (compound) statement. + * + */ + +#define DEFINE_GUARD(_name, _type, _lock, _unlock) \ + DEFINE_CLASS(_name, _type, _unlock, ({ _lock; _T; }), _type _T) + +#define guard(_name) \ + CLASS(_name, __UNIQUE_ID(guard)) + +#define scoped_guard(_name, args...) \ + for (CLASS(_name, scope)(args), \ + *done = NULL; !done; done = (void *)1) + +/* + * Additional helper macros for generating lock guards with types, either for + * locks that don't have a native type (eg. RCU, preempt) or those that need a + * 'fat' pointer (eg. spin_lock_irqsave). + * + * DEFINE_LOCK_GUARD_0(name, lock, unlock, ...) + * DEFINE_LOCK_GUARD_1(name, type, lock, unlock, ...) + * + * will result in the following type: + * + * typedef struct { + * type *lock; // 'type := void' for the _0 variant + * __VA_ARGS__; + * } class_##name##_t; + * + * As above, both _lock and _unlock are statements, except this time '_T' will + * be a pointer to the above struct. + */ + +#define __DEFINE_UNLOCK_GUARD(_name, _type, _unlock, ...) \ +typedef struct { \ + _type *lock; \ + __VA_ARGS__; \ +} class_##_name##_t; \ + \ +static inline void class_##_name##_destructor(class_##_name##_t *_T) \ +{ \ + if (_T->lock) { _unlock; } \ +} + + +#define __DEFINE_LOCK_GUARD_1(_name, _type, _lock) \ +static inline class_##_name##_t class_##_name##_constructor(_type *l) \ +{ \ + class_##_name##_t _t = { .lock = l }, *_T = &_t; \ + _lock; \ + return _t; \ +} + +#define __DEFINE_LOCK_GUARD_0(_name, _lock) \ +static inline class_##_name##_t class_##_name##_constructor(void) \ +{ \ + class_##_name##_t _t = { .lock = (void*)1 }, \ + *_T __maybe_unused = &_t; \ + _lock; \ + return _t; \ +} + +#define DEFINE_LOCK_GUARD_1(_name, _type, _lock, _unlock, ...) \ +__DEFINE_UNLOCK_GUARD(_name, _type, _unlock, __VA_ARGS__) \ +__DEFINE_LOCK_GUARD_1(_name, _type, _lock) + +#define DEFINE_LOCK_GUARD_0(_name, _lock, _unlock, ...) \ +__DEFINE_UNLOCK_GUARD(_name, void, _unlock, __VA_ARGS__) \ +__DEFINE_LOCK_GUARD_0(_name, _lock) + +#endif /* __LINUX_GUARDS_H */ diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h index 9ba951e3a6c2..d1f6f594c508 100644 --- a/include/linux/compiler-clang.h +++ b/include/linux/compiler-clang.h @@ -15,6 +15,15 @@ /* Compiler specific definitions for Clang compiler */ +/* + * Clang prior to 17 is being silly and considers many __cleanup() variables + * as unused (because they are, their sole purpose is to go out of scope). + * + * https://reviews.llvm.org/D152180 + */ +#undef __cleanup +#define __cleanup(func) __maybe_unused __attribute__((__cleanup__(func))) + /* same as gcc, this was present in clang-2.6 so we can assume it works * with any version that can compile the kernel */ diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 6f495fca9d79..74e1501ae2b1 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -93,6 +93,12 @@ */ #define __cold __attribute__((__cold__)) +/* + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-cle... + * clang: https://clang.llvm.org/docs/AttributeReference.html#cleanup + */ +#define __cleanup(func) __attribute__((__cleanup__(func))) + /* * Note the long name. * diff --git a/include/linux/device.h b/include/linux/device.h index 32f39e0ca30e..65dac0c81e47 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -31,6 +31,7 @@ #include <linux/device/bus.h> #include <linux/device/class.h> #include <linux/device/driver.h> +#include <linux/cleanup.h> #include <linux/kabi.h> #include <asm/device.h> @@ -910,6 +911,9 @@ void device_unregister(struct device *dev); void device_initialize(struct device *dev); int __must_check device_add(struct device *dev); void device_del(struct device *dev); + +DEFINE_FREE(device_del, struct device *, if (_T) device_del(_T)) + int device_for_each_child(struct device *dev, void *data, int (*fn)(struct device *dev, void *data)); int device_for_each_child_reverse(struct device *dev, void *data, @@ -1038,6 +1042,9 @@ extern int (*platform_notify_remove)(struct device *dev); */ struct device *get_device(struct device *dev); void put_device(struct device *dev); + +DEFINE_FREE(put_device, struct device *, if (_T) put_device(_T)) + bool kill_device(struct device *dev); #ifdef CONFIG_DEVTMPFS diff --git a/include/linux/file.h b/include/linux/file.h index 225982792fa2..5b4e31388648 100644 --- a/include/linux/file.h +++ b/include/linux/file.h @@ -10,6 +10,7 @@ #include <linux/types.h> #include <linux/posix_types.h> #include <linux/errno.h> +#include <linux/cleanup.h> struct file; @@ -82,6 +83,8 @@ static inline void fdput_pos(struct fd f) fdput(f); } +DEFINE_CLASS(fd, struct fd, fdput(_T), fdget(fd), int fd) + extern int f_dupfd(unsigned int from, struct file *file, unsigned flags); extern int replace_fd(unsigned fd, struct file *file, unsigned flags); extern void set_close_on_exec(unsigned int fd, int flag); @@ -90,6 +93,9 @@ extern int __get_unused_fd_flags(unsigned flags, unsigned long nofile); extern int get_unused_fd_flags(unsigned flags); extern void put_unused_fd(unsigned int fd); +DEFINE_CLASS(get_unused_fd, int, if (_T >= 0) put_unused_fd(_T), + get_unused_fd_flags(flags), unsigned flags) + extern void fd_install(unsigned int fd, struct file *file); extern int __receive_fd(int fd, struct file *file, int __user *ufd, diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h index eea9adf9dd8e..07d93a7330f9 100644 --- a/include/linux/irqflags.h +++ b/include/linux/irqflags.h @@ -13,6 +13,7 @@ #define _LINUX_TRACE_IRQFLAGS_H #include <linux/typecheck.h> +#include <linux/cleanup.h> #include <asm/irqflags.h> #include <asm/percpu.h> @@ -248,4 +249,10 @@ do { \ #define irqs_disabled_flags(flags) raw_irqs_disabled_flags(flags) +DEFINE_LOCK_GUARD_0(irq, local_irq_disable(), local_irq_enable()) +DEFINE_LOCK_GUARD_0(irqsave, + local_irq_save(_T->flags), + local_irq_restore(_T->flags), + unsigned long flags) + #endif diff --git a/include/linux/mutex.h b/include/linux/mutex.h index a8f1516ee51c..7cd802ecec05 100644 --- a/include/linux/mutex.h +++ b/include/linux/mutex.h @@ -19,6 +19,7 @@ #include <asm/processor.h> #include <linux/osq_lock.h> #include <linux/debug_locks.h> +#include <linux/cleanup.h> struct device; @@ -251,4 +252,7 @@ enum mutex_trylock_recursive_enum { extern /* __deprecated */ __must_check enum mutex_trylock_recursive_enum mutex_trylock_recursive(struct mutex *lock); +DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T)) +DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T)) + #endif /* __LINUX_MUTEX_H */ diff --git a/include/linux/percpu.h b/include/linux/percpu.h index c5bf45dead9d..aa870138c1a4 100644 --- a/include/linux/percpu.h +++ b/include/linux/percpu.h @@ -9,6 +9,7 @@ #include <linux/printk.h> #include <linux/pfn.h> #include <linux/init.h> +#include <linux/cleanup.h> #include <asm/percpu.h> @@ -140,6 +141,9 @@ extern void __init setup_per_cpu_areas(void); extern void __percpu *__alloc_percpu_gfp(size_t size, size_t align, gfp_t gfp); extern void __percpu *__alloc_percpu(size_t size, size_t align); extern void free_percpu(void __percpu *__pdata); + +DEFINE_FREE(free_percpu, void __percpu *, free_percpu(_T)) + extern phys_addr_t per_cpu_ptr_to_phys(void *addr); #define alloc_percpu_gfp(type, gfp) \ diff --git a/include/linux/preempt.h b/include/linux/preempt.h index 89d5281e060a..9ed31edacdff 100644 --- a/include/linux/preempt.h +++ b/include/linux/preempt.h @@ -8,6 +8,7 @@ */ #include <linux/linkage.h> +#include <linux/cleanup.h> #include <linux/list.h> /* @@ -354,4 +355,8 @@ static __always_inline void migrate_enable(void) int in_dbg(void); +DEFINE_LOCK_GUARD_0(preempt, preempt_disable(), preempt_enable()) +DEFINE_LOCK_GUARD_0(preempt_notrace, preempt_disable_notrace(), preempt_enable_notrace()) +DEFINE_LOCK_GUARD_0(migrate, migrate_disable(), migrate_enable()) + #endif /* __LINUX_PREEMPT_H */ diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 8716a1706351..71dcc891c72a 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -27,6 +27,7 @@ #include <linux/preempt.h> #include <linux/bottom_half.h> #include <linux/lockdep.h> +#include <linux/cleanup.h> #include <asm/processor.h> #include <linux/cpumask.h> @@ -1012,4 +1013,6 @@ rcu_head_after_call_rcu(struct rcu_head *rhp, rcu_callback_t f) extern int rcu_expedited; extern int rcu_normal; +DEFINE_LOCK_GUARD_0(rcu, rcu_read_lock(), rcu_read_unlock()) + #endif /* __LINUX_RCUPDATE_H */ diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h index 4c715be48717..debdd9c3a5e5 100644 --- a/include/linux/rwsem.h +++ b/include/linux/rwsem.h @@ -16,6 +16,8 @@ #include <linux/spinlock.h> #include <linux/atomic.h> #include <linux/err.h> +#include <linux/cleanup.h> + #ifdef CONFIG_RWSEM_SPIN_ON_OWNER #include <linux/osq_lock.h> #endif @@ -152,6 +154,13 @@ extern void up_read(struct rw_semaphore *sem); */ extern void up_write(struct rw_semaphore *sem); +DEFINE_GUARD(rwsem_read, struct rw_semaphore *, down_read(_T), up_read(_T)) +DEFINE_GUARD(rwsem_write, struct rw_semaphore *, down_write(_T), up_write(_T)) + +DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T)) +DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T)) + + /* * downgrade write lock to read lock */ diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h index 7d18b2bb9ad5..11c23dd5669a 100644 --- a/include/linux/sched/task.h +++ b/include/linux/sched/task.h @@ -142,6 +142,8 @@ static inline void put_task_struct(struct task_struct *t) __put_task_struct(t); } +DEFINE_FREE(put_task, struct task_struct *, if (_T) put_task_struct(_T)) + static inline void put_task_struct_many(struct task_struct *t, int nr) { if (refcount_sub_and_test(nr, &t->usage)) diff --git a/include/linux/slab.h b/include/linux/slab.h index d49470dbe43c..8fd9afd60ff3 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -18,7 +18,7 @@ #include <linux/workqueue.h> #include <linux/percpu-refcount.h> #include <linux/hash.h> - +#include <linux/cleanup.h> /* * Flags to pass to kmem_cache_create(). @@ -209,6 +209,8 @@ void kfree_sensitive(const void *); size_t __ksize(const void *); size_t ksize(const void *); +DEFINE_FREE(kfree, void *, if (_T) kfree(_T)) + #ifdef CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR void __check_heap_object(const void *ptr, unsigned long n, struct page *page, bool to_user); diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h index 79897841a2cc..7a95b9a990ea 100644 --- a/include/linux/spinlock.h +++ b/include/linux/spinlock.h @@ -57,6 +57,7 @@ #include <linux/stringify.h> #include <linux/bottom_half.h> #include <linux/lockdep.h> +#include <linux/cleanup.h> #include <asm/barrier.h> #include <asm/mmiowb.h> @@ -493,4 +494,35 @@ int __alloc_bucket_spinlocks(spinlock_t **locks, unsigned int *lock_mask, void free_bucket_spinlocks(spinlock_t *locks); +DEFINE_LOCK_GUARD_1(raw_spinlock, raw_spinlock_t, + raw_spin_lock(_T->lock), + raw_spin_unlock(_T->lock)) + +DEFINE_LOCK_GUARD_1(raw_spinlock_nested, raw_spinlock_t, + raw_spin_lock_nested(_T->lock, SINGLE_DEPTH_NESTING), + raw_spin_unlock(_T->lock)) + +DEFINE_LOCK_GUARD_1(raw_spinlock_irq, raw_spinlock_t, + raw_spin_lock_irq(_T->lock), + raw_spin_unlock_irq(_T->lock)) + +DEFINE_LOCK_GUARD_1(raw_spinlock_irqsave, raw_spinlock_t, + raw_spin_lock_irqsave(_T->lock, _T->flags), + raw_spin_unlock_irqrestore(_T->lock, _T->flags), + unsigned long flags) + +DEFINE_LOCK_GUARD_1(spinlock, spinlock_t, + spin_lock(_T->lock), + spin_unlock(_T->lock)) + +DEFINE_LOCK_GUARD_1(spinlock_irq, spinlock_t, + spin_lock_irq(_T->lock), + spin_unlock_irq(_T->lock)) + +DEFINE_LOCK_GUARD_1(spinlock_irqsave, spinlock_t, + spin_lock_irqsave(_T->lock, _T->flags), + spin_unlock_irqrestore(_T->lock, _T->flags), + unsigned long flags) + +#undef __LINUX_INSIDE_SPINLOCK_H #endif /* __LINUX_SPINLOCK_H */ diff --git a/include/linux/srcu.h b/include/linux/srcu.h index a0895bbf71ce..e35168e0b952 100644 --- a/include/linux/srcu.h +++ b/include/linux/srcu.h @@ -205,4 +205,9 @@ static inline void smp_mb__after_srcu_read_unlock(void) /* __srcu_read_unlock has smp_mb() internally so nothing to do here. */ } +DEFINE_LOCK_GUARD_1(srcu, struct srcu_struct, + _T->idx = srcu_read_lock(_T->lock), + srcu_read_unlock(_T->lock, _T->idx), + int idx) + #endif diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 0ad235ee96f9..9a1c5ac4ba07 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4522,7 +4522,7 @@ sub process { if|for|while|switch|return|case| volatile|__volatile__| __attribute__|format|__extension__| - asm|__asm__)$/x) + asm|__asm__|scoped_guard)$/x) { # cpp #define statements have non-optional spaces, ie # if there is a space between the name and the open -- 2.34.1

From: Thomas Gleixner <tglx@kernel.org> stable inclusion from stable-v5.10.262 commit 67aa823e3e8c229c6d374df79c804f6721cb83b6 category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/16862 CVE: CVE-2026-64560 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=... -------------------------------- commit 920f893f735e92ba3a1cd9256899a186b161928d upstream. Wongi and Jungwoo decoded and reported a non-leader exec() related race which can result in an UAF: sys_timer_delete() exec() posix_cpu_timer_del() // Observes old leader p = pid_task(pid, pid_type); de_thread() switch_leader(); release_task(old_leader) __exit_signal(old_leader) sighand = lock(old_leader, sighand); posix_cpu_timers*_exit(); sighand = lock_task_sighand(p) unhash_task(old_leader); sh = lock(p, sighand) old_leader->sighand = NULL; unlock(sighand); (p->sighand == NULL) unlock(sh) return NULL; // Returns without action if(!sighand) return 0; free_posix_timer(); This is "harmless" unless the deleted timer was armed and enqueued in p->signal because on exec() a TGID targeted timer is inherited. As sys_timer_delete() freed the underlying posix timer object run_posix_cpu_timers() or any timerqueue related add/delete operations on other timers will access the freed object's timerqueue node, which results in an UAF. There is a similar problem vs. posix_cpu_timer_set(). For regular posix timers it just transiently returns -ESRCH to user space, but for the use case in do_cpu_nanosleep() it's the same UAF just that the k_itimer is allocated on the stack. Also posix_cpu_timer_rearm() fails to rearm the timer, which means it stops to expire. While debating solutions Frederic pointed out another problem: posix_cpu_timer_del(tmr) __exit_signal(p) posix_cpu_timers*_exit(p); unhash_task(p); p->sighand = NULL; sh = lock_task_sighand(p) sighand = p->sighand; if (!sighand) return NULL; lock(sighand); if (!sh) WARN_ON_ONCE(timer_queued(tmr)); On weakly ordered architectures it is not guaranteed that posix_cpu_timer_del() will observe the stores in posix_cpu_timers*_exit() when p->sighand is observed as NULL, which means the WARN() can be a false positive. Solve these issues by: 1) Changing the store in __exit_signal() to smp_store_release(). 2) Adding a smp_acquire__after_ctrl_dep() into the !sighand path of lock_task_sighand(). 3) Creating a helper function for looking up the task and locking sighand which does not return when sighand == NULL. Instead it retries the task lookup and only if that fails it gives up. 4) Using that helper in the three affected functions. observes all preceeding stores, i.e. the stores in posix_cpu_timers*_exit() and the ones in unhash_task(). gracefully. When the task lookup returns the old leader, but sighand == NULL then it retries. In the non-leader exec() case the subsequent task lookup will observe the new leader due to #1/#2. In normal exit() scenarios the subsequent lookup fails. When the task lookup fails, the function also checks whether the timer is still enqueued and issues a warning if that's the case. Unfortunately there is nothing which can be done about it, but as the task is already not longer visible the timer should not be accessed anymore. This check also requires memory ordering, which is not provided when the first lookup fails. To achieve that the check is preceeded by a smp_rmb() which pairs with the smp_wmb() in write_seqlock() in __exit_signal(). That ensures that the stores in posix_cpu_timers*_exit() are visible. The history of the non-leader exec() issue goes back to the early days of posix CPU timers, which stored a pointer to the group leader task in the timer. That obviously fails when a non-leader exec() switches the leader. commit e0a70217107e ("posix-cpu-timers: workaround to suppress the problems with mt exec") added a temporary workaround for that in 2010 which survived about 10 years. The fix for the workaround changed the task pointer to a pid pointer, but failed to see the subtle race described above. So the Fixes tag picks that commit, which seems to be halfways accurate. Thanks to Frederic Weissbecker, Oleg Nesterov and Peter Zijlstra for review, feedback and suggestions and to Wongi and Jungwoo for the excellent bug report and analysis! Fixes: 55e8c8eb2c7b ("posix-cpu-timers: Store a reference to a pid not a task") Reported-by: Wongi Lee <qw3rtyp0@gmail.com> Reported-by: Jungwoo Lee <jwlee2217@gmail.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Reviewed-by: Oleg Nesterov <oleg@redhat.com> Cc: stable@vger.kernel.org Signed-off-by: Sasha Levin <sashal@kernel.org> Conflicts: kernel/time/posix-cpu-timers.c [ context conflicts. ] Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- kernel/exit.c | 8 +- kernel/signal.c | 10 +- kernel/time/posix-cpu-timers.c | 185 +++++++++++++++++++++------------ 3 files changed, 137 insertions(+), 66 deletions(-) diff --git a/kernel/exit.c b/kernel/exit.c index 86b083f5fd60..3427e9514968 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -154,7 +154,13 @@ static void __exit_signal(struct task_struct *tsk) * doing sigqueue_free() if we have SIGQUEUE_PREALLOC signals. */ flush_sigqueue(&tsk->pending); - tsk->sighand = NULL; + + /* + * Ensure that all preceeding state is visible. Pairs with + * the smp_acquire__after_ctrl_dep() in the sighand == NULL + * path of lock_task_sighand(). + */ + smp_store_release(&tsk->sighand, NULL); spin_unlock(&sighand->siglock); __cleanup_sighand(sighand); diff --git a/kernel/signal.c b/kernel/signal.c index 91d177e1023d..77e8c2eaab70 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1382,8 +1382,16 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk, rcu_read_lock(); for (;;) { sighand = rcu_dereference(tsk->sighand); - if (unlikely(sighand == NULL)) + if (unlikely(sighand == NULL)) { + /* + * Pairs with the smp_store_release() in + * __exit_signal(). It ensures that all state + * modifications to the task preceeding the store are + * visible to the callers of lock_task_sighand(). + */ + smp_acquire__after_ctrl_dep(); break; + } /* * This sighand can be already freed and even reused, but diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 3e08c8991833..1a2f6ef4846e 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -405,6 +405,110 @@ static int posix_cpu_timer_create(struct k_itimer *new_timer) return 0; } +/* + * Lookup the task via timer->it.cpu.pid and attempt to lock the task's sighand. + * + * This can race with the reaping of the task: + * + * CPU0 CPU1 + * + * // Finds task + * p = pid_task(pid, pid_type); __exit_signal(p) + * lock(p, sighand); + * posix_cpu_timers*_exit(); + * sighand = lock_task_sighand(p); unhash_task(p); + * p->sighand = NULL; + * unlock(sighand); + * + * In this case sighand is NULL, which means the task and the associated timer + * queue cannot be longer accessed safely. + * + * __exit_signal() invokes posix_cpu_timers_exit() and if the thread group is + * dead it also invokes posix_cpu_timers_group_exit(). These functions delete + * all pending timers from the related timer queues. The POSIX timers (k_itimer) + * themself are still accessible, but not longer connected to the task. + * + * exec() works slightly differently. The task which exec()'s terminates all + * other threads in the thread group and runs __exit_signal() on them. As the + * thread group is not dead they only clean up the per task timers via + * posix_cpu_timers_exit(). + * + * As the TGID on exec() stays the same per process timers stay queued, if they + * are armed. This works without a problem when exec() is done by the thread + * group leader. If a non-leader thread exec()'s this can end up in the + * following scenario: + * + * CPU0 CPU1 + * // Returns old leader + * p = pid_task(pid, pid_type); de_thread() + * switch_leader() + * release_task(old leader) + * __exit_signal() + * old_leader->sighand = NULL; + * // Returns NULL + * sighand = lock_task_sighand(p) + * + * That's problematic for several functions: + * + * - posix_cpu_timer_del(): If the timer is still enqueued on the task the + * underlying k_itimer will be freed which results in a UAF in + * run_posix_cpu_timers() or on timerqueue related add/delete operations. + * If the timer is not enqueued, the failure is harmless + * + * - posix_cpu_timer_set(): Independent of the enqueued state that results in a + * transient failure which is user space visible (-ESRCH) for regular posix + * timers. But for the use case in do_cpu_nanosleep() it's the same UAF + * problem just that the timer is allocated on the stack. + * + * - posix_cpu_timer_rearm(): Timer is not enqueued at that point, but this + * silently ignores the rearm request, which is a functional problem as the + * timer wont expire anymore. + */ +static struct task_struct *timer_lock_sighand(struct k_itimer *timer, unsigned long *flags) +{ + enum pid_type type = clock_pid_type(timer->it_clock); + struct cpu_timer *ctmr = &timer->it.cpu; + + guard(rcu)(); + + for (;;) { + struct task_struct *t = pid_task(timer->it.cpu.pid, type); + + /* Fail if the task cannot be found. */ + if (!t) + break; + + /* Try to lock the task's sighand */ + if (lock_task_sighand(t, flags)) + return t; + + /* + * The next PID lookup might either fail or return the new + * leader. This is correct for both exit() and exec(). + */ + } + + /* + * If the timer is still enqueued, warn. There is nothing safe to do + * here as there might be two timers in there which are removed in + * parallel and that will cause more damage than good. This should never + * happen! + * + * Ensure that the stores to the timer and timerqueue are visible: + * + * __exit_signal() + * posix_cpu_timers*_exit() + * write_seqlock(seqlock) + * smp_wmb(); <------- + * __unhash_process() | !pid_task() + * ----> smp_rmb(); + * WARN_ON_ONCE(...) + */ + smp_rmb(); + WARN_ON_ONCE(ctmr->head || timerqueue_node_queued(&ctmr->node)); + return NULL; +} + /* * Clean up a CPU-clock timer that is about to be destroyed. * This is called from timer deletion with the timer already locked. @@ -414,28 +518,13 @@ static int posix_cpu_timer_create(struct k_itimer *new_timer) static int posix_cpu_timer_del(struct k_itimer *timer) { struct cpu_timer *ctmr = &timer->it.cpu; - struct sighand_struct *sighand; struct task_struct *p; unsigned long flags; int ret = 0; - rcu_read_lock(); - p = cpu_timer_task_rcu(timer); - if (!p) - goto out; + p = timer_lock_sighand(timer, &flags); - /* - * Protect against sighand release/switch in exit/exec and process/ - * thread timer list entry concurrent read/writes. - */ - sighand = lock_task_sighand(p, &flags); - if (unlikely(sighand == NULL)) { - /* - * This raced with the reaping of the task. The exit cleanup - * should have removed this timer from the timer queue. - */ - WARN_ON_ONCE(ctmr->head || timerqueue_node_queued(&ctmr->node)); - } else { + if (likely(p)) { if (timer->it.cpu.firing) ret = TIMER_RETRY; else @@ -444,8 +533,6 @@ static int posix_cpu_timer_del(struct k_itimer *timer) unlock_task_sighand(p, &flags); } -out: - rcu_read_unlock(); if (!ret) put_pid(ctmr->pid); @@ -575,37 +662,19 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags, clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock); u64 old_expires, new_expires, old_incr, val; struct cpu_timer *ctmr = &timer->it.cpu; - struct sighand_struct *sighand; struct task_struct *p; unsigned long flags; int ret = 0; - rcu_read_lock(); - p = cpu_timer_task_rcu(timer); - if (!p) { - /* - * If p has just been reaped, we can no - * longer get any information about it at all. - */ - rcu_read_unlock(); - return -ESRCH; - } - - new_expires = timespec64_to_ns(&new->it_value); - + p = timer_lock_sighand(timer, &flags); /* - * Protect against sighand release/switch in exit/exec and p->cpu_timers - * and p->signal->cpu_timers read/write in arm_timer() + * If p has just been reaped, we can no longer get any information about + * it at all. */ - sighand = lock_task_sighand(p, &flags); - /* - * If p has just been reaped, we can no - * longer get any information about it at all. - */ - if (unlikely(sighand == NULL)) { - rcu_read_unlock(); + if (!p) return -ESRCH; - } + + new_expires = timespec64_to_ns(&new->it_value); /* * Disarm any old timer after extracting its expiry time. @@ -655,6 +724,8 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags, old->it_value.tv_sec = 0; } } + + old->it_interval = ns_to_timespec64(old_incr); } if (unlikely(ret)) { @@ -665,7 +736,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags, * it as an overrun (thanks to bump_cpu_timer above). */ unlock_task_sighand(p, &flags); - goto out; + return ret; } if (new_expires != 0 && !(timer_flags & TIMER_ABSTIME)) { @@ -682,7 +753,6 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags, arm_timer(timer, p); } - unlock_task_sighand(p, &flags); /* * Install the new reload setting, and * set up the signal and overrun bookkeeping. @@ -699,6 +769,8 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags, timer->it_overrun_last = 0; timer->it_overrun = -1; + unlock_task_sighand(p, &flags); + if (new_expires != 0 && !(val < new_expires)) { /* * The designated time already passed, so we notify @@ -708,13 +780,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags, cpu_timer_fire(timer); } - ret = 0; - out: - rcu_read_unlock(); - if (old) - old->it_interval = ns_to_timespec64(old_incr); - - return ret; + return 0; } static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp) @@ -978,19 +1044,12 @@ static void posix_cpu_timer_rearm(struct k_itimer *timer) { clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock); struct task_struct *p; - struct sighand_struct *sighand; unsigned long flags; u64 now; - rcu_read_lock(); - p = cpu_timer_task_rcu(timer); - if (!p) - goto out; - - /* Protect timer list r/w in arm_timer() */ - sighand = lock_task_sighand(p, &flags); - if (unlikely(sighand == NULL)) - goto out; + p = timer_lock_sighand(timer, &flags); + if (unlikely(!p)) + return; /* * Fetch the current sample and update the timer's expiry time. @@ -1007,8 +1066,6 @@ static void posix_cpu_timer_rearm(struct k_itimer *timer) */ arm_timer(timer, p); unlock_task_sighand(p, &flags); -out: - rcu_read_unlock(); } /** -- 2.34.1