Re: [PATCH] ArmPkg/ArmSvcLib: prevent speculative execution beyond svc

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 6/4/20 3:12 PM, Vijayenthiran Subramaniam wrote:
> Supervisor Call instruction (SVC) is used by the Arm Standalone MM
> environment to request services from the privileged software (such as
> ARM Trusted Firmware running in EL3) and also return back to the
> non-secure caller via EL3. Some Arm CPUs speculatively executes the
> instructions after the SVC instruction without crossing the privilege
> level (S-EL0). Although the results of this execution are
> architecturally discarded, adversary running on the non-secure side can
> manipulate the contents of the general purpose registers to leak the
> secure work memory through spectre like micro-architectural side channel
> attacks. This behavior is demonstrated by the SafeSide project [1] and
> [2]. Add barrier instructions after SVC to prevent speculative execution
> to mitigate such attacks.
> 
> [1]: https://github.com/google/safeside/blob/master/demos/eret_hvc_smc_wrapper.cc
> [2]: https://github.com/google/safeside/blob/master/kernel_modules/kmod_eret_hvc_smc/eret_hvc_smc_module.c
> 
> Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Merged as #663

Thanks.

> ---
>   ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S | 5 ++++-
>   ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S     | 5 ++++-
>   ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm   | 5 ++++-
>   3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
> index 7c94db3451f0..ee265f94b960 100644
> --- a/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
> +++ b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
> @@ -1,5 +1,5 @@
>   //
> -//  Copyright (c) 2012 - 2017, ARM Limited. All rights reserved.
> +//  Copyright (c) 2012 - 2020, ARM Limited. All rights reserved.
>   //
>   //  SPDX-License-Identifier: BSD-2-Clause-Patent
>   //
> @@ -25,6 +25,9 @@ ASM_PFX(ArmCallSvc):
>     ldp   x0, x1, [x0, #0]
>   
>     svc   #0
> +  // Prevent speculative execution beyond svc instruction
> +  dsb   nsh
> +  isb
>   
>     // Pop the ARM_SVC_ARGS structure address from the stack into x9
>     ldr   x9, [sp, #16]
> diff --git a/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
> index fc2886b6b53e..e81eb88f2e87 100644
> --- a/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
> +++ b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.S
> @@ -1,5 +1,5 @@
>   //
> -//  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
> +//  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>   //
>   //  SPDX-License-Identifier: BSD-2-Clause-Patent
>   //
> @@ -18,6 +18,9 @@ ASM_PFX(ArmCallSvc):
>       ldm     r0, {r0-r7}
>   
>       svc     #0
> +    // Prevent speculative execution beyond svc instruction
> +    dsb     nsh
> +    isb
>   
>       // Load the ARM_SVC_ARGS structure address from the stack into r8
>       ldr     r8, [sp]
> diff --git a/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm
> index 82d10c023ae3..d1751488b2b1 100644
> --- a/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm
> +++ b/ArmPkg/Library/ArmSvcLib/Arm/ArmSvc.asm
> @@ -1,5 +1,5 @@
>   //
> -//  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
> +//  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>   //
>   //  SPDX-License-Identifier: BSD-2-Clause-Patent
>   //
> @@ -16,6 +16,9 @@
>       ldm     r0, {r0-r7}
>   
>       svc     #0
> +    // Prevent speculative execution beyond svc instruction
> +    dsb     nsh
> +    isb
>   
>       // Load the ARM_SVC_ARGS structure address from the stack into r8
>       ldr     r8, [sp]
>