netdev - Re: [PATCH 6/8] netpoll: Allow netpoll

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: <1277459157.32034.127.camel@twins>
Date:	Fri, 25 Jun 2010 11:45:57 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	David Miller <davem@...emloft.net>, herbert@...dor.hengli.com.au,
	mst@...hat.com, frzhang@...hat.com, netdev@...r.kernel.org,
	amwang@...hat.com, shemminger@...tta.com, mpm@...enic.com,
	paulmck@...ux.vnet.ibm.com, mingo@...e.hu
Subject: Re: [PATCH 6/8] netpoll: Allow netpoll_setup/cleanup recursion

On Fri, 2010-06-25 at 01:42 -0700, Andrew Morton wrote:
> On Fri, 25 Jun 2010 10:08:56 +0200 Peter Zijlstra <peterz@...radead.org> wrote:
> 
> > On Thu, 2010-06-24 at 21:42 -0700, Andrew Morton wrote:
> > > That being said, I wonder why Herbert didn't hit this in his testing. 
> > > I suspect that he'd enabled lockdep, which hid the bug.  I haven't
> > > worked out _why_ lockdep hides the double-mutex_unlock bug, but it's a
> > > pretty bad thing to do. 
> > 
> > Most weird indeed, lockdep is supposed so shout its lungs out when
> > someone wants to unlock a lock that isn't actually owned by him (and it
> > not being locked at all certainly implies you're not the owner).
> > 
> > In fact, the below patch results in the below splat -- its also
> > something that's tested by the locking self-test:
> 
> When I enabled lockdep, the bug actually went away.  Is it possible
> that when lockdep detects this bug, it prevents mutex.count from going
> from 1 to 2?

Not lockdep itself but the DEBUG_MUTEXES code (forced by lockdep).

The difference between the normal and the debug code is that the debug
code disables all fast-path code.

The x86 fast-path code does:

 LOCK incl &lock->count
 jg done:
 call slowpath
done:

Since 1++ is >0 it will complete without calling the slow-path, would
do:

 if (__mutex_slowpath_needs_to_unlock()) /* 1 regardless of DEBUG_MUTEX */
   atomic_set(&lock->count, 1);

The question I guess is, do we want double unlocks to go silently
unnoticed? In that case we need to touch the fastpath asm.

> It could be that lockdep _did_ detect (and correct!) the bug.  But
> because I had no usable console output at the time, I didn't see it.
> 
> I did notice that the taint output was "G W".  So something warned
> about something, but I don't know what.  But that was happening with
> lockdep disabled.

Hrmm,. yeah without console output lockdep isn't going to help much,
should we maybe use the speaker to read out the dmesg :-)

> It'd be interesting to add
> 
> 	printk("%d:%d\n", __LINE__, atomic_read(&foo.count));
> 
> after the mutex_unlock()s.

1352:1


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html