Re: [PATCH kernel] powerpc/kuap: Restore AMR after replaying soft interrupts

2021-02-11 Thread Michael Ellerman 
On Tue, 2 Feb 2021 20:15:41 +1100, Alexey Kardashevskiy wrote:
> Since de78a9c "powerpc: Add a framework for Kernel Userspace Access
> Protection", user access helpers call user_{read|write}_access_{begin|end}
> when user space access is allowed.
> 
> 890274c "powerpc/64s: Implement KUAP for Radix MMU" made the mentioned
> helpers program a AMR special register to allow such access for a short
> period of time, most of the time AMR is expected to block user memory
> access by the kernel.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/kuap: Restore AMR after replaying soft interrupts
  https://git.kernel.org/powerpc/c/60a707d0c99aff4eadb7fd334c5fd21df386723e

cheers

Re: [PATCH kernel] powerpc/kuap: Restore AMR after replaying soft interrupts

2021-02-03 Thread Michal Suchánek 
Hello,

On Tue, Feb 02, 2021 at 08:15:41PM +1100, Alexey Kardashevskiy wrote:
> Since de78a9c "powerpc: Add a framework for Kernel Userspace Access
> Protection", user access helpers call user_{read|write}_access_{begin|end}
> when user space access is allowed.
> 
> 890274c "powerpc/64s: Implement KUAP for Radix MMU" made the mentioned
> helpers program a AMR special register to allow such access for a short
> period of time, most of the time AMR is expected to block user memory
> access by the kernel.
> 
> Since the code accesses the user space memory, unsafe_get_user()
> calls might_fault() which calls arch_local_irq_restore() if either
> CONFIG_PROVE_LOCKING or CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
> arch_local_irq_restore() then attempts to replay pending soft interrupts
> as KUAP regions have hardware interrupts enabled.
> If a pending interrupt happens to do user access (performance interrupts
> do that), it enables access for a short period of time so after returning
> from the replay, the user access state remains blocked and if a user page
> fault happens - "Bug: Read fault blocked by AMR!" appears and SIGSEGV is
> sent.
> 
> This saves/restores AMR when replaying interrupts.
> 
> This adds a check if AMR was not blocked when before replaying interrupts.
> 
> Found by syzkaller. The call stack for the bug is:
> 
> copy_from_user_nofault+0xf8/0x250
> perf_callchain_user_64+0x3d8/0x8d0
> perf_callchain_user+0x38/0x50
> get_perf_callchain+0x28c/0x300
> perf_callchain+0xb0/0x130
> perf_prepare_sample+0x364/0xbf0
> perf_event_output_forward+0xe0/0x280
> __perf_event_overflow+0xa4/0x240
> perf_swevent_hrtimer+0x1d4/0x1f0
> __hrtimer_run_queues+0x328/0x900
> hrtimer_interrupt+0x128/0x350
> timer_interrupt+0x180/0x600
> replay_soft_interrupts+0x21c/0x4f0
> arch_local_irq_restore+0x94/0x150
> lock_is_held_type+0x140/0x200
> ___might_sleep+0x220/0x330
> __might_fault+0x88/0x120
> do_strncpy_from_user+0x108/0x2b0
> strncpy_from_user+0x1d0/0x2a0
> getname_flags+0x88/0x2c0
> do_sys_openat2+0x2d4/0x5f0
> do_sys_open+0xcc/0x140
> system_call_exception+0x160/0x240
> system_call_common+0xf0/0x27c
> 
Can we get a Fixes tag?

Thanks

Michal
> Signed-off-by: Alexey Kardashevskiy 
> Reviewed-by: Nicholas Piggin 
> ---
> Changes:
> v3:
> * do not block/unblock if AMR was blocked
> * reverted move of AMR_KUAP_***
> * added pr_warn
> 
> v2:
> * fixed compile on hash
> * moved get/set to arch_local_irq_restore
> * block KUAP before replaying
> 
> ---
> 
> This is an example:
> 
> [ cut here ]
> Bug: Read fault blocked by AMR!
> WARNING: CPU: 0 PID: 1603 at 
> /home/aik/p/kernel/arch/powerpc/include/asm/book3s/64/kup-radix.h:145 
> __do_page_fau
> 
> Modules linked in:
> CPU: 0 PID: 1603 Comm: amr Not tainted 5.10.0-rc6_v5.10-rc6_a+fstn1 #24
> NIP:  c009ece8 LR: c009ece4 CTR: 
> REGS: cdc63560 TRAP: 0700   Not tainted  
> (5.10.0-rc6_v5.10-rc6_a+fstn1)
> MSR:  80021033   CR: 28002888  XER: 2004
> CFAR: c01fa928 IRQMASK: 1
> GPR00: c009ece4 cdc637f0 c2397600 001f
> GPR04: c20eb318  cdc63494 0027
> GPR08: c0007fe4de68 cdfe9180  0001
> GPR12: 2000 c30a  
> GPR16:    bfff
> GPR20:  c000134a4020 c19c2218 0fe0
> GPR24:   cd106200 4000
> GPR28:  0300 cdc63910 c1946730
> NIP [c009ece8] __do_page_fault+0xb38/0xde0
> LR [c009ece4] __do_page_fault+0xb34/0xde0
> Call Trace:
> [cdc637f0] [c009ece4] __do_page_fault+0xb34/0xde0 (unreliable)
> [cdc638a0] [c000c968] handle_page_fault+0x10/0x2c
> --- interrupt: 300 at strncpy_from_user+0x290/0x440
> LR = strncpy_from_user+0x284/0x440
> [cdc63ba0] [c0c3dcb0] strncpy_from_user+0x2f0/0x440 
> (unreliable)
> [cdc63c30] [c068b888] getname_flags+0x88/0x2c0
> [cdc63c90] [c0662a44] do_sys_openat2+0x2d4/0x5f0
> [cdc63d30] [c066560c] do_sys_open+0xcc/0x140
> [cdc63dc0] [c0045e10] system_call_exception+0x160/0x240
> [cdc63e20] [c000da60] system_call_common+0xf0/0x27c
> Instruction dump:
> 409c0048 3fe2ff5b 3bfff128 fac10060 fae10068 482f7a85 6000 3c62ff5b
> 7fe4fb78 3863f250 4815bbd9 6000 <0fe0> 3c62ff5b 3863f2b8 4815c8b5
> irq event stamp: 254
> hardirqs last  enabled at (253): [] 
> arch_local_irq_restore+0xa0/0x150
> hardirqs last disabled at (254): [] 
> data_access_common_virt+0x1b0/0x1d0
> softirqs last  enabled at (0): [] copy_process+0x78c/0x2120
> softirqs last disabled at (0): [<>] 0x0
> ---[ end trace ba98aec5151f3aeb ]---
> ---
>  arch/powerpc/kernel/irq.c | 27

[PATCH kernel] powerpc/kuap: Restore AMR after replaying soft interrupts

2021-02-02 Thread Alexey Kardashevskiy 
Since de78a9c "powerpc: Add a framework for Kernel Userspace Access
Protection", user access helpers call user_{read|write}_access_{begin|end}
when user space access is allowed.

890274c "powerpc/64s: Implement KUAP for Radix MMU" made the mentioned
helpers program a AMR special register to allow such access for a short
period of time, most of the time AMR is expected to block user memory
access by the kernel.

Since the code accesses the user space memory, unsafe_get_user()
calls might_fault() which calls arch_local_irq_restore() if either
CONFIG_PROVE_LOCKING or CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
arch_local_irq_restore() then attempts to replay pending soft interrupts
as KUAP regions have hardware interrupts enabled.
If a pending interrupt happens to do user access (performance interrupts
do that), it enables access for a short period of time so after returning
from the replay, the user access state remains blocked and if a user page
fault happens - "Bug: Read fault blocked by AMR!" appears and SIGSEGV is
sent.

This saves/restores AMR when replaying interrupts.

This adds a check if AMR was not blocked when before replaying interrupts.

Found by syzkaller. The call stack for the bug is:

copy_from_user_nofault+0xf8/0x250
perf_callchain_user_64+0x3d8/0x8d0
perf_callchain_user+0x38/0x50
get_perf_callchain+0x28c/0x300
perf_callchain+0xb0/0x130
perf_prepare_sample+0x364/0xbf0
perf_event_output_forward+0xe0/0x280
__perf_event_overflow+0xa4/0x240
perf_swevent_hrtimer+0x1d4/0x1f0
__hrtimer_run_queues+0x328/0x900
hrtimer_interrupt+0x128/0x350
timer_interrupt+0x180/0x600
replay_soft_interrupts+0x21c/0x4f0
arch_local_irq_restore+0x94/0x150
lock_is_held_type+0x140/0x200
___might_sleep+0x220/0x330
__might_fault+0x88/0x120
do_strncpy_from_user+0x108/0x2b0
strncpy_from_user+0x1d0/0x2a0
getname_flags+0x88/0x2c0
do_sys_openat2+0x2d4/0x5f0
do_sys_open+0xcc/0x140
system_call_exception+0x160/0x240
system_call_common+0xf0/0x27c

Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: Nicholas Piggin 
---
Changes:
v3:
* do not block/unblock if AMR was blocked
* reverted move of AMR_KUAP_***
* added pr_warn

v2:
* fixed compile on hash
* moved get/set to arch_local_irq_restore
* block KUAP before replaying

---

This is an example:

[ cut here ]
Bug: Read fault blocked by AMR!
WARNING: CPU: 0 PID: 1603 at 
/home/aik/p/kernel/arch/powerpc/include/asm/book3s/64/kup-radix.h:145 
__do_page_fau

Modules linked in:
CPU: 0 PID: 1603 Comm: amr Not tainted 5.10.0-rc6_v5.10-rc6_a+fstn1 #24
NIP:  c009ece8 LR: c009ece4 CTR: 
REGS: cdc63560 TRAP: 0700   Not tainted  (5.10.0-rc6_v5.10-rc6_a+fstn1)
MSR:  80021033   CR: 28002888  XER: 2004
CFAR: c01fa928 IRQMASK: 1
GPR00: c009ece4 cdc637f0 c2397600 001f
GPR04: c20eb318  cdc63494 0027
GPR08: c0007fe4de68 cdfe9180  0001
GPR12: 2000 c30a  
GPR16:    bfff
GPR20:  c000134a4020 c19c2218 0fe0
GPR24:   cd106200 4000
GPR28:  0300 cdc63910 c1946730
NIP [c009ece8] __do_page_fault+0xb38/0xde0
LR [c009ece4] __do_page_fault+0xb34/0xde0
Call Trace:
[cdc637f0] [c009ece4] __do_page_fault+0xb34/0xde0 (unreliable)
[cdc638a0] [c000c968] handle_page_fault+0x10/0x2c
--- interrupt: 300 at strncpy_from_user+0x290/0x440
LR = strncpy_from_user+0x284/0x440
[cdc63ba0] [c0c3dcb0] strncpy_from_user+0x2f0/0x440 (unreliable)
[cdc63c30] [c068b888] getname_flags+0x88/0x2c0
[cdc63c90] [c0662a44] do_sys_openat2+0x2d4/0x5f0
[cdc63d30] [c066560c] do_sys_open+0xcc/0x140
[cdc63dc0] [c0045e10] system_call_exception+0x160/0x240
[cdc63e20] [c000da60] system_call_common+0xf0/0x27c
Instruction dump:
409c0048 3fe2ff5b 3bfff128 fac10060 fae10068 482f7a85 6000 3c62ff5b
7fe4fb78 3863f250 4815bbd9 6000 <0fe0> 3c62ff5b 3863f2b8 4815c8b5
irq event stamp: 254
hardirqs last  enabled at (253): [] 
arch_local_irq_restore+0xa0/0x150
hardirqs last disabled at (254): [] 
data_access_common_virt+0x1b0/0x1d0
softirqs last  enabled at (0): [] copy_process+0x78c/0x2120
softirqs last disabled at (0): [<>] 0x0
---[ end trace ba98aec5151f3aeb ]---
---
 arch/powerpc/kernel/irq.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index cc7a6271b6b4..592abc798826 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -269,6 +269,23 @@ void replay_soft_interrupts(void)
}
 }
 
+#if defined(CONFIG_PPC_BOOK3S_64) &&

Re: [PATCH kernel] powerpc/kuap: Restore AMR after replaying soft interrupts

2020-12-02 Thread kernel test robot 
Hi Alexey,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on linus/master v5.10-rc6 next-20201201]
[cannot apply to scottwood/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Alexey-Kardashevskiy/powerpc-kuap-Restore-AMR-after-replaying-soft-interrupts/20201202-094132
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r024-20201202 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 
2671fccf0381769276ca8246ec0499adcb9b0355)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc64 cross compiling tool for clang build
# apt-get install binutils-powerpc64-linux-gnu
# 
https://github.com/0day-ci/linux/commit/6b38a9b10a8384beeaa820e1c935cc4cabdb951e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Alexey-Kardashevskiy/powerpc-kuap-Restore-AMR-after-replaying-soft-interrupts/20201202-094132
git checkout 6b38a9b10a8384beeaa820e1c935cc4cabdb951e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross 
ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/kernel/irq.c:31:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:10:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:604:
   arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer 
arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c),
   ^~~
   arch/powerpc/include/asm/io.h:601:3: note: expanded from macro 
'DEF_PCI_AC_NORET'
   __do_##name al; \
   ^~
   :100:1: note: expanded from here
   __do_insw
   ^
   arch/powerpc/include/asm/io.h:542:56: note: expanded from macro '__do_insw'
   #define __do_insw(p, b, n)  readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
  ~^
   In file included from arch/powerpc/kernel/irq.c:31:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:10:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:604:
   arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer 
arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
   ^~~
   arch/powerpc/include/asm/io.h:601:3: note: expanded from macro 
'DEF_PCI_AC_NORET'
   __do_##name al; \
   ^~
   :102:1: note: expanded from here
   __do_insl
   ^
   arch/powerpc/include/asm/io.h:543:56: note: expanded from macro '__do_insl'
   #define __do_insl(p, b, n)  readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
  ~^
   In file included from arch/powerpc/kernel/irq.c:31:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:10:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:604:
   arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer 
arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
   ^~
   arch/powerpc/include/asm/io.h:601:3: note: expanded from macro 
'DEF_PCI_AC_NORET'
   __do_##name al;

[PATCH kernel] powerpc/kuap: Restore AMR after replaying soft interrupts

2020-12-01 Thread Alexey Kardashevskiy 
When interrupted in raw_copy_from_user()/... after user memory access
is enabled, a nested handler may also access user memory (perf is
one example) and when it does so, it calls prevent_read_from_user()
which prevents the upper handler from accessing user memory.

This saves/restores AMR when replaying interrupts. get_kuap/set_kuap have
stubs for disabled KUAP so no ifdefs.

Found by syzkaller.

Signed-off-by: Alexey Kardashevskiy 
---

This is an example:

[ cut here ]
Bug: Read fault blocked by AMR!
WARNING: CPU: 0 PID: 1603 at 
/home/aik/p/kernel/arch/powerpc/include/asm/book3s/64/kup-radix.h:145 
__do_page_fau

Modules linked in:
CPU: 0 PID: 1603 Comm: amr Not tainted 5.10.0-rc6_v5.10-rc6_a+fstn1 #24
NIP:  c009ece8 LR: c009ece4 CTR: 
REGS: cdc63560 TRAP: 0700   Not tainted  (5.10.0-rc6_v5.10-rc6_a+fstn1)
MSR:  80021033   CR: 28002888  XER: 2004
CFAR: c01fa928 IRQMASK: 1
GPR00: c009ece4 cdc637f0 c2397600 001f
GPR04: c20eb318  cdc63494 0027
GPR08: c0007fe4de68 cdfe9180  0001
GPR12: 2000 c30a  
GPR16:    bfff
GPR20:  c000134a4020 c19c2218 0fe0
GPR24:   cd106200 4000
GPR28:  0300 cdc63910 c1946730
NIP [c009ece8] __do_page_fault+0xb38/0xde0
LR [c009ece4] __do_page_fault+0xb34/0xde0
Call Trace:
[cdc637f0] [c009ece4] __do_page_fault+0xb34/0xde0 (unreliable)
[cdc638a0] [c000c968] handle_page_fault+0x10/0x2c
--- interrupt: 300 at strncpy_from_user+0x290/0x440
LR = strncpy_from_user+0x284/0x440
[cdc63ba0] [c0c3dcb0] strncpy_from_user+0x2f0/0x440 (unreliable)
[cdc63c30] [c068b888] getname_flags+0x88/0x2c0
[cdc63c90] [c0662a44] do_sys_openat2+0x2d4/0x5f0
[cdc63d30] [c066560c] do_sys_open+0xcc/0x140
[cdc63dc0] [c0045e10] system_call_exception+0x160/0x240
[cdc63e20] [c000da60] system_call_common+0xf0/0x27c
Instruction dump:
409c0048 3fe2ff5b 3bfff128 fac10060 fae10068 482f7a85 6000 3c62ff5b
7fe4fb78 3863f250 4815bbd9 6000 <0fe0> 3c62ff5b 3863f2b8 4815c8b5
irq event stamp: 254
hardirqs last  enabled at (253): [] 
arch_local_irq_restore+0xa0/0x150
hardirqs last disabled at (254): [] 
data_access_common_virt+0x1b0/0x1d0
softirqs last  enabled at (0): [] copy_process+0x78c/0x2120
softirqs last disabled at (0): [<>] 0x0
---[ end trace ba98aec5151f3aeb ]---
---
 arch/powerpc/kernel/irq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 7d0f7682d01d..915123d861d0 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -221,6 +221,7 @@ void replay_soft_interrupts(void)
 */
unsigned char happened = local_paca->irq_happened;
struct pt_regs regs;
+   unsigned long kuap_state = get_kuap();
 
ppc_save_regs();
regs.softe = IRQS_ENABLED;
@@ -309,6 +310,7 @@ void replay_soft_interrupts(void)
trace_hardirqs_off();
goto again;
}
+   set_kuap(kuap_state);
 }
 
 notrace void arch_local_irq_restore(unsigned long mask)
-- 
2.17.1