diff options
| -rw-r--r-- | fs/exec.c | 9 | ||||
| -rw-r--r-- | include/linux/futex.h | 8 | ||||
| -rw-r--r-- | include/linux/sched/mm.h | 10 | ||||
| -rw-r--r-- | kernel/exit.c | 2 | ||||
| -rw-r--r-- | kernel/fork.c | 10 | ||||
| -rw-r--r-- | kernel/futex/core.c | 77 | ||||
| -rw-r--r-- | kernel/futex/pi.c | 121 |
7 files changed, 160 insertions, 77 deletions
diff --git a/fs/exec.c b/fs/exec.c index a14f28b15607..745f6eb5279e 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -30,6 +30,7 @@ #include <linux/mm.h> #include <linux/stat.h> #include <linux/fcntl.h> +#include <linux/futex.h> #include <linux/swap.h> #include <linux/string.h> #include <linux/init.h> @@ -854,7 +855,8 @@ static int exec_mmap(struct linux_binprm *bprm) /* Notify parent that we're no longer interested in the old VM */ tsk = current; old_mm = current->mm; - exec_mm_release(tsk, old_mm); + /* Clean up futexes and release the mm */ + mm_exit_exec_release(tsk, old_mm); ret = down_write_killable(&tsk->signal->exec_update_lock); if (ret) @@ -902,9 +904,10 @@ static int exec_mmap(struct linux_binprm *bprm) BUG_ON(active_mm != old_mm); /* Defer teardown to setup_new_exec(), outside the exec locks. */ bprm->old_mm = old_mm; - return 0; + } else { + mmdrop_lazy_tlb(active_mm); } - mmdrop_lazy_tlb(active_mm); + futex_exec_done(tsk); return 0; } diff --git a/include/linux/futex.h b/include/linux/futex.h index 51f4ccdc9092..18ed18d5cbc1 100644 --- a/include/linux/futex.h +++ b/include/linux/futex.h @@ -71,8 +71,8 @@ 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_exit_exec_release(struct task_struct *tsk); +void futex_exec_done(struct task_struct *tsk); long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout, u32 __user *uaddr2, u32 val2, u32 val3); @@ -89,8 +89,8 @@ static inline int futex_hash_free(struct mm_struct *mm) { return 0; } #else /* CONFIG_FUTEX */ 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_exit_exec_release(struct task_struct *tsk) { } +static inline void futex_exec_done(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) { diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index 95d0040df584..7fe7dfd6f8be 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -155,10 +155,12 @@ extern struct mm_struct *get_task_mm(struct task_struct *task); * succeeds. */ extern struct mm_struct *mm_access(struct task_struct *task, unsigned int mode); -/* Remove the current tasks stale references to the old mm_struct on exit() */ -extern void exit_mm_release(struct task_struct *, struct mm_struct *); -/* Remove the current tasks stale references to the old mm_struct on exec() */ -extern void exec_mm_release(struct task_struct *, struct mm_struct *); + +/* + * Remove the current tasks stale references to the old mm_struct on exit() and + * exec(). Cleans up futexes as well. + */ +extern void mm_exit_exec_release(struct task_struct *, struct mm_struct *); #ifdef CONFIG_MEMCG extern void mm_update_next_owner(struct mm_struct *mm); diff --git a/kernel/exit.c b/kernel/exit.c index f812df279ea7..e9f902be4ea6 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -582,7 +582,7 @@ static void exit_mm(void) { struct mm_struct *mm = current->mm; - exit_mm_release(current, mm); + mm_exit_exec_release(current, mm); if (!mm) return; diff --git a/kernel/fork.c b/kernel/fork.c index 1e68404bd773..e645675dd727 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1508,15 +1508,9 @@ static void mm_release(struct task_struct *tsk, struct mm_struct *mm) complete_vfork_done(tsk); } -void exit_mm_release(struct task_struct *tsk, struct mm_struct *mm) +void mm_exit_exec_release(struct task_struct *tsk, struct mm_struct *mm) { - futex_exit_release(tsk); - mm_release(tsk, mm); -} - -void exec_mm_release(struct task_struct *tsk, struct mm_struct *mm) -{ - futex_exec_release(tsk); + futex_exit_exec_release(tsk); mm_release(tsk, mm); } diff --git a/kernel/futex/core.c b/kernel/futex/core.c index 7ab106616990..a7c2a6242718 100644 --- a/kernel/futex/core.c +++ b/kernel/futex/core.c @@ -1527,14 +1527,12 @@ static void futex_cleanup_begin(struct task_struct *tsk) raw_spin_unlock_irq(&tsk->pi_lock); } -static void futex_cleanup_end(struct task_struct *tsk, int state) +static void futex_cleanup_end(struct task_struct *tsk) __releases(&tsk->futex.exit_mutex) { - /* - * Lockless store. The only side effect is that an observer might - * take another loop until it becomes visible. - */ - tsk->futex.state = state; + scoped_guard(raw_spinlock_irq, &tsk->pi_lock) + tsk->futex.state = FUTEX_STATE_DEAD; + /* * Drop the exit protection. This unblocks waiters which observed * FUTEX_STATE_EXITING to reevaluate the state. @@ -1542,29 +1540,46 @@ 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) +/* + * Invoked from mm_exit_exec_release() to cleanup the robust lists and pi state + * of the outgoing task. + * + * exec() makes it interesting for futexes because the TID of the task stays the + * same, but from a futex perspective the task has to be treated like an exiting + * task. This is especially important for the sanity check for private futexes + * in attach_to_pi_owner() which compares the owner's mm with the waiter's mm. + * + * That check would give the wrong answer if futex_cleanup_end() would + * set the state to FUTEX_STATE_OK as long as the task still has the old + * mm. + * + * After the task has switched to the new mm it sets it to + * FUTEX_STATE_OK again in futex_exec_done(). + */ +void futex_exit_exec_release(struct task_struct *tsk) { - /* - * The state handling is done for consistency, but in the case of - * exec() there is no way to prevent further damage as the PID stays - * the same. But for the unlikely and arguably buggy case that a - * futex is held on exec(), this provides at least as much state - * consistency protection which is possible. - */ futex_cleanup_begin(tsk); futex_cleanup(tsk); - /* - * Reset the state to FUTEX_STATE_OK. The task is alive and about - * exec a new binary. - */ - futex_cleanup_end(tsk, FUTEX_STATE_OK); + futex_cleanup_end(tsk); } -void futex_exit_release(struct task_struct *tsk) +/* + * exec() has switched to the new mm. Futex operations are safe again. + */ +void futex_exec_done(struct task_struct *tsk) { - futex_cleanup_begin(tsk); - futex_cleanup(tsk); - futex_cleanup_end(tsk, FUTEX_STATE_DEAD); + /* + * This store does not have to take tsk::futex::exit_mutex because the + * phase where waiters block on it during state FUTEX_STATE_EXITING has + * been finished when futex_cleanup_end() set the state to + * FUTEX_STATE_DEAD. + * + * This transitions back from FUTEX_STATE_DEAD to FUTEX_STATE_OK. The + * ordering guarantee required here is that the previous store to + * tsk::mm in the calling code cannot be reordered against this store. + */ + guard(raw_spinlock_irq)(&tsk->pi_lock); + tsk->futex.state = FUTEX_STATE_OK; } static void futex_hash_bucket_init(struct futex_hash_bucket *fhb) @@ -1844,14 +1859,18 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags) } if (!mm->futex.phash.ref) { + unsigned int __percpu *ref = alloc_percpu(unsigned int); + + if (!ref) + return -ENOMEM; + /* - * This will always be allocated by the first thread and - * therefore requires no locking. + * Tasks sharing the mm can run this concurrently, so take the + * initial reference before publishing the counter. */ - mm->futex.phash.ref = alloc_percpu(unsigned int); - if (!mm->futex.phash.ref) - return -ENOMEM; - this_cpu_inc(*mm->futex.phash.ref); /* 0 -> 1 */ + this_cpu_inc(*ref); /* 0 -> 1 */ + if (cmpxchg(&mm->futex.phash.ref, NULL, ref)) + free_percpu(ref); } fph = kvzalloc(struct_size(fph, queues, hash_slots), diff --git a/kernel/futex/pi.c b/kernel/futex/pi.c index 795011ea1202..88788e584ec8 100644 --- a/kernel/futex/pi.c +++ b/kernel/futex/pi.c @@ -193,6 +193,58 @@ void put_pi_state(struct futex_pi_state *pi_state) * pi_mutex->wait_lock * p->pi_lock * + * Futex kernel state: + * + * The kernel tracks the task state in p::futex::state to protect against exit() + * and exec(). The states are: + * + * - FUTEX_STATE_OK when the task is alive and waiters can be attached + * + * - FUTEX_STATE_EXITING when the task cleans up the robust list and PI + * state. Concurrent waiters cannot attach anymore and have to wait until the + * cleanup is finished to re-evaluate the potential changes caused by the + * robust list and PI state cleanups. + * + * - FUTEX_STATE_DEAD when the task has cleaned up the robust list. This state + * is set independent of exit() or exec(). In the exit() case the task is + * gone. In the exec() case this ensures that nothing can attach to the task + * after cleaning up the robust list and PI state before it has switched to + * the new mm. From a futex point of view the task is dead until it sets the + * state to FUTEX_STATE_OK again after switching to the new mm. + * + * The valid state transitions for exit(): + * + * FUTEX_STATE_OK -> FUTEX_STATE_EXITING -> FUTEX_STATE_DEAD + * + * The valid state transitions for exec(): + * + * FUTEX_STATE_OK -> FUTEX_STATE_EXITING -> FUTEX_STATE_DEAD -> FUTEX_STATE_OK + * + * The state has two related locks: + * + * 1) p::pi_lock + * + * p::pi_lock has to be taken by the waiter when evaluating the state to + * protect against a concurrent exit/exec cleanup by the owner. If the state + * is OK then the waiter can be attached to the owner while still holding + * pi_lock. + * + * The cleanup code has to hold it for all state transitions to ensure that + * the stores to the state cannot be reordered against previous stores on + * which the waiter correctness depends on. + * + * 2) p::futex::exit_mutex + * + * The mutex is acquired when the cleanup starts and released at the end. It + * obviously is not serializing the owner's cleanup against itself. It is + * used to avoid a live lock caused by a waiter preempting the owner's + * cleanup. Such a waiter would busy loop forever waiting for the owner to + * finish the cleanup. + * + * To prevent this, waiters have to drop all locks when observing + * FUTEX_STATE_EXITING and block on the mutex. When the owner releases the + * mutex after finishing the cleanup the waiters make progress and + * re-evaluate the situation. */ /* @@ -318,19 +370,11 @@ out_error: return ret; } -static int handle_exit_race(u32 __user *uaddr, u32 uval, - struct task_struct *tsk) +static int handle_exit_race(u32 __user *uaddr, u32 uval) { u32 uval2; /* - * If the futex exit state is not yet FUTEX_STATE_DEAD, tell the - * caller that the alleged owner is busy. - */ - if (tsk && tsk->futex.state != FUTEX_STATE_DEAD) - return -EBUSY; - - /* * Reread the user space value to handle the following situation: * * CPU0 CPU1 @@ -427,7 +471,7 @@ static int attach_to_pi_owner(u32 __user *uaddr, u32 uval, union futex_key *key, return -EAGAIN; p = find_get_task_by_vpid(pid); if (!p) - return handle_exit_race(uaddr, uval, NULL); + return handle_exit_race(uaddr, uval); if (unlikely(p->flags & PF_KTHREAD)) { put_task_struct(p); @@ -435,34 +479,55 @@ static int attach_to_pi_owner(u32 __user *uaddr, u32 uval, union futex_key *key, } /* - * We need to look at the task state to figure out, whether the - * task is exiting. To protect against the change of the task state - * in futex_exit_release(), we do this protected by p->pi_lock: + * We need to look at the task state to figure out whether the task is + * exiting. To protect against the change of the task state from + * FUTEX_STATE_OK to FUTEX_STATE_EXISTING in futex_cleanup_begin() it is + * required to do this protected by p->pi_lock, which prevents the owner + * from concurrently starting the exit cleanup. + * + * If the state is FUTEX_STATE_OK pi_lock must be held until the waiter + * is attached to protect against a concurrent exit()/exec(). */ raw_spin_lock_irq(&p->pi_lock); + + /* Validate that the task is ready for futex operations. */ if (unlikely(p->futex.state != FUTEX_STATE_OK)) { /* - * The task is on the way out. When the futex state is - * FUTEX_STATE_DEAD, we know that the task has finished - * the cleanup: + * The task is on the way out. When state is FUTEX_STATE_EXITING + * the cleanup is in progress. To avoid a live lock when the + * waiter preempted the owner, store the task pointer in + * @exiting and keep the reference on the task. The calling code + * will drop all locks, block on @p::futex::exit_mutex and wait + * for the owner to finish the cleanup. Once the owner released + * the mutex the waiter drops the reference count and + * re-evaluates the situation. */ - int ret = handle_exit_race(uaddr, uval, p); + if (p->futex.state == FUTEX_STATE_EXITING) { + raw_spin_unlock_irq(&p->pi_lock); + *exiting = p; + return -EBUSY; + } + + int ret = handle_exit_race(uaddr, uval); raw_spin_unlock_irq(&p->pi_lock); + put_task_struct(p); + return ret; + } + + if (IS_ENABLED(CONFIG_MMU) && futex_key_is_private(key)) { /* - * If the owner task is between FUTEX_STATE_EXITING and - * FUTEX_STATE_DEAD then store the task pointer and keep - * the reference on the task struct. The calling code will - * drop all locks, wait for the task to reach - * FUTEX_STATE_DEAD and then drop the refcount. This is - * required to prevent a live lock when the current task - * preempted the exiting task between the two states. + * A private futex key holds a pointer to the waiter's mm + * without holding a reference on it. So it must not be attached + * to an owner in a different address space. Otherwise that + * owner's exit cleanup could access the private hash after the + * key's mm is freed. */ - if (ret == -EBUSY) - *exiting = p; - else + if (unlikely(p->mm != key->private.mm)) { + raw_spin_unlock_irq(&p->pi_lock); put_task_struct(p); - return ret; + return -EPERM; + } } __attach_to_pi_owner(p, key, ps); |