Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Raspberry Pi 4 missing firmware mitigation for Spectre v4 #1451

Open
lilyanatia opened this issue Aug 11, 2020 · 31 comments
Open

Raspberry Pi 4 missing firmware mitigation for Spectre v4 #1451

lilyanatia opened this issue Aug 11, 2020 · 31 comments

Comments

@lilyanatia
Copy link

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
@6by9
Copy link

6by9 commented Aug 11, 2020

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.

@lilyanatia
Copy link
Author

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?

@6by9
Copy link

6by9 commented Aug 11, 2020

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

@lilyanatia
Copy link
Author

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 lilyanatia mentioned this issue Aug 11, 2020
Open
@6by9
Copy link

6by9 commented Aug 11, 2020

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.

@lilyanatia
Copy link
Author

PR created: raspberrypi/tools#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?

@alanbork
Copy link

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?

@lilyanatia
Copy link
Author

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.

@ghost
Copy link

ghost commented Sep 17, 2020

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.

@lilyanatia
Copy link
Author

raspberrypi/tools#115, which is linked three comments above yours.

@AaronDewes
Copy link

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

@lilyanatia
Copy link
Author

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.

@ell1e
Copy link

ell1e commented Sep 16, 2021

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?

@alanbork
Copy link

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.

@pelwell
Copy link
Contributor

pelwell commented Sep 16, 2021

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

@JamesH65
Copy link
Contributor

Because the issue is still open?

@ell1e
Copy link

ell1e commented Sep 16, 2021

the issue is still open

That, and the referenced raspberrypi/tools#115 hasn't been merged. Was it superseded by raspberrypi/tools#117 then?

@pelwell
Copy link
Contributor

pelwell commented Sep 16, 2021

Hmm... the PR for raspberrypi/tools#117 suggests that it could supersede raspberrypi/tools#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, raspberrypi/tools#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).

@alanbork
Copy link

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

@pelwell
Copy link
Contributor

pelwell commented Sep 16, 2021

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).

@alanbork
Copy link

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.

@ell1e
Copy link

ell1e commented Sep 16, 2021 •
edited
Loading

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.

@alanbork
Copy link

alanbork commented Sep 16, 2021 via email

@ell1e
Copy link

ell1e commented Sep 16, 2021 •
edited
Loading

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.

@pelwell
Copy link
Contributor

pelwell commented Sep 16, 2021

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

@ell1e
Copy link

ell1e commented Sep 16, 2021 •
edited
Loading

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.

@alanbork
Copy link

alanbork commented Sep 16, 2021 via email

@ell1e
Copy link

ell1e commented Sep 16, 2021 •
edited
Loading

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.

@pelwell
Copy link
Contributor

pelwell commented Sep 17, 2021

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.

@ell1e
Copy link

ell1e commented Sep 17, 2021 •
edited
Loading

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.

@alanbork
Copy link

alanbork commented Sep 17, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@lilyanatia @JamesH65 @6by9 @pelwell @alanbork @ell1e @AaronDewes