Commit 1d8f739b authored by Christophe Leroy's avatar Christophe Leroy Committed by Michael Ellerman

powerpc/kuap: Fix set direction in allow/prevent_user_access()

__builtin_constant_p() always return 0 for pointers, so on RADIX
we always end up opening both direction (by writing 0 in SPR29):

  0000000000000170 <._copy_to_user>:
  ...
   1b0:	4c 00 01 2c 	isync
   1b4:	39 20 00 00 	li      r9,0
   1b8:	7d 3d 03 a6 	mtspr   29,r9
   1bc:	4c 00 01 2c 	isync
   1c0:	48 00 00 01 	bl      1c0 <._copy_to_user+0x50>
  			1c0: R_PPC64_REL24	.__copy_tofrom_user
  ...
  0000000000000220 <._copy_from_user>:
  ...
   2ac:	4c 00 01 2c 	isync
   2b0:	39 20 00 00 	li      r9,0
   2b4:	7d 3d 03 a6 	mtspr   29,r9
   2b8:	4c 00 01 2c 	isync
   2bc:	7f c5 f3 78 	mr      r5,r30
   2c0:	7f 83 e3 78 	mr      r3,r28
   2c4:	48 00 00 01 	bl      2c4 <._copy_from_user+0xa4>
  			2c4: R_PPC64_REL24	.__copy_tofrom_user
  ...

Use an explicit parameter for direction selection, so that GCC
is able to see it is a constant:

  00000000000001b0 <._copy_to_user>:
  ...
   1f0:	4c 00 01 2c 	isync
   1f4:	3d 20 40 00 	lis     r9,16384
   1f8:	79 29 07 c6 	rldicr  r9,r9,32,31
   1fc:	7d 3d 03 a6 	mtspr   29,r9
   200:	4c 00 01 2c 	isync
   204:	48 00 00 01 	bl      204 <._copy_to_user+0x54>
  			204: R_PPC64_REL24	.__copy_tofrom_user
  ...
  0000000000000260 <._copy_from_user>:
  ...
   2ec:	4c 00 01 2c 	isync
   2f0:	39 20 ff ff 	li      r9,-1
   2f4:	79 29 00 04 	rldicr  r9,r9,0,0
   2f8:	7d 3d 03 a6 	mtspr   29,r9
   2fc:	4c 00 01 2c 	isync
   300:	7f c5 f3 78 	mr      r5,r30
   304:	7f 83 e3 78 	mr      r3,r28
   308:	48 00 00 01 	bl      308 <._copy_from_user+0xa8>
  			308: R_PPC64_REL24	.__copy_tofrom_user
  ...
Signed-off-by: default avatarChristophe Leroy <christophe.leroy@c-s.fr>
[mpe: Spell out the directions, s/KUAP_R/KUAP_READ/ etc.]
Signed-off-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/f4e88ec4941d5facb35ce75026b0112f980086c3.1579866752.git.christophe.leroy@c-s.fr
parent 6ec20aa2
...@@ -102,11 +102,13 @@ static inline void kuap_update_sr(u32 sr, u32 addr, u32 end) ...@@ -102,11 +102,13 @@ static inline void kuap_update_sr(u32 sr, u32 addr, u32 end)
isync(); /* Context sync required after mtsrin() */ isync(); /* Context sync required after mtsrin() */
} }
static inline void allow_user_access(void __user *to, const void __user *from, u32 size) static __always_inline void allow_user_access(void __user *to, const void __user *from,
u32 size, unsigned long dir)
{ {
u32 addr, end; u32 addr, end;
if (__builtin_constant_p(to) && to == NULL) BUILD_BUG_ON(!__builtin_constant_p(dir));
if (!(dir & KUAP_WRITE))
return; return;
addr = (__force u32)to; addr = (__force u32)to;
...@@ -119,11 +121,16 @@ static inline void allow_user_access(void __user *to, const void __user *from, u ...@@ -119,11 +121,16 @@ static inline void allow_user_access(void __user *to, const void __user *from, u
kuap_update_sr(mfsrin(addr) & ~SR_KS, addr, end); /* Clear Ks */ kuap_update_sr(mfsrin(addr) & ~SR_KS, addr, end); /* Clear Ks */
} }
static inline void prevent_user_access(void __user *to, const void __user *from, u32 size) static __always_inline void prevent_user_access(void __user *to, const void __user *from,
u32 size, unsigned long dir)
{ {
u32 addr = (__force u32)to; u32 addr = (__force u32)to;
u32 end = min(addr + size, TASK_SIZE); u32 end = min(addr + size, TASK_SIZE);
BUILD_BUG_ON(!__builtin_constant_p(dir));
if (!(dir & KUAP_WRITE))
return;
if (!addr || addr >= TASK_SIZE || !size) if (!addr || addr >= TASK_SIZE || !size)
return; return;
......
...@@ -77,20 +77,21 @@ static inline void set_kuap(unsigned long value) ...@@ -77,20 +77,21 @@ static inline void set_kuap(unsigned long value)
isync(); isync();
} }
static inline void allow_user_access(void __user *to, const void __user *from, static __always_inline void allow_user_access(void __user *to, const void __user *from,
unsigned long size) unsigned long size, unsigned long dir)
{ {
// This is written so we can resolve to a single case at build time // This is written so we can resolve to a single case at build time
if (__builtin_constant_p(to) && to == NULL) BUILD_BUG_ON(!__builtin_constant_p(dir));
if (dir == KUAP_READ)
set_kuap(AMR_KUAP_BLOCK_WRITE); set_kuap(AMR_KUAP_BLOCK_WRITE);
else if (__builtin_constant_p(from) && from == NULL) else if (dir == KUAP_WRITE)
set_kuap(AMR_KUAP_BLOCK_READ); set_kuap(AMR_KUAP_BLOCK_READ);
else else
set_kuap(0); set_kuap(0);
} }
static inline void prevent_user_access(void __user *to, const void __user *from, static inline void prevent_user_access(void __user *to, const void __user *from,
unsigned long size) unsigned long size, unsigned long dir)
{ {
set_kuap(AMR_KUAP_BLOCKED); set_kuap(AMR_KUAP_BLOCKED);
} }
......
...@@ -2,6 +2,10 @@ ...@@ -2,6 +2,10 @@
#ifndef _ASM_POWERPC_KUP_H_ #ifndef _ASM_POWERPC_KUP_H_
#define _ASM_POWERPC_KUP_H_ #define _ASM_POWERPC_KUP_H_
#define KUAP_READ 1
#define KUAP_WRITE 2
#define KUAP_READ_WRITE (KUAP_READ | KUAP_WRITE)
#ifdef CONFIG_PPC64 #ifdef CONFIG_PPC64
#include <asm/book3s/64/kup-radix.h> #include <asm/book3s/64/kup-radix.h>
#endif #endif
...@@ -42,9 +46,9 @@ void setup_kuap(bool disabled); ...@@ -42,9 +46,9 @@ void setup_kuap(bool disabled);
#else #else
static inline void setup_kuap(bool disabled) { } static inline void setup_kuap(bool disabled) { }
static inline void allow_user_access(void __user *to, const void __user *from, static inline void allow_user_access(void __user *to, const void __user *from,
unsigned long size) { } unsigned long size, unsigned long dir) { }
static inline void prevent_user_access(void __user *to, const void __user *from, static inline void prevent_user_access(void __user *to, const void __user *from,
unsigned long size) { } unsigned long size, unsigned long dir) { }
static inline bool static inline bool
bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write) bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
{ {
...@@ -54,24 +58,36 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write) ...@@ -54,24 +58,36 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
static inline void allow_read_from_user(const void __user *from, unsigned long size) static inline void allow_read_from_user(const void __user *from, unsigned long size)
{ {
allow_user_access(NULL, from, size); allow_user_access(NULL, from, size, KUAP_READ);
} }
static inline void allow_write_to_user(void __user *to, unsigned long size) static inline void allow_write_to_user(void __user *to, unsigned long size)
{ {
allow_user_access(to, NULL, size); allow_user_access(to, NULL, size, KUAP_WRITE);
}
static inline void allow_read_write_user(void __user *to, const void __user *from,
unsigned long size)
{
allow_user_access(to, from, size, KUAP_READ_WRITE);
} }
static inline void prevent_read_from_user(const void __user *from, unsigned long size) static inline void prevent_read_from_user(const void __user *from, unsigned long size)
{ {
prevent_user_access(NULL, from, size); prevent_user_access(NULL, from, size, KUAP_READ);
} }
static inline void prevent_write_to_user(void __user *to, unsigned long size) static inline void prevent_write_to_user(void __user *to, unsigned long size)
{ {
prevent_user_access(to, NULL, size); prevent_user_access(to, NULL, size, KUAP_WRITE);
}
static inline void prevent_read_write_user(void __user *to, const void __user *from,
unsigned long size)
{
prevent_user_access(to, from, size, KUAP_READ_WRITE);
} }
#endif /* !__ASSEMBLY__ */ #endif /* !__ASSEMBLY__ */
#endif /* _ASM_POWERPC_KUP_H_ */ #endif /* _ASM_POWERPC_KUAP_H_ */
...@@ -35,13 +35,13 @@ ...@@ -35,13 +35,13 @@
#include <asm/reg.h> #include <asm/reg.h>
static inline void allow_user_access(void __user *to, const void __user *from, static inline void allow_user_access(void __user *to, const void __user *from,
unsigned long size) unsigned long size, unsigned long dir)
{ {
mtspr(SPRN_MD_AP, MD_APG_INIT); mtspr(SPRN_MD_AP, MD_APG_INIT);
} }
static inline void prevent_user_access(void __user *to, const void __user *from, static inline void prevent_user_access(void __user *to, const void __user *from,
unsigned long size) unsigned long size, unsigned long dir)
{ {
mtspr(SPRN_MD_AP, MD_APG_KUAP); mtspr(SPRN_MD_AP, MD_APG_KUAP);
} }
......
...@@ -313,9 +313,9 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n) ...@@ -313,9 +313,9 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
unsigned long ret; unsigned long ret;
barrier_nospec(); barrier_nospec();
allow_user_access(to, from, n); allow_read_write_user(to, from, n);
ret = __copy_tofrom_user(to, from, n); ret = __copy_tofrom_user(to, from, n);
prevent_user_access(to, from, n); prevent_read_write_user(to, from, n);
return ret; return ret;
} }
#endif /* __powerpc64__ */ #endif /* __powerpc64__ */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment