[PATCH 1/3] libfdisk: gpt: avoid unnecessary pmbr check

[PATCH 1/3] libfdisk: gpt: avoid unnecessary pmbr check

[Davidlohr Bueso]

We only jump to the 'check_hybrid' label when a valid pmbr
is detected, so we need not recheck again. Move the label's
logic so it doesn't include the check.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 libfdisk/src/gpt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libfdisk/src/gpt.c b/libfdisk/src/gpt.c
index b929b84..2a6284a 100644
--- a/libfdisk/src/gpt.c
+++ b/libfdisk/src/gpt.c
@@ -503,9 +503,10 @@ static int valid_pmbr(struct fdisk_context *cxt)
 			goto check_hybrid;
 		}
 	}
-check_hybrid:
+
 	if (ret != GPT_MBR_PROTECTIVE)
 		goto done;
+check_hybrid:
 	for (i = 0 ; i < 4; i++) {
 		if ((pmbr->partition_record[i].os_type != EFI_PMBR_OSTYPE) &&
 		    (pmbr->partition_record[i].os_type != 0x00))
-- 
1.7.11.7

[PATCH 2/3] libfdisk: gpt: do not assume first pt record is always pmbr

[Davidlohr Bueso]

The 0xEE (GPT protective) partition does not always have to be
the first element in the array of MBR partition records.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 libfdisk/src/gpt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libfdisk/src/gpt.c b/libfdisk/src/gpt.c
index 2a6284a..bb9e9a7 100644
--- a/libfdisk/src/gpt.c
+++ b/libfdisk/src/gpt.c
@@ -475,7 +475,7 @@ static int gpt_mknew_header(struct fdisk_context *cxt,
  */
 static int valid_pmbr(struct fdisk_context *cxt)
 {
-	int i, ret = 0; /* invalid by default */
+	int i, part = 0, ret = 0; /* invalid by default */
 	struct gpt_legacy_mbr *pmbr = NULL;
 
 	if (!cxt->firstsector)
@@ -494,6 +494,7 @@ static int valid_pmbr(struct fdisk_context *cxt)
 	/* seems like a valid MBR was found, check DOS primary partitions */
 	for (i = 0; i < 4; i++) {
 		if (pmbr->partition_record[i].os_type == EFI_PMBR_OSTYPE) {
+			part = i;
 			/*
 			 * Ok, we at least know that there's a protective MBR,
 			 * now check if there are other partition types for
@@ -520,7 +521,7 @@ check_hybrid:
 	 * Hybrid MBRs do not necessarily comply with this.
 	 */
 	if (ret == GPT_MBR_PROTECTIVE) {
-		if (le32_to_cpu(pmbr->partition_record[0].size_in_lba) !=
+		if (le32_to_cpu(pmbr->partition_record[part].size_in_lba) !=
 		    min((uint32_t) cxt->total_sectors - 1, 0xFFFFFFFF))
 			ret = 0;
 	}
-- 
1.7.11.7

[PATCH 3/3] libfdisk: gpt: loosen check fot pmbr size in lba

[Davidlohr Bueso]

While most disk partitioning tools out there create a pMBR's size in lba
to be the lesser of the whole disk or 2Tib, Microsoft apparently does not[1].
It always sets the entry to the maximum 32-bit limitation - even though a
drive may be smaller than that.

Loosen this check and only verify that the size is either the whole disk
or 0xFFFFFFFF.  No tool in its right mind would set it to any value
other than these.

[1] http://thestarman.pcministry.com/asm/mbr/GPT.htm#GPTPT

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 libfdisk/src/gpt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libfdisk/src/gpt.c b/libfdisk/src/gpt.c
index bb9e9a7..83659cb 100644
--- a/libfdisk/src/gpt.c
+++ b/libfdisk/src/gpt.c
@@ -477,6 +477,7 @@ static int valid_pmbr(struct fdisk_context *cxt)
 {
 	int i, part = 0, ret = 0; /* invalid by default */
 	struct gpt_legacy_mbr *pmbr = NULL;
+	uint32_t sz_lba = 0;
 
 	if (!cxt->firstsector)
 		goto done;
@@ -517,12 +518,15 @@ check_hybrid:
 	/*
 	 * Protective MBRs take up the lesser of the whole disk
 	 * or 2 TiB (32bit LBA), ignoring the rest of the disk.
+	 * Some partitioning programs, nonetheless, choose to set
+	 * the size to the maximum 32-bit limitation, disregarding
+	 * the disk size.
 	 *
 	 * Hybrid MBRs do not necessarily comply with this.
 	 */
 	if (ret == GPT_MBR_PROTECTIVE) {
-		if (le32_to_cpu(pmbr->partition_record[part].size_in_lba) !=
-		    min((uint32_t) cxt->total_sectors - 1, 0xFFFFFFFF))
+		sz_lba = le32_to_cpu(pmbr->partition_record[part].size_in_lba);
+		if (sz_lba != (uint32_t) cxt->total_sectors - 1 && sz_lba != 0xFFFFFFFF)
 			ret = 0;
 	}
 done:
-- 
1.7.11.7

Re: [PATCH 1/3] libfdisk: gpt: avoid unnecessary pmbr check

[Karel Zak]

On Sat, Sep 21, 2013 at 08:45:14PM -0700, Davidlohr Bueso wrote:
>  libfdisk/src/gpt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 2/3] libfdisk: gpt: do not assume first pt record is always pmbr

[Karel Zak]

On Sat, Sep 21, 2013 at 08:45:15PM -0700, Davidlohr Bueso wrote:
> The 0xEE (GPT protective) partition does not always have to be
> the first element in the array of MBR partition records.

 Already fixed.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 3/3] libfdisk: gpt: loosen check fot pmbr size in lba

[Karel Zak]

On Sat, Sep 21, 2013 at 08:45:16PM -0700, Davidlohr Bueso wrote:
> While most disk partitioning tools out there create a pMBR's size in lba
> to be the lesser of the whole disk or 2Tib, Microsoft apparently does not[1].
> It always sets the entry to the maximum 32-bit limitation - even though a
> drive may be smaller than that.
> 
> Loosen this check and only verify that the size is either the whole disk
> or 0xFFFFFFFF.  No tool in its right mind would set it to any value
> other than these.
> 
> [1] http://thestarman.pcministry.com/asm/mbr/GPT.htm#GPTPT
> 
> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> ---
>  libfdisk/src/gpt.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com