A FUT_OFF_MMSHARED futex is a shared futex that refers to an MM. It is possible for a process to wait on a shared futex with a different MM because a FUT_OFF_INODE waiter can be requeued onto a FUT_OFF_MMSHARED futex by another process. This can cause FUT_OFF_MMSHARED waiters on a freed MM to consume wakeups intended for a newly allocated MM at the same address. Fix it by keying FUT_OFF_MMSHARED using a unique 64-bit per-MM ID. Leave private futexes as before to avoid influencing the performance of the hotpath. (Multi-threaded processes typically implicitly use FUT_OFF_MMSHARED by setting clear_child_tid such that it points into anonymous memory, which causes mm_release() in a multi-threaded mm to perform FUTEX_WAKE.) Cc: stable@vger.kernel.org Fixes: 222993395ed3 ("futex: Remove pointless mmgrap() + mmdrop()") Closes: https://lore.kernel.org/r/CAG48ez0dLBpc3QtbAhMMVNHwHL94iHh2G+h-=BVFR4dDuzZr1g@mail.gmail.com/ Signed-off-by: Jann Horn --- I've added a stable tag since, due to clear_child_tid, this can probably be used to prevent pthread_join() from completing in another process or to brute-force the addresses of pthread instances in other processes. --- include/linux/futex.h | 19 ++++++++--------- include/linux/futex_types.h | 2 ++ kernel/futex/core.c | 51 +++++++++++++++++++++++++-------------------- 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/include/linux/futex.h b/include/linux/futex.h index 18ed18d5cbc1..7eb645e49dd4 100644 --- a/include/linux/futex.h +++ b/include/linux/futex.h @@ -23,28 +23,32 @@ struct task_struct; * 01 : Shared futex (PTHREAD_PROCESS_SHARED) * mapped on a file (reference on the underlying inode) * 10 : Shared futex (PTHREAD_PROCESS_SHARED) - * (but private mapping on an mm, and reference taken on it) + * (but private mapping on an mm) */ -#define FUT_OFF_INODE 1 /* We set bit 0 if key has a reference on inode */ -#define FUT_OFF_MMSHARED 2 /* We set bit 1 if key has a reference on mm */ +#define FUT_OFF_INODE 1 /* We set bit 0 if shared key identifies an inode */ +#define FUT_OFF_MMSHARED 2 /* We set bit 1 if shared key identifies an mm */ union futex_key { + /* For FUT_OFF_INODE */ struct { u64 i_seq; unsigned long pgoff; unsigned int offset; /* unsigned int node; */ } shared; + + /* For FUT_OFF_MMSHARED or private */ struct { union { - struct mm_struct *mm; - u64 __tmp; + struct mm_struct *mm; /* for private */ + u64 mm_seq; /* for FUT_OFF_MMSHARED */ }; unsigned long address; unsigned int offset; /* unsigned int node; */ } private; + struct { u64 ptr; unsigned long word; @@ -152,11 +156,6 @@ static inline void futex_set_vdso_cs_range(struct futex_mm_data *fd, unsigned in static inline void futex_fixup_robust_unlock(struct pt_regs *regs) { } #endif /* !CONFIG_FUTEX_ROBUST_UNLOCK */ - -#if defined(CONFIG_FUTEX_PRIVATE_HASH) || defined(CONFIG_FUTEX_ROBUST_UNLOCK) void futex_mm_init(struct mm_struct *mm); -#else -static inline void futex_mm_init(struct mm_struct *mm) { } -#endif #endif /* _LINUX_FUTEX_H */ diff --git a/include/linux/futex_types.h b/include/linux/futex_types.h index d320c0571f0c..34b16f2828e7 100644 --- a/include/linux/futex_types.h +++ b/include/linux/futex_types.h @@ -85,10 +85,12 @@ struct futex_unlock_cs_ranges { }; * struct futex_mm_data - Futex related per MM data * @phash: Futex private hash related data * @unlock: Futex unlock VDSO critical sections + * @unique_id: Unique ID of the MM, for FUT_OFF_MMSHARED futexes */ struct futex_mm_data { struct futex_mm_phash phash; struct futex_unlock_cs_ranges unlock; + atomic64_t unique_id; }; #else /* CONFIG_FUTEX */ struct futex_sched_data { }; diff --git a/kernel/futex/core.c b/kernel/futex/core.c index a061f54b606d..3593d8a328e9 100644 --- a/kernel/futex/core.c +++ b/kernel/futex/core.c @@ -437,45 +437,51 @@ struct hrtimer_sleeper *futex_setup_timer(ktime_t *time, struct hrtimer_sleeper } /* - * Generate a machine wide unique identifier for this inode. + * Generate a machine wide unique identifier for this object (inode or mm). * * This relies on u64 not wrapping in the life-time of the machine; which with * 1ns resolution means almost 585 years. - * - * This further relies on the fact that a well formed program will not unmap - * the file while it has a (shared) futex waiting on it. This mapping will have - * a file reference which pins the mount and inode. - * - * If for some reason an inode gets evicted and read back in again, it will get - * a new sequence number and will _NOT_ match, even though it is the exact same - * file. - * - * It is important that futex_match() will never have a false-positive, esp. - * for PI futexes that can mess up the state. The above argues that false-negatives - * are only possible for malformed programs. */ -static u64 get_inode_sequence_number(struct inode *inode) +static u64 get_object_id(atomic64_t *object_seq) { - static atomic64_t i_seq; + static atomic64_t last_assigned_seq; u64 old; - /* Does the inode already have a sequence number? */ - old = atomic64_read(&inode->i_sequence); + /* Does the object already have a sequence number? */ + old = atomic64_read(object_seq); if (likely(old)) return old; for (;;) { - u64 new = atomic64_inc_return(&i_seq); + u64 new = atomic64_inc_return(&last_assigned_seq); if (WARN_ON_ONCE(!new)) continue; old = 0; - if (!atomic64_try_cmpxchg_relaxed(&inode->i_sequence, &old, new)) + if (!atomic64_try_cmpxchg_relaxed(object_seq, &old, new)) return old; return new; } } +/* + * This relies on the fact that a well formed program will not unmap + * the file while it has a (shared) futex waiting on it. This mapping will have + * a file reference which pins the mount and inode. + * + * If for some reason an inode gets evicted and read back in again, it will get + * a new sequence number and will _NOT_ match, even though it is the exact same + * file. + * + * It is important that futex_match() will never have a false-positive, esp. + * for PI futexes that can mess up the state. The above argues that false-negatives + * are only possible for malformed programs. + */ +static u64 get_inode_sequence_number(struct inode *inode) +{ + return get_object_id(&inode->i_sequence); +} + /** * get_futex_key() - Get parameters which are the keys for a futex * @uaddr: virtual address of the futex @@ -681,8 +687,8 @@ int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key, goto out; } - key->both.offset |= FUT_OFF_MMSHARED; /* ref taken on mm */ - key->private.mm = mm; + key->both.offset |= FUT_OFF_MMSHARED; + key->private.mm_seq = get_object_id(&mm->futex.unique_id); key->private.address = address; } else { @@ -2053,13 +2059,12 @@ static void futex_robust_unlock_init_mm(struct futex_mm_data *fd) static inline void futex_robust_unlock_init_mm(struct futex_mm_data *fd) { } #endif /* !CONFIG_FUTEX_ROBUST_UNLOCK */ -#if defined(CONFIG_FUTEX_PRIVATE_HASH) || defined(CONFIG_FUTEX_ROBUST_UNLOCK) void futex_mm_init(struct mm_struct *mm) { + atomic64_set(&mm->futex.unique_id, 0); futex_hash_init_mm(&mm->futex); futex_robust_unlock_init_mm(&mm->futex); } -#endif int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4) { --- base-commit: 6f8319e3e9a44dd537d17f41565a8453c560a581 change-id: 20260929-futex-mmshared-fix-7c8635797e15 Best regards, -- Jann Horn