chain/partiter: fix and improve gpt handling in buggy cases

Previous version had some issues in case of error situations, among
those:

- backup gpt header was not read if reading of primary failed
- alternating nature of lba_cur and lba_alt was ignored

This patch fixes this and extends the gpt handling.
The current behavior is:

- try primary header; if unreadable or checksum fails (or sanity checks
  fail unless relax flag is set) - try secondary header

- try main partition table referenced in the header that was read; if
  it's unreadable or if its crc check fails - try alternative (so if we
  managed to read primary header, table at the end would be considered
  as alternative - if we managed to read secondary header, table at the
  beginning would be treated as such)

Signed-off-by: H. Peter Anvin <hpa@zytor.com>

1 files changed, 128 insertions, 76 deletions

diff --git a/com32/chain/partiter.c b/com32/chain/partiter.c
index 1eb5350d..ee19e8be 100644
--- a/com32/chain/partiter.c
+++ b/com32/chain/partiter.c
@@ -412,34 +412,16 @@ static inline int valid_crc(uint32_t crc, const uint8_t *buf, unsigned int siz)
     return crc == crc32(crc32(0, NULL, 0), buf, siz);
 }
 
-static int gpt_check_hdr_crc(const struct disk_info * const diskinfo, struct disk_gpt_header **_gh)
+static int valid_crc_hdr(void *buf)
 {
-    struct disk_gpt_header *gh = *_gh;
-    uint64_t lba_alt;
-    uint32_t hold_crc32;
+    struct disk_gpt_header *gh = buf;
+    uint32_t crc = gh->chksum;
+    int valid;
 
-    hold_crc32 = gh->chksum;
     gh->chksum = 0;
-    if (!valid_crc(hold_crc32, (const uint8_t *)gh, gh->hdr_size)) {
-	warn("Primary GPT header checksum invalid.");
-	/* retry with backup */
-	lba_alt = gh->lba_alt;
-	free(gh);
-	if (!(gh = *_gh = disk_read_sectors(diskinfo, lba_alt, 1))) {
-	    error("Couldn't read backup GPT header.");
-	    return -1;
-	}
-	hold_crc32 = gh->chksum;
-	gh->chksum = 0;
-	if (!valid_crc(hold_crc32, (const uint8_t *)gh, gh->hdr_size)) {
-	    error("Secondary GPT header checksum invalid.");
-	    return -1;
-	}
-    }
-    /* restore old checksum */
-    gh->chksum = hold_crc32;
-
-    return 0;
+    valid = crc == crc32(crc32(0, NULL, 0), buf, gh->hdr_size);
+    gh->chksum = crc;
+    return valid;
 }
 
 static int pi_next_(struct part_iter *iter)
@@ -546,10 +528,108 @@ void pi_del(struct part_iter **_iter)
     *_iter = NULL;
 }
 
+static void try_gpt_we(const char *str, int sec)
+{
+    if (sec)
+	error(str);
+    else
+	warn(str);
+}
+
+static struct disk_gpt_header *try_gpt_hdr(const struct disk_info *di, int sec)
+{
+    const char *desc = sec ? "backup" : "primary";
+    uint64_t gpt_cur = sec ? di->lbacnt - 1 : 1;
+    struct disk_gpt_header *gpth;
+    char errbuf[64];
+
+    gpth = disk_read_sectors(di, gpt_cur, 1);
+    if (!gpth) {
+	sprintf(errbuf, "Unable to read %s GPT header.", desc);
+	try_gpt_we(errbuf, sec);
+	return NULL;
+    }
+    if(!valid_crc_hdr(gpth)) {
+	sprintf(errbuf, "Invalid checksum of %s GPT header.", desc);
+	try_gpt_we(errbuf, sec);
+	free(gpth);
+	return NULL;
+    }
+    return gpth;
+}
+
+static struct disk_gpt_part_entry *try_gpt_list(const struct disk_info *di, const struct disk_gpt_header *gpth, int alt)
+{
+    int pri = gpth->lba_cur < gpth->lba_alt;
+    const char *desc = alt ? "alternative" : "main";
+    struct disk_gpt_part_entry *gptl;
+    char errbuf[64];
+    uint64_t gpt_lsiz;	    /* size of GPT partition list in bytes */
+    uint64_t gpt_lcnt;	    /* size of GPT partition in sectors */
+    uint64_t gpt_loff;	    /* offset to GPT partition list in sectors */
+
+    gpt_lsiz = (uint64_t)gpth->part_size * gpth->part_count;
+    gpt_lcnt = (gpt_lsiz + di->bps - 1) / di->bps;
+    if (!alt) {
+	/* prefer header value for partition table if not asking for alternative */
+	gpt_loff = gpth->lba_table;
+    } else {
+	/* try to read alternative, we have to calculate its position */
+	if (!pri)
+	    gpt_loff = gpth->lba_alt + 1;
+	else
+	    gpt_loff = gpth->lba_alt - gpt_lcnt;
+    }
+
+    gptl = disk_read_sectors(di, gpt_loff, gpt_lcnt);
+    if (!gptl) {
+	sprintf(errbuf, "Unable to read %s GPT partition list.", desc);
+	try_gpt_we(errbuf, alt);
+	return NULL;
+    }
+    if (!valid_crc(gpth->table_chksum, (const uint8_t *)gptl, gpt_lsiz)) {
+	sprintf(errbuf, "Invalid checksum of %s GPT partition list.", desc);
+	try_gpt_we(errbuf, alt);
+	free(gptl);
+	return NULL;
+    }
+    return gptl;
+}
+
+static int notsane_gpt_hdr(const struct disk_info *di, const struct disk_gpt_header *gpth, int flags)
+{
+    uint64_t gpt_loff;	    /* offset to GPT partition list in sectors */
+    uint64_t gpt_lsiz;	    /* size of GPT partition list in bytes */
+    uint64_t gpt_lcnt;	    /* size of GPT partition in sectors */
+
+    if (flags & PIF_RELAX)
+	return 0;
+
+    gpt_loff = gpth->lba_table;
+    gpt_lsiz = (uint64_t)gpth->part_size * gpth->part_count;
+    gpt_lcnt = (gpt_lsiz + di->bps - 1) / di->bps;
+
+    /*
+     * disk_read_sectors allows reading of max 255 sectors, so we use
+     * it as a sanity check base. EFI doesn't specify max (AFAIK).
+     */
+    if (gpt_loff < 2 || !gpt_lsiz || gpt_lcnt > 255u ||
+	    gpth->lba_first_usable > gpth->lba_last_usable ||
+	    !sane(gpt_loff, gpt_lcnt) ||
+	    gpt_loff + gpt_lcnt > gpth->lba_first_usable ||
+	    !sane(gpth->lba_last_usable, gpt_lcnt) ||
+	    gpth->lba_last_usable + gpt_lcnt >= gpth->lba_alt ||
+	    gpth->lba_alt >= di->lbacnt ||
+	    gpth->part_size < sizeof(struct disk_gpt_part_entry))
+	return -1;
+
+    return 0;
+}
+
 /* pi_begin() - validate and and get proper iterator for a disk described by di */
 struct part_iter *pi_begin(const struct disk_info *di, int flags)
 {
-    int gptprot, ret = -1;
+    int isgpt = 0, ret = -1;
     struct part_iter *iter;
     struct disk_dos_mbr *mbr = NULL;
     struct disk_gpt_header *gpth = NULL;
@@ -561,7 +641,7 @@ struct part_iter *pi_begin(const struct disk_info *di, int flags)
 
     /* Read MBR */
     if (!(mbr = disk_read_sectors(di, 0, 1))) {
-	error("Couldn't read the first disk sector.");
+	error("Unable to read the first disk sector.");
 	goto bail;
     }
 
@@ -574,69 +654,41 @@ struct part_iter *pi_begin(const struct disk_info *di, int flags)
     }
 
     /* Check for GPT protective MBR */
-    gptprot = 0;
     for (size_t i = 0; i < 4; i++)
-	gptprot |= (mbr->table[i].ostype == 0xEE);
-    if (gptprot && !(flags & PIF_PREFMBR)) {
-	if (!(gpth = disk_read_sectors(di, 1, 1))) {
-	    error("Couldn't read potential GPT header.");
+	isgpt |= (mbr->table[i].ostype == 0xEE);
+    isgpt = isgpt && !(flags & PIF_PREFMBR);
+
+    /* Try to read GPT header */
+    if (isgpt) {
+	gpth = try_gpt_hdr(di, 0);
+	if (!gpth)
+	    /*
+	     * this read might fail if bios reports different disk size (different vm/pc)
+	     * not much we can do here to avoid it
+	     */
+	    gpth = try_gpt_hdr(di, 1);
+	if (!gpth)
 	    goto bail;
-	}
     }
 
     if (gpth && gpth->rev.uint32 == 0x00010000 &&
 	    !memcmp(gpth->sig, disk_gpt_sig_magic, sizeof gpth->sig)) {
 	/* looks like GPT v1.0 */
-	uint64_t gpt_loff;	    /* offset to GPT partition list in sectors */
-	uint64_t gpt_lsiz;	    /* size of GPT partition list in bytes */
-	uint64_t gpt_lcnt;	    /* size of GPT partition in sectors */
 #ifdef DEBUG
 	dprintf("Looks like a GPT v1.0 disk.\n");
 	disk_gpt_header_dump(gpth);
 #endif
-	/* Verify checksum, fallback to backup, then bail if invalid */
-	if (gpt_check_hdr_crc(di, &gpth))
-	    goto bail;
-
-	gpt_loff = gpth->lba_table;
-	gpt_lsiz = (uint64_t)gpth->part_size * gpth->part_count;
-	gpt_lcnt = (gpt_lsiz + di->bps - 1) / di->bps;
-
-	/*
-	 * disk_read_sectors allows reading of max 255 sectors, so we use
-	 * it as a sanity check base. EFI doesn't specify max (AFAIK).
-	 * Apart from that, some extensive sanity checks.
-	 */
-	if (!(flags & PIF_RELAX) && (
-		!gpt_loff || !gpt_lsiz || gpt_lcnt > 255u ||
-		gpth->lba_first_usable > gpth->lba_last_usable ||
-		!sane(gpt_loff, gpt_lcnt) ||
-		gpt_loff + gpt_lcnt > gpth->lba_first_usable ||
-		!sane(gpth->lba_last_usable, gpt_lcnt) ||
-		gpth->lba_last_usable + gpt_lcnt >= gpth->lba_alt ||
-		gpth->lba_alt >= di->lbacnt ||
-		gpth->part_size < sizeof *gptl)) {
-	    error("Invalid GPT header's values.");
+	if (notsane_gpt_hdr(di, gpth, flags)) {
+	    error("GPT header values are corrupted.");
 	    goto bail;
 	}
-	if (!(gptl = disk_read_sectors(di, gpt_loff, gpt_lcnt))) {
-	    error("Couldn't read GPT partition list.");
+
+	gptl = try_gpt_list(di, gpth, 0);
+	if (!gptl)
+	    gptl = try_gpt_list(di, gpth, 1);
+	if (!gptl)
 	    goto bail;
-	}
-	/* Check array checksum(s). */
-	if (!valid_crc(gpth->table_chksum, (const uint8_t *)gptl, (unsigned int)gpt_lsiz)) {
-	    warn("Checksum of the main GPT partition list is invalid, trying backup.");
-	    free(gptl);
-	    /* secondary array directly precedes secondary header */
-	    if (!(gptl = disk_read_sectors(di, gpth->lba_alt - gpt_lcnt, gpt_lcnt))) {
-		error("Couldn't read backup GPT partition list.");
-		goto bail;
-	    }
-	    if (!valid_crc(gpth->table_chksum, (const uint8_t *)gptl, gpt_lsiz)) {
-		error("Checksum of the backup GPT partition list is invalid, giving up.");
-		goto bail;
-	    }
-	}
+
 	/* looks like GPT */
 	ret = pi_gpt_ctor(iter, di, flags, gpth, gptl);
     } else {