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

From: Peter Zijlstra
Date: Tue Jun 02 2015 - 11:01:16 EST


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@xxxxxxxxxxxxx>
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@xxxxxxxxxx>
Reported-by: Jerome Marchand <jmarchan@xxxxxxxxxx>
Tested-by: Jerome Marchand <jmarchan@xxxxxxxxxx>
Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
Link: http://lkml.kernel.org/r/20150602105013.GS3644@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
---
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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/