Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932277AbbFBPBM (ORCPT ); Tue, 2 Jun 2015 11:01:12 -0400 Received: from casper.infradead.org ([85.118.1.10]:58014 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751691AbbFBPBH (ORCPT ); Tue, 2 Jun 2015 11:01:07 -0400 Date: Tue, 2 Jun 2015 17:01:04 +0200 From: Peter Zijlstra To: Jerome Marchand Cc: Ingo Molnar , linux-kernel@vger.kernel.org Subject: Re: [PATCH] fix a race between /proc/lock_stat and module unloading Message-ID: <20150602150104.GA19282@twins.programming.kicks-ass.net> References: <1432903635-27272-1-git-send-email-jmarchan@redhat.com> <20150602093043.GY19282@twins.programming.kicks-ass.net> <556D7D45.3040207@redhat.com> <20150602105013.GS3644@twins.programming.kicks-ass.net> <556DBA7C.3020909@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <556DBA7C.3020909@redhat.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3338 Lines: 102 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 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 Reported-by: Jerome Marchand Tested-by: Jerome Marchand Signed-off-by: Peter Zijlstra (Intel) 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); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/