From a2d72f70c18905aba25eb0971f6f601dd1fa5a60 Mon Sep 17 00:00:00 2001 From: "Nam T. Nguyen" Date: Fri, 22 Aug 2014 15:01:38 -0700 Subject: [PATCH] vboot: cgpt: Refer to partition entries by entries_lba. This CL accesses the partition entry array through its header's entries_lba value. Previously, we assume the primary entry array lies on third sector, and the secondary array lies (1 + 32) sectors from disk end. This assumption was fine, even Wikipedia assumed the same. But in order for us to support writing boot code to the third sector (as required by some Freescale board), the primary entry array must be moved to another location. Therefore, we must use "entries_lba" to locate the arrays from now on. BRANCH=none BUG=chromium:406432 TEST=unittest TEST=`cgpt create -p` and then `cgpt show`. Make sure the table header and entries are properly moved. Change-Id: Ia9008b0bb204f290b1f6240df562ce7d3a9bbff2 Reviewed-on: https://chromium-review.googlesource.com/213861 Reviewed-by: Bill Richardson Tested-by: Bill Richardson Commit-Queue: Nam Nguyen Tested-by: Nam Nguyen --- cgpt/cgpt_common.c | 15 ++++---- cgpt/cgpt_create.c | 11 +++--- cgpt/cgpt_show.c | 7 ++-- cgpt/cmd_create.c | 7 +++- firmware/lib/cgptlib/cgptlib_internal.c | 16 +++++---- firmware/lib/vboot_kernel.c | 46 ++++++++++++++++--------- host/include/cgpt_params.h | 1 + tests/cgptlib_test.c | 10 +++++- tests/vboot_kernel_tests.c | 28 +++++++++++---- 9 files changed, 95 insertions(+), 46 deletions(-) diff --git a/cgpt/cgpt_common.c b/cgpt/cgpt_common.c index bfc138647e..44652a3758 100644 --- a/cgpt/cgpt_common.c +++ b/cgpt/cgpt_common.c @@ -318,14 +318,15 @@ static int GptLoad(struct drive *drive, uint32_t sector_bytes) { drive->gpt.sector_bytes, GPT_HEADER_SECTOR)) { return -1; } + GptHeader* primary_header = (GptHeader*)drive->gpt.primary_header; if (CGPT_OK != Load(drive, &drive->gpt.primary_entries, - GPT_PMBR_SECTOR + GPT_HEADER_SECTOR, + primary_header->entries_lba, drive->gpt.sector_bytes, GPT_ENTRIES_SECTORS)) { return -1; } + GptHeader* secondary_header = (GptHeader*)drive->gpt.secondary_header; if (CGPT_OK != Load(drive, &drive->gpt.secondary_entries, - drive->gpt.drive_sectors - GPT_HEADER_SECTOR - - GPT_ENTRIES_SECTORS, + secondary_header->entries_lba, drive->gpt.sector_bytes, GPT_ENTRIES_SECTORS)) { return -1; } @@ -351,18 +352,19 @@ static int GptSave(struct drive *drive) { Error("Cannot write secondary header: %s\n", strerror(errno)); } } + GptHeader* primary_header = (GptHeader*)drive->gpt.primary_header; if (drive->gpt.modified & GPT_MODIFIED_ENTRIES1) { if (CGPT_OK != Save(drive, drive->gpt.primary_entries, - GPT_PMBR_SECTOR + GPT_HEADER_SECTOR, + primary_header->entries_lba, drive->gpt.sector_bytes, GPT_ENTRIES_SECTORS)) { errors++; Error("Cannot write primary entries: %s\n", strerror(errno)); } } + GptHeader* secondary_header = (GptHeader*)drive->gpt.secondary_header; if (drive->gpt.modified & GPT_MODIFIED_ENTRIES2) { if (CGPT_OK != Save(drive, drive->gpt.secondary_entries, - drive->gpt.drive_sectors - GPT_HEADER_SECTOR - - GPT_ENTRIES_SECTORS, + secondary_header->entries_lba, drive->gpt.sector_bytes, GPT_ENTRIES_SECTORS)) { errors++; Error("Cannot write secondary entries: %s\n", strerror(errno)); @@ -1155,6 +1157,7 @@ uint8_t RepairHeader(GptData *gpt, const uint32_t valid_headers) { memcpy(primary_header, secondary_header, sizeof(GptHeader)); primary_header->my_lba = GPT_PMBR_SECTOR; /* the second sector on drive */ primary_header->alternate_lba = secondary_header->my_lba; + /* TODO (namnguyen): Preserve (header, entries) padding space. */ primary_header->entries_lba = primary_header->my_lba + GPT_HEADER_SECTOR; return GPT_MODIFIED_HEADER1; } diff --git a/cgpt/cgpt_create.c b/cgpt/cgpt_create.c index 9f60b3a1f0..c9134da6cb 100644 --- a/cgpt/cgpt_create.c +++ b/cgpt/cgpt_create.c @@ -29,15 +29,16 @@ static int GptCreate(struct drive *drive, CgptCreateParams *params) { memcpy(h->signature, GPT_HEADER_SIGNATURE, GPT_HEADER_SIGNATURE_SIZE); h->revision = GPT_HEADER_REVISION; h->size = sizeof(GptHeader); - h->my_lba = 1; - h->alternate_lba = drive->gpt.drive_sectors - 1; - h->first_usable_lba = 1 + 1 + GPT_ENTRIES_SECTORS; - h->last_usable_lba = drive->gpt.drive_sectors - 1 - GPT_ENTRIES_SECTORS - 1; + h->my_lba = GPT_PMBR_SECTOR; /* The second sector on drive. */ + h->alternate_lba = drive->gpt.drive_sectors - GPT_HEADER_SECTOR; + h->entries_lba = h->my_lba + GPT_HEADER_SECTOR + params->padding; + h->first_usable_lba = h->entries_lba + GPT_ENTRIES_SECTORS; + h->last_usable_lba = (drive->gpt.drive_sectors - GPT_HEADER_SECTOR - + GPT_ENTRIES_SECTORS - 1); if (CGPT_OK != GenerateGuid(&h->disk_uuid)) { Error("Unable to generate new GUID.\n"); return -1; } - h->entries_lba = 2; h->number_of_entries = 128; h->size_of_entry = sizeof(GptEntry); diff --git a/cgpt/cgpt_show.c b/cgpt/cgpt_show.c index f5d6001e3e..3603770b3b 100644 --- a/cgpt/cgpt_show.c +++ b/cgpt/cgpt_show.c @@ -415,7 +415,8 @@ static int GptShow(struct drive *drive, CgptShowParams *params) { HeaderDetails(header, entries, indent, params->numeric); } - printf(GPT_FMT, (int)(GPT_PMBR_SECTOR + GPT_HEADER_SECTOR), + GptHeader* primary_header = (GptHeader*)drive->gpt.primary_header; + printf(GPT_FMT, (int)primary_header->entries_lba, (int)GPT_ENTRIES_SECTORS, drive->gpt.valid_entries & MASK_PRIMARY ? "" : "INVALID", "Pri GPT table"); @@ -425,8 +426,8 @@ static int GptShow(struct drive *drive, CgptShowParams *params) { EntriesDetails(drive, PRIMARY, params->numeric); /****************************** Secondary *************************/ - printf(GPT_FMT, (int)(drive->gpt.drive_sectors - GPT_HEADER_SECTOR - - GPT_ENTRIES_SECTORS), + GptHeader* secondary_header = (GptHeader*)drive->gpt.secondary_header; + printf(GPT_FMT, (int)secondary_header->entries_lba, (int)GPT_ENTRIES_SECTORS, drive->gpt.valid_entries & MASK_SECONDARY ? "" : "INVALID", "Sec GPT table"); diff --git a/cgpt/cmd_create.c b/cgpt/cmd_create.c index c751b24dce..ca1b815dee 100644 --- a/cgpt/cmd_create.c +++ b/cgpt/cmd_create.c @@ -17,6 +17,8 @@ static void Usage(void) "Options:\n" " -z Zero the sectors of the GPT table and entries\n" " -s Size (in byes) of the disk (MTD only)\n" + " -p Size (in blocks) of the disk to pad between the\n" + " primary GPT header and its entries, default 0\n" "\n", progname); } @@ -29,7 +31,7 @@ int cmd_create(int argc, char *argv[]) { char *e = 0; opterr = 0; // quiet, you - while ((c=getopt(argc, argv, ":hzs:")) != -1) + while ((c=getopt(argc, argv, ":hzsp:")) != -1) { switch (c) { @@ -39,6 +41,9 @@ int cmd_create(int argc, char *argv[]) { case 's': params.size = strtoull(optarg, &e, 0); break; + case 'p': + params.padding = strtoull(optarg, &e, 0); + break; case 'h': Usage(); diff --git a/firmware/lib/cgptlib/cgptlib_internal.c b/firmware/lib/cgptlib/cgptlib_internal.c index 2224ac143a..510a11fb08 100644 --- a/firmware/lib/cgptlib/cgptlib_internal.c +++ b/firmware/lib/cgptlib/cgptlib_internal.c @@ -88,14 +88,14 @@ int CheckHeader(GptHeader *h, int is_secondary, uint64_t drive_sectors) * secondary is at the end of the drive, preceded by its entries. */ if (is_secondary) { - if (h->my_lba != drive_sectors - 1) + if (h->my_lba != drive_sectors - GPT_HEADER_SECTOR) return 1; if (h->entries_lba != h->my_lba - GPT_ENTRIES_SECTORS) return 1; } else { - if (h->my_lba != 1) + if (h->my_lba != GPT_PMBR_SECTOR) return 1; - if (h->entries_lba != h->my_lba + 1) + if (h->entries_lba < h->my_lba + 1) return 1; } @@ -104,6 +104,7 @@ int CheckHeader(GptHeader *h, int is_secondary, uint64_t drive_sectors) * LastUsableLBA must be before the start of the secondary GPT table * array. FirstUsableLBA <= LastUsableLBA. */ + /* TODO(namnguyen): Also check for padding between header & entries. */ if (h->first_usable_lba < 2 + GPT_ENTRIES_SECTORS) return 1; if (h->last_usable_lba >= drive_sectors - 1 - GPT_ENTRIES_SECTORS) @@ -295,8 +296,8 @@ void GptRepair(GptData *gpt) if (MASK_PRIMARY == gpt->valid_headers) { /* Primary is good, secondary is bad */ Memcpy(header2, header1, sizeof(GptHeader)); - header2->my_lba = gpt->drive_sectors - 1; - header2->alternate_lba = 1; + header2->my_lba = gpt->drive_sectors - GPT_HEADER_SECTOR; + header2->alternate_lba = GPT_PMBR_SECTOR; /* Second sector. */ header2->entries_lba = header2->my_lba - GPT_ENTRIES_SECTORS; header2->header_crc32 = HeaderCrc(header2); gpt->modified |= GPT_MODIFIED_HEADER2; @@ -304,8 +305,9 @@ void GptRepair(GptData *gpt) else if (MASK_SECONDARY == gpt->valid_headers) { /* Secondary is good, primary is bad */ Memcpy(header1, header2, sizeof(GptHeader)); - header1->my_lba = 1; - header1->alternate_lba = gpt->drive_sectors - 1; + header1->my_lba = GPT_PMBR_SECTOR; /* Second sector. */ + header1->alternate_lba = gpt->drive_sectors - GPT_HEADER_SECTOR; + /* TODO (namnguyen): Preserve (header, entries) padding. */ header1->entries_lba = header1->my_lba + 1; header1->header_crc32 = HeaderCrc(header1); gpt->modified |= GPT_MODIFIED_HEADER1; diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c index 72cd2cc1fd..99bf93b38c 100644 --- a/firmware/lib/vboot_kernel.c +++ b/firmware/lib/vboot_kernel.c @@ -59,16 +59,18 @@ int AllocAndReadGptData(VbExDiskHandle_t disk_handle, GptData *gptdata) /* Read data from the drive, skipping the protective MBR */ if (0 != VbExDiskRead(disk_handle, 1, 1, gptdata->primary_header)) return 1; - if (0 != VbExDiskRead(disk_handle, 2, entries_sectors, - gptdata->primary_entries)) - return 1; - if (0 != VbExDiskRead(disk_handle, - gptdata->drive_sectors - entries_sectors - 1, - entries_sectors, gptdata->secondary_entries)) + GptHeader* primary_header = (GptHeader*)gptdata->primary_header; + if (0 != VbExDiskRead(disk_handle, primary_header->entries_lba, + entries_sectors, gptdata->primary_entries)) return 1; + if (0 != VbExDiskRead(disk_handle, gptdata->drive_sectors - 1, 1, gptdata->secondary_header)) return 1; + GptHeader* secondary_header = (GptHeader*)gptdata->secondary_header; + if (0 != VbExDiskRead(disk_handle, secondary_header->entries_lba, + entries_sectors, gptdata->secondary_entries)) + return 1; return 0; } @@ -84,8 +86,14 @@ int WriteAndFreeGptData(VbExDiskHandle_t disk_handle, GptData *gptdata) uint64_t entries_sectors = TOTAL_ENTRIES_SIZE / gptdata->sector_bytes; int ret = 1; + /* + * TODO(namnguyen): Preserve padding between primary GPT header and + * its entries. + */ + uint64_t entries_lba = GPT_PMBR_SECTOR + GPT_HEADER_SECTOR; if (gptdata->primary_header) { GptHeader *h = (GptHeader *)(gptdata->primary_header); + entries_lba = h->entries_lba; /* * Avoid even looking at this data if we don't need to. We @@ -116,7 +124,7 @@ int WriteAndFreeGptData(VbExDiskHandle_t disk_handle, GptData *gptdata) "legacy mode is enabled.\n")); } else { VBDEBUG(("Updating GPT entries 1\n")); - if (0 != VbExDiskWrite(disk_handle, 2, + if (0 != VbExDiskWrite(disk_handle, entries_lba, entries_sectors, gptdata->primary_entries)) goto fail; @@ -124,17 +132,11 @@ int WriteAndFreeGptData(VbExDiskHandle_t disk_handle, GptData *gptdata) } } - if (gptdata->secondary_entries) { - if (gptdata->modified & GPT_MODIFIED_ENTRIES2) { - VBDEBUG(("Updating GPT header 2\n")); - if (0 != VbExDiskWrite(disk_handle, - gptdata->drive_sectors - entries_sectors - 1, - entries_sectors, gptdata->secondary_entries)) - goto fail; - } - } - + entries_lba = (gptdata->drive_sectors - entries_sectors - + GPT_HEADER_SECTOR); if (gptdata->secondary_header) { + GptHeader *h = (GptHeader *)(gptdata->secondary_header); + entries_lba = h->entries_lba; if (gptdata->modified & GPT_MODIFIED_HEADER2) { VBDEBUG(("Updating GPT entries 2\n")); if (0 != VbExDiskWrite(disk_handle, @@ -144,6 +146,16 @@ int WriteAndFreeGptData(VbExDiskHandle_t disk_handle, GptData *gptdata) } } + if (gptdata->secondary_entries) { + if (gptdata->modified & GPT_MODIFIED_ENTRIES2) { + VBDEBUG(("Updating GPT header 2\n")); + if (0 != VbExDiskWrite(disk_handle, + entries_lba, entries_sectors, + gptdata->secondary_entries)) + goto fail; + } + } + ret = 0; fail: diff --git a/host/include/cgpt_params.h b/host/include/cgpt_params.h index 72de99dc8b..ac43774486 100644 --- a/host/include/cgpt_params.h +++ b/host/include/cgpt_params.h @@ -17,6 +17,7 @@ typedef struct CgptCreateParams { char *drive_name; int zap; uint64_t size; + uint64_t padding; } CgptCreateParams; typedef struct CgptAddParams { diff --git a/tests/cgptlib_test.c b/tests/cgptlib_test.c index be4c7159b7..5d46692083 100644 --- a/tests/cgptlib_test.c +++ b/tests/cgptlib_test.c @@ -699,7 +699,15 @@ static int MyLbaTest(void) h1->entries_lba++; h2->entries_lba++; RefreshCrc32(gpt); - EXPECT(1 == CheckHeader(h1, 0, gpt->drive_sectors)); + /* + * We support a padding between primary GPT header and its entries. So + * this still passes. + */ + EXPECT(0 == CheckHeader(h1, 0, gpt->drive_sectors)); + /* + * But the secondary table should fail because it would overlap the + * header, which is now lying after its entry array. + */ EXPECT(1 == CheckHeader(h2, 1, gpt->drive_sectors)); BuildTestGptData(gpt); diff --git a/tests/vboot_kernel_tests.c b/tests/vboot_kernel_tests.c index ee164ceac0..935dd869c5 100644 --- a/tests/vboot_kernel_tests.c +++ b/tests/vboot_kernel_tests.c @@ -139,6 +139,14 @@ VbError_t VbExDiskRead(VbExDiskHandle_t handle, uint64_t lba_start, /* Keep valgrind happy */ Memset(buffer, '\0', lba_count); + /* Fix up entries_lba in GPT header. */ + if (lba_start == 1 || lba_start == 1024 - 1) { + GptHeader* h = (GptHeader*)buffer; + if (lba_start == 1) + h->entries_lba = 1 + 1; + else + h->entries_lba = (1024 - 1 - 32); + } return VBERROR_SUCCESS; } @@ -245,8 +253,8 @@ static void ReadWriteGptTest(void) TEST_EQ(AllocAndReadGptData(handle, &g), 0, "AllocAndRead"); TEST_CALLS("VbExDiskRead(h, 1, 1)\n" "VbExDiskRead(h, 2, 32)\n" - "VbExDiskRead(h, 991, 32)\n" - "VbExDiskRead(h, 1023, 1)\n"); + "VbExDiskRead(h, 1023, 1)\n" + "VbExDiskRead(h, 991, 32)\n"); ResetCallLog(); /* * Valgrind complains about access to uninitialized memory here, so @@ -262,6 +270,8 @@ static void ReadWriteGptTest(void) g.modified |= GPT_MODIFIED_HEADER1 | GPT_MODIFIED_ENTRIES1; ResetCallLog(); Memset(g.primary_header, '\0', g.sector_bytes); + h = (GptHeader*)g.primary_header; + h->entries_lba = 2; TEST_EQ(WriteAndFreeGptData(handle, &g), 0, "WriteAndFree mod 1"); TEST_CALLS("VbExDiskWrite(h, 1, 1)\n" "VbExDiskWrite(h, 2, 32)\n"); @@ -272,11 +282,15 @@ static void ReadWriteGptTest(void) g.modified = -1; ResetCallLog(); Memset(g.primary_header, '\0', g.sector_bytes); + h = (GptHeader*)g.primary_header; + h->entries_lba = 2; + h = (GptHeader*)g.secondary_header; + h->entries_lba = 991; TEST_EQ(WriteAndFreeGptData(handle, &g), 0, "WriteAndFree mod all"); TEST_CALLS("VbExDiskWrite(h, 1, 1)\n" "VbExDiskWrite(h, 2, 32)\n" - "VbExDiskWrite(h, 991, 32)\n" - "VbExDiskWrite(h, 1023, 1)\n"); + "VbExDiskWrite(h, 1023, 1)\n" + "VbExDiskWrite(h, 991, 32)\n"); /* If legacy signature, don't modify GPT header/entries 1 */ ResetMocks(); @@ -286,8 +300,8 @@ static void ReadWriteGptTest(void) g.modified = -1; ResetCallLog(); TEST_EQ(WriteAndFreeGptData(handle, &g), 0, "WriteAndFree mod all"); - TEST_CALLS("VbExDiskWrite(h, 991, 32)\n" - "VbExDiskWrite(h, 1023, 1)\n"); + TEST_CALLS("VbExDiskWrite(h, 1023, 1)\n" + "VbExDiskWrite(h, 991, 32)\n"); /* Error reading */ ResetMocks(); @@ -327,6 +341,8 @@ static void ReadWriteGptTest(void) AllocAndReadGptData(handle, &g); g.modified = -1; Memset(g.primary_header, '\0', g.sector_bytes); + h = (GptHeader*)g.primary_header; + h->entries_lba = 2; TEST_NEQ(WriteAndFreeGptData(handle, &g), 0, "WriteAndFree disk fail"); ResetMocks();