Re: futex() on vdso makes process unkillable

From: Darren Hart
Date: Tue Jan 26 2010 - 09:22:10 EST


KOSAKI Motohiro wrote:
Peter Zijlstra wrote:
On Mon, 2010-01-25 at 16:27 +0900, KOSAKI Motohiro wrote:
<snip>
Futex should work both file anon anon. however I personally think vdso is not file nor anon. it is special mappings. nobody defined
futex spec on special mappings. (yes, undefined).

Personally, I think EINVAL or EFAULT are best result of vdso futexing, like as
futexing againt kernel address. but I guess another person have another thinking.

I'd like to hear futex folks's opinion.
Well, my opinion is we should remove the vdso, its ugly as hell :-)

But I think it would make most sense to extend its definition in the
direction of it being a file (for all intents and purposes its a special
DSO -- which unfortunately isn't present in any filesystem).

[ For all intents and purposes processes can already communicate through
futexes on the libc space, so being able to do so through the vsdo
really doesn't add anything ]

So the problem is that the VDSO pages do not have a page->mapping
because they lack the actual filesystem part of files, so even if (with
the recent zero-page patch from Kosaki-san) you make private COWs of the
VDSO, you'll get stuck in that loop.

So the prettiest solution is to simply place the vdso in an actual
filesystem and slowly migrate towards letting userspace map it as a
regular DSO -- /sys/lib{32,64}/libkernel.so like.

[ that has the bonus of getting rid of install_special_mapping() ]

The ugly solution is special casing the vdso in get_futex_key().
I like the creating-a-real-file solution. However, for now (and for stable), I think Kosaki's suggestion of EINVAL or EFAULT is a good stop-gap. EINVAL might play the best with existing glibc implementations.

May I confirm your mention?

If we can accept EFAULT, we don't need any change. my previous futex patch
already did. because 1) VDSO is alwasys read-only mapped 2) write mode
get_user_pages_fast() against read-only pte/vma return EFAULT.

Current linus and stable tree don't cause Mark's original problem. instead, just
return EFAULT. (Well, I'm sorry. my previous mail was unclear. I wrote v2.6.31 test
result)

If you can't accept EFAULT, we need to add vdso specific logic into get_futex_key().
Is this your intention?

That was my intention, but after looking at the glibc source, I don't see any reason for EINVAL over EFAULT. I apparently mis-remembered something there. EFAULT is fine.

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