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 ?
