Hi
Currently the "COREBOOT" FMAP cbfs region has a file named "cbfs master header" at the bottom of this fmap region and the X86 bootblock has a pointer at 0xFFFFFFFC to it. Other ARCH have a "header pointer" file at the top of that FMAP region pointing to it.
Currently this file is only used as an anchor point to use cbfs with walkcbfs_asm on X86 to access cbfs in assembly (before any C code). There are 2 uses for this at the moment: 1) updating microcode on Intel systems that don't feature FIT before setting up CAR 2) finding FSP-T (if FSP_CAR is used) before jumping to it Both the cbfstool and the C coreboot code don't rely on it anymore, so it is a legacy feature. Other cbfs FMAP region like FW_MAIN_A/B in a VBOOT setup don't feature it.
Accessing cbfs with walkcbfs_asm breaks hardware based root of trust security mechanisms like Intel bootguard/TXT/CBnT, because no verification or measurement whatsoever happens on either " cbfs master header" of "fsp-t" files. So for instance even if TXT/Bootguard measured or verified FSP-T as an IBB so that it is trusted, an attacker could insert a new cbfs file with the same name, "fsp-t" at a lower address and coreboot will run it anyway. So a static pointer to fsp-t is required. Sidenote/Rant: FSP-T continuously causes such integration problems... Blobs that set up the execution environment are just a very bad idea.
So I propose to drop the legacy "cbfs master header" file and adapt the 2 current use cases in the following way: - Reuse the Intel FIT table and implement FIT microcode updates in assembly/software. (I had this working on some point, before I decided to use walkcbfs_asm) - Either fix the location of FSP-T via for instance a Kconfig option or add a pointer to "fsp-t" at a fixed location in the bootblock and have the tooling update the pointer during the build process. I think the Kconfig option is the least amount of work and cbfstool is already overloaded with options and flags, so my preference goes to this.
Let me know what you think.
Kind regards
Arthur Heymans
Hi Arthur,
On 28.04.2021 08:41, Arthur Heymans wrote:
Hi
Currently the "COREBOOT" FMAP cbfs region has a file named "cbfs master header" at the bottom of this fmap region and the X86 bootblock has a pointer at 0xFFFFFFFC to it. Other ARCH have a "header pointer" file at the top of that FMAP region pointing to it.
Currently this file is only used as an anchor point to use cbfs with walkcbfs_asm on X86 to access cbfs in assembly (before any C code). There are 2 uses for this at the moment:
- updating microcode on Intel systems that don't feature FIT before
setting up CAR 2) finding FSP-T (if FSP_CAR is used) before jumping to it Both the cbfstool and the C coreboot code don't rely on it anymore, so it is a legacy feature. Other cbfs FMAP region like FW_MAIN_A/B in a VBOOT setup don't feature it.
The SeaBIOS payload still relies on this pointer at the 0xFFFFFFFC to find the CBFS master header. If we remove it, we will break the CBFS driver in SeaBIOS and thus lose many functionalities there, which I would like to avoid.
Accessing cbfs with walkcbfs_asm breaks hardware based root of trust security mechanisms like Intel bootguard/TXT/CBnT, because no verification or measurement whatsoever happens on either " cbfs master header" of "fsp-t" files. So for instance even if TXT/Bootguard measured or verified FSP-T as an IBB so that it is trusted, an attacker could insert a new cbfs file with the same name, "fsp-t" at a lower address and coreboot will run it anyway. So a static pointer to fsp-t is required. Sidenote/Rant: FSP-T continuously causes such integration problems... Blobs that set up the execution environment are just a very bad idea.
So I propose to drop the legacy "cbfs master header" file and adapt the 2 current use cases in the following way:
- Reuse the Intel FIT table and implement FIT microcode updates in
assembly/software. (I had this working on some point, before I decided to use walkcbfs_asm)
Agree on that, walkcbfs_asm should be removed. But why to implement microcode updates since CPU comes out of reset with microcode loaded from FIT? Platforms from pre-FIT era should be a concern? There is no pre-reset ROT for the firmware.
- Either fix the location of FSP-T via for instance a Kconfig option
or add a pointer to "fsp-t" at a fixed location in the bootblock and have the tooling update the pointer during the build process. I think the Kconfig option is the least amount of work and cbfstool is already overloaded with options and flags, so my preference goes to this.
FSP-T is not automatically relocated by cbfstool IIRC, so a Kconfig option with fixed address (set to default per microarchitecture/FSP in the SoC Kconfig) should be good. In such way we ensure FSP-T is always at the right address.
Let me know what you think.
Kind regards
Arthur Heymans
coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Best regards,
Hi Arthur,
The master header is a legacy thing and I'm in favor of removing it. That said, as you and Michal mentioned, there's some work to do first. I started https://ticket.coreboot.org/issues/306 to help track what's missing.
Patrick
Am Mi., 28. Apr. 2021 um 08:42 Uhr schrieb Arthur Heymans < arthur@aheymans.xyz>:
Hi
Currently the "COREBOOT" FMAP cbfs region has a file named "cbfs master header" at the bottom of this fmap region and the X86 bootblock has a pointer at 0xFFFFFFFC to it. Other ARCH have a "header pointer" file at the top of that FMAP region pointing to it.
Currently this file is only used as an anchor point to use cbfs with walkcbfs_asm on X86 to access cbfs in assembly (before any C code). There are 2 uses for this at the moment:
- updating microcode on Intel systems that don't feature FIT before
setting up CAR 2) finding FSP-T (if FSP_CAR is used) before jumping to it Both the cbfstool and the C coreboot code don't rely on it anymore, so it is a legacy feature. Other cbfs FMAP region like FW_MAIN_A/B in a VBOOT setup don't feature it.
Accessing cbfs with walkcbfs_asm breaks hardware based root of trust security mechanisms like Intel bootguard/TXT/CBnT, because no verification or measurement whatsoever happens on either " cbfs master header" of "fsp-t" files. So for instance even if TXT/Bootguard measured or verified FSP-T as an IBB so that it is trusted, an attacker could insert a new cbfs file with the same name, "fsp-t" at a lower address and coreboot will run it anyway. So a static pointer to fsp-t is required. Sidenote/Rant: FSP-T continuously causes such integration problems... Blobs that set up the execution environment are just a very bad idea.
So I propose to drop the legacy "cbfs master header" file and adapt the 2 current use cases in the following way:
- Reuse the Intel FIT table and implement FIT microcode updates in
assembly/software. (I had this working on some point, before I decided to use walkcbfs_asm)
- Either fix the location of FSP-T via for instance a Kconfig option or
add a pointer to "fsp-t" at a fixed location in the bootblock and have the tooling update the pointer during the build process. I think the Kconfig option is the least amount of work and cbfstool is already overloaded with options and flags, so my preference goes to this.
Let me know what you think.
Kind regards
Arthur Heymans _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Hi Patrick, Arthur.
We do have a use case in our self-crafted linux where the CBFS master header is used. I need to dig into the code and find out what needs to be done there in order to get rid of this dependency while still not break it for older builds.
Werner
Hi Werner
Sounds good.
I got rid of the SeaBIOS dependency on the CBFS master haeder: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/PSLZAMC... Maybe that can be of use for you?
Arthur
On Fri, Apr 30, 2021 at 12:38 PM Zeh, Werner werner.zeh@siemens.com wrote:
Hi Patrick, Arthur.
We do have a use case in our self-crafted linux where the CBFS master header is used. I need to dig into the code and find out what needs to be done there in order to get rid of this dependency while still not break it for older builds.
Werner