[PATCH] fs:partitions:efi.c: correct misuse force_gpt

[PATCH] fs:partitions:efi.c: correct misuse force_gpt

[wangyanqing]

The primary gpt header maybe broken, so we should
check the alternate gpt header no matter whether
there is a gpt boot parameter.(The alternate gpt header
is for this way).

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 fs/partitions/efi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index 6296b40..53fd92f 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -553,7 +553,7 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
 		good_agpt = is_gpt_valid(state,
 					 le64_to_cpu(pgpt->alternate_lba),
 					 &agpt, &aptes);
-        if (!good_agpt && force_gpt)
+        if (!good_agpt)
                 good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
 
         /* The obviously unsuccessful case */
-- 
1.7.3.4

Re: [PATCH] fs:partitions:efi.c: correct misuse force_gpt

[Andrew Morton]

On Tue, 22 Nov 2011 08:44:34 +0800
wangyanqing <udknight@gmail.com> wrote:

> The primary gpt header maybe broken, so we should
> check the alternate gpt header no matter whether
> there is a gpt boot parameter.(The alternate gpt header
> is for this way).
> 
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
>  fs/partitions/efi.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
> index 6296b40..53fd92f 100644
> --- a/fs/partitions/efi.c
> +++ b/fs/partitions/efi.c
> @@ -553,7 +553,7 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>  		good_agpt = is_gpt_valid(state,
>  					 le64_to_cpu(pgpt->alternate_lba),
>  					 &agpt, &aptes);
> -        if (!good_agpt && force_gpt)
> +        if (!good_agpt)
>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>  
>          /* The obviously unsuccessful case */

Please Cc Matt on patches to efi.c.

Neither scripts/get_maintainer.pl nor ./MAINTAINERS know that Matt
knows about efi.c, which is unfortunate.  But his name and email address are
right there at the top of the file.

Re: [PATCH] fs:partitions:efi.c: correct misuse force_gpt

[wangyanqing]

On Mon, Nov 21, 2011 at 10:33:25PM -0600, Matt Domsch wrote:

> Again, here there is some debate, what the Alternate GPT header and
> table are good for.
> 
> I put the comment in to remind myself a decade later...
> 
> /**
>  * find_valid_gpt() - Search disk for valid GPT headers and PTEs
>  * @state
>  * @gpt is a GPT header ptr, filled on return.
>  * @ptes is a PTEs ptr, filled on return.
>  * Description: Returns 1 if valid, 0 on error.
>  * If valid, returns pointers to newly allocated GPT header and PTEs.
>  * Validity depends on PMBR being valid (or being overridden by the
>  * 'gpt' kernel command line option) and finding either the Primary
>  * GPT header and PTEs valid, or the Alternate GPT header and PTEs
>  * valid.  If the Primary GPT header is not valid, the Alternate GPT header
>  * is not checked unless the 'gpt' kernel command line option is passed.
>  * This protects against devices which misreport their size, and forces
>  * the user to decide to use the Alternate GPT.
>  */
> 
> Why is this?
> 
> There were a class of hard disks (IIRC, first generation iPods at
> least), that would report their size off by one (one sector larger
> than they actually were), and attempting to read from that one-past
> sector would cause the whole device to go catatonic, which would then
> hang the boot process.
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=281905 and what
> should be in the historical kernel tree.
> 
> Your patch undoes the patch that prevented the boot from hanging in
> such situations.
> 
> Because we can't know which disks might misbehave this way, and
> because it is a catastrophic failure when it happens, we wanted to
> force the user to do something which might cause the failure to occur,
> hence it's protected behind the force_gpt flag.
Sorry I miss the comment, and you are right!

> 
> Seconarily, the primary purpose of the AGPT in my mind is so that
> userspace tools like parted can look at all the available information
> and make a decision as to whether the AGPT info is valid and should be
> used, if the PGPT is corrupt, and then can fix up either or both
> headers as necessary.  Inside the kernel, there's very little I want
> to trust the AGPT for if I can avoid it, and nothing can fix either
> header if determined to be corrupt.
Thanks for the detail explanation! Matt