On Tue, Dec 10, 2013 at 2:19 PM, Linus Torvalds
<[email protected]> wrote:
>
> Shouldn't we do something like the attached?
So I think that kernel/futex.c part of the patch might be a good idea,
but on x86-64 (which is what Dave is running), the
if (end >> __VIRTUAL_MASK_SHIFT)
test in get_user_pages_fast() should have been equivalent. And even on
32-bit, we do check the _PAGE_USER bits in the page tables, so I guess
it's all good on a get_user_pages_fast() side.
So never mind. It's not the address checking.
And I think I see what's up.
I think what happens is:
- get_user_pages_fast(address, 1, 1, &page) fails (because it's read-only)
- get_user_pages_fast(address, 1, 0, &page) succeeds and gets a large-page
- __get_user_pages_fast(address, 1, 1, &page) fails (because it's read-only).
so what triggers this is likely that Dave now does large-pages, and
one of them is a read-only mapping.
So I would suggest replacing the second "1" in the
__get_user_pages_fast() call with a "!ro" instead. So how about this
second patch instead (the access_ok() move remains).
Comments?
Linus
kernel/futex.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/futex.c b/kernel/futex.c
index 80ba086f021d..6272f560385c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -251,6 +251,9 @@ get_futex_key(u32 __user *uaddr, int fshared, union
futex_key *key, int rw)
return -EINVAL;
address -= key->both.offset;
+ if (unlikely(!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))))
+ return -EFAULT;
+
/*
* PROCESS_PRIVATE futexes are fast.
* As the mm cannot disappear under us and the 'key' only needs
@@ -259,8 +262,6 @@ get_futex_key(u32 __user *uaddr, int fshared, union
futex_key *key, int rw)
* but access_ok() should be faster than find_vma()
*/
if (!fshared) {
- if (unlikely(!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))))
- return -EFAULT;
key->private.mm = mm;
key->private.address = address;
get_futex_key_refs(key);
@@ -288,7 +289,7 @@ again:
put_page(page);
/* serialize against __split_huge_page_splitting() */
local_irq_disable();
- if (likely(__get_user_pages_fast(address, 1, 1, &page) == 1)) {
+ if (likely(__get_user_pages_fast(address, 1, !ro, &page) == 1))
{
page_head = compound_head(page);
/*
* page_head is valid pointer but we must pin
