linux-kernel - [RFC lockdep 2/4] lockdep: Allow wait context checking with empty ->held

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20201208103112.2838119-3-boqun.feng@gmail.com>
Date:   Tue,  8 Dec 2020 18:31:10 +0800
From:   Boqun Feng <boqun.feng@...il.com>
To:     linux-kernel@...r.kernel.org, rcu@...r.kernel.org
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Josh Triplett <josh@...htriplett.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Uladzislau Rezki <urezki@...il.com>,
        Boqun Feng <boqun.feng@...il.com>
Subject: [RFC lockdep 2/4] lockdep: Allow wait context checking with empty ->held_locks

Currently, the guard for !curr->lockdep_depth in check_wait_context() is
unnecessary, because the code will work well without it. Moreover, there
are cases that we will miss if we skip for curr->lockdep_depth == 0. For
example:

	<in hardirq context>
	some_irq_handler():
	  // curr->lockdep_depth == 0
	  mutex_lock(&some_mutex):
	    check_wait_context() // skip the check!

Clearly, it's a bug, but due to the skip for !curr->lockdep_depth, we
cannot detect it in check_wait_context().

Therefore, remove the !curr->lockdep_depth checks and add comments on
why it's still working without it. The idea is that if we currently
don't hold any lock, then the current context is the only one we should
use to check.

Signed-off-by: Boqun Feng <boqun.feng@...il.com>
---
 kernel/locking/lockdep.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index c1418b47f625..d4fd52b22804 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4508,7 +4508,7 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next)
 	short curr_inner;
 	int depth;
 
-	if (!curr->lockdep_depth || !next_inner || next->trylock)
+	if (!next_inner || next->trylock)
 		return 0;
 
 	if (!next_outer)
@@ -4516,6 +4516,10 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next)
 
 	/*
 	 * Find start of current irq_context..
+	 *
+	 * Note: if curr->lockdep_depth == 0, we have depth == 0 after the
+	 * "depth++" below, and will skip the second for loop, i.e. using
+	 * the current task context as the curr_inner.
 	 */
 	for (depth = curr->lockdep_depth - 1; depth >= 0; depth--) {
 		struct held_lock *prev = curr->held_locks + depth;
-- 
2.29.2