Re: [PATCH] locking/lockdep: Zap lock classes even with lock debugging disabled

[Bart Van Assche]

On Wed, 2019-04-03 at 13:44 +-0100, Will Deacon wrote:
+AD4 On Tue, Mar 26, 2019 at 12:59:12PM -0700, Bart Van Assche wrote:
+AD4 +AD4 Commit a0b0fd53e1e6 (+ACI-locking/lockdep: Free lock classes that are no longer
+AD4 +AD4 in use+ACI) changed the behavior of lockdep+AF8-free+AF8-key+AF8-range() from
+AD4 +AD4 unconditionally zapping lock classes into only zapping lock classes if
+AD4 +AD4 debug+AF8-lock +AD0APQ true. Since the new behavior can cause cat /proc/lockdep to
+AD4 +AD4 crash due to a NULL pointer dereference, restore the pre-v5.1 behavior.
+AD4 
+AD4 Can you elaborate on this NULL dereference please, and why this patch fixes
+AD4 it?

Hi Will,

Not zapping lock classes if debug+AF8-lock +AD0APQ false leaves dangling pointers in
several lockdep datastructures, e.g. lock+AF8-class::name in the all+AF8-lock+AF8-classes
list. The shell command +ACI-cat /proc/lockdep+ACI causes the kernel to iterate the
all+AF8-lock+AF8-classes list. Hence the +ACI-unable to handle kernel paging request+ACI
issue that Shenghui encountered by running cat /proc/lockdep. Please let me
know if you would like me to repost this patch with a more detailed
description.

+AD4 +AD4 Cc: Thomas Gleixner +ADw-tglx+AEA-linutronix.de+AD4
+AD4 +AD4 Cc: Will Deacon +ADw-will.deacon+AEA-arm.com+AD4
+AD4 +AD4 Cc: Waiman Long +ADw-longman+AEA-redhat.com+AD4
+AD4 +AD4 Cc: shenghui +ADw-shhuiw+AEA-foxmail.com+AD4
+AD4 +AD4 Reported-by: shenghui +ADw-shhuiw+AEA-foxmail.com+AD4
+AD4 +AD4 Fixes: a0b0fd53e1e6 (+ACI-locking/lockdep: Free lock classes that are no longer in use+ACI) +ACM v5.1-rc1.
+AD4 +AD4 Signed-off-by: Bart Van Assche +ADw-bvanassche+AEA-acm.org+AD4
+AD4 +AD4 ---
+AD4 +AD4  kernel/locking/lockdep.c +AHw 23 +-+-+-+-+-+------------------
+AD4 +AD4  1 file changed, 6 insertions(+-), 17 deletions(-)
+AD4 +AD4 
+AD4 +AD4 diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
+AD4 +AD4 index 34cdcbedda49..70480e4f8f5d 100644
+AD4 +AD4 --- a/kernel/locking/lockdep.c
+AD4 +AD4 +-+-+- b/kernel/locking/lockdep.c
+AD4 +AD4 +AEAAQA -4689,8 +-4689,7 +AEAAQA static void free+AF8-zapped+AF8-rcu(struct rcu+AF8-head +ACo-ch)
+AD4 +AD4  		return+ADs
+AD4 +AD4  
+AD4 +AD4  	raw+AF8-local+AF8-irq+AF8-save(flags)+ADs
+AD4 +AD4 -	if (+ACE-graph+AF8-lock())
+AD4 +AD4 -		goto out+AF8-irq+ADs
+AD4 +AD4 +-	arch+AF8-spin+AF8-lock(+ACY-lockdep+AF8-lock)+ADs
+AD4 
+AD4 This also throws out the recursion counting. Is that ok?

I think that that's OK. My understanding is that lockdep keeps track of
recursion to avoid that lockdep+AF8-lock is locked recursively. However, none
of the functions modified by this patch are called with that lock held.

Thanks,

Bart.