futex: Keep the PI owner of a private futex in the key's mm
Subsystems (6)
PATCH v2 1/1 futex: Keep the PI owner of a private futex in the key's mm
The futex word read by the FUTEX_LOCK_PI operations can hold an arbitrary TID. For the task looked up by that TID, attach_to_pi_owner() only checks whether it is a kernel thread and whether it is already exiting. It does not verify that the task belongs to the address space the futex key was taken from.
A private futex key is the pair (mm, address), and the mm is stored as a plain pointer without taking a reference. __attach_to_pi_owner() copies that key into the pi_state by value and links the pi_state onto the owner's futex.pi_state_list. The pi_state can therefore hold a key of an mm which is not the owner's, either because the TID named a task in another mm to begin with, or because exec_mmap() installs a new mm on the owner after the attach. de_thread() runs before exec_mmap(), so the waiter in the latter case is not a thread but a sibling that only shares the mm.
When the owner exits, exit_pi_state_list() pins the private hash of the owner's mm with guard(private_hash)(current->mm), but resolves the hash bucket from the key stored in the pi_state, which points at the waiter's mm (M below). The stored key holds no reference on M, and futex_hash_free() does not look at the fph references still outstanding.
T1 (waiter, CLONE_VM sibling) T2 (owner, mm = M -> new_mm)
clone(CLONE_VM) // creates M's private hash
execve()
exec_mmap()
exec_mm_release()
futex_exec_release() // STATE_OK
futex_lock_pi()
attach_to_pi_owner() // p->mm == key->private.mm
pi_state->key = *key // {M, addr}
list_add(&pi_state->list, &T2->futex.pi_state_list)
tsk->mm = new_mm
do_exit()
exit_pi_state_list()
guard(private_hash)(current->mm)
key = pi_state->key // M
CLASS(hbr, hbr)(&key)
hb = hbr.hb // +refcount do_exit()
exit_mm()
mmput(M)
__mmput(M)
futex_hash_free(M)
kvfree(fph)
spin_lock(&hb->lock) // UAFA private futex only has meaning inside the mm its key was taken from. Verify in attach_to_pi_owner() that the candidate owner belongs to the mm of the private key and return -ESRCH otherwise, as the TID is then simply a bogus user space value. p->mm is not protected by p->pi_lock, so it is read with READ_ONCE(), and the check is placed after the exit state check. That check alone does not stop exec, so futex_exec_release() is split into begin/end to keep FUTEX_STATE_EXITING set until the mm has been swapped. An attach in that window gets -EBUSY, retries, and then fails the check against the new mm. Fixing only one of the two leaves the same use-after-free reachable through the other path.
---
Changes in v2:
- Also keep FUTEX_STATE_EXITING set across the whole exec transition. v1 only validated at attach time, and a successful execve() invalidates that afterwards, so the same use-after-free stayed reachable with v1 alone. futex_exec_release() is split into begin/end, and every exec_mmap() unwind calls the end half.
- Rewrite the commit message to cover both paths.
- v1: https://lore.kernel.org/all/angLZrg_1yeCW0ig@v4bel/
--- fs/exec.c | 7 ++++++- include/linux/futex.h | 8 ++++++-- kernel/fork.c | 2 +- kernel/futex/core.c | 8 +++++++- kernel/futex/pi.c | 7 +++++++ 5 files changed, 27 insertions(+), 5 deletions(-)
fs/exec.c +6 -1
@@ -65,6 +65,7 @@#include <linux/io_uring.h> #include <linux/syscall_user_dispatch.h> #include <linux/coredump.h> +#include <linux/futex.h> #include <linux/time_namespace.h> #include <linux/user_events.h> #include <linux/rseq.h>
@@ -857,8 +858,10 @@ static int exec_mmap(struct linux_binprm *bprm)exec_mm_release (tsk, old_mm); ret =down_write_killable (&tsk->signal->exec_update_lock); -if (ret) +if (ret) { +futex_exec_release_end (tsk);return ret; + }if (old_mm) {/*
@@ -869,6 +872,7 @@ static int exec_mmap(struct linux_binprm *bprm) ret =mmap_read_lock_killable (old_mm);if (ret) {up_write (&tsk->signal->exec_update_lock); +futex_exec_release_end (tsk);return ret; } }
@@ -896,6 +900,7 @@ static int exec_mmap(struct linux_binprm *bprm)local_irq_enable ();lru_gen_add_mm (mm);task_unlock (tsk); +futex_exec_release_end (tsk);lru_gen_use_mm (mm);if (old_mm) {mmap_read_unlock (old_mm);
include/linux/futex.h +6 -2
@@ -72,7 +72,10 @@ static inline void futex_init_task(struct task_struct *tsk)void futex_exit_recursive (struct task_struct *tsk);void futex_exit_release (struct task_struct *tsk); -void futex_exec_release (struct task_struct *tsk); +void futex_exec_release_begin(struct task_struct *tsk) + __acquires(&tsk->futex.exit_mutex); +void futex_exec_release_end(struct task_struct *tsk) + __releases(&tsk->futex.exit_mutex);long do_futex (u32 __user *uaddr,int op,u32 val,ktime_t *timeout,u32 __user *uaddr2,u32 val2,u32 val3);
@@ -90,7 +93,8 @@ static inline int futex_hash_free(struct mm_struct *mm) { return 0; }static inline void futex_init_task (struct task_struct *tsk) { }static inline void futex_exit_recursive (struct task_struct *tsk) { }static inline void futex_exit_release (struct task_struct *tsk) { } -static inline void futex_exec_release (struct task_struct *tsk) { } +static inline void futex_exec_release_begin (struct task_struct *tsk) { } +static inline void futex_exec_release_end (struct task_struct *tsk) { }static inline long do_futex (u32 __user *uaddr,int op,u32 val,ktime_t *timeout,u32 __user *uaddr2,u32 val2,u32 val3) {
kernel/fork.c +1 -1
@@ -1510,7 +1510,7 @@ void exit_mm_release(struct task_struct *tsk, struct mm_struct *mm)void exec_mm_release (struct task_struct *tsk,struct mm_struct *mm) { -futex_exec_release (tsk); +futex_exec_release_begin (tsk);mm_release (tsk, mm); }
kernel/futex/core.c +7 -1
@@ -1539,7 +1539,8 @@ static void futex_cleanup_end(struct task_struct *tsk, int state)mutex_unlock (&tsk->futex.exit_mutex); } -void futex_exec_release (struct task_struct *tsk) +void futex_exec_release_begin (struct task_struct *tsk) + __acquires(&tsk->futex.exit_mutex) {/* * The state handling is done for consistency, but in the case of
@@ -1550,6 +1551,11 @@ void futex_exec_release(struct task_struct *tsk)*/ futex_cleanup_begin (tsk);futex_cleanup (tsk); +} + +void futex_exec_release_end (struct task_struct *tsk) + __releases(&tsk->futex.exit_mutex) +{/* * Reset the state to FUTEX_STATE_OK. The task is alive and about * exec a new binary.
kernel/futex/pi.c +7 -0
@@ -465,6 +465,13 @@ static int attach_to_pi_owner(u32 __user *uaddr, u32 uval, union futex_key *key,return ret; } +if (IS_ENABLED (CONFIG_MMU ) &&futex_key_is_private (key) && +READ_ONCE (p->mm) != key->private.mm) { +raw_spin_unlock_irq (&p->pi_lock); +put_task_struct (p); +return -ESRCH ; + } +__attach_to_pi_owner (p, key, ps);raw_spin_unlock_irq (&p->pi_lock);
Signature · 1 line
2.43.0