diff options
| author | Julius Werner <jwerner@chromium.org> | 2023-04-05 16:18:32 -0700 |
|---|---|---|
| committer | Julius Werner <jwerner@chromium.org> | 2023-04-27 23:18:57 +0000 |
| commit | d69ccaf02735e795f8b4bb5436f29d08f5e7d8cc (patch) | |
| tree | eb3e0e32307bf15dbfb3ba488e738671c82471b7 /src/arch | |
| parent | 5a802b32ea55bf76c5d89899c4198e1eb53768d7 (diff) | |
| download | coreboot-d69ccaf02735e795f8b4bb5436f29d08f5e7d8cc.tar.gz coreboot-d69ccaf02735e795f8b4bb5436f29d08f5e7d8cc.tar.bz2 coreboot-d69ccaf02735e795f8b4bb5436f29d08f5e7d8cc.zip | |
arch/x86: Disable walkcbfs_asm code when CONFIG_CBFS_VERIFICATION is set
walkcbfs_asm is a simple CBFS implementation in assembly to find a file
on a system with memory-mapped SPI flash. It seems to be mostly unused
nowadays and is only still called for early microcode loading on some
old systems (e.g. FSP 1.1 and older).
Using this implementation with CONFIG_CBFS_VERIFICATION is unsafe
because it does not verify the hashes the way the normal CBFS code does.
Therefore, to avoid potential security vulnerabilities from creeping in,
this patch makes sure the code cannot be compiled in when
CBFS_VERIFICATION is active. That means it won't be supported on the old
boards using this for microcode loading.
Ideally CONFIG_CBFS_VERIFICATION should have a `depends on` to make this
dependency more obvious in menuconfig, but the configs actually using
this code are not easy to untangle (e.g. CONFIG_MICROCODE_UPDATE_PRE_RAM
is just set everywhere by default although only very few boards are
really using it, and a lot of different old Intel CPU models are linking
in src/cpu/intel/car/non-evict/cache_as_ram.S without being united under
a single Kconfig so that's not easy to change). To keep things simple,
this patch will just prevent the code from being built and result in a
linker error if a bad combination of Kconfigs is used together. Later
patches can clean up the Kconfigs to better wrap that dependency if the
affected boards are still of enough interest to be worth that effort.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I614a1b05881aa7c1539a7f7f296855ff708db56c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/74243
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Elyes Haouas <ehaouas@noos.fr>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Diffstat (limited to 'src/arch')
| -rw-r--r-- | src/arch/x86/Makefile.inc | 2 | ||||
| -rw-r--r-- | src/arch/x86/walkcbfs.S | 4 |
2 files changed, 6 insertions, 0 deletions
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index d281037515fa..4ac30ca27015 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -112,8 +112,10 @@ add_bootblock = \ $(cbfs-autogen-attributes) $(TS_OPTIONS) $(CBFSTOOL_ADD_CMD_OPTIONS) endif +ifneq ($(CONFIG_CBFS_VERIFICATION),y) $(call src-to-obj,bootblock,$(dir)/walkcbfs.S): $(obj)/fmap_config.h bootblock-y += walkcbfs.S +endif endif # CONFIG_ARCH_BOOTBLOCK_X86_32 / CONFIG_ARCH_BOOTBLOCK_X86_64 diff --git a/src/arch/x86/walkcbfs.S b/src/arch/x86/walkcbfs.S index 208a1281b2df..f4b8d2541dfd 100644 --- a/src/arch/x86/walkcbfs.S +++ b/src/arch/x86/walkcbfs.S @@ -2,6 +2,10 @@ #include <fmap_config.h> +#if CONFIG(CBFS_VERIFICATION) +#error "walkcbfs_asm is not safe to use with CBFS verification!" +#endif + /* we use this instead of CBFS_HEADER_ALIGN because the latter is retired. */ #define CBFS_ALIGNMENT 64 |