[PATCH v6 22/25] powerpc/64s: Clear user GPRs in syscall interrupt entry

[Nicholas Piggin]

On Wed Sep 21, 2022 at 4:56 PM AEST, Rohan McLure wrote:
> Clear user state in gprs (assign to zero) to reduce the influence of user
> registers on speculation within kernel syscall handlers. Clears occur
> at the very beginning of the sc and scv 0 interrupt handlers, with
> restores occurring following the execution of the syscall handler.
>
> Signed-off-by: Rohan McLure <rmclure at linux.ibm.com>
> ---
> V2: Update summary
> V3: Remove erroneous summary paragraph on syscall_exit_prepare
> V4: Use ZEROIZE instead of NULLIFY. Clear r0 also.
> V5: Move to end of patch series.
> V6: Include clears which were previously in the syscall wrapper patch.
> Move comment on r3-r8 register save to when we alter the calling
> convention for system_call_exception.

The series looks good to here, I just need to find a bit more time to
look at the code and do some tests with the next few patches. I don't
see much problem with them, looks a lot better now with fewer ifdefs
so that's good. Possibly you could share some of those new sanitize
macros in a header file but that's a minor nit.

Coud we have this zeroize also under the same config option as the
next? I figure if we care about speculative security we want both,
and if we don't we need neither.

Thanks,
Nick

> ---
>  arch/powerpc/kernel/interrupt_64.S | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index a5dd78bdbe6d..40147558e1a6 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -106,6 +106,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	 * but this is the best we can do.
>  	 */
>  
> +	/*
> +	 * Zero user registers to prevent influencing speculative execution
> +	 * state of kernel code.
> +	 */
> +	ZEROIZE_GPR(0)
> +	ZEROIZE_GPRS(5, 12)
> +	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
>  
>  .Lsyscall_vectored_\name\()_exit:
> @@ -134,6 +141,7 @@ BEGIN_FTR_SECTION
>  	HMT_MEDIUM_LOW
>  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  
> +	REST_NVGPRS(r1)
>  	cmpdi	r3,0
>  	bne	.Lsyscall_vectored_\name\()_restore_regs
>  
> @@ -285,6 +293,13 @@ END_BTB_FLUSH_SECTION
>  	wrteei	1
>  #endif
>  
> +	/*
> +	 * Zero user registers to prevent influencing speculative execution
> +	 * state of kernel code.
> +	 */
> +	ZEROIZE_GPR(0)
> +	ZEROIZE_GPRS(5, 12)
> +	ZEROIZE_NVGPRS()
>  	bl	system_call_exception
>  
>  .Lsyscall_exit:
> @@ -325,6 +340,7 @@ BEGIN_FTR_SECTION
>  	stdcx.	r0,0,r1			/* to clear the reservation */
>  END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>  
> +	REST_NVGPRS(r1)
>  	cmpdi	r3,0
>  	bne	.Lsyscall_restore_regs
>  	/* Zero volatile regs that may contain sensitive kernel data */
> @@ -352,7 +368,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  .Lsyscall_restore_regs:
>  	ld	r3,_CTR(r1)
>  	ld	r4,_XER(r1)
> -	REST_NVGPRS(r1)
>  	mtctr	r3
>  	mtspr	SPRN_XER,r4
>  	REST_GPR(0, r1)
> -- 
> 2.34.1