Re: parted crash accessing GPT table

[Jim Meyering]

Daniel Fandrich wrote:
> On Fri, Jun 24, 2011 at 04:20:29PM +0200, Jim Meyering wrote:
>> Thank you for the complete bug report.
>> Here's the patch I'm about to push:
>
> Thanks for looking at that problem so quickly. That patch does stop the
> crash on that file presumably by rejecting all the partitions. Since sending
> that last bug report I looked more closely at the partition table that was
> generated and found a couple of off-by-one errors in the table entries.
> The attached file contains a fixed partition table which should now
> be internally consistent.
>
> The patched parted now cleanly rejects both files as-is. But if they are
> length extended to match the internal LBA sizes, parted crashes in a new
> way.  The attached file can be length extended with the command:
>
> dd if=/dev/zero of=ptest seek=2700001 bs=512 count=1
>
> The file I sent yesterday can be extended with the command:
>
> dd if=/dev/zero of=ptable seek=31031293 bs=512 count=1
>
> The backup GPT is detected as corrupt as expected, since it's at the end
> of the zero-extended device. Here's the crash dump of parted-3.0 with the
> patch on the attached file:
>
> $ /var/tmp/parted-3.0/parted/parted ptest unit co print unit s print
> WARNING: You are not superuser.  Watch out for permissions.
> Error: The backup GPT table is corrupt, but the primary appears OK, so that 
> will
> be used.
> parted: invalid token: unit
...
> Aborted

Thanks for yet another report.
Patch below, but it's incomplete: needs a stand-alone test case
which I'll add next week.

> parted doesn't seem to like that command line ("invalid token: unit") which
> I got from parted's assert error output from yesterday (is a documentation
> change needed there?) but pretty well all commands I tried caused similar
> crashes.

That's due to the way parted works in interactive mode (i.e., without
the -s option) when it hits what it calls an exception.
It interprets the second "unit" command as a response to the
"OK/Cancel?" prompt.  Such an exceptional prompt should not
interpret arguments from the command line...

Bottom line, the recommended command fails when printing
a partition table provokes an exception.
One solution might be to recommend using the -s option,
but in that case, you couldn't have "repaired" the GPT
problem in this case and might not have discovered the new bug.

Inserting 'ok's into the recommended command, with the patched
parted, I get this: (note the overlapping partitions warning --
I haven't looked into that yet)

  $ ./parted ptest unit co p ok u s p ok
  WARNING: You are not superuser.  Watch out for permissions.
  Error: The backup GPT table is corrupt, but the primary appears OK, so that 
will
  be used.
  Error: Can't have overlapping partitions.
  Model:  (file)
  Disk /w/parted/ptest: 1382MB
  Sector size (logical/physical): 512B/512B
  Partition Table: gpt

  Number  Start   End     Size    File system  Name        Flags
   1      131kB   262kB   131kB                xloader
   2      262kB   524kB   262kB                bootloader
   3      1049kB  13.6MB  12.6MB               recovery
   4      13.6MB  13.6MB  1536B                bcb
   5      13.6MB  30.4MB  16.8MB               boot
   6      30.4MB  567MB   537MB                system
   7      567MB   621MB   53.7MB               rom
   8      621MB   836MB   215MB                cache
   9      836MB   1373MB  537MB                userdata
  10      1373MB  1382MB  9818kB               media

  Error: The backup GPT table is corrupt, but the primary appears OK, so that 
will
  be used.
  Error: Can't have overlapping partitions.
  Model:  (file)
  Disk /w/parted/ptest: 2700002s
  Sector size (logical/physical): 512B/512B
  Partition Table: gpt

  Number  Start     End       Size      File system  Name        Flags
   1      256s      511s      256s                   xloader
   2      512s      1023s     512s                   bootloader
   3      2048s     26621s    24574s                 recovery
   4      26622s    26624s    3s                     bcb
   5      26625s    59385s    32761s                 boot
   6      59386s    1107961s  1048576s               system
   7      1107962s  1212817s  104856s                rom
   8      1212818s  1632249s  419432s                cache
   9      1632250s  2680825s  1048576s               userdata
  10      2680826s  2700000s  19175s                 media


>From 73c83674cf67b3579636538ad2e68fe663c4bdd3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 25 Jun 2011 08:49:58 +0200
Subject: [PATCH 1/2] libparted: fix a bug in the nilfs2 probe function

* libparted/fs/nilfs2/nilfs2.c (nilfs2_probe): Reject this partition
if we get a negative sb2 offset.  Passing a negative offset to
ped_geometry_read_alloc would evoke a failed assertion.
Bug introduced by 2010-07-09 commit d463e7de.
* NEWS: (Bug fixes): Mention it.
Reported by Daniel Fandrich in
http://thread.gmane.org/gmane.comp.gnu.parted.bugs/10466/focus=10472
---
 NEWS                         |    3 +++
 libparted/fs/nilfs2/nilfs2.c |    5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index cb61ac1..e2258ca 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU parted NEWS                                    -*- outline 
-*-

 ** Bug fixes

+  libparted: no longer aborts (failed assertion) due to a nilfs2_probe bug
+  [bug introduced in parted-2.4 with the addition of nilfs2 support]
+
   libparted: no longer aborts when reading a truncated GPT-formatted device

   libparted: works with a two-component linux kernel version number like 3.0
diff --git a/libparted/fs/nilfs2/nilfs2.c b/libparted/fs/nilfs2/nilfs2.c
index 511b155..cbc349b 100644
--- a/libparted/fs/nilfs2/nilfs2.c
+++ b/libparted/fs/nilfs2/nilfs2.c
@@ -108,13 +108,14 @@ nilfs2_probe (PedGeometry* geom)
        struct nilfs2_super_block *sb = NULL;
        struct nilfs2_super_block *sb2 = NULL;
        PedSector length = geom->length;
-       PedSector sb2off;

        /* ignore if sector size is not 512bytes for now  */
        if (geom->dev->sector_size != PED_SECTOR_SIZE_DEFAULT)
                return NULL;

-       sb2off = NILFS_SB2_OFFSET(length);
+       PedSector sb2off = NILFS_SB2_OFFSET(length);
+       if (sb2off < 0)
+               return NULL;

        if (ped_geometry_read_alloc(geom, &sb_v, 2, 1))
                sb = sb_v;
--
1.7.6.rc2.302.gc2115


>From e5aa276d8c548f6e8390f2cad47279341ebc0a9c Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 25 Jun 2011 08:52:04 +0200
Subject: [PATCH 2/2] doc: mention the origin of the other two bugs

* NEWS: Mention that the other two bugs were introduced long ago.
---
 NEWS |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index e2258ca..6c55ec9 100644
--- a/NEWS
+++ b/NEWS
@@ -8,8 +8,10 @@ GNU parted NEWS                                    -*- outline 
-*-
   [bug introduced in parted-2.4 with the addition of nilfs2 support]

   libparted: no longer aborts when reading a truncated GPT-formatted device
+  [bug present at least as far back as RHEL4's parted-1.6.19]

   libparted: works with a two-component linux kernel version number like 3.0
+  [bug present since the beginning]


 * Noteworthy changes in release 3.0 (2011-05-30) [stable]
--
1.7.6.rc2.302.gc2115