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 <rmcl...@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

Reply via email to