Bugs in parted's pte handling?

[Robert Herndon]

[Apologies for any protocol violations here.  I've just
joined the group, and don't see this addressed in the
recently archived mailing list issues.]


There seem to be bugs in the handling of partition table
entries in parted, relating to cases where

gpt->NumberOfPartitionEntries * gpt->SizeOfPartitionEntry

is not a multiple of the sector size.  This is true of all
parted versions I've looked at (1.8.1, 1.8.8, and 2.3),
although the particulars vary somewhat between versions.

In the 1.8.1 and 1.8.8 versions of parted, there is
code:

[libparted/labels/gpt.c, _generate_header()]

        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))


This truncates the sector count rather than rounding up, so
that (say) 9 entries (times 128 bytes) will cause 2 sectors
to be read in rather than 3.  The logic to scan table entries
then walks off the end into uninitialized memory, eventually
choking with:

Error: Both the primary and backup GPT tables are corrupt.  Try making a fresh table, and using Parted's rescue feature to recover partitions.

This (or similar) logic occurs in:

libparted/labels/gpt.c, gpt_read()
libparted/labels/gpt.c, _generate_header()
libparted/labels/gpt.c, gpt_write()

------

In 2.3 parted, it looks like this has been partially addressed
in gpt_read_PE_array() and gpt_alloc_metadata().  However, it
still fails with the same message:  the CRC checksum verification
done on the partition table entries doesn't use the size defined
by the GPT header; it instead uses a default size of 16k.  The
partition array then fails the checksum test and parted chokes.

The problem in 2.3 appears to be in gpt_read_PE_array(), which is
handed the address of ptes_bytes, which is set to the expected size.
However, gpt_read_PE_array() sets this value up to

    p_ent_size * gpt_disk_data->entry_count

but gpt_disk_data->entry_count is not ok, since it has not always
been initialized properly from the GPT header.

gpt_disk_data->entry_count is set in _parse_header(), but this
has not always been called at the time that gpt_read_PE_array()
has been called.  [gpt_read() calls gpt_read_headers() calls
_header_is_valid() calls check_PE_array_CRC() calls gpt_read_PE_array(),
all of which are done before gpt_read() calls _parse_header().]
So gpt_disk_data->entry_count is initialized from GPT_DEFAULT_PARTITION_ENTRIES
(128) in gpt_alloc(), instead of the actual GPT value.

Simply changing:

--- parted-2.3.RO/libparted/labels/gpt.c        2010-05-06 08:46:05.000000000 -0500
+++ parted-2.3/libparted/labels/gpt.c   2010-10-20 15:53:02.272489435 -0500
@@ -569,7 +569,7 @@
 {
   GPTDiskData *gpt_disk_data = disk->disk_specific;
   uint32_t p_ent_size = PED_LE32_TO_CPU (gpt->SizeOfPartitionEntry);
-  *ptes_bytes = p_ent_size * gpt_disk_data->entry_count;
+  *ptes_bytes = p_ent_size * PED_LE32_TO_CPU(gpt->NumberOfPartitionEntries);
   size_t ptes_sectors = ped_div_round_up (*ptes_bytes,
                                           disk->dev->sector_size);

allows parted to read labels where gpt->NumberOfPartitionEntries *
gpt->SizeOfPartitionEntry is not a sector-size multiple, but it
looks like rearranging the code may be the preferred way to address
this problem.

Robert Herndon
----
Robert Herndon | Software Developer | Quantum Corporation | Office: 651.688.4330 | address@hidden

Preserving the World's Most Important Data. Yours.

---

The information contained in this transmission may be confidential. Any disclosure, copying, or further distribution of confidential information is not permitted unless such privilege is explicitly granted in writing by Quantum. Quantum reserves the right to have electronic communications, including email and attachments, sent across its networks filtered through anti virus and spam software programs and retain such messages in order to comply with applicable data security and retention requirements. Quantum is not responsible for the proper and complete transmission of the substance of this communication or for any delay in its receipt.