diff mbox series

[3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

Message ID 20180612202411.29798-4-nm@ti.com
State Accepted
Commit dbb7caf110c4a7b9afb7cdc195ac6967d3a30adf
Headers show
Series ARM: Provide workaround setup bits for CVE-2017-5715 (A8/A15) | expand

Commit Message

Nishanth Menon June 12, 2018, 8:24 p.m. UTC
Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
function to setup the bits, we are able to override the settings.

Without this enabled, Linux kernel reports:
CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable

With this enabled, Linux kernel reports:
CPU0: Spectre v2: using ICIALLU workaround

NOTE: This by itself does not enable the workaround for CPU1 (on
OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Marek Vasut June 12, 2018, 11:06 p.m. UTC | #1
On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> function to setup the bits, we are able to override the settings.
> 
> Without this enabled, Linux kernel reports:
> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> 
> With this enabled, Linux kernel reports:
> CPU0: Spectre v2: using ICIALLU workaround
> 
> NOTE: This by itself does not enable the workaround for CPU1 (on
> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/mach-omap2/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index 3bb1ecb58de0..77820cc8d1e4 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -53,6 +53,7 @@ config OMAP54XX
>  	bool "OMAP54XX SoC"
>  	select ARM_ERRATA_798870
>  	select SYS_THUMB_BUILD
> +	select ARM_CORTEX_A15_CVE_2017_5715
>  	imply NAND_OMAP_ELM
>  	imply NAND_OMAP_GPMC
>  	imply SPL_DISPLAY_PRINT
> 

Can this be enabled for all CA15 systems somehow ? I am sure there are
more that are vulnerable.
Nishanth Menon June 13, 2018, 1:40 p.m. UTC | #2
On 23:06-20180612, Marek Vasut wrote:
> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> > Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> > function to setup the bits, we are able to override the settings.
> > 
> > Without this enabled, Linux kernel reports:
> > CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> > 
> > With this enabled, Linux kernel reports:
> > CPU0: Spectre v2: using ICIALLU workaround
> > 
> > NOTE: This by itself does not enable the workaround for CPU1 (on
> > OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> > 
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> >  arch/arm/mach-omap2/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 3bb1ecb58de0..77820cc8d1e4 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -53,6 +53,7 @@ config OMAP54XX
> >  	bool "OMAP54XX SoC"
> >  	select ARM_ERRATA_798870
> >  	select SYS_THUMB_BUILD
> > +	select ARM_CORTEX_A15_CVE_2017_5715
> >  	imply NAND_OMAP_ELM
> >  	imply NAND_OMAP_GPMC
> >  	imply SPL_DISPLAY_PRINT
> > 
> 
> Can this be enabled for all CA15 systems somehow ? I am sure there are
> more that are vulnerable.

I just dont know how to make smc call convention generic. This is the
reason why v7_arch_cp15_set_acr is setup as a weak function. you'd have
to implement it specific to SoC (in many newer SoCs, you might
potentially be able to make at least few implementations generic using
PSCI). NOTE: this is the same trouble with erratum 801819 implementation
as well.
Russell King (Oracle) June 13, 2018, 5:36 p.m. UTC | #3
On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> > Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> > function to setup the bits, we are able to override the settings.
> > 
> > Without this enabled, Linux kernel reports:
> > CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> > 
> > With this enabled, Linux kernel reports:
> > CPU0: Spectre v2: using ICIALLU workaround
> > 
> > NOTE: This by itself does not enable the workaround for CPU1 (on
> > OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> > 
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> >  arch/arm/mach-omap2/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 3bb1ecb58de0..77820cc8d1e4 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -53,6 +53,7 @@ config OMAP54XX
> >  	bool "OMAP54XX SoC"
> >  	select ARM_ERRATA_798870
> >  	select SYS_THUMB_BUILD
> > +	select ARM_CORTEX_A15_CVE_2017_5715
> >  	imply NAND_OMAP_ELM
> >  	imply NAND_OMAP_GPMC
> >  	imply SPL_DISPLAY_PRINT
> > 
> 
> Can this be enabled for all CA15 systems somehow ? I am sure there are
> more that are vulnerable.

I think you're missing the point.

Spectre affects the _entire_ system.  Working around it in just the
kernel does not mean that the system is no longer vulnerable.

Fixing the "system" means implementing the fixes also in the secure
world, which on A15 and A8 also means setting the IBE bit there.  If
the IBE bit is set in the secure world, it will also be set in the
non-secure world.

The fact that the kernel is complaining is telling you that the
system as a whole does not have the workarounds in place to mitigate
against the vulnerability.  Merely setting the IBE bit via some
secure API doesn't "magically" fix the secure world.

So, even if you were to set the IBE bit via some magic secure API,
the fact still remains: even with these workarounds in place, as I
understand it, the _system as a whole_ remains vulnerable - you
might as well _not_ have the kernel workarounds.
Marek Vasut June 13, 2018, 8:36 p.m. UTC | #4
On 06/13/2018 07:36 PM, Russell King - ARM Linux wrote:
> On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
>> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
>>> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
>>> function to setup the bits, we are able to override the settings.
>>>
>>> Without this enabled, Linux kernel reports:
>>> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
>>>
>>> With this enabled, Linux kernel reports:
>>> CPU0: Spectre v2: using ICIALLU workaround
>>>
>>> NOTE: This by itself does not enable the workaround for CPU1 (on
>>> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>> ---
>>>  arch/arm/mach-omap2/Kconfig | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>>> index 3bb1ecb58de0..77820cc8d1e4 100644
>>> --- a/arch/arm/mach-omap2/Kconfig
>>> +++ b/arch/arm/mach-omap2/Kconfig
>>> @@ -53,6 +53,7 @@ config OMAP54XX
>>>  	bool "OMAP54XX SoC"
>>>  	select ARM_ERRATA_798870
>>>  	select SYS_THUMB_BUILD
>>> +	select ARM_CORTEX_A15_CVE_2017_5715
>>>  	imply NAND_OMAP_ELM
>>>  	imply NAND_OMAP_GPMC
>>>  	imply SPL_DISPLAY_PRINT
>>>
>>
>> Can this be enabled for all CA15 systems somehow ? I am sure there are
>> more that are vulnerable.
> 
> I think you're missing the point.

Please read the patch again.

This enables it only for a specific SoC. My point being, this should be
enabled for all SoCs with CA15, not just some select few.

> Spectre affects the _entire_ system.  Working around it in just the
> kernel does not mean that the system is no longer vulnerable.
> 
> Fixing the "system" means implementing the fixes also in the secure
> world, which on A15 and A8 also means setting the IBE bit there.  If
> the IBE bit is set in the secure world, it will also be set in the
> non-secure world.
> 
> The fact that the kernel is complaining is telling you that the
> system as a whole does not have the workarounds in place to mitigate
> against the vulnerability.  Merely setting the IBE bit via some
> secure API doesn't "magically" fix the secure world.
> 
> So, even if you were to set the IBE bit via some magic secure API,
> the fact still remains: even with these workarounds in place, as I
> understand it, the _system as a whole_ remains vulnerable - you
> might as well _not_ have the kernel workarounds.
>
Nishanth Menon June 13, 2018, 9:31 p.m. UTC | #5
On 20:36-20180613, Marek Vasut wrote:
> On 06/13/2018 07:36 PM, Russell King - ARM Linux wrote:
> > On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
> >> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> >>> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> >>> function to setup the bits, we are able to override the settings.
> >>>
> >>> Without this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> >>>
> >>> With this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: using ICIALLU workaround
> >>>
> >>> NOTE: This by itself does not enable the workaround for CPU1 (on
> >>> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> >>>
> >>> Signed-off-by: Nishanth Menon <nm@ti.com>
> >>> ---
> >>>  arch/arm/mach-omap2/Kconfig | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> >>> index 3bb1ecb58de0..77820cc8d1e4 100644
> >>> --- a/arch/arm/mach-omap2/Kconfig
> >>> +++ b/arch/arm/mach-omap2/Kconfig
> >>> @@ -53,6 +53,7 @@ config OMAP54XX
> >>>  	bool "OMAP54XX SoC"
> >>>  	select ARM_ERRATA_798870
> >>>  	select SYS_THUMB_BUILD
> >>> +	select ARM_CORTEX_A15_CVE_2017_5715
> >>>  	imply NAND_OMAP_ELM
> >>>  	imply NAND_OMAP_GPMC
> >>>  	imply SPL_DISPLAY_PRINT
> >>>
> >>
> >> Can this be enabled for all CA15 systems somehow ? I am sure there are
> >> more that are vulnerable.
> > 
> > I think you're missing the point.
> 
> Please read the patch again.
> 
> This enables it only for a specific SoC. My point being, this should be
> enabled for all SoCs with CA15, not just some select few.
> 

As I had previously responded in
https://marc.info/?l=u-boot&m=152889727127549&w=2

I am not disagreeing this needs to be done for all CA15 based SoCs
(and A8s for previous patches ...), but.. I am not sure what you'd
like me to do here -> I just dont know what the SMC convention is
for all SoCs with CA15! I can help with TI SoCs for sure.. but then,
Russell has a point that this is just one part of the solution -> on
devices that provide secure services, there is definitely a need to
lock the secure entry points down as well. But, specifically to this
patch, do recommend an alternative if one exists.. will gladly follow.
Russell King (Oracle) June 13, 2018, 9:47 p.m. UTC | #6
On Wed, Jun 13, 2018 at 10:36:56PM +0200, Marek Vasut wrote:
> On 06/13/2018 07:36 PM, Russell King - ARM Linux wrote:
> > On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
> >> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> >>> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> >>> function to setup the bits, we are able to override the settings.
> >>>
> >>> Without this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> >>>
> >>> With this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: using ICIALLU workaround
> >>>
> >>> NOTE: This by itself does not enable the workaround for CPU1 (on
> >>> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> >>>
> >>> Signed-off-by: Nishanth Menon <nm@ti.com>
> >>> ---
> >>>  arch/arm/mach-omap2/Kconfig | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> >>> index 3bb1ecb58de0..77820cc8d1e4 100644
> >>> --- a/arch/arm/mach-omap2/Kconfig
> >>> +++ b/arch/arm/mach-omap2/Kconfig
> >>> @@ -53,6 +53,7 @@ config OMAP54XX
> >>>  	bool "OMAP54XX SoC"
> >>>  	select ARM_ERRATA_798870
> >>>  	select SYS_THUMB_BUILD
> >>> +	select ARM_CORTEX_A15_CVE_2017_5715
> >>>  	imply NAND_OMAP_ELM
> >>>  	imply NAND_OMAP_GPMC
> >>>  	imply SPL_DISPLAY_PRINT
> >>>
> >>
> >> Can this be enabled for all CA15 systems somehow ? I am sure there are
> >> more that are vulnerable.
> > 
> > I think you're missing the point.
> 
> Please read the patch again.

Stop this madness - I /know/ precisely what _this_ patch is doing.
My reply was to *your* comment about extending it "for all CA15
systems".

> This enables it only for a specific SoC. My point being, this should be
> enabled for all SoCs with CA15, not just some select few.

Let's try again... the short answer: NO.

The long answer:

Enabling IBE does *not* universally solve the problem for all SoCs
using CA15.  It merely enables the instructions required for
workarounds in the *kernel* part of the system to take effect.  That
leaves the rest of the system *vulnerable*.

Just in the same way that we have to apply the workarounds /not only/
at the kernel level, but also the hypervisor level for KVM to prevent
KVM being vulnerable, the workarounds _also_ need to be appled at
secure firmware level, as I tried to explain.

Nishanth's OMAP5 case is kind of special because, from what he's
said (a) there's nothing in the secure world that really matters,
and (b) there's nothing that can be done to fix the secure world
because that firmware is in ROM and can never be changed.

That isn't true "for all CA15 systems", and if we're wanting
systems to be properly fixed, then fixing the problem properly
(by fixing the secure world to set IBE *and* implement the
workarounds there) is the right thing.

Setting the IBE bit in the kernel for all CA15 means that, while
we solve the kernel part (and KVM part), the secure world will
remain vulnerable if it has no protection - and worse, people
probably haven't thought enough about this, or know enough about it,
to realise that the vulnerability still exists all the time that any
part of the system has not been fixed.  So, having the kernel print
a warning is critical.

If it was just the case that the kernel was all that was affected,
then KVM wouldn't have needed to be fixed, but the reality is it
needed to be fixed and has been.  The same applies to the secure
world.

Think about this: if you can trick the secure world into speculatively
executing a set of gadgets by manipulating the ARM register values
passed to the SMC call to read secure world memory - or any memory
you shouldn't have access to (like the kernel) then setting the IBE
bit and having the kernel fixes in place is completely meaningless.
As I said below, the system _remains_ vulnerable.

Take a look at the work going on with ARM64 syscalls - they're now
explicitly zeroing all registers on entry that are not an explicit
argument to any syscall.  The reason is to prevent userspace doing
exactly what I've described above, except with the kernel.

So, should we extend it "for all CA15 systems".  No, definitely not
without knowing exactly what the situation is for each and every one.
Having it done in firmware - the same firmware that switches the
CPU out of secure mode - is the right answer where it's possible to
do so.  That won't happen if we apply a "fix" to set IBE as a big
hammer to the kernel.

> > Spectre affects the _entire_ system.  Working around it in just the
> > kernel does not mean that the system is no longer vulnerable.
> > 
> > Fixing the "system" means implementing the fixes also in the secure
> > world, which on A15 and A8 also means setting the IBE bit there.  If
> > the IBE bit is set in the secure world, it will also be set in the
> > non-secure world.
> > 
> > The fact that the kernel is complaining is telling you that the
> > system as a whole does not have the workarounds in place to mitigate
> > against the vulnerability.  Merely setting the IBE bit via some
> > secure API doesn't "magically" fix the secure world.
> > 
> > So, even if you were to set the IBE bit via some magic secure API,
> > the fact still remains: even with these workarounds in place, as I
> > understand it, the _system as a whole_ remains vulnerable - you
> > might as well _not_ have the kernel workarounds.

And the long answer is basically what I said ^^^^^ there.
Tom Rini June 29, 2018, 8:53 p.m. UTC | #7
On Tue, Jun 12, 2018 at 03:24:10PM -0500, Nishanth Menon wrote:

> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr

> function to setup the bits, we are able to override the settings.

> 

> Without this enabled, Linux kernel reports:

> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable

> 

> With this enabled, Linux kernel reports:

> CPU0: Spectre v2: using ICIALLU workaround

> 

> NOTE: This by itself does not enable the workaround for CPU1 (on

> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.

> 

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


Applied to u-boot/master, thanks!

-- 
Tom
diff mbox series

Patch

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 3bb1ecb58de0..77820cc8d1e4 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -53,6 +53,7 @@  config OMAP54XX
 	bool "OMAP54XX SoC"
 	select ARM_ERRATA_798870
 	select SYS_THUMB_BUILD
+	select ARM_CORTEX_A15_CVE_2017_5715
 	imply NAND_OMAP_ELM
 	imply NAND_OMAP_GPMC
 	imply SPL_DISPLAY_PRINT