firmware icon indicating copy to clipboard operation
firmware copied to clipboard

Raspberry Pi 4 missing firmware mitigation for Spectre v4

Open lilyanatia opened this issue 3 years ago • 31 comments

Describe the bug the Raspberry Pi 4's Cortex-A72 cores are vulnerable to Spectre v4 (Speculative Store Bypass, CVE-2018-3639). according to ARM, there's a firmware mitigation available for this vulnerability, but the mitigation seems to not be present on the Raspberry Pi 4.

To reproduce

  1. download and build https://github.com/google/safeside
  2. run the spectre_v4 demo

Expected behaviour

Leaking the string: Does not converge

Actual behaviour

Leaking the string: It's a s3kr3t!!!
Done!

System

  • Which model of Raspberry Pi? Pi 4
  • Which OS and version (cat /etc/rpi-issue)? Arch Linux ARM aarch64
  • Which firmware version (vcgencmd version)?
Jul 13 2020 13:56:29
Copyright (c) 2012 Broadcom
version adcebbdb7b415c623931e80795ba3bae68dcc4fa (clean) (release) (start_x)
  • Which kernel version (uname -a)?
Linux marten 5.8.0-1-ARCH #1 SMP Sun Aug 9 00:03:44 UTC 2020 aarch64 GNU/Linux

lilyanatia avatar Aug 11 '20 05:08 lilyanatia

The linked page appears to say

For Cortex-A57 and Cortex-A72:

    Set bit 55 (disable load pass store) of CPUACTLR_EL1 (S3_1_C15_C2_0).

so I guess that needs to go into the ARM stub.

The issue is likely to be that this has a significant performance hit for all those who don't care about Spectre, so you now have to gain another version of the stub.

6by9 avatar Aug 11 '20 07:08 6by9

The issue is likely to be that this has a significant performance hit for all those who don't care about Spectre, so you now have to gain another version of the stub.

maybe there could be a config.txt option to enable or disable it?

lilyanatia avatar Aug 11 '20 07:08 lilyanatia

maybe there could be a config.txt option to enable or disable it?

Yes, things are possible, it just becomes further maintenance overheads.

You can already override the built-in stubs by adding an appropriate file (probably armstub8-gic.bin in your case) to /boot. That's why I was looking for the stub source files - now found https://github.com/raspberrypi/tools/tree/master/armstubs

6by9 avatar Aug 11 '20 08:08 6by9

I built armstub8-gic.bin with the following changes:

diff --git a/armstubs/armstub8.S b/armstubs/armstub8.S
index f85eb521..188c0d55 100644
--- a/armstubs/armstub8.S
+++ b/armstubs/armstub8.S
@@ -65,6 +65,9 @@
 #define SCR_VAL \
     (SCR_RW | SCR_HCE | SCR_SMD | SCR_RES1_5 | SCR_RES1_4 | SCR_NS)
 
+#define CPUACTLR_EL1		S3_1_C15_C2_0
+#define CPUACTLR_EL1_DLPS	BIT(55)
+
 #define CPUECTLR_EL1		S3_1_C15_C2_1
 #define CPUECTLR_EL1_SMPEN	BIT(6)
 
@@ -124,6 +127,10 @@ _start:
 	mov x0, #CPUECTLR_EL1_SMPEN
 	msr CPUECTLR_EL1, x0
 
+	/* mitigate Spectre v4 */
+	mov x0, #CPUACTLR_EL1_DLPS
+	msr CPUACTLR_EL1, x0
+
 #ifdef GIC
         bl      setup_gic
 #endif

and after booting with that armstub, the spectre_v4 demo does gives the desired result ("Does not converge").

lilyanatia avatar Aug 11 '20 10:08 lilyanatia

Thanks. I didn't know your knowledge level, but provided the links in case you could make use of them.

Feel free to create a PR for the stub. Ideally wrapped in a #ifdef so that it can be built only when desired. We always prefer to provide the correct attribution for contributions to the system.

Do you have any numbers on the impact that this mitigation has? I know some of them on x86 were HUGE, but I haven't been following this one.

6by9 avatar Aug 11 '20 10:08 6by9

PR created: https://github.com/raspberrypi/tools/pull/115

Do you have any numbers on the impact that this mitigation has?

so far all the testing I've done shows no impact other than Spectre v4 no longer working. do you have any recommendations of benchmarks that would be likely to show a difference?

lilyanatia avatar Aug 11 '20 18:08 lilyanatia

personally I'd want ZERO impact before enabling this by default. How many of us are using our Pi's in a multi-user environment?

alanbork avatar Sep 16 '20 22:09 alanbork

the PR wouldn't enable the mitigation by default (it's wrapped in a #ifdef). enabling it by default would definitely require more testing to determine what, if any, performance impact it has.

lilyanatia avatar Sep 17 '20 00:09 lilyanatia

the PR wouldn't enable the mitigation by default (it's wrapped in a #ifdef)

What PR? If you mean the code above, it's not wrapped in an ifdef - the one ifdef shown sets up the GIC, if GIC mode is enabled.

ghost avatar Sep 17 '20 19:09 ghost

https://github.com/raspberrypi/tools/pull/115, which is linked three comments above yours.

lilyanatia avatar Sep 17 '20 20:09 lilyanatia

According to this: https://github.com/speed47/spectre-meltdown-checker, the Raspberry Pi 4 is also vulnerable to CVE-2018-3640.

AaronDewes avatar Sep 30 '20 17:09 AaronDewes

it is, but ARM says:

This side-channel can be used to determine the values held in system registers that should not be accessible. While it is undesirable for lower exception levels to be able to access these data values, for the majority of system registers, the leakage of this information is not material.

and

Although Arm is addressing this issue in new CPU releases, in general, it is not believed that software mitigations for this issue are necessary. For system registers that are not in use when working at a particular exception level and which are felt to be sensitive, it would in principle be possible for the software of a higher exception level to substitute in dummy values into the system registers while running at that exception level. In particular, this mechanism could be used in conjunction with the mitigation for Variant 3 to ensure that the location of the VBAR_EL1 while running at EL0 is not indicative of the virtual address layout of the EL1 memory, so preventing the leakage of information useful for compromising KASLR.

mitigating CVE-2018-3640 would probably require changes to the Linux kernel, rather than firmware, and probably isn't worth the trouble.

lilyanatia avatar Sep 30 '20 22:09 lilyanatia

Any updates on this? People are using the Raspberry increasingly for normal desktop tasks, so this seems like it should be fixed (with the secure variant being the default). Does the Linux kernel contain the necessary mitigations as well?

ell1e avatar Sep 16 '21 02:09 ell1e

idk, pi4 is pretty slow as a desktop os, I'd certainly prefer it not the default, or at the least very easy to switch.

alanbork avatar Sep 16 '21 02:09 alanbork

Spectre mitigation is enabled in the kernel, and https://github.com/raspberrypi/tools/pull/117 was merged last October. Do you have some reason to believe this is not sufficient?

pelwell avatar Sep 16 '21 08:09 pelwell

Because the issue is still open?

JamesH65 avatar Sep 16 '21 09:09 JamesH65

the issue is still open

That, and the referenced https://github.com/raspberrypi/tools/pull/115 hasn't been merged. Was it superseded by https://github.com/raspberrypi/tools/pull/117 then?

ell1e avatar Sep 16 '21 11:09 ell1e

Hmm... the PR for https://github.com/raspberrypi/tools/pull/117 suggests that it could supersede https://github.com/raspberrypi/tools/pull/115, but although we've enabled the standard kernel mitigation I don't see anything that actually does set the "disable load pass store" bit of CPUACTLR_EL1.

Since the rewrite, https://github.com/raspberrypi/tools/pull/115 no longer applies and will need to be updated, but that will still leave the mitigation as optional (enabled either by an external stub file or by an as-yet-non-existent firmware config.txt setting).

pelwell avatar Sep 16 '21 16:09 pelwell

and (perhaps) more importantly, if it's in there by default, how do we disable the mitigations?

alanbork avatar Sep 16 '21 16:09 alanbork

If what is in there by default?:

but that will still leave the mitigation as optional (enabled either by an external stub file or by an as-yet-non-existent firmware config.txt setting).

pelwell avatar Sep 16 '21 16:09 pelwell

the cpu-slowing mitigation. we posted at the same time, so now that I've read your post it sounds like no spectre mitigations are enabled by default, which is good.

alanbork avatar Sep 16 '21 16:09 alanbork

enabled either by an external stub file or by an as-yet-non-existent firmware config.txt setting

Wouldn't it make the most sense if it was somehow set through the kernel mitigations option as well? Or is that not feasible? Any other way is likely to be missed by someone: if it defaults to on people who use the raspberry without untrusted code will get unexpected slowness and if it defaults to off people will set the kernel mitigations option and possibly think they're safe. Neither of these outcomes seems ideal.

ell1e avatar Sep 16 '21 16:09 ell1e

Not sure what you are suggesting, but config.txt has the best visibility I know of.

On Thu, Sep 16, 2021 at 9:27 AM Ellie @.***> wrote:

enabled either by an external stub file or by an as-yet-non-existent firmware config.txt setting

Wouldn't it make the most sense if it was somehow set through the kernel mitigations option as well? Or is that not feasible? Any other way is likely to be missed by someone: if it defaults to on people who use the raspberry without untrusted code will get unexpected slowness and if it defaults to off people will set the kernel mitigations option and think they're safe. Neither of these outcomes seems ideal.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/raspberrypi/firmware/issues/1451#issuecomment-921050951, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANPGZ5M4RTOCB2LWTFCYBU3UCILH7ANCNFSM4P2U4UFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

alanbork avatar Sep 16 '21 16:09 alanbork

The mitigations kernel option. It is for sure more known under the general Linux population, and since it needs to be set to the desired value anyway having a separate config.txt option that unexpectedly also controls a part of this has IMHO for sure less visibility. As I understand it, if someone sets mitigations=auto,nosmt the expectation would be that everything is enabled without digging into other options.

ell1e avatar Sep 16 '21 16:09 ell1e

If the kernel mitigation is enabled on the command line then detecting it and setting the magic bit accordingly is feasible,

pelwell avatar Sep 16 '21 16:09 pelwell

If that would reliably work with major distros and partitioning setups (like with LUKS/unlocking initramfs), then that sounds like the best variant to me personally, linking it to the kernel mitigations option. But maybe someone else has input on this as well.

ell1e avatar Sep 16 '21 16:09 ell1e

The whole breakdown between cmdline.txt and config.txt is confusing as heck, you never know where to look for your options. But at least config.txt allows comments so you can list what options are available and briefly describe their function. cmdline.txt is hugely less user friendly.

On Thu, Sep 16, 2021 at 9:53 AM Ellie @.***> wrote:

If that would reliably work with major distros and partitioning setups (like with LUKS/unlocking initramf), then that sounds like the best variant to me personally, linking it to the kernel mitigations. But maybe someone else has input on this as well.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/raspberrypi/firmware/issues/1451#issuecomment-921069040, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANPGZ5IYPUKCKJ4NPACIE2DUCIOJBANCNFSM4P2U4UFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

alanbork avatar Sep 16 '21 17:09 alanbork

Via rpi4-uefi + the generic Fedora ARM64 image I think the kernel options usually reside in /etc/default/grub which I think might support comments (don't quote me on that), so maybe that would be interesting to you. I am wondering now though if whatever detection @pelwell had in mind would still work when using rpi4-uefi.

ell1e avatar Sep 16 '21 17:09 ell1e

No - it wouldn't work with UEFI, grub or any other ways of passing in a kernel command line, which makes me think the config.txt setting to set the "disable load pass store" bit (and only do that) is the most general solution.

pelwell avatar Sep 17 '21 11:09 pelwell

Yes, that might be correct then @ config.txt being best. My suggestion would still be to enable it by default, but you'll probably get tons of voices (as above) disagreeing. But IMHO having an unexpected discovery you ran slower than desired is better than having an unexpected discovery you thought all mitigations were on when they were not.

ell1e avatar Sep 17 '21 17:09 ell1e