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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

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