bug#17108: [PATCH] libparted: Check AlternateLBA against LBA-1

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 04/07/2014 08:38 PM, Brian C. Lane wrote:
>> diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c 
>> index 42b0360..971c1c0 100644 --- a/libparted/labels/gpt.c +++
>> b/libparted/labels/gpt.c @@ -991,7 +991,8 @@ gpt_read (PedDisk
>> *disk) 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
>> (gpt_disk_data->AlternateLBA != gpt_disk_end && +
>> gpt_disk_data->AlternateLBA != disk->dev->length - 1) { if
>> (ped_exception_throw (PED_EXCEPTION_ERROR, -- 1.8.3.2
>> 
> 
> That can't work either:
> 
> 1 - you dropped PED_LE64_TO_CPU

Look again; I switched from primary_gpt->AlternateLBA to
gpt_disk_data->AlternateLBA, which already has been converted on the
previous line.

> 2 - if the PE count doesn't match the amount of space between the
> start and the backup.

I don't understand what you mean by this.

> 3 - when we fix the backup we *must* write it to length-1, not to
> the possibly incorrect gpt_disk_end.

We do.  This code is in the read path, not the write path.  It only
avoids throwing an incorrect error in the event that the disk has grown.

> I think we're better off dropping one or the other of these
> prompts. I've accepted the inevitability of leaving the header in
> place and using it as is, I'm fine with those bits of the patches.

As I said before, we don't want to drop this error if the backup
really is in the wrong place; we just don't want to have duplicate
errors that are both caused by the disk growing.

> The check for extra space will catch a out of position backup,
> there's no need to have two different ways to check. Either the
> backup is at length-1 or there is extra space.

No, it won't.  The extra space check only looks to see if there is
room to increase LastUsableLBA.  That may be fine, but AlternateLBA is
not the last sector of the disk.

There are two distinct errors here that have some overlap.  Picture a
venn diagram of the two errors.  The region where they overlap is when
the disk has grown, in which case, we only want to show the extra
space error, but they also have distinct, non overlapping regions.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBCgAGBQJTQ0oDAAoJEI5FoCIzSKrw4MUH/ReJGAB1wo2aSYIb7maAY3IG
YSekltL7NzGSPenGF5a5pi0mRerMojHdP/3iaXHKKtgurQ6+PBWIfg/H4nUAc9m+
QcBzewjvwiIHso6Oj26tE3o73cM2DqdpeSfSQ5pLVeW9LUWyS+wETS67Io6fqAsi
w4NPHHavAOAc7p3tbXBXa3bFHV3CawjLwcoKNJu/UNZN1wPoMc1YF/1QXG/VnZUC
HVk092PYPSExRsf6v8vIDZU1RK166+LrjFHhl6jwHYyngdLBCLBvacLZMZ9RGuXG
Vcn+MQuxuoSWbWRXjpHrzQGsS+KdhtW2iWBRwGTuU+utm9j7lDxLPEpgffupV08=
=wuwS
-----END PGP SIGNATURE-----