Re: [PATCH] fix a race between /proc/lock_stat and module unloading

[Jerome Marchand]

On 06/02/2015 12:50 PM, Peter Zijlstra wrote:
> On Tue, Jun 02, 2015 at 11:54:13AM +0200, Jerome Marchand wrote:
>>
>> I guess I jumped to conclusion here and my explanation is wrong. However
>> there is still a bug which occurs when the kernel tries to access
>> class->name is seq_stats:
>>
>> [   43.533732] BUG: unable to handle kernel paging request at
>> ffffffffa03181ce
>> [   43.534006] IP: [<ffffffff8142b489>] strnlen+0x9/0x50
> 
> Does something like so cure things?

Yes, I can't reproduce the issue anymore.

> 
> ---
>  kernel/locking/lockdep.c      |  3 ++-
>  kernel/locking/lockdep_proc.c | 22 +++++++++++++++++-----
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index a0831e1b99f4..50ed2685f937 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -3900,7 +3900,8 @@ static void zap_class(struct lock_class *class)
>  	list_del_rcu(&class->hash_entry);
>  	list_del_rcu(&class->lock_entry);
>  
> -	class->key = NULL;
> +	WRITE_ONCE(class->key, NULL);
> +	WRITE_ONCE(class->name, NULL);
>  }
>  
>  static inline int within(const void *addr, void *start, unsigned long size)
> diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
> index ef43ac4bafb5..03946779eacc 100644
> --- a/kernel/locking/lockdep_proc.c
> +++ b/kernel/locking/lockdep_proc.c
> @@ -426,10 +426,12 @@ static void seq_lock_time(struct seq_file *m, struct lock_time *lt)
>  
>  static void seq_stats(struct seq_file *m, struct lock_stat_data *data)
>  {
> -	char name[39];
> -	struct lock_class *class;
> +	struct lockdep_subclass_key *ckey;
>  	struct lock_class_stats *stats;
> +	struct lock_class *class;
>  	int i, namelen;
> +	char name[39];
> +	char *cname;
>  
>  	class = data->class;
>  	stats = &data->stats;
> @@ -440,15 +442,25 @@ static void seq_stats(struct seq_file *m, struct lock_stat_data *data)
>  	if (class->subclass)
>  		namelen -= 2;
>  
> -	if (!class->name) {
> +	rcu_read_lock_sched();
> +	cname = rcu_dereference_sched(class->name);
> +	ckey  = rcu_dereference_sched(class->key);
> +
> +	if (!cname && !ckey) {
> +		rcu_read_unlock_sched();
> +		return;
> +
> +	} else if (!cname) {
>  		char str[KSYM_NAME_LEN];
>  		const char *key_name;
>  
> -		key_name = __get_key_name(class->key, str);
> +		key_name = __get_key_name(ckey, str);
>  		snprintf(name, namelen, "%s", key_name);
>  	} else {
> -		snprintf(name, namelen, "%s", class->name);
> +		snprintf(name, namelen, "%s", cname);
>  	}
> +	rcu_read_unlock_sched();
> +
>  	namelen = strlen(name);
>  	if (class->name_version > 1) {
>  		snprintf(name+namelen, 3, "#%d", class->name_version);
>

Attachment: signature.asc
Description: OpenPGP digital signature