diff mbox

ARM: DRA7/OMAP5: Enable ACTLR[0] (Enable invalidates of BTB) for secondary cores

Message ID 20180612213611.2484-1-nm@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nishanth Menon June 12, 2018, 9:36 p.m. UTC
Call secure services to enable ACTLR[0] (Enable invalidates of BTB with
ICIALLU) when branch hardening is enabled for kernel.

Signed-off-by: Nishanth Menon <nm@ti.com>
---

Based on: next-20180612 +
Uboot series posted:  https://marc.info/?l=u-boot&m=152883522011042&w=2

With Just u-boot changes alone: OMAP5-uevm: https://pastebin.ubuntu.com/p/9yDM22bJ6n/
with kernel changes added on: https://pastebin.ubuntu.com/p/gXPBGGYRPX/

 arch/arm/mach-omap2/omap-smp.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Russell King (Oracle) June 13, 2018, 10:11 a.m. UTC | #1
On Tue, Jun 12, 2018 at 04:36:11PM -0500, Nishanth Menon wrote:
> Call secure services to enable ACTLR[0] (Enable invalidates of BTB with
> ICIALLU) when branch hardening is enabled for kernel.

As mentioned elsewhere, I don't think this is a good idea - if the secure
world is not implementing the Spectre workarounds, then the _system_ is
exploitable.

If the secure world is implementing the spectre workarounds, it will
already have enabled the IBE bit (which is r/w from secure, read only
from non-secure.)

So, basically, lack of the IBE bit being set is basically telling the
kernel that it's running on a vulnerable platform _even if the kernel
were to set it through some means_.

> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> 
> Based on: next-20180612 +
> Uboot series posted:  https://marc.info/?l=u-boot&m=152883522011042&w=2
> 
> With Just u-boot changes alone: OMAP5-uevm: https://pastebin.ubuntu.com/p/9yDM22bJ6n/
> with kernel changes added on: https://pastebin.ubuntu.com/p/gXPBGGYRPX/
> 
>  arch/arm/mach-omap2/omap-smp.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
> index 69df3620eca5..28fc80ea675b 100644
> --- a/arch/arm/mach-omap2/omap-smp.c
> +++ b/arch/arm/mach-omap2/omap-smp.c
> @@ -109,6 +109,32 @@ void omap5_erratum_workaround_801819(void)
>  static inline void omap5_erratum_workaround_801819(void) { }
>  #endif
>  
> +#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
> +static void omap5_harden_predictor(void)
> +{
> +	u32 acr, acr_mask;
> +
> +	asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
> +
> +	/*
> +	 * BIT(0) - Disables streaming. All write-allocate lines allocate in
> +	 */
> +	acr_mask = BIT(0);
> +
> +	/* do we already have it done.. if yes, skip expensive smc */
> +	if ((acr & acr_mask) == acr_mask)
> +		return;
> +
> +	acr |= acr_mask;
> +	omap_smc1(OMAP5_DRA7_MON_SET_ACR_INDEX, acr);
> +
> +	pr_debug("%s: ARM ACR setup for CVE_2017_5715 applied on CPU%d\n",
> +		 __func__, smp_processor_id());
> +}
> +#else
> +static inline void omap5_harden_predictor(void) { }
> +#endif
> +
>  static void omap4_secondary_init(unsigned int cpu)
>  {
>  	/*
> @@ -131,6 +157,8 @@ static void omap4_secondary_init(unsigned int cpu)
>  		set_cntfreq();
>  		/* Configure ACR to disable streaming WA for 801819 */
>  		omap5_erratum_workaround_801819();
> +		/* Enable ACR to allow for ICUALLU workaround */
> +		omap5_harden_predictor();
>  	}
>  
>  	/*
> -- 
> 2.15.1
>
Nishanth Menon June 13, 2018, 1:29 p.m. UTC | #2
On 10:11-20180613, Russell King - ARM Linux wrote:
> On Tue, Jun 12, 2018 at 04:36:11PM -0500, Nishanth Menon wrote:
> > Call secure services to enable ACTLR[0] (Enable invalidates of BTB with
> > ICIALLU) when branch hardening is enabled for kernel.
> 
> As mentioned elsewhere, I don't think this is a good idea - if the secure
> world is not implementing the Spectre workarounds, then the _system_ is
> exploitable.
> 
> If the secure world is implementing the spectre workarounds, it will
> already have enabled the IBE bit (which is r/w from secure, read only
> from non-secure.)
> 
> So, basically, lack of the IBE bit being set is basically telling the
> kernel that it's running on a vulnerable platform _even if the kernel
> were to set it through some means_.

On GP devices OMAP5/DRA7, there is no possibility to update secure side
since "secure world" is ROM and there are no override mechanisms possible.
on HS devices, I agree, appropriate PPA will do the workarounds as well.

However, this patch is to enable the IBE enable on GP device for _a_
core can only be done via SMC services that ROM provides for
specifically the reasons you have already stated. u-boot will only
enable the IBE for the boot core, by the time the secondary cores start
up, u-boot is long gone.. so someone has to invoke the SMC call to
enable the IBE bit for the secondary core.

This is what the patch does.

If the above explanation makes sense, I will add that to the commit log
as well.
Tony Lindgren June 25, 2018, 8:03 a.m. UTC | #3
* Nishanth Menon <nm@ti.com> [180613 13:31]:
> On 10:11-20180613, Russell King - ARM Linux wrote:
> > On Tue, Jun 12, 2018 at 04:36:11PM -0500, Nishanth Menon wrote:
> > > Call secure services to enable ACTLR[0] (Enable invalidates of BTB with
> > > ICIALLU) when branch hardening is enabled for kernel.
> > 
> > As mentioned elsewhere, I don't think this is a good idea - if the secure
> > world is not implementing the Spectre workarounds, then the _system_ is
> > exploitable.
> > 
> > If the secure world is implementing the spectre workarounds, it will
> > already have enabled the IBE bit (which is r/w from secure, read only
> > from non-secure.)
> > 
> > So, basically, lack of the IBE bit being set is basically telling the
> > kernel that it's running on a vulnerable platform _even if the kernel
> > were to set it through some means_.
> 
> On GP devices OMAP5/DRA7, there is no possibility to update secure side
> since "secure world" is ROM and there are no override mechanisms possible.
> on HS devices, I agree, appropriate PPA will do the workarounds as well.
> 
> However, this patch is to enable the IBE enable on GP device for _a_
> core can only be done via SMC services that ROM provides for
> specifically the reasons you have already stated. u-boot will only
> enable the IBE for the boot core, by the time the secondary cores start
> up, u-boot is long gone.. so someone has to invoke the SMC call to
> enable the IBE bit for the secondary core.
> 
> This is what the patch does.
> 
> If the above explanation makes sense, I will add that to the commit log
> as well.

Probably good idea to also add a comment to the code that this
is for the secondary core.

Regards,

Tony
Tony Lindgren June 25, 2018, 8:04 a.m. UTC | #4
* Tony Lindgren <tony@atomide.com> [180625 01:03]:
> * Nishanth Menon <nm@ti.com> [180613 13:31]:
> > 
> > If the above explanation makes sense, I will add that to the commit log
> > as well.
> 
> Probably good idea to also add a comment to the code that this
> is for the secondary core.

Or rename the function to make it specific to secondary core.

Regards,

Tony
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
index 69df3620eca5..28fc80ea675b 100644
--- a/arch/arm/mach-omap2/omap-smp.c
+++ b/arch/arm/mach-omap2/omap-smp.c
@@ -109,6 +109,32 @@  void omap5_erratum_workaround_801819(void)
 static inline void omap5_erratum_workaround_801819(void) { }
 #endif
 
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+static void omap5_harden_predictor(void)
+{
+	u32 acr, acr_mask;
+
+	asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
+
+	/*
+	 * BIT(0) - Disables streaming. All write-allocate lines allocate in
+	 */
+	acr_mask = BIT(0);
+
+	/* do we already have it done.. if yes, skip expensive smc */
+	if ((acr & acr_mask) == acr_mask)
+		return;
+
+	acr |= acr_mask;
+	omap_smc1(OMAP5_DRA7_MON_SET_ACR_INDEX, acr);
+
+	pr_debug("%s: ARM ACR setup for CVE_2017_5715 applied on CPU%d\n",
+		 __func__, smp_processor_id());
+}
+#else
+static inline void omap5_harden_predictor(void) { }
+#endif
+
 static void omap4_secondary_init(unsigned int cpu)
 {
 	/*
@@ -131,6 +157,8 @@  static void omap4_secondary_init(unsigned int cpu)
 		set_cntfreq();
 		/* Configure ACR to disable streaming WA for 801819 */
 		omap5_erratum_workaround_801819();
+		/* Enable ACR to allow for ICUALLU workaround */
+		omap5_harden_predictor();
 	}
 
 	/*