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?
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
place.
So I'd like the patch to include an explanation of exactly why the two
cases are different.
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: