Attention is currently required from: Krystian Hebel. Hello Krystian Hebel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/57073
to review the following change.
Change subject: util/cbfstool/flashmap/fmap.c: fix fmaptool endianness bugs on BE ......................................................................
util/cbfstool/flashmap/fmap.c: fix fmaptool endianness bugs on BE
Signed-off-by: Marek Kasiewicz marek.kasiewicz@3mdeb.com
util/cbfstool/flashmap/fmap.c: add missing htole16()
This isn't necessary in this particular case, but it is safer to add this in case someone copies the code to other tests.
Change-Id: Id49843a10aae5586688140fc746e77e47a04a1e3 Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com --- M 3rdparty/arm-trusted-firmware M 3rdparty/chromeec M 3rdparty/vboot M util/cbfstool/flashmap/fmap.c 4 files changed, 35 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/57073/1
diff --git a/3rdparty/arm-trusted-firmware b/3rdparty/arm-trusted-firmware index 586aafa..96404aa 160000 --- a/3rdparty/arm-trusted-firmware +++ b/3rdparty/arm-trusted-firmware @@ -1 +1 @@ -Subproject commit 586aafa3a4b13971339f78e430075592c3fe74b5 +Subproject commit 96404aa27efbf1c9051d515075a60c3cf4fa47be diff --git a/3rdparty/chromeec b/3rdparty/chromeec index 4c21b57..1e800ac 160000 --- a/3rdparty/chromeec +++ b/3rdparty/chromeec @@ -1 +1 @@ -Subproject commit 4c21b57eb9619cc3dc86d11226917d25f62f1bc8 +Subproject commit 1e800ac838504c0d2950c7aa90cdfe7bde251545 diff --git a/3rdparty/vboot b/3rdparty/vboot index ccc56f4..b38e3a6 160000 --- a/3rdparty/vboot +++ b/3rdparty/vboot @@ -1 +1 @@ -Subproject commit ccc56f46c51fa9d0c2b3c086ace97c14fe887c32 +Subproject commit b38e3a63a8b1d42fd707e4c23e71c3f3ed84e6ad diff --git a/util/cbfstool/flashmap/fmap.c b/util/cbfstool/flashmap/fmap.c index b7a748c..4755a06 100644 --- a/util/cbfstool/flashmap/fmap.c +++ b/util/cbfstool/flashmap/fmap.c @@ -35,7 +35,7 @@ if (!fmap) return -1;
- return sizeof(*fmap) + (fmap->nareas * sizeof(struct fmap_area)); + return sizeof(*fmap) + (le16toh(fmap->nareas) * sizeof(struct fmap_area)); }
/* Make a best-effort assessment if the given fmap is real */ @@ -47,8 +47,8 @@ if (fmap->ver_major != FMAP_VER_MAJOR) return 0; /* a basic consistency check: flash should be larger than fmap */ - if (fmap->size < - sizeof(*fmap) + fmap->nareas * sizeof(struct fmap_area)) + if (le32toh(fmap->size) < + sizeof(*fmap) + le16toh(fmap->nareas) * sizeof(struct fmap_area)) return 0;
/* fmap-alikes along binary data tend to fail on having a valid, @@ -177,14 +177,14 @@ kv_pair_fmt(kv, "fmap_ver_major", "%d", fmap->ver_major); kv_pair_fmt(kv, "fmap_ver_minor","%d", fmap->ver_minor); kv_pair_fmt(kv, "fmap_base", "0x%016llx", - (unsigned long long)fmap->base); - kv_pair_fmt(kv, "fmap_size", "0x%04x", fmap->size); + (unsigned long long)le64toh(fmap->base)); + kv_pair_fmt(kv, "fmap_size", "0x%04x", le32toh(fmap->size)); kv_pair_fmt(kv, "fmap_name", "%s", fmap->name); - kv_pair_fmt(kv, "fmap_nareas", "%d", fmap->nareas); + kv_pair_fmt(kv, "fmap_nareas", "%d", le16toh(fmap->nareas)); kv_pair_print(kv); kv_pair_free(kv);
- for (i = 0; i < fmap->nareas; i++) { + for (i = 0; i < le16toh(fmap->nareas); i++) { struct kv_pair *pair; uint16_t flags; char *str; @@ -194,16 +194,16 @@ return -1;
kv_pair_fmt(pair, "area_offset", "0x%08x", - fmap->areas[i].offset); + le32toh(fmap->areas[i].offset)); kv_pair_fmt(pair, "area_size", "0x%08x", - fmap->areas[i].size); + le32toh(fmap->areas[i].size)); kv_pair_fmt(pair, "area_name", "%s", fmap->areas[i].name); kv_pair_fmt(pair, "area_flags_raw", "0x%02x", - fmap->areas[i].flags); + le16toh(fmap->areas[i].flags));
/* Print descriptive strings for flags rather than the field */ - flags = fmap->areas[i].flags; + flags = le16toh(fmap->areas[i].flags); str = fmap_flags_to_string(flags); if (str == NULL) { kv_pair_free(pair); @@ -265,8 +265,8 @@ memcpy(&fmap->signature, FMAP_SIGNATURE, strlen(FMAP_SIGNATURE)); fmap->ver_major = FMAP_VER_MAJOR; fmap->ver_minor = FMAP_VER_MINOR; - fmap->base = base; - fmap->size = size; + fmap->base = htole64(base); + fmap->size = htole32(size); memccpy(&fmap->name, name, '\0', FMAP_STRLEN);
return fmap; @@ -289,7 +289,7 @@ return -1;
/* too many areas */ - if ((*fmap)->nareas >= 0xffff) + if (le16toh((*fmap)->nareas) >= 0xffff) return -1;
orig_size = fmap_size(*fmap); @@ -301,12 +301,12 @@
area = (struct fmap_area *)((uint8_t *)*fmap + orig_size); memset(area, 0, sizeof(*area)); - memcpy(&area->offset, &offset, sizeof(area->offset)); - memcpy(&area->size, &size, sizeof(area->size)); memccpy(&area->name, name, '\0', FMAP_STRLEN); - memcpy(&area->flags, &flags, sizeof(area->flags)); + area->offset = htole32(offset); + area->size = htole32(size); + area->flags = htole16(flags);
- (*fmap)->nareas++; + (*fmap)->nareas = htole16(le16toh((*fmap)->nareas) + 1); return new_size; }
@@ -319,7 +319,7 @@ if (!fmap || !name) return NULL;
- for (i = 0; i < fmap->nareas; i++) { + for (i = 0; i < le16toh(fmap->nareas); i++) { if (!strcmp((const char *)fmap->areas[i].name, name)) { area = &fmap->areas[i]; break; @@ -358,12 +358,12 @@ goto fmap_create_test_exit; }
- if (fmap->base != base) { + if (le64toh(fmap->base) != base) { printf("FAILURE: base is incorrect\n"); goto fmap_create_test_exit; }
- if (fmap->size != 0x100000) { + if (le32toh(fmap->size) != 0x100000) { printf("FAILURE: size is incorrect\n"); goto fmap_create_test_exit; } @@ -373,7 +373,7 @@ goto fmap_create_test_exit; }
- if (fmap->nareas != 0) { + if (le16toh(fmap->nareas) != 0) { printf("FAILURE: number of areas is incorrect\n"); goto fmap_create_test_exit; } @@ -414,10 +414,10 @@ uint16_t nareas_orig; /* test_area will be used by fmap_csum_test and find_area_test */ struct fmap_area test_area = { - .offset = 0x400, - .size = 0x10000, + .offset = htole32(0x400), + .size = htole32(0x10000), .name = "test_area_1", - .flags = FMAP_AREA_STATIC, + .flags = htole16(FMAP_AREA_STATIC), };
status = fail; @@ -428,26 +428,26 @@ goto fmap_append_area_test_exit; }
- nareas_orig = (*fmap)->nareas; - (*fmap)->nareas = ~(0); + nareas_orig = le16toh((*fmap)->nareas); + (*fmap)->nareas = htole16(~(0)); if (fmap_append_area(fmap, 0, 0, (const uint8_t *)"foo", 0) >= 0) { printf("FAILURE: failed to abort with too many areas\n"); goto fmap_append_area_test_exit; } - (*fmap)->nareas = nareas_orig; + (*fmap)->nareas = htole16(nareas_orig);
total_size = sizeof(**fmap) + sizeof(test_area); if (fmap_append_area(fmap, - test_area.offset, - test_area.size, + le32toh(test_area.offset), + le32toh(test_area.size), test_area.name, - test_area.flags + le16toh(test_area.flags) ) != total_size) { printf("failed to append area\n"); goto fmap_append_area_test_exit; }
- if ((*fmap)->nareas != 1) { + if (le16toh((*fmap)->nareas) != 1) { printf("FAILURE: failed to increment number of areas\n"); goto fmap_append_area_test_exit; }