From: Ana Bobillo <[email protected]> validate_gpt_header() computes the CRC of a GPT header over header_size bytes, as the header gives it, and nothing checks the size of the partition entries before they are used:
- the header is read into a buffer of one block, on the stack behind part_get_info(), part_print_efi(), gpt_repair_headers() and the gpt command, so a header_size larger than the block size makes the CRC read past the end of that buffer - alloc_read_gpt_entries() allocates and reads num_partition_entries * sizeof_partition_entry bytes, computed in 32 bits, but the entries are then used as an array of num_partition_entries gpt_entry structures: with a smaller entry size, or a product that wraps, part_get_gpt_pte() and part_print_efi() read past the end of the allocation. On a 32-bit board, rounding a product close to 4GiB up to whole blocks also wraps, to a tiny allocation that the entries are then read into. As Linux does in is_gpt_valid(), refuse a header whose header_size is smaller than the header or larger than the block it is read from, one whose sizeof_partition_entry is not sizeof(gpt_entry), and one whose entry array is larger than 4MiB (KMALLOC_MAX_SIZE in Linux with 4KiB pages, 32768 entries). Larger entries are allowed by the UEFI specification, but U-Boot has never read them correctly, and Linux does not accept them either. The header size is checked before the CRC is computed over it. All the checks are in validate_gpt_header(), so they also apply to the header that is_valid_gpt_buf() checks when fastboot is sent a GPT. That path still takes partition_entry_lba from the header without checking it against the size of the download, which is left for a follow-up. Add a test that gives the primary header of a GPT on a blkmap device each of these sizes, with CRCs to match, and checks that it is refused and that the backup header is used instead. Signed-off-by: Ana Bobillo <[email protected]> Reviewed-by: Simon Glass <[email protected]> --- (no changes since v2) Changes in v2: - Include linux/sizes.h, rather than rely on blk.h for SZ_4M (Simon) - Don't word the commit message as if the fastboot path were fully covered: its partition_entry_lba is left for a follow-up (Simon) - Check the header allocation with ut_assertnonnull() in the test, so that ut_assertok() never sees a known -ENOMEM (as Coverity CID 661179 reports for lib_test_gpt_blkmap()) disk/part_efi.c | 47 +++++++++++-- test/lib/part_efi.c | 163 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+), 5 deletions(-) diff --git a/disk/part_efi.c b/disk/part_efi.c index d8751297fbe..85945634915 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -28,8 +28,15 @@ #include <linux/compiler.h> #include <linux/ctype.h> #include <linux/printk.h> +#include <linux/sizes.h> #include <u-boot/crc.h> +/* + * Largest partition entry array accepted, as in Linux (KMALLOC_MAX_SIZE with + * 4KiB pages): 32768 entries, far more than any tool writes + */ +#define GPT_ENTRIES_MAX_SIZE SZ_4M + /* GUID for basic data partitons */ #if CONFIG_IS_ENABLED(EFI_PARTITION) static const efi_guid_t partition_basic_data_guid = PARTITION_BASIC_DATA_GUID; @@ -89,10 +96,11 @@ static int get_bootable(gpt_entry *p) } static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba, - lbaint_t lastlba) + lbaint_t lastlba, unsigned long blksz) { uint32_t crc32_backup = 0; uint32_t calc_crc32; + u32 header_size, entry_size, num_entries; /* Check the GPT header signature */ if (le64_to_cpu(gpt_h->signature) != GPT_HEADER_SIGNATURE_UBOOT) { @@ -103,12 +111,22 @@ static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba, return -1; } + /* + * The CRC covers header_size bytes, so the header has to fit in the + * one block that was read into gpt_h + */ + header_size = le32_to_cpu(gpt_h->header_size); + if (header_size < sizeof(gpt_header) || header_size > blksz) { + log_debug("GPT: header_size incorrect: %u not in %zu..%lu\n", + header_size, sizeof(gpt_header), blksz); + return -1; + } + /* Check the GUID Partition Table CRC */ memcpy(&crc32_backup, &gpt_h->header_crc32, sizeof(crc32_backup)); memset(&gpt_h->header_crc32, 0, sizeof(gpt_h->header_crc32)); - calc_crc32 = efi_crc32((const unsigned char *)gpt_h, - le32_to_cpu(gpt_h->header_size)); + calc_crc32 = efi_crc32((const unsigned char *)gpt_h, header_size); memcpy(&gpt_h->header_crc32, &crc32_backup, sizeof(crc32_backup)); @@ -143,6 +161,24 @@ static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba, return -1; } + /* + * The partition entries are used as an array of gpt_entry, read into + * a buffer whose size is the product of these two fields, worked out + * in 32 bits and then rounded up to whole blocks + */ + entry_size = le32_to_cpu(gpt_h->sizeof_partition_entry); + if (entry_size != sizeof(gpt_entry)) { + log_debug("GPT: sizeof_partition_entry incorrect: %u != %zu\n", + entry_size, sizeof(gpt_entry)); + return -1; + } + num_entries = le32_to_cpu(gpt_h->num_partition_entries); + if (num_entries > GPT_ENTRIES_MAX_SIZE / entry_size) { + log_debug("GPT: num_partition_entries too large: %u\n", + num_entries); + return -1; + } + debug("GPT: first_usable_lba: %llX last_usable_lba: %llX last lba: " LBAF "\n", le64_to_cpu(gpt_h->first_usable_lba), le64_to_cpu(gpt_h->last_usable_lba), lastlba); @@ -925,7 +961,7 @@ int is_valid_gpt_buf(struct blk_desc *desc, void *buf) /* determine start of GPT Header in the buffer */ gpt_h = buf + (GPT_PRIMARY_PARTITION_TABLE_LBA * desc->blksz); if (validate_gpt_header(gpt_h, GPT_PRIMARY_PARTITION_TABLE_LBA, - desc->lba)) + desc->lba, desc->blksz)) return -1; /* determine start of GPT Entries in the buffer */ @@ -1095,7 +1131,8 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head, return 2; } - if (validate_gpt_header(pgpt_head, (lbaint_t)lba, desc->lba)) + if (validate_gpt_header(pgpt_head, (lbaint_t)lba, desc->lba, + desc->blksz)) return 0; if (desc->sig_type == SIG_TYPE_NONE) { diff --git a/test/lib/part_efi.c b/test/lib/part_efi.c index 82153fdabfd..42de9109360 100644 --- a/test/lib/part_efi.c +++ b/test/lib/part_efi.c @@ -12,6 +12,7 @@ #include <part.h> #include <part_efi.h> #include <asm/byteorder.h> +#include <u-boot/crc.h> #include <test/lib.h> #include <test/test.h> #include <test/ut.h> @@ -280,4 +281,166 @@ static int lib_test_gpt_blkmap(struct unit_test_state *uts) } LIB_TEST(lib_test_gpt_blkmap, 0); +/** + * gpt_sizes_check() - check the size fields of a GPT header one by one + * + * Give the primary header of a good table each set of sizes in turn, with + * both CRCs recomputed over what those sizes cover, as a crafted disk would + * have them. Only the checks on the sizes themselves can then refuse it. + * + * @uts: test state + * @dev: blkmap device with nothing mapped yet + * @lba: size of the device in blocks + * @buf: memory to back the device with, @lba blocks of DEFAULT_BLKSZ + * @hdr: two blocks of memory, to rewrite the header in + * Return: 0 if OK, other value on error + */ +static int gpt_sizes_check(struct unit_test_state *uts, struct udevice *dev, + lbaint_t lba, void *buf, gpt_header *hdr) +{ + static const struct { + u32 header_size; + u32 num_entries; + u32 entry_size; + int ret; + } cases[] = { + { sizeof(gpt_header), GPT_ENTRY_NUMBERS, sizeof(gpt_entry), 0 }, + /* the reserved rest of the block may be part of the header */ + { DEFAULT_BLKSZ, GPT_ENTRY_NUMBERS, sizeof(gpt_entry), 0 }, + /* a header longer than the block it is read from */ + { DEFAULT_BLKSZ + 1, GPT_ENTRY_NUMBERS, sizeof(gpt_entry), -1 }, + { sizeof(gpt_header) - 1, GPT_ENTRY_NUMBERS, sizeof(gpt_entry), + -1 }, + /* entries are only ever read as a gpt_entry array */ + { sizeof(gpt_header), GPT_ENTRY_NUMBERS, sizeof(gpt_entry) / 2, + -1 }, + { sizeof(gpt_header), GPT_ENTRY_NUMBERS, sizeof(gpt_entry) * 2, + -1 }, + /* an array whose size comes to a single entry in 32 bits */ + { sizeof(gpt_header), U32_MAX / sizeof(gpt_entry) + 2, + sizeof(gpt_entry), -1 }, + }; + + char str_disk_guid[UUID_STR_LEN + 1] = + "8d60b397-1bb6-4d33-80ee-b1587d24c2f8"; + const unsigned long blksz = DEFAULT_BLKSZ; + struct disk_partition parts[1], info; + struct blk_desc *desc; + struct udevice *blk; + gpt_entry *gpt_pte; + void *entries; + int i, ret; + + ut_assertok(blkmap_map_mem(dev, 0, lba, buf)); + ut_assertok(blk_get_from_parent(dev, &blk)); + desc = dev_get_uclass_plat(blk); + + memset(parts, '\0', sizeof(parts)); + strcpy((char *)parts[0].name, "first"); + parts[0].size = 0x100; + disk_partition_set_uuid(&parts[0], + "a2a0d0eb-e5b9-3344-87c0-68b6b72699c7"); + ut_assertok(gpt_restore(desc, str_disk_guid, parts, 1)); + + for (i = 0; i < ARRAY_SIZE(cases); i++) { + u32 header_size = cases[i].header_size; + u32 array_size; + + /* start from the primary header as it is on the disk */ + memset(hdr, '\0', 2 * blksz); + ut_asserteq(1, blk_dread(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, + 1, hdr)); + + hdr->header_size = cpu_to_le32(header_size); + hdr->num_partition_entries = cpu_to_le32(cases[i].num_entries); + hdr->sizeof_partition_entry = cpu_to_le32(cases[i].entry_size); + + /* the array CRC covers the size as the parser works it out */ + array_size = cases[i].num_entries * cases[i].entry_size; + entries = buf + le64_to_cpu(hdr->partition_entry_lba) * blksz; + hdr->partition_entry_array_crc32 = + cpu_to_le32(crc32(0, entries, array_size)); + hdr->header_crc32 = 0; + hdr->header_crc32 = cpu_to_le32(crc32(0, (void *)hdr, + header_size)); + ut_asserteq(1, blk_dwrite(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, + 1, hdr)); + + /* + * Read it back into a buffer as large as the one above, so + * that a header longer than its block is refused by the size + * check rather than by a CRC over whatever follows the buffer + */ + memset(hdr, '\0', 2 * blksz); + gpt_pte = NULL; + ret = gpt_verify_headers(desc, hdr, &gpt_pte); + free(gpt_pte); + ut_assertf(ret == cases[i].ret, + "case %d: header %u, %u entries of %u: %d\n", i, + header_size, cases[i].num_entries, + cases[i].entry_size, ret); + + /* + * fastboot checks a GPT it is sent in memory the same way. + * There a header longer than its block is also refused by its + * CRC, which then takes in the first entry that follows it. + */ + ut_asserteq(cases[i].ret, is_valid_gpt_buf(desc, buf)); + + /* either way the disk still reads, from the backup if needed */ + desc->part_type = PART_TYPE_UNKNOWN; + part_init(desc); + ut_asserteq(PART_TYPE_EFI, desc->part_type); + ut_assertok(part_get_info(desc, 1, &info)); + ut_asserteq_str((char *)parts[0].name, (char *)info.name); + ut_asserteq_64(parts[0].size, info.size); + } + + return 0; +} + +/* + * A GPT header gives its own size and the size of its partition entries, and + * its CRCs cover only what those say, so a CRC does not show that either size + * is one the parser can use. Check that a primary header with a size the + * parser cannot use is refused, CRCs or not, and that the backup is used. + */ +static int lib_test_gpt_sizes(struct unit_test_state *uts) +{ + const unsigned long blksz = DEFAULT_BLKSZ; + const lbaint_t lba = 0x400; + struct udevice *dev; + gpt_header *hdr; + int ret, err; + void *buf; + + buf = memalign(ARCH_DMA_MINALIGN, lba * blksz); + ut_assertnonnull(buf); + memset(buf, '\0', lba * blksz); + + hdr = malloc_cache_aligned(2 * blksz); + if (!hdr) { + free(buf); + ut_assertnonnull(hdr); + } + ret = blkmap_create(GPT_TEST_LABEL, &dev); + if (ret) { + free(hdr); + free(buf); + ut_assertok(ret); + } + + /* clean up here, as lib_test_gpt_blkmap() explains */ + ret = gpt_sizes_check(uts, dev, lba, buf, hdr); + err = blkmap_destroy(dev); + free(hdr); + free(buf); + if (ret) + return ret; + ut_assertok(err); + + return 0; +} +LIB_TEST(lib_test_gpt_sizes, 0); + #endif /* BLKMAP */
