[PATCH] futex: futex_find_get_task remove credentails check

[PATCH] futex: futex_find_get_task remove credentails check

[Michal Hocko]

Hi stable team,
could you consider including the following patch (Linus tree commit:
7a0ea09ad5352efce8fe79ed853150449903b9f5).

The original discussion which led to this commit can be found at
http://lkml.org/lkml/2010/6/23/52.

In short:
The original pi locking implementation (since it got to the kernel)
contains a credential check (in futex_find_get_task) if we want to
create a PI state for already held lock. This test fails if the lock
owner has a different (e)uid than the process for which we want to
create the state.
The lock operation then fails with ESRCH which is the error code 
which is returned if the process holding a lock doesn't exist. 
The userspace (glibc) doesn't expect this behavior for shared robust PI
futexes and fails with an assert or hang the task in the end-less loop.

The test case is attached in the referenced thread.

The credential test, which is removed by this patch, doesn't look
correct and it limits the functionality without any good reason. There
are no security consequences as well because the only thing that should
matter in the shared futex-es is accessibility to the shared memory.

The patch applies as is on top of Vanilla 2.6.32, but let me know if you
want to base it on top of the any of the stable trees.


---
>From 7a0ea09ad5352efce8fe79ed853150449903b9f5 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.cz>
Date: Wed, 30 Jun 2010 09:51:19 +0200
Subject: [PATCH] futex: futex_find_get_task remove credentails check

futex_find_get_task is currently used (through lookup_pi_state) from two
contexts, futex_requeue and futex_lock_pi_atomic.  None of the paths
looks it needs the credentials check, though.  Different (e)uids
shouldn't matter at all because the only thing that is important for
shared futex is the accessibility of the shared memory.

The credentail check results in glibc assert failure or process hang (if
glibc is compiled without assert support) for shared robust pthread
mutex with priority inheritance if a process tries to lock already held
lock owned by a process with a different euid:

pthread_mutex_lock.c:312: __pthread_mutex_lock_full: Assertion `(-(e)) != 3 || !robust' failed.

The problem is that futex_lock_pi_atomic which is called when we try to
lock already held lock checks the current holder (tid is stored in the
futex value) to get the PI state.  It uses lookup_pi_state which in turn
gets task struct from futex_find_get_task.  ESRCH is returned either
when the task is not found or if credentials check fails.

futex_lock_pi_atomic simply returns if it gets ESRCH.  glibc code,
however, doesn't expect that robust lock returns with ESRCH because it
should get either success or owner died.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
Acked-by: Darren Hart <dvhltc@us.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Nick Piggin <npiggin@suse.de>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 kernel/futex.c |   17 ++++-------------
 1 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index e7a35f1..6a3a5fa 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -429,20 +429,11 @@ static void free_pi_state(struct futex_pi_state *pi_state)
 static struct task_struct * futex_find_get_task(pid_t pid)
 {
 	struct task_struct *p;
-	const struct cred *cred = current_cred(), *pcred;
 
 	rcu_read_lock();
 	p = find_task_by_vpid(pid);
-	if (!p) {
-		p = ERR_PTR(-ESRCH);
-	} else {
-		pcred = __task_cred(p);
-		if (cred->euid != pcred->euid &&
-		    cred->euid != pcred->uid)
-			p = ERR_PTR(-ESRCH);
-		else
-			get_task_struct(p);
-	}
+	if (p)
+		get_task_struct(p);
 
 	rcu_read_unlock();
 
@@ -564,8 +555,8 @@ lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
 	if (!pid)
 		return -ESRCH;
 	p = futex_find_get_task(pid);
-	if (IS_ERR(p))
-		return PTR_ERR(p);
+	if (!p)
+		return -ESRCH;
 
 	/*
 	 * We need to look at the task state flags to figure out,
-- 
1.7.1

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Ingo Molnar]

* Michal Hocko <mhocko@suse.cz> wrote:

> Hi stable team,
> could you consider including the following patch (Linus tree commit:
> 7a0ea09ad5352efce8fe79ed853150449903b9f5).

Ack.

	Ingo

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Thomas Gleixner]

On Thu, 8 Jul 2010, Ingo Molnar wrote:

> 
> * Michal Hocko <mhocko@suse.cz> wrote:
> 
> > Hi stable team,
> > could you consider including the following patch (Linus tree commit:
> > 7a0ea09ad5352efce8fe79ed853150449903b9f5).
> 
> Ack.

Ack

	tglx

Re: [stable] [PATCH] futex: futex_find_get_task remove credentails check

[Greg KH]

On Thu, Jul 08, 2010 at 02:51:04PM +0200, Michal Hocko wrote:
> Hi stable team,
> could you consider including the following patch (Linus tree commit:
> 7a0ea09ad5352efce8fe79ed853150449903b9f5).

Now queued up.

thanks,

greg k-h

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Michal Hocko]

On Thu 08-07-10 11:43:53, Peter Zijlstra wrote:
> On Thu, 2010-07-08 at 11:39 +0200, Michal Hocko wrote:
> > OK, so do you need an ACK from Thomas, or can you grab the patch and
> > push it through one of your trees?
> > 
> Have a look at Linus' tree.

Ahh, you mean 7a0ea09ad5352efce8fe79ed853150449903b9f5. I didn't get any
email about the committing.

Thanks!

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Peter Zijlstra]

On Thu, 2010-07-08 at 11:39 +0200, Michal Hocko wrote:
> OK, so do you need an ACK from Thomas, or can you grab the patch and
> push it through one of your trees?
> 
Have a look at Linus' tree.

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Michal Hocko]

On Thu 08-07-10 11:32:41, Ingo Molnar wrote:
> 
> * Michal Hocko <mhocko@suse.cz> wrote:
> 
> > On Wed 30-06-10 09:43:27, Darren Hart wrote:
> > > On 06/30/2010 02:55 AM, Michal Hocko wrote:
> > > >On Wed 30-06-10 09:01:15, Michal Hocko wrote:
> > > >>On Tue 29-06-10 09:41:02, Linus Torvalds wrote:
> > > >>>On Tue, Jun 29, 2010 at 1:42 AM, Michal Hocko<mhocko@suse.cz>  wrote:
> > > >>>>
> > > >>>>futex_find_get_task is currently used (through lookup_pi_state) from two
> > > >>>>contexts, futex_requeue and futex_lock_pi_atomic. While credentials check
> > > >>>>makes sense in the first code path, the second one is more problematic
> > > >>>>because this check requires that the PI lock holder (pid parameter) has
> > > >>>>the same uid and euid as the process's euid which is trying to lock the
> > > >>>>same futex (current).
> > > >>>
> > > >>>So exactly why does it make sense to check the credentials in the
> > > >>>first code path then?
> > > >>
> > > >>I though that requeue needs this for security reasons (don't let requeue
> > > >>process for other user), but when I thought about that again you are
> > > >>right and the only what matters should be accessibility of the shared
> > > >>memory.
> > > >
> > > >And here is the patch which does the thing.
> > > >
> > > >--
> > > >
> > > > From 082c5ad2c482a8e78b61b17e213e750b006176aa Mon Sep 17 00:00:00 2001
> > > >From: Michal Hocko<mhocko@suse.cz>
> > > >Date: Wed, 30 Jun 2010 09:51:19 +0200
> > > >Subject: [PATCH] futex: futex_find_get_task remove credentails check
> > > >
> > > >futex_find_get_task is currently used (through lookup_pi_state) from two
> > > >contexts, futex_requeue and futex_lock_pi_atomic. None of the paths
> > > >looks it needs the credentials check, though. Different (e)uids
> > > >shouldn't matter at all because the only thing that is important for
> > > >shared futex is the accessibility of the shared memory.
> > > >
> > > >The credentail check results in glibc assert failure or process hang (if
> > > >glibc is compiled without assert support) for shared robust pthread
> > > >mutex with priority inheritance if a process tries to lock already held
> > > >lock owned by a process with a different euid:
> > > >
> > > >pthread_mutex_lock.c:312: __pthread_mutex_lock_full: Assertion `(-(e)) != 3 || !robust' failed.
> > > >
> > > >The problem is that futex_lock_pi_atomic which is called when we try to
> > > >lock already held lock checks the current holder (tid is stored in the
> > > >futex value) to get the PI state. It uses lookup_pi_state which in turn
> > > >gets task struct from futex_find_get_task. ESRCH is returned either when
> > > >the task is not found or if credentials check fails.
> > > >futex_lock_pi_atomic simply returns if it gets ESRCH. glibc code,
> > > >however,  doesn't expect that robust lock returns with ESRCH because it
> > > >should get either success or owner died.
> > > >
> > > >Signed-off-by: Michal Hocko<mhocko@suse.cz>
> > > 
> > > Without hearing back from Ingo on the original intent of the
> > > credentials check, this looks right to me.
> > 
> > Could you comment on that Ingo, please?
> 
> I think that's more of a question to Thomas :-)
> 
> My memories are hazy and nothing springs out as some credible original intent. 
> So please assume it doesnt exist :-)

OK, so do you need an ACK from Thomas, or can you grab the patch and
push it through one of your trees?

> 
> 	Ingo

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Ingo Molnar]

* Michal Hocko <mhocko@suse.cz> wrote:

> On Wed 30-06-10 09:43:27, Darren Hart wrote:
> > On 06/30/2010 02:55 AM, Michal Hocko wrote:
> > >On Wed 30-06-10 09:01:15, Michal Hocko wrote:
> > >>On Tue 29-06-10 09:41:02, Linus Torvalds wrote:
> > >>>On Tue, Jun 29, 2010 at 1:42 AM, Michal Hocko<mhocko@suse.cz>  wrote:
> > >>>>
> > >>>>futex_find_get_task is currently used (through lookup_pi_state) from two
> > >>>>contexts, futex_requeue and futex_lock_pi_atomic. While credentials check
> > >>>>makes sense in the first code path, the second one is more problematic
> > >>>>because this check requires that the PI lock holder (pid parameter) has
> > >>>>the same uid and euid as the process's euid which is trying to lock the
> > >>>>same futex (current).
> > >>>
> > >>>So exactly why does it make sense to check the credentials in the
> > >>>first code path then?
> > >>
> > >>I though that requeue needs this for security reasons (don't let requeue
> > >>process for other user), but when I thought about that again you are
> > >>right and the only what matters should be accessibility of the shared
> > >>memory.
> > >
> > >And here is the patch which does the thing.
> > >
> > >--
> > >
> > > From 082c5ad2c482a8e78b61b17e213e750b006176aa Mon Sep 17 00:00:00 2001
> > >From: Michal Hocko<mhocko@suse.cz>
> > >Date: Wed, 30 Jun 2010 09:51:19 +0200
> > >Subject: [PATCH] futex: futex_find_get_task remove credentails check
> > >
> > >futex_find_get_task is currently used (through lookup_pi_state) from two
> > >contexts, futex_requeue and futex_lock_pi_atomic. None of the paths
> > >looks it needs the credentials check, though. Different (e)uids
> > >shouldn't matter at all because the only thing that is important for
> > >shared futex is the accessibility of the shared memory.
> > >
> > >The credentail check results in glibc assert failure or process hang (if
> > >glibc is compiled without assert support) for shared robust pthread
> > >mutex with priority inheritance if a process tries to lock already held
> > >lock owned by a process with a different euid:
> > >
> > >pthread_mutex_lock.c:312: __pthread_mutex_lock_full: Assertion `(-(e)) != 3 || !robust' failed.
> > >
> > >The problem is that futex_lock_pi_atomic which is called when we try to
> > >lock already held lock checks the current holder (tid is stored in the
> > >futex value) to get the PI state. It uses lookup_pi_state which in turn
> > >gets task struct from futex_find_get_task. ESRCH is returned either when
> > >the task is not found or if credentials check fails.
> > >futex_lock_pi_atomic simply returns if it gets ESRCH. glibc code,
> > >however,  doesn't expect that robust lock returns with ESRCH because it
> > >should get either success or owner died.
> > >
> > >Signed-off-by: Michal Hocko<mhocko@suse.cz>
> > 
> > Without hearing back from Ingo on the original intent of the
> > credentials check, this looks right to me.
> 
> Could you comment on that Ingo, please?

I think that's more of a question to Thomas :-)

My memories are hazy and nothing springs out as some credible original intent. 
So please assume it doesnt exist :-)

	Ingo

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Michal Hocko]

On Wed 30-06-10 09:43:27, Darren Hart wrote:
> On 06/30/2010 02:55 AM, Michal Hocko wrote:
> >On Wed 30-06-10 09:01:15, Michal Hocko wrote:
> >>On Tue 29-06-10 09:41:02, Linus Torvalds wrote:
> >>>On Tue, Jun 29, 2010 at 1:42 AM, Michal Hocko<mhocko@suse.cz>  wrote:
> >>>>
> >>>>futex_find_get_task is currently used (through lookup_pi_state) from two
> >>>>contexts, futex_requeue and futex_lock_pi_atomic. While credentials check
> >>>>makes sense in the first code path, the second one is more problematic
> >>>>because this check requires that the PI lock holder (pid parameter) has
> >>>>the same uid and euid as the process's euid which is trying to lock the
> >>>>same futex (current).
> >>>
> >>>So exactly why does it make sense to check the credentials in the
> >>>first code path then?
> >>
> >>I though that requeue needs this for security reasons (don't let requeue
> >>process for other user), but when I thought about that again you are
> >>right and the only what matters should be accessibility of the shared
> >>memory.
> >
> >And here is the patch which does the thing.
> >
> >--
> >
> > From 082c5ad2c482a8e78b61b17e213e750b006176aa Mon Sep 17 00:00:00 2001
> >From: Michal Hocko<mhocko@suse.cz>
> >Date: Wed, 30 Jun 2010 09:51:19 +0200
> >Subject: [PATCH] futex: futex_find_get_task remove credentails check
> >
> >futex_find_get_task is currently used (through lookup_pi_state) from two
> >contexts, futex_requeue and futex_lock_pi_atomic. None of the paths
> >looks it needs the credentials check, though. Different (e)uids
> >shouldn't matter at all because the only thing that is important for
> >shared futex is the accessibility of the shared memory.
> >
> >The credentail check results in glibc assert failure or process hang (if
> >glibc is compiled without assert support) for shared robust pthread
> >mutex with priority inheritance if a process tries to lock already held
> >lock owned by a process with a different euid:
> >
> >pthread_mutex_lock.c:312: __pthread_mutex_lock_full: Assertion `(-(e)) != 3 || !robust' failed.
> >
> >The problem is that futex_lock_pi_atomic which is called when we try to
> >lock already held lock checks the current holder (tid is stored in the
> >futex value) to get the PI state. It uses lookup_pi_state which in turn
> >gets task struct from futex_find_get_task. ESRCH is returned either when
> >the task is not found or if credentials check fails.
> >futex_lock_pi_atomic simply returns if it gets ESRCH. glibc code,
> >however,  doesn't expect that robust lock returns with ESRCH because it
> >should get either success or owner died.
> >
> >Signed-off-by: Michal Hocko<mhocko@suse.cz>
> 
> Without hearing back from Ingo on the original intent of the
> credentials check, this looks right to me.

Could you comment on that Ingo, please?

> 
> Acked-by: Darren Hart <dvhltc@us.ibm.com>
> 
> 
> >---
> >  kernel/futex.c |   17 ++++-------------
> >  1 files changed, 4 insertions(+), 13 deletions(-)
> >
> >diff --git a/kernel/futex.c b/kernel/futex.c
> >index e7a35f1..6a3a5fa 100644
> >--- a/kernel/futex.c
> >+++ b/kernel/futex.c
> >@@ -429,20 +429,11 @@ static void free_pi_state(struct futex_pi_state *pi_state)
> >  static struct task_struct * futex_find_get_task(pid_t pid)
> >  {
> >  	struct task_struct *p;
> >-	const struct cred *cred = current_cred(), *pcred;
> >
> >  	rcu_read_lock();
> >  	p = find_task_by_vpid(pid);
> >-	if (!p) {
> >-		p = ERR_PTR(-ESRCH);
> >-	} else {
> >-		pcred = __task_cred(p);
> >-		if (cred->euid != pcred->euid&&
> >-		    cred->euid != pcred->uid)
> >-			p = ERR_PTR(-ESRCH);
> >-		else
> >-			get_task_struct(p);
> >-	}
> >+	if (p)
> >+		get_task_struct(p);
> >
> >  	rcu_read_unlock();
> >
> >@@ -564,8 +555,8 @@ lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
> >  	if (!pid)
> >  		return -ESRCH;
> >  	p = futex_find_get_task(pid);
> >-	if (IS_ERR(p))
> >-		return PTR_ERR(p);
> >+	if (!p)
> >+		return -ESRCH;
> >
> >  	/*
> >  	 * We need to look at the task state flags to figure out,
> 
> 
> -- 
> Darren Hart
> IBM Linux Technology Center
> Real-Time Linux Team

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: [PATCH] futex: futex_find_get_task remove credentails check

[Darren Hart]

On 06/30/2010 02:55 AM, Michal Hocko wrote:
> On Wed 30-06-10 09:01:15, Michal Hocko wrote:
>> On Tue 29-06-10 09:41:02, Linus Torvalds wrote:
>>> On Tue, Jun 29, 2010 at 1:42 AM, Michal Hocko<mhocko@suse.cz>  wrote:
>>>>
>>>> futex_find_get_task is currently used (through lookup_pi_state) from two
>>>> contexts, futex_requeue and futex_lock_pi_atomic. While credentials check
>>>> makes sense in the first code path, the second one is more problematic
>>>> because this check requires that the PI lock holder (pid parameter) has
>>>> the same uid and euid as the process's euid which is trying to lock the
>>>> same futex (current).
>>>
>>> So exactly why does it make sense to check the credentials in the
>>> first code path then?
>>
>> I though that requeue needs this for security reasons (don't let requeue
>> process for other user), but when I thought about that again you are
>> right and the only what matters should be accessibility of the shared
>> memory.
>
> And here is the patch which does the thing.
>
> --
>
>  From 082c5ad2c482a8e78b61b17e213e750b006176aa Mon Sep 17 00:00:00 2001
> From: Michal Hocko<mhocko@suse.cz>
> Date: Wed, 30 Jun 2010 09:51:19 +0200
> Subject: [PATCH] futex: futex_find_get_task remove credentails check
>
> futex_find_get_task is currently used (through lookup_pi_state) from two
> contexts, futex_requeue and futex_lock_pi_atomic. None of the paths
> looks it needs the credentials check, though. Different (e)uids
> shouldn't matter at all because the only thing that is important for
> shared futex is the accessibility of the shared memory.
>
> The credentail check results in glibc assert failure or process hang (if
> glibc is compiled without assert support) for shared robust pthread
> mutex with priority inheritance if a process tries to lock already held
> lock owned by a process with a different euid:
>
> pthread_mutex_lock.c:312: __pthread_mutex_lock_full: Assertion `(-(e)) != 3 || !robust' failed.
>
> The problem is that futex_lock_pi_atomic which is called when we try to
> lock already held lock checks the current holder (tid is stored in the
> futex value) to get the PI state. It uses lookup_pi_state which in turn
> gets task struct from futex_find_get_task. ESRCH is returned either when
> the task is not found or if credentials check fails.
> futex_lock_pi_atomic simply returns if it gets ESRCH. glibc code,
> however,  doesn't expect that robust lock returns with ESRCH because it
> should get either success or owner died.
>
> Signed-off-by: Michal Hocko<mhocko@suse.cz>

Without hearing back from Ingo on the original intent of the credentials 
check, this looks right to me.

Acked-by: Darren Hart <dvhltc@us.ibm.com>


> ---
>   kernel/futex.c |   17 ++++-------------
>   1 files changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index e7a35f1..6a3a5fa 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -429,20 +429,11 @@ static void free_pi_state(struct futex_pi_state *pi_state)
>   static struct task_struct * futex_find_get_task(pid_t pid)
>   {
>   	struct task_struct *p;
> -	const struct cred *cred = current_cred(), *pcred;
>
>   	rcu_read_lock();
>   	p = find_task_by_vpid(pid);
> -	if (!p) {
> -		p = ERR_PTR(-ESRCH);
> -	} else {
> -		pcred = __task_cred(p);
> -		if (cred->euid != pcred->euid&&
> -		    cred->euid != pcred->uid)
> -			p = ERR_PTR(-ESRCH);
> -		else
> -			get_task_struct(p);
> -	}
> +	if (p)
> +		get_task_struct(p);
>
>   	rcu_read_unlock();
>
> @@ -564,8 +555,8 @@ lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
>   	if (!pid)
>   		return -ESRCH;
>   	p = futex_find_get_task(pid);
> -	if (IS_ERR(p))
> -		return PTR_ERR(p);
> +	if (!p)
> +		return -ESRCH;
>
>   	/*
>   	 * We need to look at the task state flags to figure out,


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

[PATCH] futex: futex_find_get_task remove credentails check

[Michal Hocko]

On Wed 30-06-10 09:01:15, Michal Hocko wrote:
> On Tue 29-06-10 09:41:02, Linus Torvalds wrote:
> > On Tue, Jun 29, 2010 at 1:42 AM, Michal Hocko <mhocko@suse.cz> wrote:
> > >
> > > futex_find_get_task is currently used (through lookup_pi_state) from two
> > > contexts, futex_requeue and futex_lock_pi_atomic. While credentials check
> > > makes sense in the first code path, the second one is more problematic
> > > because this check requires that the PI lock holder (pid parameter) has
> > > the same uid and euid as the process's euid which is trying to lock the
> > > same futex (current).
> > 
> > So exactly why does it make sense to check the credentials in the
> > first code path then? 
> 
> I though that requeue needs this for security reasons (don't let requeue
> process for other user), but when I thought about that again you are
> right and the only what matters should be accessibility of the shared
> memory.

And here is the patch which does the thing.

--

>From 082c5ad2c482a8e78b61b17e213e750b006176aa Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.cz>
Date: Wed, 30 Jun 2010 09:51:19 +0200
Subject: [PATCH] futex: futex_find_get_task remove credentails check

futex_find_get_task is currently used (through lookup_pi_state) from two
contexts, futex_requeue and futex_lock_pi_atomic. None of the paths
looks it needs the credentials check, though. Different (e)uids
shouldn't matter at all because the only thing that is important for
shared futex is the accessibility of the shared memory.

The credentail check results in glibc assert failure or process hang (if
glibc is compiled without assert support) for shared robust pthread
mutex with priority inheritance if a process tries to lock already held
lock owned by a process with a different euid:

pthread_mutex_lock.c:312: __pthread_mutex_lock_full: Assertion `(-(e)) != 3 || !robust' failed.

The problem is that futex_lock_pi_atomic which is called when we try to
lock already held lock checks the current holder (tid is stored in the
futex value) to get the PI state. It uses lookup_pi_state which in turn
gets task struct from futex_find_get_task. ESRCH is returned either when
the task is not found or if credentials check fails.
futex_lock_pi_atomic simply returns if it gets ESRCH. glibc code,
however,  doesn't expect that robust lock returns with ESRCH because it
should get either success or owner died.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 kernel/futex.c |   17 ++++-------------
 1 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index e7a35f1..6a3a5fa 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -429,20 +429,11 @@ static void free_pi_state(struct futex_pi_state *pi_state)
 static struct task_struct * futex_find_get_task(pid_t pid)
 {
 	struct task_struct *p;
-	const struct cred *cred = current_cred(), *pcred;
 
 	rcu_read_lock();
 	p = find_task_by_vpid(pid);
-	if (!p) {
-		p = ERR_PTR(-ESRCH);
-	} else {
-		pcred = __task_cred(p);
-		if (cred->euid != pcred->euid &&
-		    cred->euid != pcred->uid)
-			p = ERR_PTR(-ESRCH);
-		else
-			get_task_struct(p);
-	}
+	if (p)
+		get_task_struct(p);
 
 	rcu_read_unlock();
 
@@ -564,8 +555,8 @@ lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
 	if (!pid)
 		return -ESRCH;
 	p = futex_find_get_task(pid);
-	if (IS_ERR(p))
-		return PTR_ERR(p);
+	if (!p)
+		return -ESRCH;
 
 	/*
 	 * We need to look at the task state flags to figure out,
-- 
1.7.1


-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic