Re: [PATCH v2 16/24] locking/lockdep: Retain the class key and name while freeing a lock class

[Bart Van Assche]

On Tue, 2018-12-04 at 15:31 -0500, Waiman Long wrote:
+AD4 On 12/04/2018 02:08 PM, Bart Van Assche wrote:
+AD4 +AD4 On Tue, 2018-12-04 at 13:57 -0500, Waiman Long wrote:
+AD4 +AD4 +AD4 On 12/03/2018 07:28 PM, Bart Van Assche wrote:
+AD4 +AD4 +AD4 +AD4 The next patch in this series uses the class name in code that
+AD4 +AD4 +AD4 +AD4 detects lock class use-after-free. Hence retain the class name for
+AD4 +AD4 +AD4 +AD4 lock classes that are being freed.
+AD4 +AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 +AD4 Cc: Peter Zijlstra +ADw-peterz+AEA-infradead.org+AD4
+AD4 +AD4 +AD4 +AD4 Cc: Waiman Long +ADw-longman+AEA-redhat.com+AD4
+AD4 +AD4 +AD4 +AD4 Cc: Johannes Berg +ADw-johannes+AEA-sipsolutions.net+AD4
+AD4 +AD4 +AD4 +AD4 Signed-off-by: Bart Van Assche +ADw-bvanassche+AEA-acm.org+AD4
+AD4 +AD4 +AD4 +AD4 ---
+AD4 +AD4 +AD4 +AD4  kernel/locking/lockdep.c +AHw 4 +----
+AD4 +AD4 +AD4 +AD4  1 file changed, 1 insertion(+-), 3 deletions(-)
+AD4 +AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 +AD4 diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
+AD4 +AD4 +AD4 +AD4 index ecd92969674c..92bdb187987f 100644
+AD4 +AD4 +AD4 +AD4 --- a/kernel/locking/lockdep.c
+AD4 +AD4 +AD4 +AD4 +-+-+- b/kernel/locking/lockdep.c
+AD4 +AD4 +AD4 +AD4 +AEAAQA -4147,10 +-4147,8 +AEAAQA static void zap+AF8-class(struct lock+AF8-class +ACo-class)
+AD4 +AD4 +AD4 +AD4  	 +ACo Unhash the class and remove it from the all+AF8-lock+AF8-classes list:
+AD4 +AD4 +AD4 +AD4  	 +ACo-/
+AD4 +AD4 +AD4 +AD4  	hlist+AF8-del+AF8-rcu(+ACY-class-+AD4-hash+AF8-entry)+ADs
+AD4 +AD4 +AD4 +AD4 +-	class-+AD4-hash+AF8-entry.pprev +AD0 NULL+ADs
+AD4 +AD4 +AD4 +AD4  	list+AF8-del(+ACY-class-+AD4-lock+AF8-entry)+ADs
+AD4 +AD4 +AD4 +AD4 -page 
+AD4 +AD4 +AD4 +AD4 -	RCU+AF8-INIT+AF8-POINTER(class-+AD4-key, NULL)+ADs
+AD4 +AD4 +AD4 +AD4 -	RCU+AF8-INIT+AF8-POINTER(class-+AD4-name, NULL)+ADs
+AD4 +AD4 +AD4 +AD4  +AH0
+AD4 +AD4 +AD4 +AD4  
+AD4 +AD4 +AD4 +AD4  static inline int within(const void +ACo-addr, void +ACo-start, unsigned long size)
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 Using the name after module unload can be problematic if it points to
+AD4 +AD4 +AD4 memory space occupied by the unloaded kernel module. The memory page may
+AD4 +AD4 +AD4 be invalid with the potential of crashing the kernel. You may have to
+AD4 +AD4 +AD4 duplicate the name if it is really from the module address space.
+AD4 +AD4 
+AD4 +AD4 The class name shouldn't be used by any of the code I introduced in the
+AD4 +AD4 lockdep infrastructure after unloading a kernel module finished unless if
+AD4 +AD4 some of the kernel module code is still running after unloading the kernel
+AD4 +AD4 module finished. I think if that happens that the kernel module author is
+AD4 +AD4 to blame and not lockdep :-)
+AD4 +AD4 
+AD4 +AD4 Bart.
+AD4 
+AD4 You do use the name once in a WARN+AF8-ON check in check+AF8-prev+AF8-add(). You
+AD4 commit log did say the class name will be used.

Hi Waiman,

I will update the commit message and I will add a comment in check+AF8-prev+AF8-add()
that explains that the newly added warning may trigger a use-after-free.

Bart.