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

[Bart Van Assche]

On Fri, 2018-12-07 at 09:50 -0500, Waiman Long wrote:
+AD4 On 12/07/2018 05:21 AM, Peter Zijlstra wrote:
+AD4 +AD4 On Thu, Dec 06, 2018 at 05:11:40PM -0800, Bart Van Assche wrote:
+AD4 +AD4 +AD4 The next patch in this series uses the class name in code that
+AD4 +AD4 +AD4 detects calls to lock+AF8-acquire() while a class key is being freed. Hence
+AD4 +AD4 +AD4 retain the class name for lock classes that are being freed.
+AD4 +AD4 
+AD4 +AD4 From readin the discussion with v2+ADs you meant to say: 'uses the class
+AD4 +AD4 name pointer', right? You're not actually ever going to dereference it,
+AD4 +AD4 since that would be a UaF.
+AD4 +AD4 
+AD4 +AD4 +AD4 Cc: Peter Zijlstra +ADw-peterz+AEA-infradead.org+AD4
+AD4 +AD4 +AD4 Cc: Waiman Long +ADw-longman+AEA-redhat.com+AD4
+AD4 +AD4 +AD4 Cc: Johannes Berg +ADw-johannes+AEA-sipsolutions.net+AD4
+AD4 +AD4 +AD4 Signed-off-by: Bart Van Assche +ADw-bvanassche+AEA-acm.org+AD4
+AD4 +AD4 +AD4 ---
+AD4 +AD4 +AD4  kernel/locking/lockdep.c +AHw 4 +----
+AD4 +AD4 +AD4  1 file changed, 1 insertion(+-), 3 deletions(-)
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
+AD4 +AD4 +AD4 index ecd92969674c..92bdb187987f 100644
+AD4 +AD4 +AD4 --- a/kernel/locking/lockdep.c
+AD4 +AD4 +AD4 +-+-+- b/kernel/locking/lockdep.c
+AD4 +AD4 +AD4 +AEAAQA -4147,10 +-4147,8 +AEAAQA static void zap+AF8-class(struct lock+AF8-class +ACo-class)
+AD4 +AD4 +AD4  	 +ACo Unhash the class and remove it from the all+AF8-lock+AF8-classes list:
+AD4 +AD4 +AD4  	 +ACo-/
+AD4 +AD4 +AD4  	hlist+AF8-del+AF8-rcu(+ACY-class-+AD4-hash+AF8-entry)+ADs
+AD4 +AD4 +AD4 +-	class-+AD4-hash+AF8-entry.pprev +AD0 NULL+ADs
+AD4 +AD4 +AD4  	list+AF8-del(+ACY-class-+AD4-lock+AF8-entry)+ADs
+AD4 +AD4 +AD4 -
+AD4 +AD4 +AD4 -	RCU+AF8-INIT+AF8-POINTER(class-+AD4-key, NULL)+ADs
+AD4 +AD4 +AD4 -	RCU+AF8-INIT+AF8-POINTER(class-+AD4-name, NULL)+ADs
+AD4 +AD4 +AD4  +AH0
+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 2.20.0.rc2.403.gdbc3b29805-goog
+AD4 +AD4 +AD4 
+AD4 
+AD4 I still prefer keeping the clearing statements. Leaving key behind
+AD4 should be OK as it is just a long value to be compared against. The
+AD4 variable name is a different story as it is a pointer that will
+AD4 reference bytes that it points to unless you save the name in a safe
+AD4 storage area and change the pointer to point there.

I will drop this patch and update later patches accordingly.

Bart.