From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754184Ab1JUJpi (ORCPT ); Fri, 21 Oct 2011 05:45:38 -0400 Received: from mail-bw0-f46.google.com ([209.85.214.46]:64831 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753131Ab1JUJph (ORCPT ); Fri, 21 Oct 2011 05:45:37 -0400 Date: Fri, 21 Oct 2011 17:45:20 +0800 From: Yong Zhang To: David Rientjes Cc: Sergey Senozhatsky , Tejun Heo , Ingo Molnar , Borislav Petkov , Peter Zijlstra , linux-kernel@vger.kernel.org, Andrew Morton Subject: Re: WARNING: at kernel/lockdep.c:690 __lock_acquire+0x168/0x164b() Message-ID: <20111021094520.GA9884@zhy> Reply-To: Yong Zhang References: <20111020183913.GA21918@liondog.tnic> <20111020185329.GA3586@swordfish.minsk.epam.com> <20111020190701.GB3586@swordfish.minsk.epam.com> <20111020212353.GZ25124@google.com> <20111020213644.GB25124@google.com> <20111020230040.GC3586@swordfish.minsk.epam.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 21, 2011 at 02:14:34AM -0700, David Rientjes wrote: > How does it mask the race condition? Before the memset(), the ->name > field was never _cleared_ in lockdep_init_map() like it is now, it was > only stored. A typcal race condition will like this: CPU A CPU B lock_set_subclass(lockA); lock_set_class(lockA); lockdep_init_map(lockA); /* lockA->name is cleared */ memset(lockA); __lock_acquire(lockA); /* lockA->class_cache[] is cleared */ register_lock_class(lockA); look_up_lock_class(lockA); WARN_ON_ONCE(class->name != lock->name); lock->name = name; And a untested patch is below: BTW, now the patch could cure (I guess) the very issue reported in this thread. But it don't cover the case which change the key and the relevant lock_class has existed, I don't think out a way how to fix it yet :) But the fact is we have no such caller yet, the only call site of lock_set_subclass() is double_unlock_balance(). Thanks, Yong --- From: Yong Zhang Subject: [PATCH] lockdep: On-demand initialization for lock_set_class() Since commit f59de89 [lockdep: Clear whole lockdep_map on initialization], lockdep_init_map() will clear all the struct. But it will break lock_set_class()/lock_set_subclass(). A typical race condition is like below: CPU A CPU B lock_set_subclass(lockA); lock_set_class(lockA); lockdep_init_map(lockA); /* lockA->name is cleared */ memset(lockA); __lock_acquire(lockA); /* lockA->class_cache[] is cleared */ register_lock_class(lockA); look_up_lock_class(lockA); WARN_ON_ONCE(class->name != lock->name); lock->name = name; Reported-by: Sergey Senozhatsky Reported-by: Borislav Petkov Signed-off-by: Yong Zhang Cc: Tejun Heo Cc: David Rientjes Cc: Ingo Molnar Cc: Peter Zijlstra --- kernel/lockdep.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/kernel/lockdep.c b/kernel/lockdep.c index 91d67ce..bc7dd1e 100644 --- a/kernel/lockdep.c +++ b/kernel/lockdep.c @@ -3160,7 +3160,10 @@ __lock_set_class(struct lockdep_map *lock, const char *name, return print_unlock_inbalance_bug(curr, lock, ip); found_it: - lockdep_init_map(lock, name, key, 0); + /* only changing lock->name make no sense */ + WARN_ON(lock->key == key && lock->name != name); + if (lock->key != key) + lockdep_init_map(lock, name, key, 0); class = register_lock_class(lock, subclass, 0); hlock->class_idx = class - lock_classes + 1; -- 1.7.5.4