linux-kernel - [PATCH] RCU: don't turn off lockdep when find suspicious rcu_dereference

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: <4BCD646B.1080206@cn.fujitsu.com>
Date:	Tue, 20 Apr 2010 16:23:07 +0800
From:	Lai Jiangshan <laijs@...fujitsu.com>
To:	paulmck@...ux.vnet.ibm.com, Ingo Molnar <mingo@...e.hu>
CC:	Eric Paris <eparis@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Eric Paris <eparis@...isplace.org>,
	Miles Lane <miles.lane@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH] RCU: don't turn off lockdep when find suspicious rcu_dereference_check()
 usage

Paul E. McKenney wrote:
> On Mon, Apr 19, 2010 at 09:25:29PM -0400, Eric Paris wrote:
>> On Mon, 2010-04-19 at 16:01 -0700, Paul E. McKenney wrote:
>>
>>> Yep, different code path to the same location.  Does the following
>>> patch help?
>>>
>>> 							Thanx, Paul
>>>
>>> ------------------------------------------------------------------------
>>>
>>> commit 2836f18139267ea918ed2cf39023fb0eb38c4361
>>> Author: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
>>> Date:   Mon Apr 19 15:59:50 2010 -0700
>>>
>>>     rcu: fix RCU lockdep splat on freezer_fork path
>>>     
>>>     Add an RCU read-side critical section to suppress this false positive.
>>>     
>>>     Located-by: Eric Paris <eparis@...isplace.org>
>>>     Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
>> That one is also fixed so feel free to add a tested or something from
>> me.  But we've got another, weeeee!  If there some way I could get all
>> of these at once?

This patch fits your requirement.

> 
> Sure!  I -think- that if you remove the first "if" statement in
> lockdep_rcu_dereference() in kernel/lockdep.c, you will get lots of them
> all at once.  Maybe more than your console log is able to hold...
> 
> So another approach would be to print only the first 100 or some such.
> 
> It -looks- to me that you could make __debug_locks_off() atomically
> decrement a counter rather than just setting it to zero, see
> include/linux/debug_locks.h.  I suspect that atomic_dec_not_zero()
> would work very well for you here.
> 

[PATCH] RCU: don't turn off lockdep when find suspicious rcu_dereference_check() usage

When suspicious rcu_dereference_check() usage is detected, lockdep is still
available actually, so we should not call debug_locks_off() in
lockdep_rcu_dereference().

For get rid of too much "suspicious rcu_dereference_check() usage"
output when the "if(!debug_locks_off())" statement is removed. This patch uses
static variable '__warned's for very usage of "rcu_dereference*()".

One variable per usage, so, Now, we can get multiple complaint
when we detect multiple different suspicious rcu_dereference_check() usage.

Requested-by: Eric Paris <eparis@...hat.com>
Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com>
---
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 9f1ddfe..30b8d20 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -193,6 +193,15 @@ static inline int rcu_read_lock_sched_held(void)
 
 #ifdef CONFIG_PROVE_RCU
 
+#define __do_rcu_dereference_check(c)					\
+	do {								\
+		static bool __warned;					\
+		if (debug_lockdep_rcu_enabled() && !__warned && !(c)) {	\
+			__warned = true;				\
+			lockdep_rcu_dereference(__FILE__, __LINE__);	\
+		}							\
+	} while (0)
+
 /**
  * rcu_dereference_check - rcu_dereference with debug checking
  * @p: The pointer to read, prior to dereferencing
@@ -222,8 +231,7 @@ static inline int rcu_read_lock_sched_held(void)
  */
 #define rcu_dereference_check(p, c) \
 	({ \
-		if (debug_lockdep_rcu_enabled() && !(c)) \
-			lockdep_rcu_dereference(__FILE__, __LINE__); \
+		__do_rcu_dereference_check(c); \
 		rcu_dereference_raw(p); \
 	})
 
@@ -240,8 +248,7 @@ static inline int rcu_read_lock_sched_held(void)
  */
 #define rcu_dereference_protected(p, c) \
 	({ \
-		if (debug_lockdep_rcu_enabled() && !(c)) \
-			lockdep_rcu_dereference(__FILE__, __LINE__); \
+		__do_rcu_dereference_check(c); \
 		(p); \
 	})
 
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 78325f8..cc52ffe 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -3788,8 +3788,6 @@ void lockdep_rcu_dereference(const char *file, const int line)
 {
 	struct task_struct *curr = current;
 
-	if (!debug_locks_off())
-		return;
 	printk("\n===================================================\n");
 	printk(  "[ INFO: suspicious rcu_dereference_check() usage. ]\n");
 	printk(  "---------------------------------------------------\n");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/