make 'user_access_begin()' do 'access_ok()'

commit 594cc251fdd0d231d342d88b2fdff4bc42fb0690 upstream.

Originally, the rule used to be that you'd have to do access_ok()
separately, and then user_access_begin() before actually doing the
direct (optimized) user access.

But experience has shown that people then decide not to do access_ok()
at all, and instead rely on it being implied by other operations or
similar.  Which makes it very hard to verify that the access has
actually been range-checked.

If you use the unsafe direct user accesses, hardware features (either
SMAP - Supervisor Mode Access Protection - on x86, or PAN - Privileged
Access Never - on ARM) do force you to use user_access_begin().  But
nothing really forces the range check.

By putting the range check into user_access_begin(), we actually force
people to do the right thing (tm), and the range check vill be visible
near the actual accesses.  We have way too long a history of people
trying to avoid them.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miles Chen <miles.chen@mediatek.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 9718303410614a8a503408b290d5617bc656d0c5..d871e424fa50737f3b1764f8c39507ab1f4da6f5 100644 (file)
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -711,7 +711,17 @@ extern struct movsl_mask {
  * checking before using them, but you have to surround them with the
  * user_access_begin/end() pair.
  */
-#define user_access_begin()    __uaccess_begin()
+static __must_check inline bool user_access_begin(int type,
+                                                 const void __user *ptr,
+                                                 size_t len)
+{
+       if (unlikely(!access_ok(type, ptr, len)))
+               return 0;
+       __uaccess_begin();
+       return 1;
+}
+
+#define user_access_begin(a, b, c)     user_access_begin(a, b, c)
 #define user_access_end()      __uaccess_end()
 
 #define unsafe_put_user(x, ptr, err_label)                                     \

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d99d05a91032e6cf97462f0a6ced5493488a8307..bf13299ebb5584ae821b1cc0f920bdee13bbf59b 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1566,7 +1566,9 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
                 * happened we would make the mistake of assuming that the
                 * relocations were valid.
                 */
-               user_access_begin();
+               if (!user_access_begin(VERIFY_WRITE, urelocs, size))
+                       goto end_user;
+
                for (copied = 0; copied < nreloc; copied++)
                        unsafe_put_user(-1,
                                        &urelocs[copied].presumed_offset,
@@ -2601,6 +2603,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
        struct drm_i915_gem_execbuffer2 *args = data;
        struct drm_i915_gem_exec_object2 *exec2_list;
        struct drm_syncobj **fences = NULL;
+       const size_t count = args->buffer_count;
        int err;
 
        if (args->buffer_count < 1 || args->buffer_count > SIZE_MAX / sz - 1) {
@@ -2649,7 +2652,17 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
                unsigned int i;
 
                /* Copy the new buffer offsets back to the user's exec list. */
-               user_access_begin();
+               /*
+                * Note: count * sizeof(*user_exec_list) does not overflow,
+                * because we checked 'count' in check_buffer_count().
+                *
+                * And this range already got effectively checked earlier
+                * when we did the "copy_from_user()" above.
+                */
+               if (!user_access_begin(VERIFY_WRITE, user_exec_list,
+                                      count * sizeof(*user_exec_list)))
+                       goto end_user;
+
                for (i = 0; i < args->buffer_count; i++) {
                        if (!(exec2_list[i].offset & UPDATE))
                                continue;

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 251e655d407fadfb43f604c032c2b026a9a1fc38..ec4f0053d6d8ebafee1383a22010d96b6b057ff2 100644 (file)
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -267,7 +267,7 @@ extern long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count);
        probe_kernel_read(&retval, addr, sizeof(retval))
 
 #ifndef user_access_begin
-#define user_access_begin() do { } while (0)
+#define user_access_begin(type, ptr, len) access_ok(type, ptr, len)
 #define user_access_end() do { } while (0)
 #define unsafe_get_user(x, ptr, err) do { if (unlikely(__get_user(x, ptr))) goto err; } while (0)
 #define unsafe_put_user(x, ptr, err) do { if (unlikely(__put_user(x, ptr))) goto err; } while (0)

diff --git a/kernel/compat.c b/kernel/compat.c
index 7e83733d4c95c854c1a4acf2b8b7a96bf91c0d59..45ae3ace49c294ae086869627cdf2bed7186bdd9 100644 (file)
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -437,10 +437,9 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
        bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
        nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
 
-       if (!access_ok(VERIFY_READ, umask, bitmap_size / 8))
+       if (!user_access_begin(VERIFY_READ, umask, bitmap_size / 8))
                return -EFAULT;
 
-       user_access_begin();
        while (nr_compat_longs > 1) {
                compat_ulong_t l1, l2;
                unsafe_get_user(l1, umask++, Efault);
@@ -467,10 +466,9 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
        bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
        nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
 
-       if (!access_ok(VERIFY_WRITE, umask, bitmap_size / 8))
+       if (!user_access_begin(VERIFY_WRITE, umask, bitmap_size / 8))
                return -EFAULT;
 
-       user_access_begin();
        while (nr_compat_longs > 1) {
                unsigned long m = *mask++;
                unsafe_put_user((compat_ulong_t)m, umask++, Efault);

diff --git a/kernel/exit.c b/kernel/exit.c
index d1baf9c96c3e6d27b5c8766d96de4f6a06ad398f..7bee1dd596fa02ebc72f6c29ffb12d5a81b09a45 100644 (file)
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1597,10 +1597,9 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
        if (!infop)
                return err;
 
-       if (!access_ok(VERIFY_WRITE, infop, sizeof(*infop)))
+       if (!user_access_begin(VERIFY_WRITE, infop, sizeof(*infop)))
                return -EFAULT;
 
-       user_access_begin();
        unsafe_put_user(signo, &infop->si_signo, Efault);
        unsafe_put_user(0, &infop->si_errno, Efault);
        unsafe_put_user(info.cause, &infop->si_code, Efault);
@@ -1725,10 +1724,9 @@ COMPAT_SYSCALL_DEFINE5(waitid,
        if (!infop)
                return err;
 
-       if (!access_ok(VERIFY_WRITE, infop, sizeof(*infop)))
+       if (!user_access_begin(VERIFY_WRITE, infop, sizeof(*infop)))
                return -EFAULT;
 
-       user_access_begin();
        unsafe_put_user(signo, &infop->si_signo, Efault);
        unsafe_put_user(0, &infop->si_errno, Efault);
        unsafe_put_user(info.cause, &infop->si_code, Efault);

diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index e304b54c9c7dd3a994ec98857503f1d49259f148..b8570a11776d3fdbdc6938f5fa95ae285fe72d88 100644 (file)
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -115,10 +115,11 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
 
                kasan_check_write(dst, count);
                check_object_size(dst, count, false);
-               user_access_begin();
-               retval = do_strncpy_from_user(dst, src, count, max);
-               user_access_end();
-               return retval;
+               if (user_access_begin(VERIFY_READ, src, max)) {
+                       retval = do_strncpy_from_user(dst, src, count, max);
+                       user_access_end();
+                       return retval;
+               }
        }
        return -EFAULT;
 }

diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 184f80f7bacfa96ce500f9ad40cb11aeaea06627..f5fa5b266ea27695c9c32d80c224f5701a796537 100644 (file)
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -114,10 +114,11 @@ long strnlen_user(const char __user *str, long count)
                unsigned long max = max_addr - src_addr;
                long retval;
 
-               user_access_begin();
-               retval = do_strnlen_user(str, count, max);
-               user_access_end();
-               return retval;
+               if (user_access_begin(VERIFY_READ, str, max)) {
+                       retval = do_strnlen_user(str, count, max);
+                       user_access_end();
+                       return retval;
+               }
        }
        return 0;
 }