Le 05/02/2021 à 03:16, Nicholas Piggin a écrit :
Excerpts from Michael Ellerman's message of February 5, 2021 10:22 am:
Nicholas Piggin <npig...@gmail.com> writes:
Excerpts from Christophe Leroy's message of February 4, 2021 6:03 pm:
Le 04/02/2021 à 04:27, Nicholas Piggin a écrit :
Excerpts from Christophe Leroy's message of February 4, 2021 2:25 am:
Le 25/02/2020 à 18:35, Nicholas Piggin a écrit :
...
+
+ /*
+ * We don't need to restore AMR on the way back to userspace for KUAP.
+ * The value of AMR only matters while we're in the kernel.
+ */
+ kuap_restore_amr(regs);
Is that correct to restore KUAP state here ? Shouldn't we have it at lower
level in assembly ?
Isn't there a risk that someone manages to call interrupt_exit_kernel_prepare()
or the end of it in
a way or another, and get the previous KUAP state restored by this way ?
I'm not sure if there much more risk if it's here rather than the
instruction being in another place in the code.
There's a lot of user access around the kernel too if you want to find a
gadget to unlock KUAP then I suppose there is a pretty large attack
surface.
My understanding is that user access scope is strictly limited, for instance we
enforce the
begin/end of user access to be in the same function, and we refrain from
calling any other function
inside the user access window. x86 even have 'objtool' to enforce it at build
time. So in theory
there is no way to get out of the function while user access is open.
Here with the interrupt exit function it is free beer. You have a place where
you re-open user
access and return with a simple blr. So that's open bar. If someone manages to
just call the
interrupt exit function, then user access remains open
Hmm okay maybe that's a good point.
I don't think it's a very attractive gadget, it's not just a plain blr,
it does a full stack frame tear down before the return. And there's no
LR reloads anywhere very close.
Obviously it depends on what the compiler decides to do, it's possible
it could be a usable gadget. But there are other places that are more
attractive I think, eg:
c00000000061d768: a6 03 3d 7d mtspr 29,r9
c00000000061d76c: 2c 01 00 4c isync
c00000000061d770: 00 00 00 60 nop
c00000000061d774: 30 00 21 38 addi r1,r1,48
c00000000061d778: 20 00 80 4e blr
So I don't think we should redesign the code *purely* because we're
worried about interrupt_exit_kernel_prepare() being a useful gadget. If
we can come up with a way to restructure it that reads well and is
maintainable, and also reduces the chance of it being a good gadget then
sure.
Okay. That would be good if we can keep it in C, the pkeys + kuap combo
is fairly complicated and we might want to something cleverer with it,
so that would make it even more difficult in asm.
Ok.
For ppc32, I prefer to keep it in assembly for the time being and move everything from ASM to C at
once after porting syscall and interrupts to C and wrappers.
Hope this is OK for you.
Christophe
