Re: [GIT PULL] core kernel fixes

From: Linus Torvalds
Date: Sat Jun 20 2009 - 14:52:31 EST




On Sat, 20 Jun 2009, Ingo Molnar wrote:
>
> Please pull the latest core-fixes-for-linus git tree from:

I need to think about this one.

> +/*
> + * get_user_writeable - get user page and verify RW access
> + * @uaddr: pointer to faulting user space address
> + *
> + * We cannot write to the user space address and get_user just faults
> + * the page in, but does not tell us whether the mapping is writeable.
> + *
> + * We can not rely on access_ok() for private futexes as it is just a
> + * range check and we can neither rely on get_user_pages() as there
> + * might be a mprotect(PROT_READ) for that mapping after
> + * get_user_pages() and before the fault in the atomic write access.
> + */
> +static int get_user_writeable(u32 __user *uaddr)
> +{
> + unsigned long addr = (unsigned long)uaddr;
> + struct page *page;
> + int ret;
> +
> + ret = get_user_pages_fast(addr, 1, 1, &page);
> + if (ret > 0)
> + put_page(page);
> +
> + return ret;
> +}

This is some seriously crazy sh*t, man. What drugs were you on, and are
the police on to you? Because whatever drugs they were, I seriously doube
they are legal even with a prescription.

There's somethign wrong in futex land. This whole retry crap has been so
incredibly broken so many times, and this particular fix looks so horribly
ugly that I really need to ask people: "is the loop really sane?"

I also think that the above is a singularly stupid way to do what you want
done. It's slow, it's nasty, it's complicated.

On x86, the natural way to do what you want done is ONE SINGLE
INSTRUCTION! As far as I can tell, the above crazy function is 100%
equivalent to this:

asm __inline__("lock ; addl $0,%0":"+m" (*uaddr): :"memory", "cc");

which really makes me think that using "get_user_pages_fast()" for it is
some truly crazy crap.

Sure, we don't have any architecture-generic way to do that, but still..

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