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 <wfrichar@chromium.org>
Tested-by: Bill Richardson <wfrichar@chromium.org>
Commit-Queue: Nam Nguyen <namnguyen@chromium.org>
Tested-by: Nam Nguyen <namnguyen@chromium.org>
diff --git a/cgpt/cgpt_common.c b/cgpt/cgpt_common.c
index bfc1386..44652a3 100644
--- a/cgpt/cgpt_common.c
+++ b/cgpt/cgpt_common.c
@@ -318,14 +318,15 @@
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 @@
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 @@
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 9f60b3a..c9134da 100644
--- a/cgpt/cgpt_create.c
+++ b/cgpt/cgpt_create.c
@@ -29,15 +29,16 @@
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 f5d6001..3603770 100644
--- a/cgpt/cgpt_show.c
+++ b/cgpt/cgpt_show.c
@@ -415,7 +415,8 @@
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 @@
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 c751b24..ca1b815 100644
--- a/cgpt/cmd_create.c
+++ b/cgpt/cmd_create.c
@@ -17,6 +17,8 @@
"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 @@
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 @@
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 2224ac1..510a11f 100644
--- a/firmware/lib/cgptlib/cgptlib_internal.c
+++ b/firmware/lib/cgptlib/cgptlib_internal.c
@@ -88,14 +88,14 @@
* 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 @@
* 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 @@
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 @@
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 72cd2cc..99bf93b 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -59,16 +59,18 @@
/* 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))
+ 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 - entries_sectors - 1,
- entries_sectors, gptdata->secondary_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 @@
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 @@
"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 @@
}
}
- 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 @@
}
}
+ 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 72de99d..ac43774 100644
--- a/host/include/cgpt_params.h
+++ b/host/include/cgpt_params.h
@@ -17,6 +17,7 @@
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 be4c715..5d46692 100644
--- a/tests/cgptlib_test.c
+++ b/tests/cgptlib_test.c
@@ -699,7 +699,15 @@
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 ee164ce..935dd86 100644
--- a/tests/vboot_kernel_tests.c
+++ b/tests/vboot_kernel_tests.c
@@ -139,6 +139,14 @@
/* 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 @@
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 @@
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 @@
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 @@
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 @@
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();