linux-kernel - [PATCH] locking/mutex: fix the MUTEX_FLAG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Jun 2021 23:51:55 +0800
From:   Yanfei Xu <yanfei.xu@...driver.com>
To:     peterz@...radead.org, mingo@...hat.com, longman@...hat.com,
        boqun.feng@...il.com
Cc:     linux-kernel@...r.kernel.org
Subject: [PATCH] locking/mutex: fix the MUTEX_FLAG_HANDOFF bit is cleared unexpected

When the mutex unlock path is excuted with WAITERS bit and without
HANDOFF bit set, it will wake up the first task in wait_list. If
there are some tasks not in wait_list are stealing lock, it is very
likely successfully due to the task field of lock is NULL and flags
field is non-NULL. Then the HANDOFF bit will be cleared. But if the
HANDOFF bit was just set by the waked task in wait_list, this clearing
is unexcepted.

__mutex_lock_common                   __mutex_lock_common
  __mutex_trylock                       schedule_preempt_disabled
    /*steal lock successfully*/         __mutex_set_flag(lock,MUTEX_FLAG_HANDOFF)
    __mutex_trylock_or_owner
      if (task==NULL)
      flags &= ~MUTEX_FLAG_HANDOFF
      atomic_long_cmpxchg_acquire
                                        __mutex_trylock  //failed
                                        mutex_optimistic_spin  //failed
                                        schedule_preempt_disabled  //sleep without HANDOFF bit

So the HANDOFF bit should be set as late as possible, here we defer
it util the task is going to be scheduled.

Signed-off-by: Yanfei Xu <yanfei.xu@...driver.com>
---

Hi maintainers,

I am not very sure if I missed or misunderstanded something, please help
to review. Many thanks!

 kernel/locking/mutex.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 013e1b08a1bf..e57d920e96bf 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -1033,17 +1033,17 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 		}
 
 		spin_unlock(&lock->wait_lock);
+
+		if (first)
+			__mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
 		schedule_preempt_disabled();
 
 		/*
 		 * ww_mutex needs to always recheck its position since its waiter
 		 * list is not FIFO ordered.
 		 */
-		if (ww_ctx || !first) {
+		if (ww_ctx || !first)
 			first = __mutex_waiter_is_first(lock, &waiter);
-			if (first)
-				__mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
-		}
 
 		set_current_state(state);
 		/*
-- 
2.27.0