Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755486Ab3IMWCZ (ORCPT ); Fri, 13 Sep 2013 18:02:25 -0400 Received: from g4t0015.houston.hp.com ([15.201.24.18]:38745 "EHLO g4t0015.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754852Ab3IMWCY (ORCPT ); Fri, 13 Sep 2013 18:02:24 -0400 Message-ID: <1379109742.2197.67.camel@buesod1.americas.hpqcorp.net> Subject: Re: GPT detection regression in efi.c from commit 27a7c64 From: Davidlohr Bueso To: Matt Porter Cc: Karel Zak , Matt Fleming , Linux Kernel Mailing List , torvalds@linux-foundation.org Date: Fri, 13 Sep 2013 15:02:22 -0700 In-Reply-To: <20130913213607.GB8502@ohporter.com> References: <20130913145033.GA8502@ohporter.com> <1379089736.2197.14.camel@buesod1.americas.hpqcorp.net> <52334506.9030802@linaro.org> <1379095648.2197.27.camel@buesod1.americas.hpqcorp.net> <523354F3.70001@linaro.org> <20130913181720.GA1614@x2.net.home> <1379096972.2197.31.camel@buesod1.americas.hpqcorp.net> <52335A7A.6000406@linaro.org> <1379100399.2197.34.camel@buesod1.americas.hpqcorp.net> <20130913213607.GB8502@ohporter.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.4.4 (3.4.4-2.fc17) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4434 Lines: 118 On Fri, 2013-09-13 at 17:36 -0400, Matt Porter wrote: [...] > > > > diff --git a/block/partitions/efi.c b/block/partitions/efi.c > > index 1a5ec9a..9bae49c 100644 > > --- a/block/partitions/efi.c > > +++ b/block/partitions/efi.c > > @@ -186,6 +186,7 @@ invalid: > > */ > > static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors) > > { > > + uint32_t sz = 0; > > int i, part = 0, ret = 0; /* invalid by default */ > > > > if (!mbr || le16_to_cpu(mbr->signature) != MSDOS_MBR_SIGNATURE) > > @@ -216,12 +217,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(mbr->partition_record[part].size_in_lba) != > > - min((uint32_t) total_sectors - 1, 0xFFFFFFFF)) > > + sz = le32_to_cpu(mbr->partition_record[part].size_in_lba); > > + if (sz != (uint32_t) total_sectors - 1 || sz != 0xFFFFFFFF) > > Almost! You'll want to get that brown paper bag out cause I've worn it > for this same type of bug before. Add this fix in and you can have my > > Tested-by: Matt Porter > > diff --git a/block/partitions/efi.c b/block/partitions/efi.c > index 9bae49c..1eb09ee 100644 > --- a/block/partitions/efi.c > +++ b/block/partitions/efi.c > @@ -225,7 +225,7 @@ check_hybrid: > */ > if (ret == GPT_MBR_PROTECTIVE) { > sz = le32_to_cpu(mbr->partition_record[part].size_in_lba); > - if (sz != (uint32_t) total_sectors - 1 || sz != 0xFFFFFFFF) > + if (sz != (uint32_t) total_sectors - 1 && sz != 0xFFFFFFFF) > ret = 0; Oh my, yes, sorry about that. I obviously only built-tested this patch. Linus, if you're ok with it, please go ahead and pick this up. Thanks, Davidlohr 8<------------------------------------------------- From: Davidlohr Bueso Subject: [PATCH] partitions/efi: loosen pmbr size in lba check Matt found that commit 27a7c64 (partitions/efi: account for pmbr size in lba) caused his GPT formatted eMMC device not to boot. The reason is that this commit enforced Linux to always check the lesser of the whole disk or 2Tib for the pMBR size in LBA. While most disk partitioning tools out there create a pMBR with these characteristics, Microsoft does not, as it always sets the entry to the maximum 32-bit limitation - even though a drive may be smaller than that[1]. 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 Reported-and-tested-by: Matt Porter Signed-off-by: Davidlohr Bueso --- block/partitions/efi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/block/partitions/efi.c b/block/partitions/efi.c index 1a5ec9a..1eb09ee 100644 --- a/block/partitions/efi.c +++ b/block/partitions/efi.c @@ -186,6 +186,7 @@ invalid: */ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors) { + uint32_t sz = 0; int i, part = 0, ret = 0; /* invalid by default */ if (!mbr || le16_to_cpu(mbr->signature) != MSDOS_MBR_SIGNATURE) @@ -216,12 +217,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(mbr->partition_record[part].size_in_lba) != - min((uint32_t) total_sectors - 1, 0xFFFFFFFF)) + sz = le32_to_cpu(mbr->partition_record[part].size_in_lba); + if (sz != (uint32_t) total_sectors - 1 && sz != 0xFFFFFFFF) ret = 0; } done: -- 1.7.11.7 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/