[PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING

[Michael Ellerman]

Christophe Leroy <christophe.leroy at csgroup.eu> writes:
> +Aneesh
>
> Le 29/01/2021 à 07:52, Zorro Lang a écrit :
..
>> [   96.200296] ------------[ cut here ]------------
>> [   96.200304] Bug: Read fault blocked by KUAP!
>> [   96.200309] WARNING: CPU: 3 PID: 1876 at arch/powerpc/mm/fault.c:229 bad_kernel_fault+0x180/0x310
>
>> [   96.200734] NIP [c000000000849424] fault_in_pages_readable+0x104/0x350
>> [   96.200741] LR [c00000000084952c] fault_in_pages_readable+0x20c/0x350
>> [   96.200747] --- interrupt: 300
>
>
> Problem happens in a section where userspace access is supposed to be granted, so the patch you 
> proposed is definitely not the right fix.
>
> c000000000849408:	2c 01 00 4c 	isync
> c00000000084940c:	a6 03 3d 7d 	mtspr   29,r9  <== granting userspace access permission
> c000000000849410:	2c 01 00 4c 	isync
> c000000000849414:	00 00 36 e9 	ld      r9,0(r22)
> c000000000849418:	20 00 29 81 	lwz     r9,32(r9)
> c00000000084941c:	00 02 29 71 	andi.   r9,r9,512
> c000000000849420:	78 d3 5e 7f 	mr      r30,r26
> ==> c000000000849424:	00 00 bf 8b 	lbz     r29,0(r31)  <== accessing userspace
> c000000000849428:	10 00 82 41 	beq     c000000000849438 <fault_in_pages_readable+0x118>
> c00000000084942c:	2c 01 00 4c 	isync
> c000000000849430:	a6 03 bd 7e 	mtspr   29,r21  <== clearing userspace access permission
> c000000000849434:	2c 01 00 4c 	isync
>
> My first guess is that the problem is linked to the following function, see the comment
>
> /*
>   * For kernel thread that doesn't have thread.regs return
>   * default AMR/IAMR values.
>   */
> static inline u64 current_thread_amr(void)
> {
> 	if (current->thread.regs)
> 		return current->thread.regs->amr;
> 	return AMR_KUAP_BLOCKED;
> }
>
> Above function was introduced by commit 48a8ab4eeb82 ("powerpc/book3s64/pkeys: Don't update SPRN_AMR 
> when in kernel mode")

Yeah that's a bit of a curly one.

At some point io_uring did kthread_use_mm(), which is supposed to mean
the kthread can operate on behalf of the original process that submitted
the IO.

But because KUAP is implemented using memory protection keys, it depends
on the value of the AMR register, which is not part of the mm, it's in
thread.regs->amr.

And what's worse by the time we're in kthread_use_mm() we no longer have
access to the thread.regs->amr of the original process that submitted
the IO.

We also can't simply move the AMR into the mm, precisely because it's
per thread, not per mm.

So TBH I don't know how we're going to fix this.

I guess we could return AMR=unblocked for kernel threads, but that's
arguably a bug because it allows a process to circumvent memory keys by
asking the kernel to do the access.


cheers