From: Michael Ellerman <mpe@ellerman•id.au>
To: Christophe Leroy <christophe.leroy@csgroup•eu>,
Zorro Lang <zlang@redhat•com>,
"Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com>
Cc: Jens Axboe <axboe@kernel•dk>,
linuxppc-dev@lists•ozlabs.org,
Nicholas Piggin <npiggin@gmail•com>
Subject: Re: [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING
Date: Sat, 30 Jan 2021 22:22:39 +1100 [thread overview]
Message-ID: <87r1m2d5z4.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <18dd441b-440a-fe95-0907-d8cec5b49410@csgroup.eu>
Christophe Leroy <christophe.leroy@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
next prev parent reply other threads:[~2021-01-30 11:26 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-27 14:56 [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING Zorro Lang
2021-01-27 16:38 ` Christophe Leroy
2021-01-27 19:29 ` Jens Axboe
2021-01-28 0:18 ` Nicholas Piggin
2021-01-28 3:13 ` Zorro Lang
2021-01-28 3:06 ` Jens Axboe
2021-01-28 13:52 ` Zorro Lang
2021-01-28 14:42 ` Jens Axboe
2021-01-28 14:44 ` Christophe Leroy
2021-01-28 15:20 ` Zorro Lang
2021-01-29 6:52 ` Zorro Lang
2021-01-29 12:26 ` Christophe Leroy
2021-01-30 11:22 ` Michael Ellerman [this message]
2021-01-30 13:54 ` Nicholas Piggin
2021-02-02 4:45 ` Aneesh Kumar K.V
2021-02-02 5:55 ` Aneesh Kumar K.V
2021-02-02 6:02 ` Christophe Leroy
2021-02-02 6:16 ` Aneesh Kumar K.V
2021-02-02 6:20 ` Christophe Leroy
2021-02-02 6:30 ` Aneesh Kumar K.V
2021-02-02 9:49 ` Nicholas Piggin
2021-02-04 16:53 ` Jens Axboe
2021-02-04 17:01 ` Aneesh Kumar K.V
2021-02-04 17:03 ` Jens Axboe
2021-02-04 17:57 ` Zorro Lang
2021-02-04 17:42 ` Aneesh Kumar K.V
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r1m2d5z4.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=aneesh.kumar@linux$(echo .)ibm.com \
--cc=axboe@kernel$(echo .)dk \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=npiggin@gmail$(echo .)com \
--cc=zlang@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox