On Tue, Dec 10, 2013 at 1:57 PM, Linus Torvalds
<torva...@linux-foundation.org> wrote:
>
> So it looks like __get_user_pages_fast() fails, and keeps failing.
Hmm.. Is any of the addresses unchecked, perhaps?
__get_user_pages_fast() does an access_ok() check, while
get_user_pages_fast() does *not* seem to do one.
That looks a bit dangerous. Yeah, users should have checked the
address range, but there really is no reason not to do it in
get_user_pages_fast().
And it looks like the futex code is actually seriously buggered. It
only does the access_ok() check for the non-shared case.
Why?
Shouldn't we do something like the attached?
Linus
arch/x86/mm/gup.c | 4 +---
kernel/futex.c | 5 +++--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/x86/mm/gup.c b/arch/x86/mm/gup.c
index dd74e46828c0..61e726597599 100644
--- a/arch/x86/mm/gup.c
+++ b/arch/x86/mm/gup.c
@@ -326,10 +326,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages,
int write,
if (end < start)
goto slow_irqon;
-#ifdef CONFIG_X86_64
- if (end >> __VIRTUAL_MASK_SHIFT)
+ if (end > TASK_SIZE_MAX)
goto slow_irqon;
-#endif
/*
* XXX: batch / limit 'nr', to avoid large irq off latency
diff --git a/kernel/futex.c b/kernel/futex.c
index 80ba086f021d..23c12a35dca2 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);
