Subject: Re: bug#17108: [PATCH 1/4] libparted: Check AlternateLBA against LBA-1

From: Phillip Susi <[email-masked]>
Date: Wed, 26 Mar 2014 22:25:03 -0400

On 03/26/2014 01:56 PM, Brian C. Lane wrote:
> commit 9e9588c71e introduced a problem with how the backup gpt 
> partition's location was checked, and where it was re-written. It
> was using LastUsableLBA plus an offset based on the number of
> partition entries. This will not always match up with the end of
> the disk. eg. during t0210 where the number of partitions is set to
> 9 instead of 128.

I mentioned recently that t0210 was failing, but when I looked it it,
it looked to me like the script was broken and was underflowing a
variable, causing it to write the backup to a location near 2 TiB,
rather than at the end of the much smaller "disk".  In other words,
the script initially creates a 2M loop file, and after running
gpt-header-munge on it, it is now a 2 TiB file.  This is clearly an
error in gpt-heade-munge, rather than parted.

> The only way to check the AlternateLBA is against the size of the
> disk. If it isn't in LBA - 1 then it is in the wrong place.

While this is true, the patch below regresses what I fixed before,
which is that if the AlternateLBA says it does not go at the end of
the disk, and we don't fix that, then we write it to the end of the
disk anyhow.  This is incorrect.  If we don't fix AlternateLBA to
point to the right location relative to the end of the disk, then we
need to actually write the backup to where AlternateLBA says it is,
even if that isn't where it should be.  With this patch, we write the
backup to where it should be, but the header now claims it is in
another location.

> The outcome was that t0210 and t0211 were failing becase the
> location that it tried to move the backup to was also invalid.
> 
> * libparted/labels/gpt.c (gpt_read): Fix check for backup gpt
> location --- libparted/labels/gpt.c | 10 ++-------- 1 file changed,
> 2 insertions(+), 8 deletions(-)
> 
> diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c index
> 42b0360..3e8e603 100644 --- a/libparted/labels/gpt.c +++
> b/libparted/labels/gpt.c @@ -985,13 +985,7 @@ gpt_read (PedDisk
> *disk) { /* Both are valid.  */ #ifndef DISCOVER_ONLY -
> PedSector gpt_disk_end = PED_LE64_TO_CPU
> (primary_gpt->LastUsableLBA) + 1; -      gpt_disk_end +=
> ((PedSector) (PED_LE32_TO_CPU
> (primary_gpt->NumberOfPartitionEntries)) * -
> (PedSector) (PED_LE32_TO_CPU (primary_gpt->SizeOfPartitionEntry))
> / -                       disk->dev->sector_size); - -
> gpt_disk_data->AlternateLBA = PED_LE64_TO_CPU
> (primary_gpt->AlternateLBA); -      if (PED_LE64_TO_CPU
> (primary_gpt->AlternateLBA) != gpt_disk_end) +	if (PED_LE64_TO_CPU
> (primary_gpt->AlternateLBA) < disk->dev->length - 1) { if
> (ped_exception_throw (PED_EXCEPTION_ERROR, @@ -1002,7 +996,7 @@
> gpt_read (PedDisk *disk) { ptt_clear_sectors (disk->dev, 
> PED_LE64_TO_CPU (primary_gpt->AlternateLBA), 1); -
> gpt_disk_data->AlternateLBA = gpt_disk_end; +
> gpt_disk_data->AlternateLBA = disk->dev->length - 1; write_back =
> 1; } }