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

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

[Hyunwoo Kim]

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")
Cc: stable@kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 kernel/futex/pi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/futex/pi.c b/kernel/futex/pi.c
index 795011ea1202f1..d97611196b34ac 100644
--- a/kernel/futex/pi.c
+++ b/kernel/futex/pi.c
@@ -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);
 
-- 
2.43.0