From c7ddc999fc076bf6871e3b5e641c07f17b0b1746 Mon Sep 17 00:00:00 2001 From: Mathew King Date: Thu, 8 Aug 2019 14:59:25 -0600 Subject: [PATCH] ifdtool: Add validate option to ifdtool Add an option to ifdtool which validates that the flash regions defined in the descriptor match the coresponding areas in the FMAP. BUG=chromium:992215 TEST=Ran 'ifdtool -t' with a good bios image and verify no issues run 'ifdtool -t' with a bad bios image and verify expected issues Signed-off-by: Mathew King Change-Id: Idebf105dee1b8f829d54bd65c82867af7aa4aded Reviewed-on: https://review.coreboot.org/c/coreboot/+/34802 Tested-by: build bot (Jenkins) Reviewed-by: Stefan Reinauer --- .../binary_extraction.md} | 0 Documentation/ifdtool/index.md | 5 ++ Documentation/ifdtool/layout.md | 66 +++++++++++++++ Documentation/index.md | 1 - Documentation/util.md | 2 +- util/ifdtool/Makefile | 11 ++- util/ifdtool/ifdtool.c | 80 ++++++++++++++++--- util/ifdtool/ifdtool.h | 1 + 8 files changed, 150 insertions(+), 16 deletions(-) rename Documentation/{Binary_Extraction.md => ifdtool/binary_extraction.md} (100%) create mode 100644 Documentation/ifdtool/index.md create mode 100644 Documentation/ifdtool/layout.md diff --git a/Documentation/Binary_Extraction.md b/Documentation/ifdtool/binary_extraction.md similarity index 100% rename from Documentation/Binary_Extraction.md rename to Documentation/ifdtool/binary_extraction.md diff --git a/Documentation/ifdtool/index.md b/Documentation/ifdtool/index.md new file mode 100644 index 0000000000..e6e6057a74 --- /dev/null +++ b/Documentation/ifdtool/index.md @@ -0,0 +1,5 @@ + +Contents: + +* [Intel IFD Binary Extraction](binary_extraction.md) +* [IFD Layout](layout.md) \ No newline at end of file diff --git a/Documentation/ifdtool/layout.md b/Documentation/ifdtool/layout.md new file mode 100644 index 0000000000..950db6f7ff --- /dev/null +++ b/Documentation/ifdtool/layout.md @@ -0,0 +1,66 @@ +# IFD Layout + +A coreboot image for an Intel SoC contains two separate definitions of the +layout of the flash. The Intel Flash Descriptor (IFD) which defines offsets and +sizes of various regions of flash and the [coreboot FMAP](../lib/flashmap.md). + +The FMAP should define all of the of the regions defined by the IFD to ensure +that those regions are accounted for by coreboot and will not be accidentally +modified. + +## IFD mapping + +The names of the IFD regions in the FMAP should follow the convention of +starting with the prefix `SI_` which stands for `silicon initialization` as a +way to categorize anything required by the SoC but not provided by coreboot. + +|IFD Region index|IFD Region name|FMAP Name|Notes| +|---|---|---|---| +|0|Flash Descriptor|SI_DESC|Always the top 4KB of flash| +|1|BIOS|SI_BIOS|This is the region that contains coreboot| +|2|Intel ME|SI_ME|| +|3|Gigabit Ethernet|SI_GBE|| +|4|Platform Data|SI_PDR|| +|8|EC Firmware|SI_EC|Most Chrome OS devices do not use this region; EC firmware is stored BIOS region of flash| + +## Validation + +The ifdtool can be used to manipulate a firmware image with a IFD. This tool +will not take into account the FMAP while modifying the image which can lead to +unexpected and hard to debug issues with the firmware image. For example if the +ME region is defined at 6 MB in the IFD but the FMAP only allocates 4 MB for the +ME, then when the ME is added by the ifdtool 6 MB will be written which could +overwrite 2 MB of the BIOS. + +In order to validate that the FMAP and the IFD are compatible the ifdtool +provides --validate (-t) option. `ifdtool -t` will read both the IFD and the +FMAP in the image and for every non empty region in the IFD if that region is +defined in the FMAP but the offset or size is different then the tool will +return an error. + +Example: + +```console +foo@bar:~$ ifdtool -t bad_image.bin +Region mismatch between bios and SI_BIOS + Descriptor region bios: + offset: 0x00400000 + length: 0x01c00000 + FMAP area SI_BIOS: + offset: 0x00800000 + length: 0x01800000 +Region mismatch between me and SI_ME + Descriptor region me: + offset: 0x00103000 + length: 0x002f9000 + FMAP area SI_ME: + offset: 0x00103000 + length: 0x006f9000 +Region mismatch between pd and SI_PDR + Descriptor region pd: + offset: 0x003fc000 + length: 0x00004000 + FMAP area SI_PDR: + offset: 0x007fc000 + length: 0x00004000 +``` \ No newline at end of file diff --git a/Documentation/index.md b/Documentation/index.md index b880c1c4d5..39c8d11f9a 100644 --- a/Documentation/index.md +++ b/Documentation/index.md @@ -169,7 +169,6 @@ Contents: * [coreboot at conferences](community/conferences.md) * [Payloads](payloads.md) * [Distributions](distributions.md) -* [Intel IFD Binary Extraction](Binary_Extraction.md) * [Dealing with Untrusted Input in SMM](technotes/2017-02-dealing-with-untrusted-input-in-smm.md) * [GPIO toggling in ACPI AML](acpi/gpio.md) * [Adding devices to a device tree](acpi/devicetree.md) diff --git a/Documentation/util.md b/Documentation/util.md index f8fabc1ec3..3a09b00101 100644 --- a/Documentation/util.md +++ b/Documentation/util.md @@ -47,7 +47,7 @@ Controller (EC). `C` * __genprof__ - Format function tracing logs `Bash` `C` * __gitconfig__ - Initialize git repository submodules install git hooks `Bash` -* __ifdtool__ - Extract and dump Intel Firmware Descriptor information +* [__ifdtool__](ifdtool/index.md) - Extract and dump Intel Firmware Descriptor information `C` * __intelmetool__ - Dump interesting things about Management Engine even if hidden `C` diff --git a/util/ifdtool/Makefile b/util/ifdtool/Makefile index cc52b1ed4a..a4f0af6217 100644 --- a/util/ifdtool/Makefile +++ b/util/ifdtool/Makefile @@ -18,10 +18,16 @@ PROGRAM = ifdtool CC = gcc INSTALL = /usr/bin/env install PREFIX = /usr/local -CFLAGS = -O2 -g -Wall -Wextra -Wmissing-prototypes -Werror -I../../src/commonlib/include +CFLAGS = -O2 -g -Wall -Wextra -Wmissing-prototypes -Werror +CFLAGS += -I../../src/commonlib/include +CFLAGS += -I../cbfstool/flashmap +CFLAGS += -include ../../src/commonlib/include/commonlib/compiler.h LDFLAGS = OBJS = ifdtool.o +OBJS += fmap.o +OBJS += kv_pair.o +OBJS += valstr.o all: dep $(PROGRAM) @@ -38,6 +44,9 @@ dep: %.o: %.c $(CC) $(CFLAGS) -c -o $@ $< +%.o: ../cbfstool/flashmap/%.c + $(CC) $(CFLAGS) -c -o $@ $< + install: $(PROGRAM) mkdir -p $(DESTDIR)$(PREFIX)/bin $(INSTALL) $(PROGRAM) $(DESTDIR)$(PREFIX)/bin diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c index 83caa693ab..0e83c760d8 100644 --- a/util/ifdtool/ifdtool.c +++ b/util/ifdtool/ifdtool.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "ifdtool.h" #ifndef O_BINARY @@ -46,15 +47,15 @@ static int selected_chip = 0; static int platform = -1; static const struct region_name region_names[MAX_REGIONS] = { - { "Flash Descriptor", "fd", "flashregion_0_flashdescriptor.bin" }, - { "BIOS", "bios", "flashregion_1_bios.bin" }, - { "Intel ME", "me", "flashregion_2_intel_me.bin" }, - { "GbE", "gbe", "flashregion_3_gbe.bin" }, - { "Platform Data", "pd", "flashregion_4_platform_data.bin" }, - { "Reserved", "res1", "flashregion_5_reserved.bin" }, - { "Reserved", "res2", "flashregion_6_reserved.bin" }, - { "Reserved", "res3", "flashregion_7_reserved.bin" }, - { "EC", "ec", "flashregion_8_ec.bin" }, + { "Flash Descriptor", "fd", "flashregion_0_flashdescriptor.bin", "SI_DESC" }, + { "BIOS", "bios", "flashregion_1_bios.bin", "SI_BIOS" }, + { "Intel ME", "me", "flashregion_2_intel_me.bin", "SI_ME" }, + { "GbE", "gbe", "flashregion_3_gbe.bin", "SI_GBE" }, + { "Platform Data", "pd", "flashregion_4_platform_data.bin", "SI_PDR" }, + { "Reserved", "res1", "flashregion_5_reserved.bin", NULL }, + { "Reserved", "res2", "flashregion_6_reserved.bin", NULL }, + { "Reserved", "res3", "flashregion_7_reserved.bin", NULL }, + { "EC", "ec", "flashregion_8_ec.bin", "SI_EC" }, }; /* port from flashrom */ @@ -804,6 +805,51 @@ static void write_regions(char *image, int size) } } +static void validate_layout(char *image, int size) +{ + uint i, errors = 0; + struct fmap *fmap; + long int fmap_loc = fmap_find((uint8_t *)image, size); + const frba_t *frba = find_frba(image, size); + + if (fmap_loc < 0 || !frba) + exit(EXIT_FAILURE); + + fmap = (struct fmap *)(image + fmap_loc); + + for (i = 0; i < max_regions; i++) { + if (region_names[i].fmapname == NULL) + continue; + + region_t region = get_region(frba, i); + + if (region.size == 0) + continue; + + const struct fmap_area *area = + fmap_find_area(fmap, region_names[i].fmapname); + + if (!area) + continue; + + if ((uint)region.base != area->offset || + (uint)region.size != area->size) { + printf("Region mismatch between %s and %s\n", + region_names[i].terse, area->name); + printf(" Descriptor region %s:\n", region_names[i].terse); + printf(" offset: 0x%08x\n", region.base); + printf(" length: 0x%08x\n", region.size); + printf(" FMAP area %s:\n", area->name); + printf(" offset: 0x%08x\n", area->offset); + printf(" length: 0x%08x\n", area->size); + errors++; + } + } + + if (errors > 0) + exit(EXIT_FAILURE); +} + static void write_image(const char *filename, char *image, int size) { char new_filename[FILENAME_MAX]; // allow long file names @@ -1359,6 +1405,7 @@ static void print_usage(const char *name) printf("\n" " -d | --dump: dump intel firmware descriptor\n" " -f | --layout dump regions into a flashrom layout file\n" + " -t | --validate Validate that the firmware descriptor layout matches the fmap layout\n" " -x | --extract: extract intel fd modules\n" " -i | --inject : inject file into region \n" " -n | --newlayout update regions using a flashrom layout file\n" @@ -1388,7 +1435,7 @@ int main(int argc, char *argv[]) { int opt, option_index = 0; int mode_dump = 0, mode_extract = 0, mode_inject = 0, mode_spifreq = 0; - int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0; + int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0, mode_validate = 0; int mode_layout = 0, mode_newlayout = 0, mode_density = 0; int mode_altmedisable = 0, altmedisable = 0; char *region_type_string = NULL, *region_fname = NULL; @@ -1413,10 +1460,11 @@ int main(int argc, char *argv[]) {"version", 0, NULL, 'v'}, {"help", 0, NULL, 'h'}, {"platform", 0, NULL, 'p'}, + {"validate", 0, NULL, 't'}, {0, 0, 0, 0} }; - while ((opt = getopt_long(argc, argv, "df:D:C:M:xi:n:s:p:eluvh?", + while ((opt = getopt_long(argc, argv, "df:D:C:M:xi:n:s:p:eluvth?", long_options, &option_index)) != EOF) { switch (opt) { case 'd': @@ -1593,6 +1641,9 @@ int main(int argc, char *argv[]) } fprintf(stderr, "Platform is: %s\n", optarg); break; + case 't': + mode_validate = 1; + break; case 'v': print_version(); exit(EXIT_SUCCESS); @@ -1608,7 +1659,7 @@ int main(int argc, char *argv[]) if ((mode_dump + mode_layout + mode_extract + mode_inject + mode_newlayout + (mode_spifreq | mode_em100 | mode_unlocked | - mode_locked) + mode_altmedisable) > 1) { + mode_locked) + mode_altmedisable + mode_layout) > 1) { fprintf(stderr, "You may not specify more than one mode.\n\n"); print_usage(argv[0]); exit(EXIT_FAILURE); @@ -1616,7 +1667,7 @@ int main(int argc, char *argv[]) if ((mode_dump + mode_layout + mode_extract + mode_inject + mode_newlayout + mode_spifreq + mode_em100 + mode_locked + - mode_unlocked + mode_density + mode_altmedisable) == 0) { + mode_unlocked + mode_density + mode_altmedisable + mode_validate) == 0) { fprintf(stderr, "You need to specify a mode.\n\n"); print_usage(argv[0]); exit(EXIT_FAILURE); @@ -1667,6 +1718,9 @@ int main(int argc, char *argv[]) if (mode_extract) write_regions(image, size); + if (mode_validate) + validate_layout(image, size); + if (mode_inject) inject_region(filename, image, size, region_type, region_fname); diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h index f3b9a53e06..195a09cce4 100644 --- a/util/ifdtool/ifdtool.h +++ b/util/ifdtool/ifdtool.h @@ -165,4 +165,5 @@ struct region_name { const char *pretty; const char *terse; const char *filename; + const char *fmapname; };