Re: [parted-devel] [parted 1.8.8] msdos label and file system corruption issues with > 2TB disk

[Michael Reed]

Comments below.

Jim Meyering wrote:
> Michael Reed <address@hidden> wrote:
>> (I'm copying parted-devel as this ties into GPT-MBR hybrid discussion in that
>> there is a need to be able to boot BIOS based systems from disks larger than
>> 2TB and still be able to address all the storage.)
>>
>> Using an on board hardware raid5 "disk" of 5,851,561,984 512 byte sectors,
>> or roughly 2.9TB, we installed name_brand_Enterprise_distro.  We created an
>> 800 MB swap partition (1), a 25GB root partition (2), and the rest of the 
>> space
>> to a large scratch xfs file system.
>>
>> The OS installation went fine and upon reboot the system complained that the
>> large scratch file system was, essentially, corrupt.  Here's my analysis of
>> what happened.
>>
>> Via analysis of the MSDOS partition table, I can state that it can address
>> a maximum of 4TB of space.  Each partition entry consists of a starting block
>> and a length.  Both of these fields are 32 bits in length.  No individual
>> partition can begin beyond 2TB.  No individual partition can be longer than
>> 2TB.  In order to address 4TB, at least two partition table entries must be
>> used, for example:
>>
>>      start           length
>>      0x00000000      0xfffffffe
>>      0xffffffff      0xffffffff
>>
>> (It's interesting to note that Linux will handle 4TB of space defined via
>> the msdos label, and Windows 2003/2008 appears to stop at 2TB.)
>>
>> Working with parted after the system was up I can draw these conclusions:
>>
>>      1) parted doesn't complain that a partition length is too long
>>         for the msdos label in which it will be stored.
>>
>>      2) parted "truncates" the partition
>>
>>      3) parted "lies" to the OS about the size of the partition.
> 
> Thank you for the detailed bug report!
> With the patch below, I've fixed Parted so it won't do that any more.

I will try to find real hardware with which to test this today.  I'll
let you know how it goes.  Please hold off on posting the patch until
I can play with it.  You'll hear back from me Monday (CA business
hours) at the latest.

Does this effect the ability to create > 2TB with GPT labels?

Thank you for taking the time to do this work.

Comments within the patch....

> 
> The patch to parted is deceptively simple looking.
> Just finding the right place in the call tree to insert the
> tests was not easy.  Name/semantic mismatches (and lack of comments)
> led me to experiment with two others first.
> 
> An early version of the patch did not have this guard condition:
> 
>   if (!(part->type & PED_PARTITION_METADATA)
> 
> Everything appeared to work until I tried to *print*
> a partition table containing the largest (2TB-1-sector) partition.
> Without the guard, it'd complain about the starting sector being too large.
> The actual sector number may have been 2^32+2k.
> 
> Designing the tests was a challenge, too.
> Just creating a file that parted can use as a "device" in which to
> create such a large partition (>= 2TB) is tricky, since many common
> file system types have a maximum file size of just under 2TB.
> But thanks to sparse files, the actual space required is just a few MB.
> 
> Since I ended up choosing to create and mount an XFS partition, the test
> does useful work only when run by root.  You may also need to install
> the xfsprogs package to get mkfs.xfs.  You can run the test manually
> like this:
> 
>   (cd tests && sudo ./t4100-msdos-partition-limits.sh --verbose)
> 
> Here's the change-set I expect to push today or tomorrow:
> -----------------
>>From 1bf00ea84e80d9f04aed3c4a15ddb307b1807f5e Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Thu, 10 Jan 2008 14:51:56 +0100
> Subject: [PATCH] Enforce inherent limitations of the dos partition table 
> format.
> 
> * libparted/disk.c (_check_partition): Enforce the 32-bit limitation
> on a partition's starting sector number and length (in sectors).
> With the usual 512-byte sector size, this limits the maximum
> partition size to just under 2TB.
> * tests/t4100-msdos-partition-limits.sh: New file.  Test the above.
> * tests/Makefile.am (TESTS): Add t4100-msdos-partition-limits.sh.
> 
> Signed-off-by: Jim Meyering <address@hidden>
> ---
>  libparted/disk.c                      |   41 ++++++++-
>  tests/Makefile.am                     |    3 +-
>  tests/t4100-msdos-partition-limits.sh |  169 
> +++++++++++++++++++++++++++++++++
>  3 files changed, 211 insertions(+), 2 deletions(-)
>  create mode 100755 tests/t4100-msdos-partition-limits.sh
> 
> diff --git a/libparted/disk.c b/libparted/disk.c
> index 087fbbf..135e230 100644
> --- a/libparted/disk.c
> +++ b/libparted/disk.c
> @@ -1,6 +1,6 @@
>   /*
>      libparted - a library for manipulating disk partitions
> -    Copyright (C) 1999, 2000, 2001, 2002, 2003, 2005, 2007
> +    Copyright (C) 1999, 2000, 2001, 2002, 2003, 2005, 2007, 2008
>                    Free Software Foundation, Inc.
> 
>      This program is free software; you can redistribute it and/or modify
> @@ -1735,6 +1735,45 @@ _check_partition (PedDisk* disk, PedPartition* part)
>               return 0;
>       }
> 
> +     if (!(part->type & PED_PARTITION_METADATA)
> +            && strcmp (disk->type->name, "msdos") == 0) {
> +             /* Enforce some restrictions inherent in the DOS
> +                partition table format.  Without these, one would be able
> +                to create a 2TB partition (or larger), and it would work,
> +                but only until the next reboot.  This was insidious: the
> +                too-large partition would work initially, because with
> +                Linux-2.4.x and newer we set the partition start sector
> +                and length (in sectors) accurately and directly via the
> +                BLKPG ioctl.  However, only the last 32 bits of each
> +                number would be written to the partition table, and the
> +                next time the system would read/use those corrupted numbers
> +                it would usually complain about an invalid partition.
> +                   The same applies to the starting sector number.  */
> +
> +             /* The partition length, in sectors, must fit in 32 bytes.  */

You probably want this to be "32 bits".

> +             if (part->geom.length > UINT32_MAX) {
> +                     ped_exception_throw (
> +                             PED_EXCEPTION_ERROR,
> +                             PED_EXCEPTION_CANCEL,
> +                             _("partition length of %jd sectors exceeds"
> +                                  " the DOS-partition-table-imposed maximum"
> +                                  " of 2^32-1"),
> +                             part->geom.length);
> +                     return 0;
> +             }
> +
> +             /* The starting sector number must fit in 32 bytes.  */
> +             if (part->geom.start > UINT32_MAX) {
> +                     ped_exception_throw (
> +                             PED_EXCEPTION_ERROR,
> +                             PED_EXCEPTION_CANCEL,
> +                             _("starting sector number, %jd exceeds"
> +                                  " the DOS-partition-table-imposed maximum"
> +                                  " of 2^32-1"), part->geom.start);
> +                     return 0;
> +             }
> +     }
> +
>       return 1;
>  }
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 3a3020e..b9cd205 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -7,7 +7,8 @@ TESTS = \
>    t2000-mkfs.sh \
>    t2100-mkswap.sh \
>    t3000-constraints.sh \
> -  t3100-resize-ext2-partion.sh
> +  t3100-resize-ext2-partion.sh \
> +  t4100-msdos-partition-limits.sh
> 
>  EXTRA_DIST = \
>    $(TESTS) test-lib.sh mkdtemp
> diff --git a/tests/t4100-msdos-partition-limits.sh 
> b/tests/t4100-msdos-partition-limits.sh
> new file mode 100755
> index 0000000..13e32af
> --- /dev/null
> +++ b/tests/t4100-msdos-partition-limits.sh
> @@ -0,0 +1,169 @@
> +#!/bin/sh
> +
> +# Copyright (C) 2008 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +test_description='msdos: enforce limits on partition start sector and length'
> +
> +# Need root privileges to use mount.
> +privileges_required_=1
> +
> +. ./init.sh
> +
> +####################################################
> +# Create and mount a file system capable of dealing with >=2TB files.
> +# We must be able to create a file with an apparent length of 2TB or larger.
> +# It needn't be a large file system.
> +fs=fs_file
> +mp=`pwd`/mount-point
> +n=128
> +
> +test_expect_success \
> +    'create an XFS file system' \
> +    '
> +    dd if=/dev/zero of=$fs bs=1MB count=2 seek=20 &&
> +    mkfs.xfs -q $fs &&
> +    mkdir "$mp"
> +
> +    '
> +
> +# Unmount upon interrupt, failure, etc., as well as upon normal completion.
> +cleanup_() { cd "$test_dir_" && umount "$mp" > /dev/null 2>&1; }
> +
> +test_expect_success \
> +    'mount it' \
> +    '
> +    mount -o loop $fs "$mp" &&
> +    cd "$mp"
> +
> +    '
> +dev=loop-file
> +
> +do_mkpart()
> +{
> +  start_sector=$1
> +  end_sector=$2
> +  # echo '********' $(echo $end_sector - $start_sector + 1 |bc)
> +  dd if=/dev/zero of=$dev bs=1b count=2k seek=$end_sector 2> /dev/null &&
> +  parted -s $dev mklabel msdos &&
> +  parted -s $dev mkpart p xfs ${start_sector}s ${end_sector}s
> +}
> +
> +# Specify the starting sector number and length in sectors,
> +# rather than start and end.
> +do_mkpart_start_and_len()
> +{
> +  start_sector=$1
> +  len=$2
> +  end_sector=$(echo $start_sector + $len - 1|bc)
> +  do_mkpart $start_sector $end_sector
> +}
> +
> +test_expect_success \
> +    'a partition length of 2^32-1 works.' \
> +    '
> +    end=$(echo $n+2^32-2|bc) &&
> +    do_mkpart $n $end
> +    '
> +
> +cat > exp <<EOF
> +Model:  (file)
> +Disk: 4294969470s
> +Sector size (logical/physical): 512B/512B
> +Partition Table: msdos
> +
> +Number  Start  End          Size         Type     File system  Flags
> + 1      ${n}s   ${end}s  4294967295s  primary
> +
> +EOF
> +
> +test_expect_success \
> +    'print the result' \
> +    'parted -s $dev unit s p > out 2>&1 &&
> +     sed "s/Disk .*:/Disk:/;s/ *$//" out > k && mv k out &&
> +     diff -u out exp
> +    '
> +
> +test_expect_failure \
> +    'a partition length of exactly 2^32 sectors provokes failure.' \
> +    'do_mkpart $n $(echo $n+2^32-1|bc) > err 2>&1'
> +
> +msg='Error: partition length of 4294967296 sectors exceeds the '\
> +'DOS-partition-table-imposed maximum of 2^32-1'
> +test_expect_success \
> +    'check for new diagnostic' \
> +    'echo "$msg" > exp && diff -u err exp'
> +
> +# FIXME: investigate this.
> +# Unexpectedly to me, both of these failed with this same diagnostic:
> +#
> +#   Error: partition length of 4294967296 sectors exceeds the \
> +#   DOS-partition-table-imposed maximum of 2^32-1" > exp &&
> +#
> +# I expected the one below to fail with a length of _4294967297_.
> +# Debugging, I see that _check_partition *does* detect this,
> +# but the diagnostic doesn't get displayed because of the wonders
> +# of parted's exception mechanism.
> +
> +test_expect_failure \
> +    'a partition length of 2^32+1 sectors provokes failure.' \
> +    'do_mkpart $n $(echo $n+2^32|bc) > err 2>&1'
> +
> +test_expect_success \
> +    'check for new diagnostic' \
> +    'echo "$msg" > exp && diff -u err exp'
> +
> +# =========================================================
> +# Now consider partition starting sector numbers.
> +msg='Error: starting sector number, 4294967296 exceeds the '\
> +'DOS-partition-table-imposed maximum of 2^32-1'
> +
> +test_expect_success \
> +    'a partition start sector number of 2^32-1 works.' \
> +    'do_mkpart_start_and_len $(echo 2^32-1|bc) 1000'
> +
> +cat > exp <<EOF
> +Model:  (file)
> +Disk: 4294970342s
> +Sector size (logical/physical): 512B/512B
> +Partition Table: msdos
> +
> +Number  Start        End          Size   Type     File system  Flags
> + 1      4294967295s  4294968294s  1000s  primary
> +
> +EOF
> +
> +test_expect_success \
> +    'print the result' \
> +    'parted -s $dev unit s p > out 2>&1 &&
> +     sed "s/Disk .*:/Disk:/;s/ *$//" out > k && mv k out &&
> +     diff -u out exp
> +    '
> +
> +test_expect_failure \
> +    'a partition start sector number of 2^32 must fail.' \
> +    'do_mkpart_start_and_len $(echo 2^32|bc) 1000 > err 2>&1'
> +test_expect_success \
> +    'check for new diagnostic' \
> +    'echo "$msg" > exp && diff -u err exp'
> +
> +test_expect_failure \
> +    'a partition start sector number of 2^32+1 must fail, too.' \
> +    'do_mkpart_start_and_len $(echo 2^32+1|bc) 1000 > err 2>&1'
> +test_expect_success \
> +    'check for new diagnostic' \
> +    'echo "$msg" > exp && diff -u err exp'
> +
> +test_done
> --
> 1.5.4.rc2.85.g71fd
> 
> _______________________________________________
> parted-devel mailing list
> address@hidden
> http://lists.alioth.debian.org/mailman/listinfo/parted-devel
> 
>