powerpc/uaccess: Perform barrier_nospec() in KUAP allowance helpers
authorChristophe Leroy <christophe.leroy@csgroup.eu>
Sun, 7 Feb 2021 10:08:11 +0000 (10:08 +0000)
committerMichael Ellerman <mpe@ellerman.id.au>
Mon, 8 Feb 2021 14:10:15 +0000 (01:10 +1100)
barrier_nospec() in uaccess helpers is there to protect against
speculative accesses around access_ok().

When using user_access_begin() sequences together with
unsafe_get_user() like macros, barrier_nospec() is called for
every single read although we know the access_ok() is done
onece.

Since all user accesses must be granted by a call to either
allow_read_from_user() or allow_read_write_user() which will
always happen after the access_ok() check, move the barrier_nospec()
there.

Reported-by: Christopher M. Riedl <cmr@codefail.de>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/c72f014730823b413528e90ab6c4d3bcb79f8497.1612692067.git.christophe.leroy@csgroup.eu
arch/powerpc/include/asm/kup.h
arch/powerpc/include/asm/uaccess.h

index bf221a2..7ec21af 100644 (file)
@@ -91,6 +91,7 @@ static __always_inline void setup_kup(void)
 
 static inline void allow_read_from_user(const void __user *from, unsigned long size)
 {
+       barrier_nospec();
        allow_user_access(NULL, from, size, KUAP_READ);
 }
 
@@ -102,6 +103,7 @@ static inline void allow_write_to_user(void __user *to, unsigned long size)
 static inline void allow_read_write_user(void __user *to, const void __user *from,
                                         unsigned long size)
 {
+       barrier_nospec();
        allow_user_access(to, from, size, KUAP_READ_WRITE);
 }
 
index 501c9a7..46123ae 100644 (file)
@@ -315,7 +315,6 @@ do {                                                                \
        __chk_user_ptr(__gu_addr);                              \
        if (!is_kernel_addr((unsigned long)__gu_addr))          \
                might_fault();                                  \
-       barrier_nospec();                                       \
        if (do_allow)                                                           \
                __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err);      \
        else                                                                    \
@@ -333,10 +332,8 @@ do {                                                               \
        __typeof__(size) __gu_size = (size);                            \
                                                                        \
        might_fault();                                                  \
-       if (access_ok(__gu_addr, __gu_size)) {                          \
-               barrier_nospec();                                       \
+       if (access_ok(__gu_addr, __gu_size))                            \
                __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
-       }                                                               \
        (x) = (__force __typeof__(*(ptr)))__gu_val;                             \
                                                                        \
        __gu_err;                                                       \
@@ -350,7 +347,6 @@ do {                                                                \
        __typeof__(size) __gu_size = (size);                    \
                                                                \
        __chk_user_ptr(__gu_addr);                              \
-       barrier_nospec();                                       \
        __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
        (x) = (__force __typeof__(*(ptr)))__gu_val;                     \
                                                                \
@@ -395,7 +391,6 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
 {
        unsigned long ret;
 
-       barrier_nospec();
        allow_read_write_user(to, from, n);
        ret = __copy_tofrom_user(to, from, n);
        prevent_read_write_user(to, from, n);
@@ -412,19 +407,15 @@ static inline unsigned long raw_copy_from_user(void *to,
 
                switch (n) {
                case 1:
-                       barrier_nospec();
                        __get_user_size(*(u8 *)to, from, 1, ret);
                        break;
                case 2:
-                       barrier_nospec();
                        __get_user_size(*(u16 *)to, from, 2, ret);
                        break;
                case 4:
-                       barrier_nospec();
                        __get_user_size(*(u32 *)to, from, 4, ret);
                        break;
                case 8:
-                       barrier_nospec();
                        __get_user_size(*(u64 *)to, from, 8, ret);
                        break;
                }
@@ -432,7 +423,6 @@ static inline unsigned long raw_copy_from_user(void *to,
                        return 0;
        }
 
-       barrier_nospec();
        allow_read_from_user(from, n);
        ret = __copy_tofrom_user((__force void __user *)to, from, n);
        prevent_read_from_user(from, n);