linux-kernel - Re: [PATCH v3] timer: implement lockdep deadlock detection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090129123804.GA22239@elte.hu>
Date:	Thu, 29 Jan 2009 13:38:04 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Johannes Berg <johannes@...solutions.net>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Linux Kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] timer: implement lockdep deadlock detection


* Johannes Berg <johannes@...solutions.net> wrote:

> On Wed, 2009-01-28 at 11:59 +0100, Johannes Berg wrote:
> 
> > > > I actually got "trying to register non-static key" on my powerpc64
> > > > machine. Is there a possibility that functions are not static??
> > > 
> > > Hmm, weird, afaict static_obj() includes both text and data, for the
> > > core kernel as well as modules.
> > 
> > Yeah, I'd think it should. I'll run it by the powerpc list when I get it
> > again, it only seems to happen very rarely.
> 
> It's actually a generic bug.
> 
> delayed work structs are initialised like this:
> 
> #define __DELAYED_WORK_INITIALIZER(n, f) {                      \
>         .work = __WORK_INITIALIZER((n).work, (f)),              \
>         .timer = TIMER_INITIALIZER(NULL, 0, 0),                 \
>         }
> 
> #define DECLARE_DELAYED_WORK(n, f)                              \
>         struct delayed_work n = __DELAYED_WORK_INITIALIZER(n, f)
> 
> 
> Note the NULL function, which I used for the key of timers. Thus, the
> key is NULL, and the name is "NULL".
> 
> Now, this means that my code for the run timer:
> 
> 				struct lockdep_map lockdep_map =
> 					timer->lockdep_map;
> 
> will actually have lockdep_map here with a NULL key. Once it gets into
> 
> 			lock_map_acquire(&lockdep_map);
> 
> it'll try to register &lockdep_map as the key.
> 
> Interestingly, that doesn't seem to be a problem on x86_64, which would
> appear to be a bug, the stack certainly isn't a static location.
> 
> The patch below fixes it by using the file/lineno of the static
> definition as both the name and the key -- using it as the name means
> you have a good chance of finding it if something goes wrong, and using
> it as the key means we have a good key for it. The patch looks horrible
> though. Any better ideas? If not, I think we should roll this into the
> original patch.

I suspect this invalidates v3 - mind resending a full v4 patch?

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/