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

From: Jerome Marchand
Date: Tue Jun 02 2015 - 10:15:47 EST


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