Re: [PATCH] RFC: futex fault handling and futex key references (NOTFOR INCLUSION)

From: Darren Hart
Date: Sat Jan 10 2009 - 00:54:20 EST


Peter Zijlstra wrote:
On Thu, 2009-01-08 at 23:52 -0800, Darren Hart wrote:
While trying to bend my brain around the various layers of fault handling in
futex.c, I think I may have uncovered some logical errors (or at least stale
code sections). I've attached two patches that address the alleged problems
against linux-tip/core/futexes. They are based on the following assumption:

Since the uaddr passed to a futex function isn't updated within the function,
and the mm doesn't change while we're in there,

That's not quite true, you _can_ change the memory map by issuing
concurrent mmap/munmap/mremap etc.. calls from another thread.

Well, what I meant was that the pointer "current->mm" doesn't change, since that is what we store in the futex key union.

The thing is, afaik futexes have never been completely safe wrt
concurrent mm modifications -- that is, as long as we fail the futex op
with -EFAULT or similar and not crash the kernel we're good.

there should never be a need to
make repeat calls to futex_get_key(). Even if the queue_lock is dropped, the
futex_q might lose it's waiter (requeued) but the key stays the same.

Yes, so when we assume the mmap stable (and fail the op whenever that
assumption proves false) we can say the futex keys are stable and should
never need recomputation.

And if I understand correctly, we would catch this scenario any time we try to use uaddr and find it faulting (during the cmpxchg* calls for example). If the mmap changes too much, we'll exhaust our fault tolerance (3) and exit with -EFAULT back to userspace.

Sound right?

Thanks for the review Peter,

--
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 http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/