[PATCH v5 02/10] arm64: smccc: clear the Xn registers after SMC calls

[Jens Wiklander]

On Mon, Sep 26, 2022 at 03:08:19PM +0100, Abdellatif El Khlifi wrote:
> set to zero the x0-x17 registers
> 
> As per the SMCCC v1.2 spec, unused result and scratch registers
> can leak information after an SMC call. We can mitigate against
> this risk by returning zero in each register.
> 
> The leakage we are referring to is data leakage across exception
> levels. The intent is to prevent lower exception levels (EL1/EL0)
> from reading the SMC data exchanged at EL2.
> 
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Cc: Jens Wiklander <jens.wiklander at linaro.org>
> ---
> 
> Changelog:
> ===============
> 
> v4:
> 
> * move the clearing code into a new macro: clear_gp_regs
> 
> v3:
> 
> * clear the Xn registers after SMC calls
> 
> 
>  arch/arm/cpu/armv8/smccc-call.S | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv8/smccc-call.S b/arch/arm/cpu/armv8/smccc-call.S
> index ec6f299bc9..32f3eb8eeb 100644
> --- a/arch/arm/cpu/armv8/smccc-call.S
> +++ b/arch/arm/cpu/armv8/smccc-call.S
> @@ -50,6 +50,12 @@ ENDPROC(__arm_smccc_hvc)
>  
>  #ifdef CONFIG_ARM64
>  
> +	.macro	clear_gp_regs
> +	.irp	n,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17
> +	mov	x\n, xzr
> +	.endr
> +	.endm
> +
>  	.macro SMCCC_1_2 instr
>  	/* Save `res` and free a GPR that won't be clobbered */
>  	stp     x1, x19, [sp, #-16]!
> @@ -84,6 +90,9 @@ ENDPROC(__arm_smccc_hvc)
>  	stp	x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
>  	stp	x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
>  
> +	/* x0-x17 registers can leak information after an SMC or HVC call. Let's clear them */
> +	clear_gp_regs
> +

This should in my opinion not be needed. The higher exception level should
only return what it indends to return and certainly not rely on lower
exception levels to try to hide eventual unintentionally revealed
secrets.

In an earlier conversation you said:
> The leakage we are referring to is data leakage across exception levels.
> The intent is to prevent lower exception levels (EL1/EL0) to read the
> data exchanged at EL2.
> 
> The linux kernel clears the general purpose registers before switching
> to EL0. As far as I know u-boot doesn't.
> 
> So, the code above makes sure the registers are cleared.

U-Boot is as far as I know not changing to EL0. Do you have a real example
where this cleaning actually would be needed? If it's needed I'd expect
the cleaning to be done just before changing exception level.

Cheers,
Jens


>  	/* Restore original x19 */
>  	ldp     xzr, x19, [sp], #16
>  	ret
> -- 
> 2.17.1
>