linux-kernel - Re: [patch 0/3] futex/rtmutex: Fix issues exposed by trinity

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: <5373DD6F.40506@redhat.com>
Date:	Wed, 14 May 2014 17:17:35 -0400
From:	"Carlos O'Donell" <carlos@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	Darren Hart <dvhart@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	Dave Jones <davej@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Darren Hart <darren@...art.com>,
	Davidlohr Bueso <davidlohr@...com>,
	Ingo Molnar <mingo@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Clark Williams <williams@...hat.com>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Roland McGrath <roland@...k.frob.com>,
	Jakub Jelinek <jakub@...hat.com>,
	Michael Kerrisk <mtk.manpages@...il.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: [patch 0/3] futex/rtmutex: Fix issues exposed by trinity

On 05/14/2014 05:22 AM, Peter Zijlstra wrote:
>>> I believe the thinking goes that if we get to here, then the lock is in an
>>> inconsistent state (between kernel and userspace). I don't have an answer for
>>> why pausing forever would be preferable to returning an error however...
>>
>> What error would we return?
> 
> EDEADLK is a valid user return for pthread_mutex_lock() as per:
> 
>   http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_lock.html

How is that correct? It isn't a deadlock we've detected but inconsistent
state between glibc and the kernel. In this case glibc should assert.
Delaying indefinitely with pause() never seems correct (despite that being
what we do today).
 
>> This particular case is a serious error for which we have no good error code
>> to return to userspace. It's an implementation defect, a bug, we should probably
>> assert instead of pausing.
> 
> No, its perfectly fine to have a lock sequence abort with -EDEADLK.
> Userspace should release its locks and re-attempt.

I agree. If I can prove that it's actually a deadlock, and
that unlock/relock will work to fix it, then we can arrange for glibc
to return EDEADLK.

> You can implement usable locking schemes using this error, like
> wound/wait locking.

Agreed.

>> We can't cancel the stuck thread because pthread_mutex_lock is not a cancellation
>> point.
>>
>> In practice the rest of the application can make forward progress with a single
>> thread stuck. You can attach the debugger and inspect state, so it's useful
>> from that perspective.
> 
> That's just totally braindead. Return EDEADLK to userspace already, let
> the user deal with it.

Not all cases where EDEADLK returns is it such a case that the user
can make forward progress, it might be a corrupt state, in which case
if we detect the corrupt state I would assert. Otherwise, yes, we can
return EDEADLK and let the user figure it out.

Does that make sense?

Cheers,
Carlos.

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