Re: [RFC v2 5/7] arm64: initialize and switch ptrauth kernel keys

On 06/06/2019 17:44, Catalin Marinas wrote:

On Wed, May 29, 2019 at 08:03:30PM +0100, Kristina Martsenko wrote:

quoted

 - Added ISB after key install in kernel_exit, in case in the future C function
   calls are added after the macro

[...]

quoted

diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
index e3bfddfe80b6..f595da9661a4 100644
--- a/arch/arm64/include/asm/asm_pointer_auth.h
+++ b/arch/arm64/include/asm/asm_pointer_auth.h

@@ -25,11 +25,24 @@ alternative_if ARM64_HAS_ADDRESS_AUTH
 	ldp	\tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APDB]
 	msr_s	SYS_APDBKEYLO_EL1, \tmp2
 	msr_s	SYS_APDBKEYHI_EL1, \tmp3
+	isb
 alternative_else_nop_endif
 alternative_if ARM64_HAS_GENERIC_AUTH
 	ldp	\tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APGA]
 	msr_s	SYS_APGAKEYLO_EL1, \tmp2
 	msr_s	SYS_APGAKEYHI_EL1, \tmp3
+	isb
+alternative_else_nop_endif

I couldn't find the previous discussions, so why are the ISBs needed
here? Is this macro not invoked only on the kernel_exit path?

It is invoked only in kernel_exit. There weren't any previous
discussions, I just started thinking that in the future someone could
add a call to a C function in kernel_exit after this macro (there are a
few C calls in kernel_exit already). If the function is compiled with
ptrauth instructions, and the above key system register writes take
effect in the middle of the function, then authentication would fail,
because we would enter the function with one key but exit with another.

This is probably overly cautious, so I'd be happy to remove the ISBs if
you prefer. Could also add a comment in kernel_exit. Also, thinking
about it now, we only use APIA key in the kernel, so the ISB after APGA
key is actually unnecessary, so I'll go ahead and remove that one.

Thanks,
Kristina

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel