Blame SOURCES/CVE-2021-3347.patch

738669
From 517d5c245c9805b56f73c7fa0e23e8853fe22da6 Mon Sep 17 00:00:00 2001
738669
From: Artem Savkov <asavkov@redhat.com>
738669
Date: Fri, 21 May 2021 14:20:32 +0200
738669
Subject: [RHEL7.9 KPATCH] CVE-2021-3347 Use after free via PI futex state
738669
738669
Kernels:
738669
3.10.0-1160.el7
738669
3.10.0-1160.2.1.el7
738669
3.10.0-1160.2.2.el7
738669
3.10.0-1160.6.1.el7
738669
3.10.0-1160.11.1.el7
738669
3.10.0-1160.15.2.el7
738669
3.10.0-1160.21.1.el7
738669
3.10.0-1160.24.1.el7
738669
3.10.0-1160.25.1.el7
738669
738669
Changes since last build:
738669
[x86_64]:
738669
futex.o: changed function: do_futex
738669
futex.o: changed function: fixup_owner
738669
futex.o: changed function: fixup_pi_state_owner.isra.16
738669
futex.o: changed function: free_pi_state
738669
futex.o: changed function: futex_lock_pi.isra.20
738669
futex.o: changed function: futex_wait_requeue_pi.constprop.22
738669
futex.o: new function: pi_state_update_owner
738669
738669
[ppc64le]:
738669
futex.o: changed function: do_futex
738669
futex.o: changed function: fixup_owner
738669
futex.o: changed function: fixup_pi_state_owner.isra.9
738669
futex.o: changed function: free_pi_state
738669
futex.o: changed function: futex_lock_pi.isra.16
738669
futex.o: changed function: futex_wait_requeue_pi.constprop.17
738669
futex.o: changed function: unqueue_me_pi
738669
futex.o: new function: pi_state_update_owner
738669
738669
---------------------------
738669
738669
Modifications: added -fno-optimize-sibling-calls to fixup_owner()
738669
738669
commit d2fb2a9cf682bdba4b66103fb079c13a04039430
738669
Author: Donghai Qiao <dqiao@redhat.com>
738669
Date:   Thu May 20 16:35:49 2021 -0400
738669
738669
    futex: Handle faults correctly for PI futexes
738669
738669
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1935108
738669
    Upstream status: 34b1a1ce1458f50ef27c54e28eb9b1947012907a
738669
    CVE: CVE-2021-3347
738669
738669
    Conflicts:
738669
    The original patch is intent to make the state of rtmutex and pi_state consistent
738669
    if the kernel is unable to update the user space futex word, rather than unlocking
738669
    the rtmutex and leaving pi_state out of synched. As a result, this original fix
738669
    removed part of the code which was introduced by 16ffa12d7 ("futex: Pull
738669
    rt_mutex_futex_unlock() out from under hb->lock") to the functions futex_lock_pi()
738669
    and futex_wait_requeue_pi() to avoid the inconsistency. So the conflicts are related
738669
    to the following two commits, though git blame displayed a much longer list which
738669
    shows the chain of dependency in the history.
738669
738669
    16ffa12d7425 ("futex: Pull rt_mutex_futex_unlock() out from under hb->lock")
738669
    c236c8e95a3d ("futex: Fix potential use-after-free in FUTEX_REQUEUE_PI")
738669
738669
    commit 34b1a1ce1458f50ef27c54e28eb9b1947012907a
738669
    Author: Thomas Gleixner <tglx@linutronix.de>
738669
    Date:   Mon, 18 Jan 2021 19:01:21 +0100
738669
738669
        futex: Handle faults correctly for PI futexes
738669
738669
        fixup_pi_state_owner() tries to ensure that the state of the rtmutex,
738669
        pi_state and the user space value related to the PI futex are consistent
738669
        before returning to user space. In case that the user space value update
738669
        faults and the fault cannot be resolved by faulting the page in via
738669
        fault_in_user_writeable() the function returns with -EFAULT and leaves
738669
        the rtmutex and pi_state owner state inconsistent.
738669
738669
        A subsequent futex_unlock_pi() operates on the inconsistent pi_state and
738669
        releases the rtmutex despite not owning it which can corrupt the RB tree of
738669
        the rtmutex and cause a subsequent kernel stack use after free.
738669
738669
        It was suggested to loop forever in fixup_pi_state_owner() if the fault
738669
        cannot be resolved, but that results in runaway tasks which is especially
738669
        undesired when the problem happens due to a programming error and not due
738669
        to malice.
738669
738669
        As the user space value cannot be fixed up, the proper solution is to make
738669
        the rtmutex and the pi_state consistent so both have the same owner. This
738669
        leaves the user space value out of sync. Any subsequent operation on the
738669
        futex will fail because the 10th rule of PI futexes (pi_state owner and
738669
        user space value are consistent) has been violated.
738669
738669
        As a consequence this removes the inept attempts of 'fixing' the situation
738669
        in case that the current task owns the rtmutex when returning with an
738669
        unresolvable fault by unlocking the rtmutex which left pi_state::owner and
738669
        rtmutex::owner out of sync in a different and only slightly less dangerous
738669
        way.
738669
738669
        Fixes: 1b7558e457ed ("futexes: fix fault handling in futex_lock_pi")
738669
        Reported-by: gzobqq@gmail.com
738669
        Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
738669
        Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
738669
        Cc: stable@vger.kernel.org
738669
738669
    Signed-off-by: Donghai Qiao <dqiao@redhat.com>
738669
738669
commit 25077b49b47c1cdf224b54c837172ff820e8be88
738669
Author: Donghai Qiao <dqiao@redhat.com>
738669
Date:   Thu May 20 16:30:16 2021 -0400
738669
738669
    futex: Provide and use pi_state_update_owner()
738669
738669
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1935108
738669
    Upstream status: c5cade200ab9a2a3be9e7f32a752c8d86b502ec7
738669
    CVE: CVE-2021-3347
738669
738669
    Conflicts:
738669
    Updating the owner of pi_state requires that we remove the pi_state structure from
738669
    the old owner's pi_state_list then add it to the new owner's pi_state_list. Because
738669
    this action takes place in multiple occassions in the current upstream futex.c, so
738669
    the similar code is duplicated in all these places. The purpose of this patch is to
738669
    eliminate these code duplications with a new routine pi_state_update_owner().
738669
738669
    The conflicts in 7.9.z are caused by the differences in places where updating owner
738669
    takes place. After sorting out the details, the relevant commit IDs as below :
738669
738669
    734009e96d19 ("futex: Change locking rules")
738669
    b4abf91047cf ("rtmutex: Make wait_lock irq safe")
738669
738669
    commit c5cade200ab9a2a3be9e7f32a752c8d86b502ec7
738669
    Author: Thomas Gleixner <tglx@linutronix.de>
738669
    Date:   Tue, 19 Jan 2021 15:21:35 +0100
738669
738669
        futex: Provide and use pi_state_update_owner()
738669
738669
        Updating pi_state::owner is done at several places with the same
738669
        code. Provide a function for it and use that at the obvious places.
738669
738669
        This is also a preparation for a bug fix to avoid yet another copy of the
738669
        same code or alternatively introducing a completely unpenetratable mess of
738669
        gotos.
738669
738669
        Originally-by: Peter Zijlstra <peterz@infradead.org>
738669
        Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
738669
        Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
738669
        Cc: stable@vger.kernel.org
738669
738669
    Signed-off-by: Donghai Qiao <dqiao@redhat.com>
738669
738669
commit 69414a50f8bad2063b89981110fb374733209d9d
738669
Author: Donghai Qiao <dqiao@redhat.com>
738669
Date:   Wed May 19 14:24:04 2021 -0400
738669
738669
    futex: Replace pointless printk in fixup_owner()
738669
738669
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1935108
738669
    Upstream status: 04b79c55201f02ffd675e1231d731365e335c307
738669
    CVE: CVE-2021-3347
738669
738669
    commit 04b79c55201f02ffd675e1231d731365e335c307
738669
    Author: Thomas Gleixner <tglx@linutronix.de>
738669
    Date:   Tue, 19 Jan 2021 16:06:10 +0100
738669
738669
        futex: Replace pointless printk in fixup_owner()
738669
738669
        If that unexpected case of inconsistent arguments ever happens then the
738669
        futex state is left completely inconsistent and the printk is not really
738669
        helpful. Replace it with a warning and make the state consistent.
738669
738669
        Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
738669
        Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
738669
        Cc: stable@vger.kernel.org
738669
738669
    Signed-off-by: Donghai Qiao <dqiao@redhat.com>
738669
738669
commit 7e96fb06469c95628039ead2591f82e88af5da10
738669
Author: Donghai Qiao <dqiao@redhat.com>
738669
Date:   Wed May 19 14:19:05 2021 -0400
738669
738669
    futex: Ensure the correct return value from futex_lock_pi()
738669
738669
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1935108
738669
    Upstream status: 12bb3f7f1b03d5913b3f9d4236a488aa7774dfe9
738669
    CVE: CVE-2021-3347
738669
738669
    Conflicts:
738669
    This original upstream patch relies heavily on c1e2f0eaf015 ("futex: Avoid
738669
    violating the 10th rule of futex") which is one of the upstream commits listed
738669
    below. But the backport for c1e2f0eaf015 requires we resolve very complex chain
738669
    of dependencies across multiple critical kernel source files therefore the risk
738669
    is considered too high for 7.9.z.
738669
738669
    Instead of pulling together tons of the relevant commits in to 7.9.z, we just
738669
    want to take a light risk approach by digesting the fix 12bb3f7f1b03 ("futex:
738669
    Ensure the correct return value from futex_lock_pi()") for 7.9.z. All we need
738669
    to do is to make the changed functions fixup_owner() and fixup_pi_state_owner()
738669
    of 7.9.z return the required values as this upstream fix suggests in every
738669
    circumstance. This way, we can cleanly cut this CVE patch set with merely four
738669
    patches, without having to backport tons of patches in the chain of dependency.
738669
738669
    Besides, an extra change made to fixup_owner() (see HUNK -2063,13 +2062,11 in
738669
    this backport patch) is to eliminate a mistake made by upstream, where the
738669
    specification of a local variable "ret" was removed from that function, but
738669
    there was still a dereference to "ret" as shown by that HUNK.
738669
738669
    16ffa12d7425 ("futex: Pull rt_mutex_futex_unlock() out from under hb->lock")
738669
    c1e2f0eaf015 ("futex: Avoid violating the 10th rule of futex")
738669
    734009e96d19 ("futex: Change locking rules")
738669
    d7c5ed73b19c ("futex: Remove needless goto's")
738669
    6b4f4bc9cb22 ("locking/futex: Allow low-level atomic operations to return -EAGAIN")
738669
738669
    commit 12bb3f7f1b03d5913b3f9d4236a488aa7774dfe9
738669
    Author: Thomas Gleixner <tglx@linutronix.de>
738669
    Date:   Wed, 20 Jan 2021 16:00:24 +0100
738669
738669
        futex: Ensure the correct return value from futex_lock_pi()
738669
738669
        In case that futex_lock_pi() was aborted by a signal or a timeout and the
738669
        task returned without acquiring the rtmutex, but is the designated owner of
738669
        the futex due to a concurrent futex_unlock_pi() fixup_owner() is invoked to
738669
        establish consistent state. In that case it invokes fixup_pi_state_owner()
738669
        which in turn tries to acquire the rtmutex again. If that succeeds then it
738669
        does not propagate this success to fixup_owner() and futex_lock_pi()
738669
        returns -EINTR or -ETIMEOUT despite having the futex locked.
738669
738669
        Return success from fixup_pi_state_owner() in all cases where the current
738669
        task owns the rtmutex and therefore the futex and propagate it correctly
738669
        through fixup_owner(). Fixup the other callsite which does not expect a
738669
        positive return value.
738669
738669
        Fixes: c1e2f0eaf015 ("futex: Avoid violating the 10th rule of futex")
738669
        Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
738669
        Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
738669
        Cc: stable@vger.kernel.org
738669
738669
    Signed-off-by: Donghai Qiao <dqiao@redhat.com>
738669
738669
Signed-off-by: Artem Savkov <asavkov@redhat.com>
738669
Acked-by: Joe Lawrence <joe.lawrence@redhat.com>
738669
Acked-by: Yannick Cote <ycote@redhat.com>
738669
---
738669
 kernel/futex.c | 123 +++++++++++++++++++++++++------------------------
738669
 1 file changed, 63 insertions(+), 60 deletions(-)
738669
738669
diff --git a/kernel/futex.c b/kernel/futex.c
738669
index 877831775d7aa..8ec57c357ca58 100644
738669
--- a/kernel/futex.c
738669
+++ b/kernel/futex.c
738669
@@ -640,6 +640,29 @@ static struct futex_pi_state * alloc_pi_state(void)
738669
 	return pi_state;
738669
 }
738669
 
738669
+static void pi_state_update_owner(struct futex_pi_state *pi_state,
738669
+				  struct task_struct *new_owner)
738669
+{
738669
+	struct task_struct *old_owner = pi_state->owner;
738669
+
738669
+	lockdep_assert_held(&pi_state->pi_mutex.wait_lock);
738669
+
738669
+	if (old_owner) {
738669
+		raw_spin_lock_irq(&old_owner->pi_lock);
738669
+		WARN_ON(list_empty(&pi_state->list));
738669
+		list_del_init(&pi_state->list);
738669
+		raw_spin_unlock_irq(&old_owner->pi_lock);
738669
+	}
738669
+
738669
+	if (new_owner) {
738669
+		raw_spin_lock_irq(&new_owner->pi_lock);
738669
+		WARN_ON(!list_empty(&pi_state->list));
738669
+		list_add(&pi_state->list, &new_owner->pi_state_list);
738669
+		pi_state->owner = new_owner;
738669
+		raw_spin_unlock_irq(&new_owner->pi_lock);
738669
+	}
738669
+}
738669
+
738669
 static void free_pi_state(struct futex_pi_state *pi_state)
738669
 {
738669
 	if (!atomic_dec_and_test(&pi_state->refcount))
738669
@@ -650,10 +673,7 @@ static void free_pi_state(struct futex_pi_state *pi_state)
738669
 	 * and has cleaned up the pi_state already
738669
 	 */
738669
 	if (pi_state->owner) {
738669
-		raw_spin_lock_irq(&pi_state->owner->pi_lock);
738669
-		list_del_init(&pi_state->list);
738669
-		raw_spin_unlock_irq(&pi_state->owner->pi_lock);
738669
-
738669
+		pi_state_update_owner(pi_state, NULL);
738669
 		rt_mutex_proxy_unlock(&pi_state->pi_mutex, pi_state->owner);
738669
 	}
738669
 
738669
@@ -791,7 +811,8 @@ void exit_pi_state_list(struct task_struct *curr)
738669
  *	FUTEX_OWNER_DIED bit. See [4]
738669
  *
738669
  * [10] There is no transient state which leaves owner and user space
738669
- *	TID out of sync.
738669
+ *	TID out of sync. Except one error case where the kernel is denied
738669
+ *	write access to the user address, see fixup_pi_state_owner().
738669
  */
738669
 static int
738669
 lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
738669
@@ -1168,16 +1189,7 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this)
738669
 		return ret;
738669
 	}
738669
 
738669
-	raw_spin_lock_irq(&pi_state->owner->pi_lock);
738669
-	WARN_ON(list_empty(&pi_state->list));
738669
-	list_del_init(&pi_state->list);
738669
-	raw_spin_unlock_irq(&pi_state->owner->pi_lock);
738669
-
738669
-	raw_spin_lock_irq(&new_owner->pi_lock);
738669
-	WARN_ON(!list_empty(&pi_state->list));
738669
-	list_add(&pi_state->list, &new_owner->pi_state_list);
738669
-	pi_state->owner = new_owner;
738669
-	raw_spin_unlock_irq(&new_owner->pi_lock);
738669
+	pi_state_update_owner(pi_state, new_owner);
738669
 
738669
 	raw_spin_unlock(&pi_state->pi_mutex.wait_lock);
738669
 	rt_mutex_unlock(&pi_state->pi_mutex);
738669
@@ -1953,20 +1965,9 @@ retry:
738669
 	 * We fixed up user space. Now we need to fix the pi_state
738669
 	 * itself.
738669
 	 */
738669
-	if (pi_state->owner != NULL) {
738669
-		raw_spin_lock_irq(&pi_state->owner->pi_lock);
738669
-		WARN_ON(list_empty(&pi_state->list));
738669
-		list_del_init(&pi_state->list);
738669
-		raw_spin_unlock_irq(&pi_state->owner->pi_lock);
738669
-	}
738669
+	pi_state_update_owner(pi_state, newowner);
738669
 
738669
-	pi_state->owner = newowner;
738669
-
738669
-	raw_spin_lock_irq(&newowner->pi_lock);
738669
-	WARN_ON(!list_empty(&pi_state->list));
738669
-	list_add(&pi_state->list, &newowner->pi_state_list);
738669
-	raw_spin_unlock_irq(&newowner->pi_lock);
738669
-	return 0;
738669
+	return newowner == current;
738669
 
738669
 	/*
738669
 	 * To handle the page fault we need to drop the hash bucket
738669
@@ -1989,10 +1990,26 @@ handle_fault:
738669
 	 * Check if someone else fixed it for us:
738669
 	 */
738669
 	if (pi_state->owner != oldowner)
738669
-		return 0;
738669
+		return newowner == current;
738669
+
738669
+	if (ret) {
738669
+		/*
738669
+		 * fault_in_user_writeable() failed so user state is immutable. At
738669
+		 * best we can make the kernel state consistent but user state will
738669
+		 * be most likely hosed and any subsequent unlock operation will be
738669
+		 * rejected due to PI futex rule [10].
738669
+		 *
738669
+		 * Ensure that the rtmutex owner is also the pi_state owner despite
738669
+		 * the user space value claiming something different. There is no
738669
+		 * point in unlocking the rtmutex if current is the owner as it
738669
+		 * would need to wait until the next waiter has taken the rtmutex
738669
+		 * to guarantee consistent state. Keep it simple. Userspace asked
738669
+		 * for this wreckaged state.
738669
+		 */
738669
+		pi_state_update_owner(pi_state, rt_mutex_owner(&pi_state->pi_mutex));
738669
 
738669
-	if (ret)
738669
 		return ret;
738669
+	}
738669
 
738669
 	goto retry;
738669
 }
738669
@@ -2014,10 +2031,10 @@ static long futex_wait_restart(struct restart_block *restart);
738669
  *  0 - success, lock not taken;
738669
  * <0 - on error (-EFAULT)
738669
  */
738669
+__attribute__((optimize("-fno-optimize-sibling-calls")))
738669
 static int fixup_owner(u32 __user *uaddr, struct futex_q *q, int locked)
738669
 {
738669
 	struct task_struct *owner;
738669
-	int ret = 0;
738669
 
738669
 	if (locked) {
738669
 		/*
738669
@@ -2025,8 +2042,8 @@ static int fixup_owner(u32 __user *uaddr, struct futex_q *q, int locked)
738669
 		 * did a lock-steal - fix up the PI-state in that case:
738669
 		 */
738669
 		if (q->pi_state->owner != current)
738669
-			ret = fixup_pi_state_owner(uaddr, q, current);
738669
-		goto out;
738669
+			return fixup_pi_state_owner(uaddr, q, current);
738669
+		return 1;
738669
 	}
738669
 
738669
 	/*
738669
@@ -2040,8 +2057,7 @@ static int fixup_owner(u32 __user *uaddr, struct futex_q *q, int locked)
738669
 		 * rt_mutex waiters list.
738669
 		 */
738669
 		if (rt_mutex_trylock(&q->pi_state->pi_mutex)) {
738669
-			locked = 1;
738669
-			goto out;
738669
+			return 1;
738669
 		}
738669
 
738669
 		/*
738669
@@ -2054,22 +2070,18 @@ static int fixup_owner(u32 __user *uaddr, struct futex_q *q, int locked)
738669
 		if (!owner)
738669
 			owner = rt_mutex_next_owner(&q->pi_state->pi_mutex);
738669
 		raw_spin_unlock(&q->pi_state->pi_mutex.wait_lock);
738669
-		ret = fixup_pi_state_owner(uaddr, q, owner);
738669
-		goto out;
738669
+
738669
+		return fixup_pi_state_owner(uaddr, q, owner);
738669
 	}
738669
 
738669
 	/*
738669
 	 * Paranoia check. If we did not take the lock, then we should not be
738669
-	 * the owner of the rt_mutex.
738669
+	 * the owner of the rt_mutex. Warn and establish consistent state.
738669
 	 */
738669
-	if (rt_mutex_owner(&q->pi_state->pi_mutex) == current)
738669
-		printk(KERN_ERR "fixup_owner: ret = %d pi-mutex: %p "
738669
-				"pi-state %p\n", ret,
738669
-				q->pi_state->pi_mutex.owner,
738669
-				q->pi_state->owner);
738669
+	if (WARN_ON_ONCE(rt_mutex_owner(&q->pi_state->pi_mutex) == current))
738669
+		return fixup_pi_state_owner(uaddr, q, current);
738669
 
738669
-out:
738669
-	return ret ? ret : locked;
738669
+	return 0;
738669
 }
738669
 
738669
 /**
738669
@@ -2363,13 +2375,6 @@ retry_private:
738669
 	if (res)
738669
 		ret = (res < 0) ? res : 0;
738669
 
738669
-	/*
738669
-	 * If fixup_owner() faulted and was unable to handle the fault, unlock
738669
-	 * it and return the fault to userspace.
738669
-	 */
738669
-	if (ret && (rt_mutex_owner(&q.pi_state->pi_mutex) == current))
738669
-		rt_mutex_unlock(&q.pi_state->pi_mutex);
738669
-
738669
 	/* Unqueue and drop the lock */
738669
 	unqueue_me_pi(&q);
738669
 
738669
@@ -2666,6 +2671,11 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
738669
 			spin_lock(q.lock_ptr);
738669
 			ret = fixup_pi_state_owner(uaddr2, &q, current);
738669
 			spin_unlock(q.lock_ptr);
738669
+			/*
738669
+			 * Adjust the return value. It's either -EFAULT or
738669
+			 * success (1) but the caller expects 0 for success.
738669
+			 */
738669
+			ret = ret < 0 ? ret : 0;
738669
 		}
738669
 	} else {
738669
 		/*
738669
@@ -2695,14 +2705,7 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
738669
 		unqueue_me_pi(&q);
738669
 	}
738669
 
738669
-	/*
738669
-	 * If fixup_pi_state_owner() faulted and was unable to handle the
738669
-	 * fault, unlock the rt_mutex and return the fault to userspace.
738669
-	 */
738669
-	if (ret == -EFAULT) {
738669
-		if (pi_mutex && rt_mutex_owner(pi_mutex) == current)
738669
-			rt_mutex_unlock(pi_mutex);
738669
-	} else if (ret == -EINTR) {
738669
+	if (ret == -EINTR) {
738669
 		/*
738669
 		 * We've already been requeued, but cannot restart by calling
738669
 		 * futex_lock_pi() directly. We could restart this syscall, but
738669
-- 
738669
2.26.3
738669