[parted-devel] gpt_probe(): possible bug.

[rahul dev]

Hi Karel,


> > 
> > > From: rahul dev <rahul_dev_agg at yahoo.co.in>
> > > Subject: [parted-devel] gpt_probe(): possible
> bug.
> > > To: parted-devel at lists.alioth.debian.org
> > > Date: Friday, 31 July, 2009, 5:43 PM
> > > Guys,
> > > 
> > >   gpt_probe() function doesn't check for
> > > GPT_HEADER_SIGNATURE in the backup copy of
> partition table
> > > if the gpt signature is not found in the primary
> table.
> > >
> > > if (ped_device_read(dev, pth_raw, 1,
> GPT_HEADER_SECTORS)
> > > || ped_device_read(dev, pth_raw, dev->length -
> 1,
> > > GPT_HEADER_SECTORS)) {
> > >        gpt = pth_new_from_raw
> > > (dev, pth_raw);
> > >        if (gpt->Signature ==
> > > PED_CPU_TO_LE64(GPT_HEADER_SIGNATURE))
> > >            
> > >    gpt_sig_found = 1;
> > > }
> > > 
> > > If the signature is not found in the primary
> header, it
> > > doesn't check for it in the secondary header.
> > > 
> > > Is this a bug ?
> 
>  Yes. Maybe I'm wrong, but I see more bugs in the GPT
> validation.
> 
>          Extensible Firmware
> Interface Specification
>          11.2.2.1 EFI
> Partition Header
>          ....
>          The following test
> must be performed to determine if a GUID Partition
>          Table is valid:
> 
>          • Check the GUID
> Partition Table Signature
>          • Check the GUID
> Partition Table CRC
>          • Check that the
> MyLBA entry points to the LBA that contains the GUID
>          Partition Table
>          • Check the CRC of
> the GUID Partition Entry Array
> 
>  I don't see MyLBA check and GPT Entry array CRC check
> in  _header_is_valid().
>  The GPT Entry array CRC is nowhere checked in the code. It
> seems like a 
>  serious bug.
> 
>  The other (less important) problem is that gpt_read() is
> using
>  gpt->AlternateLBA from invalid header (=does not pass
> validation in
>  _read_header() + _header_is_valid()). For example Linux
> kernel does
>  not use any information from the invalid primary header.
> It seems that 
>  more paranoid solution is to use a backup header from last
> sector and
>  ignore AlternateLBA from invalid primary header.
> 
>  (Sorry, I don't time to write a patch right now.)

Thanks for taking a look at this.
Looks like you may add few more things to your bug list.

As per the EFI spec, "A minimum of 16,384 bytes of space must be reserved for the GUID Partition Entry Array". This check is also missing.

More serious bug is probably there in gpt_read(). it is assumed that the 'ptes_size' will always be an exact multiple of sector_size. See the code below:

ptes_size = sizeof (GuidPartitionEntry_t) * gpt_disk_data->entry_count;
ptes = (GuidPartitionEntry_t*) ped_malloc (ptes_size);
if (!ped_device_read (disk->dev, ptes,
                      PED_LE64_TO_CPU(gpt->PartitionEntryLBA),
                      ptes_size / disk->dev->sector_size))

In case, the ptes_size is not an exact multiple of sector_size, the partition entries in the last sector will not be read.


thanks,
rahul



      See the Web&#39;s breaking stories, chosen by people like you. Check out Yahoo! Buzz. http://in.buzz.yahoo.com/