linux-kernel - Re: [PATCH] [RESEND 2] Take over futex of dead task only if FUTEX

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: <alpine.LFD.2.02.1210250936270.2756@ionos>
Date:	Thu, 25 Oct 2012 10:14:23 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Darren Hart <dvhart@...ux.intel.com>
cc:	Siddhesh Poyarekar <siddhesh.poyarekar@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] [RESEND 2] Take over futex of dead task only if
 FUTEX_WAITERS is not set

On Wed, 24 Oct 2012, Darren Hart wrote:
> On 10/23/2012 01:29 PM, Thomas Gleixner wrote:
> > Now the proposed change
> > 
> > -	if (unlikely(ownerdied || !(curval & FUTEX_TID_MASK))) {
> > +       if (unlikely(ownerdied ||
> > +                       !(curval & (FUTEX_TID_MASK | FUTEX_WAITERS)))) {
> > 
> > solves the problem, but it's not obvious why and it wreckages the
> > "stale WAITERS bit" case.
> 
> 
> In what scenario does the WAITERS bit become stale for pi futexes? This
> corner case seems rather core to your solution, so I would like to
> understand it a bit better.

Hmm. I can't reconstruct the scenario anymore, though this has been an
issue some time ago. Either we fixed up that case in course of the big
restructuring of the code or it's just there and I cant decode
it. Brain hurts.

> > Now there is a different solution to that problem. Do not look at the
> > user space value at all and enforce a lookup of possibly available
> > pi_state. If pi_state can be found, then the new incoming locker T3
> > blocks on that pi_state and legitimately races with T2 to acquire the
> > rt_mutex and the pi_state and therefor the proper ownership of the
> > user space futex.
> 
> My first concern here is performance impact by forcing the pi_state
> lookup, however, if we got this far, we already took the syscall, and
> our performance sucks anyway. Correctness obviously trumps performance here.

Right, the pi_state lookup is cheap compared to the syscall, locking ....

And even without the stale WAITERS bit issue, the bit below is a good
reason to do it this way.

> > Another benefit of changing the code this way is that it makes it less
> > dependent on untrusted user space values and therefor minimizes the
> > possible wreckage which might be inflicted.
> 
> That's a definite plus!

Indeed.

> I was surprised at how fast you were able to page all this in after all
> that travel - or is this what you did for 12 hours on the plane?

Nah. I'm just too paranoid to apply any futex patch w/o understanding
the root cause of it.  Darn, if I only could remember how that stale
waiters bit issue got inflicted ....

Thanks,

	tglx


--
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/