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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090718111948.GD27287@elte.hu>
Date:	Sat, 18 Jul 2009 13:19:49 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Johannes Berg <johannes@...solutions.net>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	Jiri Slaby <jirislaby@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] wireless: wl12xx, fix lock imbalance


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

> On Mon, 2009-07-13 at 23:51 +0200, Jiri Slaby wrote:
> > On 07/13/2009 11:49 PM, Johannes Berg wrote:
> > > On Mon, 2009-07-13 at 23:44 +0200, Jiri Slaby wrote:
> > > 
> > >>> I've had local hacks
> > >>> many times to make sparse aware of mutexes, is there a reason they are
> > >>> not annotated with __acquire(s)/__release(s) like spinlocks etc.?
> > >>
> > >> Mutexes are often locked/unlocked interprocedural which I think sparse
> > >> can't do much about.
> > > 
> > > Well, you annotate those functions too, of course.
> > 
> > Sorry, I don't understand. What functions I annotate?
> 
> Well those that take the mutex, e.g.
> 
> void acquire_foo(struct foo *f)
> {
> 	mutex_lock(&f->mtx);
> }
> 
> 
> turns to
> 
> void acquire_foo(struct foo *f)
> 	__acquires(f->mtx)
> {
> 	mutex_lock(&f->mtx);
> }
> 
> johannes

Yes. And in fact 'nice' code wants to be either annotated explicitly 
as 'I am taking locks', or should be balanced.

I was thinking about also using lockdep plus the function-graph 
tracer for that (in the dynamic lock debugging department).

It would work like this: __acquires()/__releases() would also emit 
section markers like __lockfunc, and lockdep would warn about 
functions that return with unbalanced locks, irqs or preempt counts 
and do not declare themselves as locking related functions.

This would help catch imbalances at their source.

Plus static tools like Jiri is working on are very useful as well. I 
think Coverty does that too and it's a pity we dont have free tools 
for that. In fact Covery will sweep clean the kernel of such bugs, 
giving OSS tools like 'stanse' the false impression that there are 
no such bugs. There are such bugs - there's a constant influx of 
them. So please work on this, it looks very useful.

	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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ