diff mbox series

powerpc/uaccess: Perform barrier_nospec() in KUAP allowance helpers

Message ID c72f014730823b413528e90ab6c4d3bcb79f8497.1612692067.git.christophe.leroy@csgroup.eu (mailing list archive)
State Accepted
Headers show
Series powerpc/uaccess: Perform barrier_nospec() in KUAP allowance helpers | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (44158b256b30415079588d0fcb1bccbdc2ccd009)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 72 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Christophe Leroy Feb. 7, 2021, 10:08 a.m. UTC
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>
---
 arch/powerpc/include/asm/kup.h     |  2 ++
 arch/powerpc/include/asm/uaccess.h | 12 +-----------
 2 files changed, 3 insertions(+), 11 deletions(-)

Comments

Michael Ellerman Feb. 10, 2021, 12:57 p.m. UTC | #1
On Sun, 7 Feb 2021 10:08:11 +0000 (UTC), Christophe Leroy wrote:
> 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.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/uaccess: Perform barrier_nospec() in KUAP allowance helpers
      https://git.kernel.org/powerpc/c/8524e2e76441fc615a3b5c1415823e051cc79eae

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index bf221a2a523e..7ec21af49a45 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -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);
 }
 
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 501c9a79038c..46123ae6a4c9 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -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);