Message ID | 20201203054724.44838-1-aik@ozlabs.ru (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [kernel,v2] powerpc/kuap: Restore AMR after replaying soft interrupts | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (a1aeabd25a36d9e019381278e543e2d538dd44a7) |
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 | fail | Build failed! |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 52 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > 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 on RADIX but there are > none for hash-only configs (BOOK3E) so this adds stubs and moves > AMR_KUAP_BLOCK_xxx. > > Found by syzkaller. More likely to break with enabled > CONFIG_DEBUG_ATOMIC_SLEEP, the call chain is > timer_interrupt -> ktime_get -> read_seqcount_begin -> local_irq_restore. Can you test this with https://github.com/kvaneesh/linux/commits/hash-kuap-reworked-2 We do save restore AMR on interrupt entry and exit. -aneesh
Hi Alexey, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.10-rc6 next-20201201] [cannot apply to powerpc/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/20201203-135010 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 34816d20f173a90389c8a7e641166d8ea9dce70a config: powerpc-wii_defconfig (attached as .config) compiler: powerpc-linux-gcc (GCC) 9.3.0 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 # https://github.com/0day-ci/linux/commit/d2a99260ef6d241abe6fb961ee3ed84bbc5db7f5 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/20201203-135010 git checkout d2a99260ef6d241abe6fb961ee3ed84bbc5db7f5 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from arch/powerpc/include/asm/uaccess.h:9, from include/linux/uaccess.h:11, from include/linux/sched/task.h:11, from include/linux/sched/signal.h:9, from include/linux/rcuwait.h:6, from include/linux/percpu-rwsem.h:7, from include/linux/fs.h:33, from include/linux/compat.h:17, from arch/powerpc/kernel/asm-offsets.c:14: arch/powerpc/include/asm/kup.h: In function 'get_kuap': >> arch/powerpc/include/asm/kup.h:19:26: warning: conversion from 'long long unsigned int' to 'long unsigned int' changes value from '13835058055282163712' to '0' [-Woverflow] 19 | #define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) | ^ arch/powerpc/include/asm/kup.h:73:9: note: in expansion of macro 'AMR_KUAP_BLOCKED' 73 | return AMR_KUAP_BLOCKED; | ^~~~~~~~~~~~~~~~ -- In file included from arch/powerpc/include/asm/uaccess.h:9, from include/linux/uaccess.h:11, from include/linux/sched/task.h:11, from include/linux/sched/signal.h:9, from include/linux/rcuwait.h:6, from include/linux/percpu-rwsem.h:7, from include/linux/fs.h:33, from include/linux/huge_mm.h:8, from include/linux/mm.h:687, from include/linux/ring_buffer.h:5, from include/linux/trace_events.h:6, from include/trace/trace_events.h:21, from include/trace/define_trace.h:102, from include/trace/events/sched.h:656, from kernel/sched/core.c:10: arch/powerpc/include/asm/kup.h: In function 'get_kuap': >> arch/powerpc/include/asm/kup.h:19:26: warning: conversion from 'long long unsigned int' to 'long unsigned int' changes value from '13835058055282163712' to '0' [-Woverflow] 19 | #define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) | ^ arch/powerpc/include/asm/kup.h:73:9: note: in expansion of macro 'AMR_KUAP_BLOCKED' 73 | return AMR_KUAP_BLOCKED; | ^~~~~~~~~~~~~~~~ kernel/sched/core.c: In function 'ttwu_stat': kernel/sched/core.c:2419:13: warning: variable 'rq' set but not used [-Wunused-but-set-variable] 2419 | struct rq *rq; | ^~ -- In file included from arch/powerpc/include/asm/uaccess.h:9, from include/linux/uaccess.h:11, from include/linux/sched/task.h:11, from include/linux/sched/signal.h:9, from include/linux/sched/cputime.h:5, from kernel/sched/sched.h:11, from kernel/sched/fair.c:23: arch/powerpc/include/asm/kup.h: In function 'get_kuap': >> arch/powerpc/include/asm/kup.h:19:26: warning: conversion from 'long long unsigned int' to 'long unsigned int' changes value from '13835058055282163712' to '0' [-Woverflow] 19 | #define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) | ^ arch/powerpc/include/asm/kup.h:73:9: note: in expansion of macro 'AMR_KUAP_BLOCKED' 73 | return AMR_KUAP_BLOCKED; | ^~~~~~~~~~~~~~~~ kernel/sched/fair.c: At top level: kernel/sched/fair.c:5368:6: warning: no previous prototype for 'init_cfs_bandwidth' [-Wmissing-prototypes] 5368 | void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {} | ^~~~~~~~~~~~~~~~~~ kernel/sched/fair.c:11150:6: warning: no previous prototype for 'free_fair_sched_group' [-Wmissing-prototypes] 11150 | void free_fair_sched_group(struct task_group *tg) { } | ^~~~~~~~~~~~~~~~~~~~~ kernel/sched/fair.c:11152:5: warning: no previous prototype for 'alloc_fair_sched_group' [-Wmissing-prototypes] 11152 | int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent) | ^~~~~~~~~~~~~~~~~~~~~~ kernel/sched/fair.c:11157:6: warning: no previous prototype for 'online_fair_sched_group' [-Wmissing-prototypes] 11157 | void online_fair_sched_group(struct task_group *tg) { } | ^~~~~~~~~~~~~~~~~~~~~~~ kernel/sched/fair.c:11159:6: warning: no previous prototype for 'unregister_fair_sched_group' [-Wmissing-prototypes] 11159 | void unregister_fair_sched_group(struct task_group *tg) { } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ -- In file included from arch/powerpc/include/asm/uaccess.h:9, from include/linux/uaccess.h:11, from include/linux/sched/task.h:11, from include/linux/sched/signal.h:9, from include/linux/sched/cputime.h:5, from kernel/sched/sched.h:11, from kernel/sched/rt.c:6: arch/powerpc/include/asm/kup.h: In function 'get_kuap': >> arch/powerpc/include/asm/kup.h:19:26: warning: conversion from 'long long unsigned int' to 'long unsigned int' changes value from '13835058055282163712' to '0' [-Woverflow] 19 | #define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) | ^ arch/powerpc/include/asm/kup.h:73:9: note: in expansion of macro 'AMR_KUAP_BLOCKED' 73 | return AMR_KUAP_BLOCKED; | ^~~~~~~~~~~~~~~~ kernel/sched/rt.c: At top level: kernel/sched/rt.c:253:6: warning: no previous prototype for 'free_rt_sched_group' [-Wmissing-prototypes] 253 | void free_rt_sched_group(struct task_group *tg) { } | ^~~~~~~~~~~~~~~~~~~ kernel/sched/rt.c:255:5: warning: no previous prototype for 'alloc_rt_sched_group' [-Wmissing-prototypes] 255 | int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent) | ^~~~~~~~~~~~~~~~~~~~ kernel/sched/rt.c:668:6: warning: no previous prototype for 'sched_rt_bandwidth_account' [-Wmissing-prototypes] 668 | bool sched_rt_bandwidth_account(struct rt_rq *rt_rq) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ -- In file included from arch/powerpc/include/asm/uaccess.h:9, from include/linux/uaccess.h:11, from include/linux/sched/task.h:11, from include/linux/sched/signal.h:9, from include/linux/rcuwait.h:6, from include/linux/percpu-rwsem.h:7, from include/linux/fs.h:33, from include/linux/compat.h:17, from arch/powerpc/kernel/asm-offsets.c:14: arch/powerpc/include/asm/kup.h: In function 'get_kuap': >> arch/powerpc/include/asm/kup.h:19:26: warning: conversion from 'long long unsigned int' to 'long unsigned int' changes value from '13835058055282163712' to '0' [-Woverflow] 19 | #define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) | ^ arch/powerpc/include/asm/kup.h:73:9: note: in expansion of macro 'AMR_KUAP_BLOCKED' 73 | return AMR_KUAP_BLOCKED; | ^~~~~~~~~~~~~~~~ vim +19 arch/powerpc/include/asm/kup.h 16 17 #define AMR_KUAP_BLOCK_READ UL(0x4000000000000000) 18 #define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000) > 19 #define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) 20 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Le 03/12/2020 à 06:47, Alexey Kardashevskiy a écrit : > 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 on RADIX but there are > none for hash-only configs (BOOK3E) so this adds stubs and moves > AMR_KUAP_BLOCK_xxx. > > Found by syzkaller. More likely to break with enabled > CONFIG_DEBUG_ATOMIC_SLEEP, the call chain is > timer_interrupt -> ktime_get -> read_seqcount_begin -> local_irq_restore. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > Changes: > 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: c00000000009ece8 LR: c00000000009ece4 CTR: 0000000000000000 > REGS: c00000000dc63560 TRAP: 0700 Not tainted (5.10.0-rc6_v5.10-rc6_a+fstn1) > MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 28002888 XER: 20040000 > CFAR: c0000000001fa928 IRQMASK: 1 > GPR00: c00000000009ece4 c00000000dc637f0 c000000002397600 000000000000001f > GPR04: c0000000020eb318 0000000000000000 c00000000dc63494 0000000000000027 > GPR08: c00000007fe4de68 c00000000dfe9180 0000000000000000 0000000000000001 > GPR12: 0000000000002000 c0000000030a0000 0000000000000000 0000000000000000 > GPR16: 0000000000000000 0000000000000000 0000000000000000 bfffffffffffffff > GPR20: 0000000000000000 c0000000134a4020 c0000000019c2218 0000000000000fe0 > GPR24: 0000000000000000 0000000000000000 c00000000d106200 0000000040000000 > GPR28: 0000000000000000 0000000000000300 c00000000dc63910 c000000001946730 > NIP [c00000000009ece8] __do_page_fault+0xb38/0xde0 > LR [c00000000009ece4] __do_page_fault+0xb34/0xde0 > Call Trace: > [c00000000dc637f0] [c00000000009ece4] __do_page_fault+0xb34/0xde0 (unreliable) > [c00000000dc638a0] [c00000000000c968] handle_page_fault+0x10/0x2c > --- interrupt: 300 at strncpy_from_user+0x290/0x440 > LR = strncpy_from_user+0x284/0x440 > [c00000000dc63ba0] [c000000000c3dcb0] strncpy_from_user+0x2f0/0x440 (unreliable) > [c00000000dc63c30] [c00000000068b888] getname_flags+0x88/0x2c0 > [c00000000dc63c90] [c000000000662a44] do_sys_openat2+0x2d4/0x5f0 > [c00000000dc63d30] [c00000000066560c] do_sys_open+0xcc/0x140 > [c00000000dc63dc0] [c000000000045e10] system_call_exception+0x160/0x240 > [c00000000dc63e20] [c00000000000da60] system_call_common+0xf0/0x27c > Instruction dump: > 409c0048 3fe2ff5b 3bfff128 fac10060 fae10068 482f7a85 60000000 3c62ff5b > 7fe4fb78 3863f250 4815bbd9 60000000 <0fe00000> 3c62ff5b 3863f2b8 4815c8b5 > irq event stamp: 254 > hardirqs last enabled at (253): [<c000000000019550>] arch_local_irq_restore+0xa0/0x150 > hardirqs last disabled at (254): [<c000000000008a10>] data_access_common_virt+0x1b0/0x1d0 > softirqs last enabled at (0): [<c0000000001f6d5c>] copy_process+0x78c/0x2120 > softirqs last disabled at (0): [<0000000000000000>] 0x0 > ---[ end trace ba98aec5151f3aeb ]--- > --- > arch/powerpc/include/asm/book3s/64/kup-radix.h | 3 --- > arch/powerpc/include/asm/kup.h | 10 ++++++++++ > arch/powerpc/kernel/irq.c | 6 ++++++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h > index a39e2d193fdc..4ad607461b75 100644 > --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h > +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h > @@ -5,9 +5,6 @@ > #include <linux/const.h> > #include <asm/reg.h> > > -#define AMR_KUAP_BLOCK_READ UL(0x4000000000000000) > -#define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000) > -#define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) > #define AMR_KUAP_SHIFT 62 > > #ifdef __ASSEMBLY__ > diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h > index 0d93331d0fab..e63930767a96 100644 > --- a/arch/powerpc/include/asm/kup.h > +++ b/arch/powerpc/include/asm/kup.h > @@ -14,6 +14,10 @@ > #define KUAP_CURRENT_WRITE 8 > #define KUAP_CURRENT (KUAP_CURRENT_READ | KUAP_CURRENT_WRITE) > > +#define AMR_KUAP_BLOCK_READ UL(0x4000000000000000) > +#define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000) > +#define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) > + Those macro are specific to BOOK3S_64, they have nothing to do in asm/kup.h, must stay in the file included just below > #ifdef CONFIG_PPC_BOOK3S_64 > #include <asm/book3s/64/kup-radix.h> > #endif > @@ -64,6 +68,12 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write) > } > > static inline void kuap_check_amr(void) { } > +static inline unsigned long get_kuap(void) > +{ > + return AMR_KUAP_BLOCKED; > +} > + The above is not generic, it is specific to book3s 64, AMR doesn't exist on book3s/32 or on 8xx. > +static inline void set_kuap(unsigned long value) { } > > /* > * book3s/64/kup-radix.h defines these functions for the !KUAP case to flush > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > index 7d0f7682d01d..d9fd46da04d6 100644 > --- a/arch/powerpc/kernel/irq.c > +++ b/arch/powerpc/kernel/irq.c > @@ -314,6 +314,7 @@ void replay_soft_interrupts(void) > notrace void arch_local_irq_restore(unsigned long mask) > { > unsigned char irq_happened; > + unsigned long kuap_state; > > /* Write the new soft-enabled value */ > irq_soft_mask_set(mask); > @@ -373,9 +374,14 @@ notrace void arch_local_irq_restore(unsigned long mask) > irq_soft_mask_set(IRQS_ALL_DISABLED); > trace_hardirqs_off(); > > + kuap_state = get_kuap(); > + set_kuap(AMR_KUAP_BLOCKED); > + > replay_soft_interrupts(); > local_paca->irq_happened = 0; > > + set_kuap(kuap_state); > + > trace_hardirqs_on(); > irq_soft_mask_set(IRQS_ENABLED); > __hard_irq_enable(); >
On 03/12/2020 17:38, Aneesh Kumar K.V wrote: > Alexey Kardashevskiy <aik@ozlabs.ru> writes: > >> 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 on RADIX but there are >> none for hash-only configs (BOOK3E) so this adds stubs and moves >> AMR_KUAP_BLOCK_xxx. >> >> Found by syzkaller. More likely to break with enabled >> CONFIG_DEBUG_ATOMIC_SLEEP, the call chain is >> timer_interrupt -> ktime_get -> read_seqcount_begin -> local_irq_restore. > > Can you test this with https://github.com/kvaneesh/linux/commits/hash-kuap-reworked-2 Yup, broken: [ 8.813115] ------------[ cut here ]------------ [ 8.813499] Bug: Read fault blocked by AMR! [ 8.813532] WARNING: CPU: 0 PID: 1113 at /home/aik/p/kernel/arch/powerpc/include/asm/book3s/64/kup.h:369 __do_page_fault+0xd 34/0xdf0 [ 8.814248] Modules linked in: [ 8.814459] CPU: 0 PID: 1113 Comm: amr Not tainted 5.10.0-rc4_aneesh--hash-kuap-reworked-2_a+fstn1 #61 [ 8.815075] NIP: c0000000000a4674 LR: c0000000000a4670 CTR: 0000000000000000 [ 8.815632] REGS: c00000000d44f530 TRAP: 0700 Not tainted (5.10.0-rc4_aneesh--hash-kuap-reworked-2_a+fstn1) > We do save restore AMR on interrupt entry and exit. This is nested interrupt. we get interrupted while copying to/from user. The first handler has AMR off, then it goes to strncpy_from_user, enables AMR, page fault happens and another interrupt arrives - and if the new interrupt also wants strncpy_from_user/co, it will enable AMR again (which is ok), do copy and then disable AMR; and then we go back and resume the first strncpy_from_user which thinks AMR is enabling while it is not. I just see how strncpy_from_user is called while strncpy_from_user is running. Nick understands the mechanics better. I can give you an initramdisk which crashes in a millisecond, ping me in slack.
On 03/12/2020 19:03, Christophe Leroy wrote: > > > Le 03/12/2020 à 06:47, Alexey Kardashevskiy a écrit : >> 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 on RADIX but there are >> none for hash-only configs (BOOK3E) so this adds stubs and moves >> AMR_KUAP_BLOCK_xxx. >> >> Found by syzkaller. More likely to break with enabled >> CONFIG_DEBUG_ATOMIC_SLEEP, the call chain is >> timer_interrupt -> ktime_get -> read_seqcount_begin -> local_irq_restore. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> Changes: >> 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: c00000000009ece8 LR: c00000000009ece4 CTR: 0000000000000000 >> REGS: c00000000dc63560 TRAP: 0700 Not tainted >> (5.10.0-rc6_v5.10-rc6_a+fstn1) >> MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 28002888 XER: 20040000 >> CFAR: c0000000001fa928 IRQMASK: 1 >> GPR00: c00000000009ece4 c00000000dc637f0 c000000002397600 >> 000000000000001f >> GPR04: c0000000020eb318 0000000000000000 c00000000dc63494 >> 0000000000000027 >> GPR08: c00000007fe4de68 c00000000dfe9180 0000000000000000 >> 0000000000000001 >> GPR12: 0000000000002000 c0000000030a0000 0000000000000000 >> 0000000000000000 >> GPR16: 0000000000000000 0000000000000000 0000000000000000 >> bfffffffffffffff >> GPR20: 0000000000000000 c0000000134a4020 c0000000019c2218 >> 0000000000000fe0 >> GPR24: 0000000000000000 0000000000000000 c00000000d106200 >> 0000000040000000 >> GPR28: 0000000000000000 0000000000000300 c00000000dc63910 >> c000000001946730 >> NIP [c00000000009ece8] __do_page_fault+0xb38/0xde0 >> LR [c00000000009ece4] __do_page_fault+0xb34/0xde0 >> Call Trace: >> [c00000000dc637f0] [c00000000009ece4] __do_page_fault+0xb34/0xde0 >> (unreliable) >> [c00000000dc638a0] [c00000000000c968] handle_page_fault+0x10/0x2c >> --- interrupt: 300 at strncpy_from_user+0x290/0x440 >> LR = strncpy_from_user+0x284/0x440 >> [c00000000dc63ba0] [c000000000c3dcb0] strncpy_from_user+0x2f0/0x440 >> (unreliable) >> [c00000000dc63c30] [c00000000068b888] getname_flags+0x88/0x2c0 >> [c00000000dc63c90] [c000000000662a44] do_sys_openat2+0x2d4/0x5f0 >> [c00000000dc63d30] [c00000000066560c] do_sys_open+0xcc/0x140 >> [c00000000dc63dc0] [c000000000045e10] system_call_exception+0x160/0x240 >> [c00000000dc63e20] [c00000000000da60] system_call_common+0xf0/0x27c >> Instruction dump: >> 409c0048 3fe2ff5b 3bfff128 fac10060 fae10068 482f7a85 60000000 3c62ff5b >> 7fe4fb78 3863f250 4815bbd9 60000000 <0fe00000> 3c62ff5b 3863f2b8 4815c8b5 >> irq event stamp: 254 >> hardirqs last enabled at (253): [<c000000000019550>] >> arch_local_irq_restore+0xa0/0x150 >> hardirqs last disabled at (254): [<c000000000008a10>] >> data_access_common_virt+0x1b0/0x1d0 >> softirqs last enabled at (0): [<c0000000001f6d5c>] >> copy_process+0x78c/0x2120 >> softirqs last disabled at (0): [<0000000000000000>] 0x0 >> ---[ end trace ba98aec5151f3aeb ]--- >> --- >> arch/powerpc/include/asm/book3s/64/kup-radix.h | 3 --- >> arch/powerpc/include/asm/kup.h | 10 ++++++++++ >> arch/powerpc/kernel/irq.c | 6 ++++++ >> 3 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h >> b/arch/powerpc/include/asm/book3s/64/kup-radix.h >> index a39e2d193fdc..4ad607461b75 100644 >> --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h >> +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h >> @@ -5,9 +5,6 @@ >> #include <linux/const.h> >> #include <asm/reg.h> >> -#define AMR_KUAP_BLOCK_READ UL(0x4000000000000000) >> -#define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000) >> -#define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) >> #define AMR_KUAP_SHIFT 62 >> #ifdef __ASSEMBLY__ >> diff --git a/arch/powerpc/include/asm/kup.h >> b/arch/powerpc/include/asm/kup.h >> index 0d93331d0fab..e63930767a96 100644 >> --- a/arch/powerpc/include/asm/kup.h >> +++ b/arch/powerpc/include/asm/kup.h >> @@ -14,6 +14,10 @@ >> #define KUAP_CURRENT_WRITE 8 >> #define KUAP_CURRENT (KUAP_CURRENT_READ | KUAP_CURRENT_WRITE) >> +#define AMR_KUAP_BLOCK_READ UL(0x4000000000000000) >> +#define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000) >> +#define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) >> + > > Those macro are specific to BOOK3S_64, they have nothing to do in > asm/kup.h, must stay in the file included just below > >> #ifdef CONFIG_PPC_BOOK3S_64 >> #include <asm/book3s/64/kup-radix.h> >> #endif >> @@ -64,6 +68,12 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long >> address, bool is_write) >> } >> static inline void kuap_check_amr(void) { } >> +static inline unsigned long get_kuap(void) >> +{ >> + return AMR_KUAP_BLOCKED; >> +} >> + > > The above is not generic, it is specific to book3s 64, AMR doesn't exist > on book3s/32 or on 8xx. ah ok, I did not want more ifdefs there but looks like it is unavoidable, I'll repost if there are no other comments. Thanks,
Hi Alexey, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.10-rc6 next-20201204] [cannot apply to powerpc/next 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/20201203-135010 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 34816d20f173a90389c8a7e641166d8ea9dce70a config: powerpc64-randconfig-r023-20201204 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 32c501dd88b62787d3a5ffda7aabcf4650dbe3cd) 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/d2a99260ef6d241abe6fb961ee3ed84bbc5db7f5 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/20201203-135010 git checkout d2a99260ef6d241abe6fb961ee3ed84bbc5db7f5 # 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 <lkp@intel.com> All errors (new ones prefixed by >>): In file included from arch/powerpc/kernel/asm-offsets.c:14: In file included from include/linux/compat.h:17: In file included from include/linux/fs.h:33: In file included from include/linux/percpu-rwsem.h:7: In file included from include/linux/rcuwait.h:6: In file included from include/linux/sched/signal.h:9: In file included from include/linux/sched/task.h:11: In file included from include/linux/uaccess.h:11: In file included from arch/powerpc/include/asm/uaccess.h:9: >> arch/powerpc/include/asm/kup.h:71:29: error: redefinition of 'get_kuap' static inline unsigned long get_kuap(void) ^ arch/powerpc/include/asm/book3s/64/kup-radix.h:152:29: note: previous definition is here static inline unsigned long get_kuap(void) ^ In file included from arch/powerpc/kernel/asm-offsets.c:14: In file included from include/linux/compat.h:17: In file included from include/linux/fs.h:33: In file included from include/linux/percpu-rwsem.h:7: In file included from include/linux/rcuwait.h:6: In file included from include/linux/sched/signal.h:9: In file included from include/linux/sched/task.h:11: In file included from include/linux/uaccess.h:11: In file included from arch/powerpc/include/asm/uaccess.h:9: >> arch/powerpc/include/asm/kup.h:76:20: error: redefinition of 'set_kuap' static inline void set_kuap(unsigned long value) { } ^ arch/powerpc/include/asm/book3s/64/kup-radix.h:157:20: note: previous definition is here static inline void set_kuap(unsigned long value) { } ^ In file included from arch/powerpc/kernel/asm-offsets.c:21: include/linux/mman.h:155:9: warning: division by zero is undefined [-Wdivision-by-zero] _calc_vm_trans(flags, MAP_LOCKED, VM_LOCKED ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/mman.h:133:21: note: expanded from macro '_calc_vm_trans' : ((x) & (bit1)) / ((bit1) / (bit2)))) ^ ~~~~~~~~~~~~~~~~~ include/linux/mman.h:156:9: warning: division by zero is undefined [-Wdivision-by-zero] _calc_vm_trans(flags, MAP_SYNC, VM_SYNC ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/mman.h:133:21: note: expanded from macro '_calc_vm_trans' : ((x) & (bit1)) / ((bit1) / (bit2)))) ^ ~~~~~~~~~~~~~~~~~ 2 warnings and 2 errors generated. -- In file included from arch/powerpc/kernel/asm-offsets.c:14: In file included from include/linux/compat.h:17: In file included from include/linux/fs.h:33: In file included from include/linux/percpu-rwsem.h:7: In file included from include/linux/rcuwait.h:6: In file included from include/linux/sched/signal.h:9: In file included from include/linux/sched/task.h:11: In file included from include/linux/uaccess.h:11: In file included from arch/powerpc/include/asm/uaccess.h:9: >> arch/powerpc/include/asm/kup.h:71:29: error: redefinition of 'get_kuap' static inline unsigned long get_kuap(void) ^ arch/powerpc/include/asm/book3s/64/kup-radix.h:152:29: note: previous definition is here static inline unsigned long get_kuap(void) ^ In file included from arch/powerpc/kernel/asm-offsets.c:14: In file included from include/linux/compat.h:17: In file included from include/linux/fs.h:33: In file included from include/linux/percpu-rwsem.h:7: In file included from include/linux/rcuwait.h:6: In file included from include/linux/sched/signal.h:9: In file included from include/linux/sched/task.h:11: In file included from include/linux/uaccess.h:11: In file included from arch/powerpc/include/asm/uaccess.h:9: >> arch/powerpc/include/asm/kup.h:76:20: error: redefinition of 'set_kuap' static inline void set_kuap(unsigned long value) { } ^ arch/powerpc/include/asm/book3s/64/kup-radix.h:157:20: note: previous definition is here static inline void set_kuap(unsigned long value) { } ^ In file included from arch/powerpc/kernel/asm-offsets.c:21: include/linux/mman.h:155:9: warning: division by zero is undefined [-Wdivision-by-zero] _calc_vm_trans(flags, MAP_LOCKED, VM_LOCKED ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/mman.h:133:21: note: expanded from macro '_calc_vm_trans' : ((x) & (bit1)) / ((bit1) / (bit2)))) ^ ~~~~~~~~~~~~~~~~~ include/linux/mman.h:156:9: warning: division by zero is undefined [-Wdivision-by-zero] _calc_vm_trans(flags, MAP_SYNC, VM_SYNC ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/mman.h:133:21: note: expanded from macro '_calc_vm_trans' : ((x) & (bit1)) / ((bit1) / (bit2)))) ^ ~~~~~~~~~~~~~~~~~ 2 warnings and 2 errors generated. make[2]: *** [scripts/Makefile.build:117: arch/powerpc/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [Makefile:1198: prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [Makefile:185: __sub-make] Error 2 make: Target 'prepare' not remade because of errors. vim +/get_kuap +71 arch/powerpc/include/asm/kup.h 69 70 static inline void kuap_check_amr(void) { } > 71 static inline unsigned long get_kuap(void) 72 { 73 return AMR_KUAP_BLOCKED; 74 } 75 > 76 static inline void set_kuap(unsigned long value) { } 77 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h index a39e2d193fdc..4ad607461b75 100644 --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h @@ -5,9 +5,6 @@ #include <linux/const.h> #include <asm/reg.h> -#define AMR_KUAP_BLOCK_READ UL(0x4000000000000000) -#define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000) -#define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) #define AMR_KUAP_SHIFT 62 #ifdef __ASSEMBLY__ diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h index 0d93331d0fab..e63930767a96 100644 --- a/arch/powerpc/include/asm/kup.h +++ b/arch/powerpc/include/asm/kup.h @@ -14,6 +14,10 @@ #define KUAP_CURRENT_WRITE 8 #define KUAP_CURRENT (KUAP_CURRENT_READ | KUAP_CURRENT_WRITE) +#define AMR_KUAP_BLOCK_READ UL(0x4000000000000000) +#define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000) +#define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE) + #ifdef CONFIG_PPC_BOOK3S_64 #include <asm/book3s/64/kup-radix.h> #endif @@ -64,6 +68,12 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write) } static inline void kuap_check_amr(void) { } +static inline unsigned long get_kuap(void) +{ + return AMR_KUAP_BLOCKED; +} + +static inline void set_kuap(unsigned long value) { } /* * book3s/64/kup-radix.h defines these functions for the !KUAP case to flush diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c index 7d0f7682d01d..d9fd46da04d6 100644 --- a/arch/powerpc/kernel/irq.c +++ b/arch/powerpc/kernel/irq.c @@ -314,6 +314,7 @@ void replay_soft_interrupts(void) notrace void arch_local_irq_restore(unsigned long mask) { unsigned char irq_happened; + unsigned long kuap_state; /* Write the new soft-enabled value */ irq_soft_mask_set(mask); @@ -373,9 +374,14 @@ notrace void arch_local_irq_restore(unsigned long mask) irq_soft_mask_set(IRQS_ALL_DISABLED); trace_hardirqs_off(); + kuap_state = get_kuap(); + set_kuap(AMR_KUAP_BLOCKED); + replay_soft_interrupts(); local_paca->irq_happened = 0; + set_kuap(kuap_state); + trace_hardirqs_on(); irq_soft_mask_set(IRQS_ENABLED); __hard_irq_enable();
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 on RADIX but there are none for hash-only configs (BOOK3E) so this adds stubs and moves AMR_KUAP_BLOCK_xxx. Found by syzkaller. More likely to break with enabled CONFIG_DEBUG_ATOMIC_SLEEP, the call chain is timer_interrupt -> ktime_get -> read_seqcount_begin -> local_irq_restore. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: 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: c00000000009ece8 LR: c00000000009ece4 CTR: 0000000000000000 REGS: c00000000dc63560 TRAP: 0700 Not tainted (5.10.0-rc6_v5.10-rc6_a+fstn1) MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 28002888 XER: 20040000 CFAR: c0000000001fa928 IRQMASK: 1 GPR00: c00000000009ece4 c00000000dc637f0 c000000002397600 000000000000001f GPR04: c0000000020eb318 0000000000000000 c00000000dc63494 0000000000000027 GPR08: c00000007fe4de68 c00000000dfe9180 0000000000000000 0000000000000001 GPR12: 0000000000002000 c0000000030a0000 0000000000000000 0000000000000000 GPR16: 0000000000000000 0000000000000000 0000000000000000 bfffffffffffffff GPR20: 0000000000000000 c0000000134a4020 c0000000019c2218 0000000000000fe0 GPR24: 0000000000000000 0000000000000000 c00000000d106200 0000000040000000 GPR28: 0000000000000000 0000000000000300 c00000000dc63910 c000000001946730 NIP [c00000000009ece8] __do_page_fault+0xb38/0xde0 LR [c00000000009ece4] __do_page_fault+0xb34/0xde0 Call Trace: [c00000000dc637f0] [c00000000009ece4] __do_page_fault+0xb34/0xde0 (unreliable) [c00000000dc638a0] [c00000000000c968] handle_page_fault+0x10/0x2c --- interrupt: 300 at strncpy_from_user+0x290/0x440 LR = strncpy_from_user+0x284/0x440 [c00000000dc63ba0] [c000000000c3dcb0] strncpy_from_user+0x2f0/0x440 (unreliable) [c00000000dc63c30] [c00000000068b888] getname_flags+0x88/0x2c0 [c00000000dc63c90] [c000000000662a44] do_sys_openat2+0x2d4/0x5f0 [c00000000dc63d30] [c00000000066560c] do_sys_open+0xcc/0x140 [c00000000dc63dc0] [c000000000045e10] system_call_exception+0x160/0x240 [c00000000dc63e20] [c00000000000da60] system_call_common+0xf0/0x27c Instruction dump: 409c0048 3fe2ff5b 3bfff128 fac10060 fae10068 482f7a85 60000000 3c62ff5b 7fe4fb78 3863f250 4815bbd9 60000000 <0fe00000> 3c62ff5b 3863f2b8 4815c8b5 irq event stamp: 254 hardirqs last enabled at (253): [<c000000000019550>] arch_local_irq_restore+0xa0/0x150 hardirqs last disabled at (254): [<c000000000008a10>] data_access_common_virt+0x1b0/0x1d0 softirqs last enabled at (0): [<c0000000001f6d5c>] copy_process+0x78c/0x2120 softirqs last disabled at (0): [<0000000000000000>] 0x0 ---[ end trace ba98aec5151f3aeb ]--- --- arch/powerpc/include/asm/book3s/64/kup-radix.h | 3 --- arch/powerpc/include/asm/kup.h | 10 ++++++++++ arch/powerpc/kernel/irq.c | 6 ++++++ 3 files changed, 16 insertions(+), 3 deletions(-)