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

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, 4 Dec 2018 13:57:49 -0500
From:   Waiman Long <longman@...hat.com>
To:     Bart Van Assche <bvanassche@....org>, mingo@...hat.com
Cc:     peterz@...radead.org, tj@...nel.org, johannes.berg@...el.com,
        linux-kernel@...r.kernel.org,
        Johannes Berg <johannes@...solutions.net>
Subject: Re: [PATCH v2 16/24] locking/lockdep: Retain the class key and name
 while freeing a lock class

On 12/03/2018 07:28 PM, Bart Van Assche wrote:
> The next patch in this series uses the class name in code that
> detects lock class use-after-free. Hence retain the class name for
> lock classes that are being freed.
>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Waiman Long <longman@...hat.com>
> Cc: Johannes Berg <johannes@...solutions.net>
> Signed-off-by: Bart Van Assche <bvanassche@....org>
> ---
>  kernel/locking/lockdep.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index ecd92969674c..92bdb187987f 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -4147,10 +4147,8 @@ static void zap_class(struct lock_class *class)
>  	 * Unhash the class and remove it from the all_lock_classes list:
>  	 */
>  	hlist_del_rcu(&class->hash_entry);
> +	class->hash_entry.pprev = NULL;
>  	list_del(&class->lock_entry);
> -page 
> -	RCU_INIT_POINTER(class->key, NULL);
> -	RCU_INIT_POINTER(class->name, NULL);
>  }
>  
>  static inline int within(const void *addr, void *start, unsigned long size)

Using the name after module unload can be problematic if it points to
memory space occupied by the unloaded kernel module. The memory page may
be invalid with the potential of crashing the kernel. You may have to
duplicate the name if it is really from the module address space.

Cheers,
Longman