| Message ID | 20180612213611.2484-1-nm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
* 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 <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 --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(); } /*
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(+)