* Re: [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts
2020-12-03 5:47 [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts Alexey Kardashevskiy
@ 2020-12-03 6:38 ` Aneesh Kumar K.V
2020-12-03 8:10 ` Alexey Kardashevskiy
2020-12-03 7:41 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Aneesh Kumar K.V @ 2020-12-03 6:38 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy, Nicholas Piggin
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts
2020-12-03 6:38 ` Aneesh Kumar K.V
@ 2020-12-03 8:10 ` Alexey Kardashevskiy
0 siblings, 0 replies; 7+ messages in thread
From: Alexey Kardashevskiy @ 2020-12-03 8:10 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Nicholas Piggin
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.
--
Alexey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts
2020-12-03 5:47 [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts Alexey Kardashevskiy
2020-12-03 6:38 ` Aneesh Kumar K.V
@ 2020-12-03 7:41 ` kernel test robot
2020-12-03 8:03 ` Christophe Leroy
2020-12-04 21:02 ` kernel test robot
3 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2020-12-03 7:41 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Alexey Kardashevskiy, kbuild-all, Nicholas Piggin
[-- Attachment #1: Type: text/plain, Size: 9162 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16271 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts
2020-12-03 5:47 [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts Alexey Kardashevskiy
2020-12-03 6:38 ` Aneesh Kumar K.V
2020-12-03 7:41 ` kernel test robot
@ 2020-12-03 8:03 ` Christophe Leroy
2020-12-03 8:13 ` Alexey Kardashevskiy
2020-12-04 21:02 ` kernel test robot
3 siblings, 1 reply; 7+ messages in thread
From: Christophe Leroy @ 2020-12-03 8:03 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Nicholas Piggin
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();
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts
2020-12-03 8:03 ` Christophe Leroy
@ 2020-12-03 8:13 ` Alexey Kardashevskiy
0 siblings, 0 replies; 7+ messages in thread
From: Alexey Kardashevskiy @ 2020-12-03 8:13 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev; +Cc: Nicholas Piggin
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,
--
Alexey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts
2020-12-03 5:47 [PATCH kernel v2] powerpc/kuap: Restore AMR after replaying soft interrupts Alexey Kardashevskiy
` (2 preceding siblings ...)
2020-12-03 8:03 ` Christophe Leroy
@ 2020-12-04 21:02 ` kernel test robot
3 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2020-12-04 21:02 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Alexey Kardashevskiy, clang-built-linux, kbuild-all, Nicholas Piggin
[-- Attachment #1: Type: text/plain, Size: 7629 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41007 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread