linux-kernel - RFC [PATCH 1/1] locking/lockdep: Fix nest lock warning on unlock

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]

Date:   Tue, 11 Dec 2018 18:25:06 -0800
From:   Derek Basehore <dbasehore@...omium.org>
To:     peterz@...radead.org
Cc:     mingo@...hat.com, will.deacon@....com,
        linux-kernel@...r.kernel.org,
        Derek Basehore <dbasehore@...omium.org>
Subject: RFC [PATCH 1/1] locking/lockdep: Fix nest lock warning on unlock

The function __lock_acquire checks that the nest lock is held passed
in as an argument. The issue with this is that __lock_acquire is used
for internal bookkeeping on lock_release. This produces a false
positive lockdep warning on unlock. Since you explicitly don't need to
hold the nest lock on unlock, this is an issue.

This fixes the problem by only checking the nest lock on the actual
lock acquire step.

Signed-off-by: Derek Basehore <dbasehore@...omium.org>
---
 kernel/locking/lockdep.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 1efada2dd9dd..2e7297ee6596 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3155,15 +3155,15 @@ EXPORT_SYMBOL_GPL(lockdep_init_map);
 struct lock_class_key __lockdep_no_validate__;
 EXPORT_SYMBOL_GPL(__lockdep_no_validate__);
 
-static int
-print_lock_nested_lock_not_held(struct task_struct *curr,
-				struct held_lock *hlock,
+static void
+print_lock_nested_lock_not_held(struct lockdep_map *lock,
+				struct lockdep_map *nest_lock,
 				unsigned long ip)
 {
 	if (!debug_locks_off())
-		return 0;
+		return;
 	if (debug_locks_silent)
-		return 0;
+		return;
 
 	pr_warn("\n");
 	pr_warn("==================================\n");
@@ -3171,22 +3171,21 @@ print_lock_nested_lock_not_held(struct task_struct *curr,
 	print_kernel_ident();
 	pr_warn("----------------------------------\n");
 
-	pr_warn("%s/%d is trying to lock:\n", curr->comm, task_pid_nr(curr));
-	print_lock(hlock);
+	pr_warn("%s/%d is trying to lock:\n", current->comm,
+			task_pid_nr(current));
+	pr_warn("%s\n", lock->name);
 
 	pr_warn("\nbut this task is not holding:\n");
-	pr_warn("%s\n", hlock->nest_lock->name);
+	pr_warn("%s\n", nest_lock->name);
 
 	pr_warn("\nstack backtrace:\n");
 	dump_stack();
 
 	pr_warn("\nother info that might help us debug this:\n");
-	lockdep_print_held_locks(curr);
+	lockdep_print_held_locks(current);
 
 	pr_warn("\nstack backtrace:\n");
 	dump_stack();
-
-	return 0;
 }
 
 static int __lock_is_held(const struct lockdep_map *lock, int read);
@@ -3335,9 +3334,6 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 	}
 	chain_key = iterate_chain_key(chain_key, class_idx);
 
-	if (nest_lock && !__lock_is_held(nest_lock, -1))
-		return print_lock_nested_lock_not_held(curr, hlock, ip);
-
 	if (!validate_chain(curr, lock, hlock, chain_head, chain_key))
 		return 0;
 
@@ -3843,6 +3839,9 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 	trace_lock_acquire(lock, subclass, trylock, read, check, nest_lock, ip);
 	__lock_acquire(lock, subclass, trylock, read, check,
 		       irqs_disabled_flags(flags), nest_lock, ip, 0, 0);
+	if (nest_lock && !__lock_is_held(nest_lock, -1))
+		print_lock_nested_lock_not_held(lock, nest_lock, ip);
+
 	current->lockdep_recursion = 0;
 	raw_local_irq_restore(flags);
 }
-- 
2.20.0.rc2.403.gdbc3b29805-goog