linux-kernel - [PATCH] locking/semaphore: Use raw_spin_trylock_irqsave() in down

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]

Message-ID: <20250120193608.2312690-1-longman@redhat.com>
Date: Mon, 20 Jan 2025 14:36:08 -0500
From: Waiman Long <longman@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Will Deacon <will.deacon@....com>,
	Boqun Feng <boqun.feng@...il.com>
Cc: linux-kernel@...r.kernel.org,
	Waiman Long <longman@...hat.com>
Subject: [PATCH] locking/semaphore: Use raw_spin_trylock_irqsave() in down_trylock()

A circular lock dependency splat has been seen with down_trylock().

[ 4011.795602] ======================================================
[ 4011.795603] WARNING: possible circular locking dependency detected
[ 4011.795607] 6.12.0-41.el10.s390x+debug
[ 4011.795612] ------------------------------------------------------
[ 4011.795613] dd/32479 is trying to acquire lock:
[ 4011.795617] 0015a20accd0d4f8 ((console_sem).lock){-.-.}-{2:2}, at: down_trylock+0x26/0x90
[ 4011.795636]
[ 4011.795636] but task is already holding lock:
[ 4011.795637] 000000017e461698 (&zone->lock){-.-.}-{2:2}, at: rmqueue_bulk+0xac/0x8f0
[ 4011.795644]
[ 4011.795644] which lock already depends on the new lock.
  :
[ 4011.796025]   (console_sem).lock --> hrtimer_bases.lock --> &zone->lock
[ 4011.796025]
[ 4011.796029]  Possible unsafe locking scenario:
[ 4011.796029]
[ 4011.796030]        CPU0
[ 4011.796031]        ----
[ 4011.796032]   lock(&zone->lock);
[ 4011.796034]                                lock(hrtimer_bases.lock);
[ 4011.796036]                                lock(&zone->lock);
[ 4011.796038]   lock((console_sem).lock);
[ 4011.796039]
[ 4011.796039]  *** DEADLOCK ***

The calling sequence was
  rmqueue_pcplist()
  => __rmqueue_pcplist()
    => rmqueue_bulk()
      => expand()
	=> __add_to_free_list()
	  => __warn_printk()
	    => ...
	      => console_trylock()
		=> __down_trylock_console_sem()
		  => down_trylock()
		    => _raw_spin_lock_irqsave()

Normally, a trylock call should avoid this kind of circular lock
dependency splat, but down_trylock() is an exception. Fix this problem
by using raw_spin_trylock_irqsave() in down_trylock() to make it behave
like the other trylock calls.

Signed-off-by: Waiman Long <longman@...hat.com>
---
 kernel/locking/semaphore.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
index 34bfae72f295..cb27cbf5162f 100644
--- a/kernel/locking/semaphore.c
+++ b/kernel/locking/semaphore.c
@@ -127,6 +127,7 @@ EXPORT_SYMBOL(down_killable);
  *
  * NOTE: This return value is inverted from both spin_trylock and
  * mutex_trylock!  Be careful about this when converting code.
+ * I.e. 0 on success, 1 on failure.
  *
  * Unlike mutex_trylock, this function can be used from interrupt context,
  * and the semaphore can be released by any task or interrupt.
@@ -136,7 +137,8 @@ int __sched down_trylock(struct semaphore *sem)
 	unsigned long flags;
 	int count;
 
-	raw_spin_lock_irqsave(&sem->lock, flags);
+	if (!raw_spin_trylock_irqsave(&sem->lock, flags))
+		return 1;
 	count = sem->count - 1;
 	if (likely(count >= 0))
 		sem->count = count;
-- 
2.47.1