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.

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

AMD SPI lock check #4094

Open
kerneis-anssi opened this issue Dec 16, 2021 · 15 comments
Open

AMD SPI lock check #4094

kerneis-anssi opened this issue Dec 16, 2021 · 15 comments

Comments

@kerneis-anssi
Copy link
Collaborator

kerneis-anssi commented Dec 16, 2021

I have investigated how to implement a check that the BIOS sets the correct flags on AMD platforms to prevent SPI overwrite.

AMD PPR for family 17h has the following text:

9.1.7.3.1 Enable SPI ROM Protection

  1. Program D14F3x050 FCH::ITF::LPC::RomProtect to enable protection for Read or Write memory accesses to SPI
    flash memory space. Up to four memory ranges specified by Rom Protect registers can be protected.
  2. Program SPIx04 FCH::ITF::SPI::SPIRestrictedCmd or SPIx08 FCH::ITF::SPI::SPIRestrictedCmd2 with SPI
    commands that are required before doing write accesses to the SPI ROM. Use commands such as WR_EN,
    WR_Status and Erase when programming these restricted command registers.
  3. Program SPIx00 FCH::ITF::SPI::SPICntrl0[SpiHostAccessRomEn] = 0 to protect the registers above from being
    reprogrammed.
  4. Program SPIx1D FCH::ITF::SPI::AltSPICS[SpiProtectEn0] = 1 to apply Read/Write protection on ranges defined
    by Rom Protect registers (D14F3x50, D14F3x54, D14F3x58, D14F3x5C).
  5. Program SPIx1D FCH::ITF::SPI::AltSPICS[SpiProtectLock] = 1 to make bits 3, 4, and 5 non-writable.
    Accesses are now blocked by the SPI host preventing write accesses to the SPI ROM.

RomProtect in point 1 lives in PCI configuration space, so should be accessible via sysfs. However, the rest of the registers are mapped in main memory: SPIx04 means SPI_BASE_ADDRESS + 0x04, where SPI_BASE_ADDRESS is outside of PCI config space (in practice it's 0xFEC10000, but it's also stored as a PCI config if we don't want to hard code it). Therefore, I don't see a way to access it on a locked down or strict_dev_mem kernel. Debian — and I guess other distributions — has a patch to enable lock down automatically when secure boot is enabled.

In principle, this is something similar to Intel's BIOS Lock Enable and friends, and we'd like the kernel to export it. Given that the latest attempt I could find to do that for Intel was abandoned a year ago, I'm not sure how much hope there is on that front. cc @dgutson

There is one driver right now in the kernel that accesses this memory space: the amd-spi driver. I'm not clear how SPI drivers are plumbed in the kernel, but looking at the code and the AMD spec, I can guess that SPI_BASE_ADDRESS ends up being assigned to amd_spi.io_remap_addr in that code. The code does not access or checks the lock flags though, and I'm don't think it belongs to a SPI driver to do that. We could try and reach out to the AMD maintainer of that driver.

Any thought?

@kerneis-anssi
Copy link
Collaborator Author

On a related note, AMD also documents (somewhat poorly) the memory mappings of SPI ROM. There are in fact a couple registers that hold the low and high addresses where the ROM is mapped.
I've noticed the recent attempt to expose this in the kernel, where @hughsie chimed in. I think, at least in the case of AMD, this could be used to restrict the driver to a specific CPU family (as Greg KH requested) and avoid hardcoding an address, instead deriving it from a documented source. It doesn't solve the maintainer's issue of exposing this to userspace and having to maintain it long-term, of course. If you are aware of plans to rework that patch series, I'd be happy to provide input for the AMD side of thing and my (limited) experimental results.

@kerneis-anssi kerneis-anssi self-assigned this Dec 16, 2021
@lemonpy
Copy link

lemonpy commented Dec 16, 2021

I'm not sure if hans-gert.dahmen will continue with the patch. About Intel, we will work with the maintainer on a way to expose SPI flags and content to userspace in a safe way on paltforms with hardware sequencing link after this patch is merged.

@superm1
Copy link
Member

superm1 commented Dec 16, 2021

Therefore, I don't see a way to access it on a locked down or strict_dev_mem kernel. Debian — and I guess other distributions — has a patch to enable lock down automatically when secure boot is enabled.

Yeah no relying on accessing things you can't from userspace when locked down.

AMD PPR for family 17h has the following text:

FYI, for at least one product the PPR is also published for family 19h recently.

It's slightly different, so yes the behavior would need to be model dependent.

@kerneis-anssi
Copy link
Collaborator Author

kerneis-anssi commented Dec 16, 2021

FYI, for at least one product the PPR is also published for family 19h recently.

Thanks I missed that and only had the useless « preliminary » version.

For the record, the corresponding section (looks like they changed the naming convention but the procedure is the same):

7.1.5.3.1 Enable SPI ROM Protection

  1. Program D14F3x050 FCH::LPCPCICFG::rom_protect_0 to enable protection for Read or Write memory accesses
    to SPI flash memory space. Up to four memory ranges specified by Rom Protect registers can be protected.
  2. Program SPIx04 FCH::LPCHOSTSPIREG::spi_restrictedcmd_register or SPIx08
    FCH::LPCHOSTSPIREG::spi_restrictedcmd2_register with SPI commands that are required before doing write
    accesses to the SPI ROM. Use commands such as WR_EN, WR_Status and Erase when programming these
    restricted command registers.
  3. Program SPIx00 FCH::LPCHOSTSPIREG::spi_cntrl0_register[spiaccessmacromen] = 0 to protect the registers
    above from being reprogrammed.
  4. Program SPIx1D FCH::LPCHOSTSPIREG::alt_spi_cs_register[spiprotecten0] = 1 to apply Read/Write protection
    on ranges defined by Rom Protect registers (D14F3x50, D14F3x54, D14F3x58, D14F3x5C).
  5. Program SPIx1D FCH::LPCHOSTSPIREG::alt_spi_cs_register[spiprotectlock] = 1 to make bits 3, 4, and 5 non-
    writable.
    Accesses are now blocked by the SPI host preventing write accesses to the SPI ROM.

So I guess the best approach here is to (a) keep an eye on changes to Intel driver to see if a common or similar approach can be found for both, and (b) reach out to AMD maintainer to think about exposing this information.

@hughsie
Copy link
Member

hughsie commented Dec 16, 2021

However, the rest of the registers are mapped in main memory

Is that io space? @zaolin seemed to think that you could read some of that even with things locked down -- although no idea if that's goig to work longer term. I also think the work by @dgutson is probably the way we want to go; i.e. have a sysfs class that drivers can just "implement" -- I believe the consensus was that the concept was sound but there was some bikesheding on the name itself. The idea was to ask @kees for some help actually getting this over the finishing line. Some context for Kees -- https://lwn.net/Articles/824829/

I'm also really crossing my fingers for a mtd-ified intel-spi -- thanks @lemonpy for continuing to push this forward.

@flanfly
Copy link
Collaborator

flanfly commented Jan 21, 2022

While I'd welcome the intel-spi driver becoming useful, I wouldn't bet on mtd exposing all flash protection bits. I'm not an expert on AMD but for Intel there is a myriad of different ways to lock the flash (protected range regs., global write enable, flash chip native write protections).

@kerneis-anssi If the SPI_BASE_ADDRESS is a PCI BAR you can access the memory region through /dev/mem, given the kernel is not in the most strict lockdown mode. Alternatively you could use VFIO to map that BAR into userspace with the additional upside of being able to receive interrupts. This way you could implement the whole SPI driver in userspace. The flashrom project does that for hardware sequencing on the Intel SPI controller albeit via /dev/mem. The implementation for VFIO should be more or less the same.

@hughsie
Copy link
Member

hughsie commented Feb 8, 2022

@kerneis-anssi is there anything I can help unblock here? I've got AMD hardware now and can help move this along myself if that would be helpful.

@kerneis-anssi
Copy link
Collaborator Author

I'm not sure what @flanfly calls "the most strict lockdown mode". Basically there are three flavors of lockdown:

  • STRICT_DEVMEM, which is not really a form of lockdown, just preventing /dev/mem from being accessed (except for PCI address space).
  • kernel lockdown in integrity mode
  • kernel lockdown in confidentiality mode.

Opening /dev/mem (let alone reading any part of it) is disabled in both integrity and confidentiality mode: see https://elixir.bootlin.com/linux/v5.4.23/source/security/lockdown/lockdown.c#L19 (DEV_MEM is before INTEGRITY_MAX in the list).

I need is to be able to read memory area 0xFEC10000—0xFEC10020, which corresponds to SPI_BASE and appears as pnp 00:03 in /proc/iomem. I see no way of doing that on a locked down kernel except through a kernel module. I know how to access the value of SPI_BASE via /sus/bus/pci/devices/**/config but I don't know a way of jumping from there to the memory-mapped extended config. Am I missing something obvious?

@kerneis-anssi
Copy link
Collaborator Author

@flanfly Even though I don't need to implement the SPI driver at all (because I don't care about dumping the rom at this stage), I'm very interested when you mention "you could use VFIO to map that BAR into userspace". Do you have any example code where this is done, or pointers to API documentation?

@kees
Copy link
Contributor

kees commented Feb 9, 2022

Probably the best way to deal with this kind of this is with a legit kernel driver and expose a sysfs node for the setting you're interested in. i.e. I recommend getting the original patch series mentioned in the description dusted off, fixed up, and resent:
https://lore.kernel.org/all/20200930163714.12879-1-daniel.gutson@eclypsium.com/

@kees
Copy link
Contributor

kees commented Feb 9, 2022

And while you're at it, this one too:
https://lore.kernel.org/all/20201028214359.384918-1-daniel.gutson@eclypsium.com/

@kerneis-anssi
Copy link
Collaborator Author

kerneis-anssi commented Feb 9, 2022

We'll look into it with a colleague of mine. Also got a tip that ROM protection may change on future AMD boards, so it makes even more sense to try and have a single sysfs feature for the various implementations instead of having the complexity in fwupd.

Edit: it's not new, and it's not documented at all :-( but here you go, the only details available about ROM Armor: https://twitter.com/vpikhur/status/1458519177115832328

@orangecms
Copy link

for reading out info via chipsec, @kerneis-anssi's script in a gist:
https://gist.github.com/orangecms/6c4d586778eca411192ab74856c85676

@jmcelroy01
Copy link

When fwupd checks for SPI write protection status, is that something that can be toggled via kernel configuration? e.g., would CONFIG_SPI = n or CONFIG_SPI_SPIDEV = n enable SPI write protection? I have an AMD laptop reporting SPI replay protection as enabled but SPI write protection as disabled, and am trying to determine if there is a bug anywhere. The kernel has lockdown enabled (integrity) and the BIOS doesn't have any SPI toggles. Pardon my asking here, there was little discussion of this specific topic anywhere else that I could find.

@superm1
Copy link
Member

superm1 commented Jul 16, 2023

When fwupd checks for SPI write protection status, is that something that can be toggled via kernel configuration? e.g., would CONFIG_SPI = n or CONFIG_SPI_SPIDEV = n enable SPI write protection? I have an AMD laptop reporting SPI replay protection as enabled but SPI write protection as disabled, and am trying to determine if there is a bug anywhere. The kernel has lockdown enabled (integrity) and the BIOS doesn't have any SPI toggles. Pardon my asking here, there was little discussion of this specific topic anywhere else that I could find.

It's very unlikely to be a bug in fwupd or the kernel. They reflect the state exported by the platform.
fwupd reads a sysfs file exported by the kernel. The kernel gets this information from a register populated by the PSP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants