Re: [PATCH] futex: futex_find_get_task make credentials check conditional

From: Darren Hart
Date: Tue Jun 29 2010 - 12:58:35 EST

On 06/29/2010 09:41 AM, Linus Torvalds wrote:
On Tue, Jun 29, 2010 at 1:42 AM, Michal Hocko<mhocko@xxxxxxx> 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? Shouldn't the futex issue in the end depend on
whether you have a shared page or not - and not on credentials at all?
Any two processes that share a futex in the same shared page should be
able to use that without any regard for whether they are the same
user. That's kind of the point, no?

I agree and haven't been able to come up with a need for the test either, but I wanted to hear back from Ingo as the he authored the original check.

I was trying to see if futex_lock_pi() could somehow be abused, but if so, I don't see it:

TaskUserA TaskUserB
*addrB = TID_OF(TaskUserA)

TaskUserB would lookup the pi_state, not find it as addrB and addrA don't hash to the same key, create a new pi_state and mark TaskUserA as the owner, then block.

Once TaskUserA exits, the pi_list will contain the pi_state for the addrB futex. This is "bad", but the kernel cleans it up, releases the lock - but doesn't wake TaskUserB. That seems acceptable to me since TaskUserB is in the wrong here.

IOW, I personally dislike these kinds of conditional checks,
especially since the discussion (at least the part I've seen) hasn't
made it clear why it should be conditional - or exist - in the first

So I'd like the patch to include an explanation of exactly why the two
cases are different.

Agreed, waiting on Ingo at the moment.

The other thing I'd like to see is to move the whole cred checking up
a level. There's no reason to check the credentials in
futex_find_get_task() that I can see - why not do it in the caller
instead? IOW, I think futex_find_get_task() should just look something
like this instead:

/me beats head on desk, duh. Still, I'm hoping this isn't necessary and we can lose the credentials checking entirely.


Darren Hart
IBM Linux Technology Center
Real-Time Linux Team
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at