Re: Minimum size of GPT partition table header

[rahul dev]

Hi Jim,

> >
> >    There seems to be a bug in function
> _header_is_valid() in file gpt.c.
> >
> > static int
> > _header_is_valid (PedDisk const *disk,
> GuidPartitionTableHeader_t *gpt,
> >               
>    PedSector my_lba)
> > {
> >   uint32_t crc, origcrc;
> >   PedDevice const *dev = disk->dev;
> >
> >   if (PED_LE64_TO_CPU
> (gpt->Signature) != GPT_HEADER_SIGNATURE)
> >     return 0;
> >   /*
> >    * "While the GUID Partition Table Header's
> size may increase
> >    * in the future it cannot span more than
> one block on the
> >    * device."  EFI Specification,
> version 1.10, 11.2.2.1
> >    */
> >   if (PED_LE32_TO_CPU
> (gpt->HeaderSize) < pth_get_size_static (dev)
> >       || PED_LE32_TO_CPU
> (gpt->HeaderSize) > dev->sector_size)
> >     return 0;
> >
> >
> > The check gpt->HeaderSize < pth_get_size_static
> seems to be incorrect.
> > I think minimum size of gpt header is 92 bytes. So,
> correct check should be
> > gtp->HeaderSize < 92.
> >
> > As mentioned in the EFI spec that the header size may
> increase in future, 
> > pth_get_size_static() will also return the increased
> size. In that case, if we are running parted on a disk
> partitioned by old versions, the above check would fail. For
> older versions, gpt->HeaderSize is 92, whereas
> pth_get_size_static() will be > 92 for newer versions
> (where gpt hdr size has been increased).
> 
> Thanks for reviewing the code.
> However, currently, pth_get_size_static() always returns 92,
> and I do
> not expect it (or the official header size) to change any
> time soon.
> 
> Has this code caused an actual problem for you?

One more question here.
Should we allow read of gpt table in case the header size > 92 ? This is 
because if the header size has been increased, the header revision should also 
be increased. So, we should fail reading such a partition table (saying hdr 
revision not supported).
Even if allow reading such a table, we should not allow any modifications to 
it. While writing the gpt header we modify the header size, so original 
contents (beyong 92 offset) will be lost/overwritten.

I am seeing this problem for gpt partitions created by Solaris "zpool create" 
command. It creates a gpt hdr with size 512 bytes (but doesn't change the 
revision which is incorect). Apart from this it has only 9 partition table 
entries (which doesn't comply with the EFI spec). Moreover, it doesn't place 
the backup gpt at the end of disk. Using libparted, I can read this table. But, 
if I modify such a table the original gpt hdr contents are lost. 
 
I feel that we should fail both read/write of a table whose hdr size > 92. 
What's your opinion ?