_primary_constraint(): start_geom is uninitialized if min_geom==0 && cylinder_size < dev->length. Debian bug #602568

[Jean-Christian de Rivaz]

Hello,

I try to fix a bug I have found with parted-2.3 while using a special 
USB key (Model: disk2go PURE II) that expose two storage devices: a 
small 5MB device and a regular 2GB device. Here are the 5MB one viewed 
by using fdisk:

fdisk -l /dev/sdb

Disk /dev/sdb: 5 MB, 5242880 bytes
256 heads, 32 sectors/track, 1 cylinders
Units = cylinders of 8192 * 512 = 4194304 bytes
Disk identifier: 0x00000000

   Device Boot      Start         End      Blocks   Id  System
/dev/sdb1               1           1        4080    6  FAT16

The first 5MB device have, acoording to parted-2.3, a cylinder size 
(16065) not smaller than the device length (10240), causing the 
PedGeometry start_geom of the function _primary_constraint() (file 
libparted/labels/dos.c) to stay uninitialized. I suspect that the 
condition "cylinder_size < dev->length" have bee introduced for the 
memory-mapped test as explained into the comment above the condition. 
Unfortunately, it seem that the true real world contain at least a type 
of USB key sold with a storage device that go wrong with this condition.

I don't know the parted code enough to propose better patch than the 
attached one that remove the condition. Obviously it will cause the 
memory-mapped test to fail. With the proposed patch, parted-2.3 work as 
expected when compiled without optimization:

address@hidden:~/parted-testing/build-O0$ parted/parted /dev/sdb print
WARNING: You are not superuser.  Watch out for permissions.
Error: Can't have the end before the start! (start sector=16065 
length=-5825)
Model: disk2go PURE II (scsi)
Disk /dev/sdb: 5243kB
Sector size (logical/physical): 512B/512B
Partition Table: msdos

Number  Start  End     Size    Type     File system  Flags
 1      5120B  5243kB  5238kB  primary


The bug was first reported to Debian: 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=602568 but this is 
reproductible without any Debian patch.

Warning: as this debug screen capture show, this bug occur only if 
parted is compiled without any optimization: 
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=71;filename=Capture-parted-2.3.png;att=1;bug=602568

Regards,

Jean-Christian de Rivaz

--- libparted/labels/dos.c.orig 2010-05-25 15:42:21.000000000 +0200
+++ libparted/labels/dos.c      2010-11-08 14:56:33.000000000 +0100
@@ -1638,13 +1638,14 @@
                                        dev->length - min_geom->end))
                        return NULL;
        } else {
+               /* TO BE FIXED as testing if (cylinder_size < dev->length)
+                  is not enough. See Debian bug #602568. Original comment:
                /* Do not assume that length is larger than 1 cylinder's
                   worth of sectors.  This is useful when testing with
                   a memory-mapped "disk" (a la scsi_debug) that is say,
                   2048 sectors long.  */
-               if (cylinder_size < dev->length
-                   && !ped_geometry_init (&start_geom, dev, cylinder_size,
-                                          dev->length - cylinder_size))
+               if (!ped_geometry_init (&start_geom, dev, cylinder_size,
+                                       dev->length - cylinder_size))
                        return NULL;
                if (!ped_geometry_init (&end_geom, dev, 0, dev->length))
                        return NULL;