Skip to contentSkip to conversation

futex: Require the PI owner of a private futex to share the key's mm

PATCH Superseded Hyunwoo Kim 1 participant 1 message Aug 9 Futex

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 futex: Require the PI owner of a private futex to share the key's mm

The futex word read by the FUTEX_LOCK_PI operations can hold an arbitrary TID. attach_to_pi_owner() only checks whether the task looked up by that TID 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, so a key scoped to the waiter's mm ends up on a task in a different 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). Nothing holds a reference that keeps that mm alive.

T1 (waiter, mm = M) T2 (owner, mm != M)

prctl(PR_FUTEX_HASH, SET_SLOTS, 2) /* creates M's private hash */
futex_lock_pi()

    attach_to_pi_owner()
      pi_state->key = *key       /* {M, addr} */
      list_add(&pi_state->list, &T2->futex.pi_state_list)

                                 do_exit()
                                   exit_pi_state_list()
                                     guard(private_hash)(current->mm)
                                       /* T2's mm, not M */
                                     raw_spin_lock_irq(&curr->pi_lock)
                                     key = pi_state->key   // M 
                                     CLASS(hbr, hbr)(&key)
                                     hb = hbr.hb
                                     raw_spin_unlock_irq(&curr->pi_lock)
  do_exit()
    exit_mm()
      mmput(M) -> __mmput(M)
        futex_hash_free(M)
          kvfree(fph)

                                     spin_lock(&hb->lock)  // UAF

Once M loses its last mm_users reference, futex_hash_free() in __mmput() frees the private hash with kvfree() unconditionally, ignoring the fph references that are still outstanding. When the owner then reaches spin_lock() with the stale hb, it writes into the freed queues[0].lock.

A private futex only has meaning inside the mm its key was taken from, so a task that does not share that mm cannot be its owner. 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(). The check is placed after the exit state check. Placing it before would return -ESRCH for a task whose current->mm has already been cleared by exit_mm(), instead of letting handle_exit_race() decide.

Fixes 80367ad01d93 ("futex: Add basic infrastructure for local task local hash")
---
 kernel/futex/pi.c | 7 +++++++
 1 file changed, 7 insertions(+)
kernel/futex/pi.c +7 -0
index 795011ea1202f1..d97611196b34ac 100644
@@ -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