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

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

[Jerome Marchand]

When opening /proc/lock_stat, lock_stat_open() makes a copy of
all_lock_classes list in the form of an array of ad hoc structures
lock_stat_data that reference lock_class, so it can be sorted and
passed to seq_read(). However, nothing prevents module unloading code
to free some of these lock_class structures before seq_read() tries to
access them.

Copying the all lock_class structures instead of just their pointers
would be an easy fix, but it seems quite wasteful. This patch copies
only the needed data into the lock_stat_data structure.

Signed-off-by: Jerome Marchand <jmarchan@redhat.com>
---
 kernel/locking/lockdep_proc.c | 88 ++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index ef43ac4..c2eb8e8 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -363,7 +363,9 @@ static const struct file_operations proc_lockdep_stats_operations = {
 #ifdef CONFIG_LOCK_STAT
 
 struct lock_stat_data {
-	struct lock_class *class;
+	char name[39];
+	unsigned long contention_point[LOCKSTAT_POINTS];
+	unsigned long contending_point[LOCKSTAT_POINTS];
 	struct lock_class_stats stats;
 };
 
@@ -426,39 +428,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;
+	char *name = data->name;
 	struct lock_class_stats *stats;
-	int i, namelen;
+	int i, namelen = strlen(data->name);
 
-	class = data->class;
 	stats = &data->stats;
 
-	namelen = 38;
-	if (class->name_version > 1)
-		namelen -= 2; /* XXX truncates versions > 9 */
-	if (class->subclass)
-		namelen -= 2;
-
-	if (!class->name) {
-		char str[KSYM_NAME_LEN];
-		const char *key_name;
-
-		key_name = __get_key_name(class->key, str);
-		snprintf(name, namelen, "%s", key_name);
-	} else {
-		snprintf(name, namelen, "%s", class->name);
-	}
-	namelen = strlen(name);
-	if (class->name_version > 1) {
-		snprintf(name+namelen, 3, "#%d", class->name_version);
-		namelen += 2;
-	}
-	if (class->subclass) {
-		snprintf(name+namelen, 3, "/%d", class->subclass);
-		namelen += 2;
-	}
-
 	if (stats->write_holdtime.nr) {
 		if (stats->read_holdtime.nr)
 			seq_printf(m, "%38s-W:", name);
@@ -490,32 +465,32 @@ static void seq_stats(struct seq_file *m, struct lock_stat_data *data)
 	for (i = 0; i < LOCKSTAT_POINTS; i++) {
 		char ip[32];
 
-		if (class->contention_point[i] == 0)
+		if (data->contention_point[i] == 0)
 			break;
 
 		if (!i)
 			seq_line(m, '-', 40-namelen, namelen);
 
 		snprintf(ip, sizeof(ip), "[<%p>]",
-				(void *)class->contention_point[i]);
+				(void *)data->contention_point[i]);
 		seq_printf(m, "%40s %14lu %29s %pS\n",
 			   name, stats->contention_point[i],
-			   ip, (void *)class->contention_point[i]);
+			   ip, (void *)data->contention_point[i]);
 	}
 	for (i = 0; i < LOCKSTAT_POINTS; i++) {
 		char ip[32];
 
-		if (class->contending_point[i] == 0)
+		if (data->contending_point[i] == 0)
 			break;
 
 		if (!i)
 			seq_line(m, '-', 40-namelen, namelen);
 
 		snprintf(ip, sizeof(ip), "[<%p>]",
-				(void *)class->contending_point[i]);
+				(void *)data->contending_point[i]);
 		seq_printf(m, "%40s %14lu %29s %pS\n",
 			   name, stats->contending_point[i],
-			   ip, (void *)class->contending_point[i]);
+			   ip, (void *)data->contending_point[i]);
 	}
 	if (i) {
 		seq_puts(m, "\n");
@@ -593,6 +568,44 @@ static const struct seq_operations lockstat_ops = {
 	.show	= ls_show,
 };
 
+static void copy_lock_class(struct lock_stat_data *data,
+			    struct lock_class *class)
+{
+	char *name = data->name;
+	int namelen = 38;
+
+	if (class->name_version > 1)
+		namelen -= 2; /* XXX truncates versions > 9 */
+	if (class->subclass)
+		namelen -= 2;
+
+	if (!class->name) {
+		char str[KSYM_NAME_LEN];
+		const char *key_name;
+
+		key_name = __get_key_name(class->key, str);
+		snprintf(name, namelen, "%s", key_name);
+	} else {
+		snprintf(name, namelen, "%s", class->name);
+	}
+	namelen = strlen(name);
+	if (class->name_version > 1) {
+		snprintf(name+namelen, 3, "#%d", class->name_version);
+		namelen += 2;
+	}
+	if (class->subclass) {
+		snprintf(name+namelen, 3, "/%d", class->subclass);
+		namelen += 2;
+	}
+
+	memcpy(data->contention_point, class->contention_point,
+	       sizeof(class->contention_point));
+	memcpy(data->contending_point, class->contending_point,
+	       sizeof(class->contending_point));
+
+	data->stats = lock_stats(class);
+}
+
 static int lock_stat_open(struct inode *inode, struct file *file)
 {
 	int res;
@@ -608,8 +621,7 @@ static int lock_stat_open(struct inode *inode, struct file *file)
 		struct seq_file *m = file->private_data;
 
 		list_for_each_entry(class, &all_lock_classes, lock_entry) {
-			iter->class = class;
-			iter->stats = lock_stats(class);
+			copy_lock_class(iter, class);
 			iter++;
 		}
 		data->iter_end = iter;
-- 
1.9.3

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

[Peter Zijlstra]

On Fri, May 29, 2015 at 02:47:15PM +0200, Jerome Marchand wrote:
> When opening /proc/lock_stat, lock_stat_open() makes a copy of
> all_lock_classes list in the form of an array of ad hoc structures
> lock_stat_data that reference lock_class, so it can be sorted and
> passed to seq_read(). However, nothing prevents module unloading code
> to free some of these lock_class structures before seq_read() tries to
> access them.

Well, how about lock_class being from a static array in lockdep.c:138 ?

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

[Jerome Marchand]

[-- Attachment #1: Type: text/plain, Size: 5066 bytes --]

On 06/02/2015 11:30 AM, Peter Zijlstra wrote:
> On Fri, May 29, 2015 at 02:47:15PM +0200, Jerome Marchand wrote:
>> When opening /proc/lock_stat, lock_stat_open() makes a copy of
>> all_lock_classes list in the form of an array of ad hoc structures
>> lock_stat_data that reference lock_class, so it can be sorted and
>> passed to seq_read(). However, nothing prevents module unloading code
>> to free some of these lock_class structures before seq_read() tries to
>> access them.
> 
> Well, how about lock_class being from a static array in lockdep.c:138 ?
> 
> 

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
[   43.534006] PGD 1e14067 PUD 1e15063 PMD 79153067 PTE 0
[   43.534006] Oops: 0000 [#1] SMP
[   43.534006] Modules linked in: ip6t_rpfilter ip6t_REJECT
nf_reject_ipv6 xt_conntrack ebtable_nat ebtable_broute bridge stp llc
ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6
nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw
ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4
nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security
iptable_raw ppdev iosf_mbi crct10dif_pclmul crc32_pclmul crc32c_intel
ghash_clmulni_intel serio_raw virtio_balloon virtio_console parport_pc
parport pvpanic i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc
virtio_blk virtio_net qxl drm_kms_helper ttm drm virtio_pci virtio_ring
virtio ata_generic pata_acpi [last unloaded: zram]
[   43.534006] CPU: 0 PID: 2125 Comm: cat Not tainted
3.19.4-200.fc21.x86_64+debug #1
[   43.534006] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   43.534006] task: ffff88007a468000 ti: ffff88007a374000 task.ti:
ffff88007a374000
[   43.534006] RIP: 0010:[<ffffffff8142b489>]  [<ffffffff8142b489>]
strnlen+0x9/0x50
[   43.534006] RSP: 0018:ffff88007a377ba8  EFLAGS: 00010286
[   43.534006] RAX: ffffffff81c7ac25 RBX: ffff88007a377ce9 RCX:
0000000000000000
[   43.534006] RDX: ffffffffa03181ce RSI: ffffffffffffffff RDI:
ffffffffa03181ce
[   43.534006] RBP: ffff88007a377ba8 R08: 000000000000ffff R09:
000000000000ffff
[   43.534006] R10: 0000000000000000 R11: 0000000000000000 R12:
ffffffffa03181ce
[   43.534006] R13: ffff88007a377d0f R14: 00000000ffffffff R15:
0000000000000000
[   43.534006] FS:  00007f6e132f3700(0000) GS:ffff88007d200000(0000)
knlGS:0000000000000000
[   43.534006] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   43.534006] CR2: ffffffffa03181ce CR3: 000000007b777000 CR4:
00000000001406f0
[   43.534006] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[   43.534006] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[   43.534006] Stack:
[   43.534006]  ffff88007a377be8 ffffffff8142d92f ffff88007a377c81
ffff88007a377ce9
[   43.534006]  ffff88007a377d0f ffff88007a377c78 ffffffff81cd0158
ffffffff81cd0158
[   43.534006]  ffff88007a377c68 ffffffff8142f0d9 ffff88007a377ce9
ffff88007b128800
[   43.534006] Call Trace:
[   43.534006]  [<ffffffff8142d92f>] string.isra.7+0x3f/0xf0
[   43.534006]  [<ffffffff8142f0d9>] vsnprintf+0x199/0x5b0
[   43.534006]  [<ffffffff812a329c>] ? seq_printf+0x4c/0x70
[   43.534006]  [<ffffffff8142f593>] snprintf+0x43/0x60
[   43.534006]  [<ffffffff812a33c8>] ? seq_puts+0x48/0x70
[   43.534006]  [<ffffffff8111278c>] seq_stats+0x7c/0x520
[   43.534006]  [<ffffffff8110db4c>] ? mark_held_locks+0x7c/0xb0
[   43.534006]  [<ffffffff8187486c>] ? mutex_lock_nested+0x28c/0x440
[   43.534006]  [<ffffffff8110dcbd>] ? trace_hardirqs_on_caller+0x13d/0x1e0
[   43.534006]  [<ffffffff81112c47>] ls_show+0x17/0x120
[   43.534006]  [<ffffffff812c1822>] ? fsnotify+0x462/0x820
[   43.534006]  [<ffffffff812c1458>] ? fsnotify+0x98/0x820
[   43.534006]  [<ffffffff812a2cf6>] seq_read+0x316/0x400
[   43.534006]  [<ffffffff812f06a8>] proc_reg_read+0x48/0x70
[   43.534006]  [<ffffffff81277378>] __vfs_read+0x18/0x50
[   43.534006]  [<ffffffff8127743d>] vfs_read+0x8d/0x150
[   43.534006]  [<ffffffff8127755c>] SyS_read+0x5c/0xd0
[   43.534006]  [<ffffffff81879389>] system_call_fastpath+0x12/0x17
[   43.534006] Code: 40 00 48 83 c0 01 80 38 00 75 f7 48 29 f8 5d c3 31
c0 5d c3 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 85 f6 48 89 e5
74 3f <80> 3f 00 74 3a 48 8d 47 01 48 01 fe eb 13 66 0f 1f 84 00 00 00
[   43.534006] RIP  [<ffffffff8142b489>] strnlen+0x9/0x50
[   43.534006]  RSP <ffff88007a377ba8>
[   43.534006] CR2: ffffffffa03181ce
[   43.534006] ---[ end trace 609a4a4bd210562d ]---

So I guess it's actually just class->name that get freed underneath us.
The following script easily triggers the bug unless my patch is applied:

#! /bin/sh

while true; do
    modprobe zram;
    modprobe -r zram;
done &

while true; do
    cat /proc/lock_stat > /dev/null ;
done




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Peter Zijlstra]

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?

---
 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);

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

[Jerome Marchand]

[-- Attachment #1: Type: text/plain, Size: 2782 bytes --]

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Peter Zijlstra]

On Tue, Jun 02, 2015 at 04:15:24PM +0200, Jerome Marchand wrote:
> Yes, I can't reproduce the issue anymore.

Great; queued the below. Slight changes

 - s/WRITE_ONCE/RCU_INIT_POINTER/ which should be similar but more descriptive
 - const char *cname to avoid a compile warn.

---
Subject: lockdep: Fix a race between /proc/lock_stat and module unload
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 2 Jun 2015 12:50:13 +0200

The lock_class iteration of /proc/lock_stat is not serialized against
the lockdep_free_key_range() call from module unload.

Therefore it can happen that we find a class of which ->name/->key are
no longer valid.

There is a further bug in zap_class() that left ->name dangling. Cure
this. Use RCU_INIT_POINTER() because NULL.

Since lockdep_free_key_range() is rcu_sched serialized, we can read
both ->name and ->key under rcu_read_lock_sched() (preempt-disable)
and be assured that if we observe a !NULL value it stays safe to use
for as long as we hold that lock.

If we observe both NULL, skip the entry.

Cc: Ingo Molnar <mingo@redhat.com>
Reported-by: Jerome Marchand <jmarchan@redhat.com>
Tested-by: Jerome Marchand <jmarchan@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150602105013.GS3644@twins.programming.kicks-ass.net
---
 kernel/locking/lockdep.c      |    3 ++-
 kernel/locking/lockdep_proc.c |   22 +++++++++++++++++-----
 2 files changed, 19 insertions(+), 6 deletions(-)

--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3900,7 +3900,8 @@ static void zap_class(struct lock_class
 	list_del_rcu(&class->hash_entry);
 	list_del_rcu(&class->lock_entry);
 
-	class->key = NULL;
+	RCU_INIT_POINTER(class->key, NULL);
+	RCU_INIT_POINTER(class->name, NULL);
 }
 
 static inline int within(const void *addr, void *start, unsigned long size)
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -426,10 +426,12 @@ static void seq_lock_time(struct seq_fil
 
 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;
+	const char *cname;
 	int i, namelen;
+	char name[39];
 
 	class = data->class;
 	stats = &data->stats;
@@ -440,15 +442,25 @@ static void seq_stats(struct seq_file *m
 	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);

[tip:locking/urgent] lockdep: Fix a race between /proc/ lock_stat and module unload

[tip-bot for Peter Zijlstra]

Commit-ID:  cee34d88cabd1ba5fc93e09b5b12232bc9338c7c
Gitweb:     http://git.kernel.org/tip/cee34d88cabd1ba5fc93e09b5b12232bc9338c7c
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Tue, 2 Jun 2015 12:50:13 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 7 Jun 2015 15:46:30 +0200

lockdep: Fix a race between /proc/lock_stat and module unload

The lock_class iteration of /proc/lock_stat is not serialized against
the lockdep_free_key_range() call from module unload.

Therefore it can happen that we find a class of which ->name/->key are
no longer valid.

There is a further bug in zap_class() that left ->name dangling. Cure
this. Use RCU_INIT_POINTER() because NULL.

Since lockdep_free_key_range() is rcu_sched serialized, we can read
both ->name and ->key under rcu_read_lock_sched() (preempt-disable)
and be assured that if we observe a !NULL value it stays safe to use
for as long as we hold that lock.

If we observe both NULL, skip the entry.

Reported-by: Jerome Marchand <jmarchan@redhat.com>
Tested-by: Jerome Marchand <jmarchan@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20150602105013.GS3644@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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 a0831e1..aaeae88 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;
+	RCU_INIT_POINTER(class->key, NULL);
+	RCU_INIT_POINTER(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 ef43ac4..d83d798 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;
+	const char *cname;
 	int i, namelen;
+	char name[39];
 
 	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);