[parted-devel] [PATCH 1/2] libparted: enforce dos partition limit

Jim Meyering jim at meyering.net
Wed Jan 18 13:12:27 UTC 2012


Phillip Susi wrote:
> From: Phillip Susi <psusi at cfl.rr.com>
>
> The msdos partition table claimed a maximum partition count of 16
> but would allow you to go beyond that.  This resulted in the kernel
> not being informed of those partitions.  Corrected to enforce the
> limit.
> ---
>  NEWS                               |    4 ++
>  libparted/labels/dos.c             |   12 +++++--
>  tests/Makefile.am                  |    1 +
>  tests/t9042-dos-partition-limit.sh |   69 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 83 insertions(+), 3 deletions(-)
>  create mode 100644 tests/t9042-dos-partition-limit.sh
>
> diff --git a/NEWS b/NEWS
> index 58a5f89..194903b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -7,6 +7,10 @@ GNU parted NEWS                                    -*- outline -*-
>    parted has improved support for partitionable loopback devices
>
>  ** Bug fixes
> +  The msdos partition table claimed a maximum partition count of 16
> +  but would allow you to go beyond that.  This resulted in the kernel
> +  not being informed of those partitions.  Corrected to enforce the
> +  limit.
...
> +n=$((n_MiB * sectors_per_MiB))
> +printf "BYT;\n$scsi_dev:${n}s:scsi:$ss:$ss:gpt:Linux scsi_debug;\n" \
> +  > exp || fail=1
> +
> +parted -s $scsi_dev mklabel msdos || fail=1
> +parted -s -a min $scsi_dev mkpart extended 2046s 100% || fail=1
> +
> +i=1
> +while :; do
> +    end=$((start + partition_sectors - 2))
> +    parted -s -a min $scsi_dev mkpart logical ${start}s ${end}s || fail=1
> +    printf "$i:${start}s:${end}s:${partition_sectors}s::p$i:;\n" >> exp
> +    test $i = $((n_partitions - 1)) && break
> +    start=$((start + partition_sectors))
> +    i=$((i+1))
> +done
> +
> +start=$((start + partition_sectors))
> +end=$((start + partition_sectors - 2))
> +
> +#try one more partition than allowed, make sure it fails
> +
> +parted -s -a min $scsi_dev mkpart logical ${start}s ${end}s > out 2>&1
> +cat <<EOF > exp
> +Error: Can not create any more partitions
> +EOF
> +compare exp out || fail=1

Thanks for the fix and added test.  And NEWS.
Almost perfect.
I've appended this to the commit log:

    * NEWS (Bug fixes): Mention it.
    * libparted/labels/dos.c (next_primary): Return -1 upon failure.
    (next_logical): Stop no later than MAX_TOTAL_PART.
    Throw exception and return -1 upon failure.
    (msdos_partition_enumerate): Convert a negative partition number
    return value from either of the above two to failure (return 0).
    * tests/t9042-dos-partition-limit.sh: New file.
    * tests/Makefile.am (TESTS): Add it.

and fixed the test:
  - changed "gpt" artifact to "msdos"
  - adjust the expected first partition output
  - adjust the per-partition expected output generation
  - generate actual listing output and use that expected output we generated

I.e., I've folded in this incremental:

diff --git a/tests/t9042-dos-partition-limit.sh b/tests/t9042-dos-partition-limit.sh
index e7e23a7..07e8f64 100644
--- a/tests/t9042-dos-partition-limit.sh
+++ b/tests/t9042-dos-partition-limit.sh
@@ -39,7 +39,9 @@ scsi_debug_setup_ sector_size=$ss dev_size_mb=$n_MiB > dev-name ||
 scsi_dev=$(cat dev-name)

 n=$((n_MiB * sectors_per_MiB))
-printf "BYT;\n$scsi_dev:${n}s:scsi:$ss:$ss:gpt:Linux scsi_debug;\n" \
+printf '%s\n' "BYT;" \
+    "$scsi_dev:${n}s:scsi:$ss:$ss:msdos:Linux scsi_debug;" \
+    "1:$((start-2))s:$((start-2+4098-1))s:4098s:::lba;" \
   > exp || fail=1

 parted -s $scsi_dev mklabel msdos || fail=1
@@ -49,12 +51,15 @@ i=1
 while :; do
     end=$((start + partition_sectors - 2))
     parted -s -a min $scsi_dev mkpart logical ${start}s ${end}s || fail=1
-    printf "$i:${start}s:${end}s:${partition_sectors}s::p$i:;\n" >> exp
+    printf "$((i+4)):${start}s:${end}s:$((end-start+1))s:::;\n" >> exp
     test $i = $((n_partitions - 1)) && break
     start=$((start + partition_sectors))
     i=$((i+1))
 done

+parted -m -s $scsi_dev u s p > out || fail=1
+compare exp out || fail=1
+
 start=$((start + partition_sectors))
 end=$((start + partition_sectors - 2))

Here's the complete patch:

>From b4351c7b7205afb6017d495407e49a1e20a80389 Mon Sep 17 00:00:00 2001
From: Phillip Susi <psusi at cfl.rr.com>
Date: Fri, 13 Jan 2012 16:21:51 -0500
Subject: [PATCH] libparted: enforce dos partition limit

The msdos partition table claimed a maximum partition count of 16
but would allow you to go beyond that.  This resulted in the kernel
not being informed of those partitions.  Corrected to enforce the
limit.

* NEWS (Bug fixes): Mention it.
* libparted/labels/dos.c (next_primary): Return -1 upon failure.
(next_logical): Stop no later than MAX_TOTAL_PART.
Throw exception and return -1 upon failure.
(msdos_partition_enumerate): Convert a negative partition number
return value from either of the above two to failure (return 0).
* tests/t9042-dos-partition-limit.sh: New file.
* tests/Makefile.am (TESTS): Add it.
---
 NEWS                               |    4 ++
 libparted/labels/dos.c             |   12 ++++-
 tests/Makefile.am                  |    1 +
 tests/t9042-dos-partition-limit.sh |   74 ++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 3 deletions(-)
 create mode 100644 tests/t9042-dos-partition-limit.sh

diff --git a/NEWS b/NEWS
index 58a5f89..194903b 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,10 @@ GNU parted NEWS                                    -*- outline -*-
   parted has improved support for partitionable loopback devices

 ** Bug fixes
+  The msdos partition table claimed a maximum partition count of 16
+  but would allow you to go beyond that.  This resulted in the kernel
+  not being informed of those partitions.  Corrected to enforce the
+  limit.

   parted now exits nonzero for certain failures already diagnosed as "Error".
   For example, before this change, parted would exit successfully in spite
diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
index fc89170..3f3effe 100644
--- a/libparted/labels/dos.c
+++ b/libparted/labels/dos.c
@@ -2398,17 +2398,22 @@ next_primary (const PedDisk* disk)
 		if (!ped_disk_get_partition (disk, i))
 			return i;
 	}
-	return 0;
+	return -1;
 }

 static int _GL_ATTRIBUTE_PURE
 next_logical (const PedDisk* disk)
 {
 	int	i;
-	for (i=5; 1; i++) {
+	for (i=5; i<=MAX_TOTAL_PART; i++) {
 		if (!ped_disk_get_partition (disk, i))
 			return i;
 	}
+	ped_exception_throw (
+		PED_EXCEPTION_ERROR, PED_EXCEPTION_CANCEL,
+		_("cannot create any more partitions"),
+		disk->dev->path);
+	return -1;
 }

 static int
@@ -2427,7 +2432,8 @@ msdos_partition_enumerate (PedPartition* part)
 		part->num = next_logical (part->disk);
 	else
 		part->num = next_primary (part->disk);
-
+	if (part->num == -1)
+		return 0;
 	return 1;
 }

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 34b3bc5..77a1cae 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -63,6 +63,7 @@ TESTS = \
   t9030-align-check.sh \
   t9040-many-partitions.sh \
   t9041-undetected-in-use-16th-partition.sh \
+  t9042-dos-partition-limit.sh \
   t9050-partition-table-types.sh

 EXTRA_DIST = \
diff --git a/tests/t9042-dos-partition-limit.sh b/tests/t9042-dos-partition-limit.sh
new file mode 100644
index 0000000..07e8f64
--- /dev/null
+++ b/tests/t9042-dos-partition-limit.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+# Ensure that parted enforces msdos partition limit
+
+# Copyright (C) 2010-2012 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/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../parted
+
+require_root_
+require_scsi_debug_module_
+
+grep '^#define USE_BLKID 1' "$CONFIG_HEADER" > /dev/null ||
+  skip_ 'this system lacks a new-enough libblkid'
+
+ss=$sector_size_
+partition_sectors=256  # sectors per partition
+n_partitions=13        # how many partitions to create
+start=2048             # start sector for the first partition
+
+n_sectors=$(($start + n_partitions * partition_sectors))
+
+sectors_per_MiB=$((1024 * 1024 / ss))
+n_MiB=$(((n_sectors + sectors_per_MiB - 1) / sectors_per_MiB))
+# create memory-backed device
+scsi_debug_setup_ sector_size=$ss dev_size_mb=$n_MiB > dev-name ||
+  skip_ 'failed to create scsi_debug device'
+scsi_dev=$(cat dev-name)
+
+n=$((n_MiB * sectors_per_MiB))
+printf '%s\n' "BYT;" \
+    "$scsi_dev:${n}s:scsi:$ss:$ss:msdos:Linux scsi_debug;" \
+    "1:$((start-2))s:$((start-2+4098-1))s:4098s:::lba;" \
+  > exp || fail=1
+
+parted -s $scsi_dev mklabel msdos || fail=1
+parted -s -a min $scsi_dev mkpart extended 2046s 100% || fail=1
+
+i=1
+while :; do
+    end=$((start + partition_sectors - 2))
+    parted -s -a min $scsi_dev mkpart logical ${start}s ${end}s || fail=1
+    printf "$((i+4)):${start}s:${end}s:$((end-start+1))s:::;\n" >> exp
+    test $i = $((n_partitions - 1)) && break
+    start=$((start + partition_sectors))
+    i=$((i+1))
+done
+
+parted -m -s $scsi_dev u s p > out || fail=1
+compare exp out || fail=1
+
+start=$((start + partition_sectors))
+end=$((start + partition_sectors - 2))
+
+#try one more partition than allowed, make sure it fails
+
+parted -s -a min $scsi_dev mkpart logical ${start}s ${end}s > out 2>&1
+cat <<EOF > exp
+Error: cannot create any more partitions
+EOF
+compare exp out || fail=1
+
+Exit $fail
--
1.7.9.rc1.2.gccfe4



More information about the parted-devel mailing list