Hans De Goede spotted another way to make parted
misbehave with hybrid GPT/MBR partition tables.
The fix is tiny. Everything else is test factorization
and adding the new one:
1/2 tests: factor utility functions into "library"
2/2 gpt: "read-only" operation could clobber the pMBR in another way
>From 7a06dae72e336b16e6a412ddebe72fe03be2a3a9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Wed, 10 Feb 2010 17:09:49 +0100
Subject: [PATCH 1/2] tests: factor utility functions into "library"
...since we're about to use them from a second test.
* tests/t-local.sh (peek_, poke_, gpt1_pte_name_offset_): New functions.
(gpt_corrupt_primary_table_, gpt_restore_primary_table_): New functions.
* tests/t0280-gpt-corrupt.sh: Remove local copies of those functions.
Use the new ones.
---
tests/t-local.sh | 57 ++++++++++++++++++++++++++++++++++++++++++++
tests/t0280-gpt-corrupt.sh | 35 +++------------------------
2 files changed, 61 insertions(+), 31 deletions(-)
diff --git a/tests/t-local.sh b/tests/t-local.sh
index 096e67c..2e4ca4b 100644
--- a/tests/t-local.sh
+++ b/tests/t-local.sh
@@ -97,4 +97,61 @@ require_512_byte_sector_size_()
|| skip_test_ FS test with sector size != 512
}
+peek_()
+{
+ case $# in 2) ;; *) echo "usage: peek_ FILE 0_BASED_OFFSET" >&2; exit 1;;
esac
+ case $2 in *[^0-9]*) echo "peek_: invalid offset: $2" >&2; exit 1 ;; esac
+ dd if="$1" bs=1 skip="$2" count=1
+}
+
+poke_()
+{
+ case $# in 3) ;; *) echo "usage: poke_ FILE 0_BASED_OFFSET BYTE" >&2; exit
1;;
+ esac
+ case $2 in *[^0-9]*) echo "poke_: invalid offset: $2" >&2; exit 1 ;; esac
+ case $3 in ?) ;; *) echo "poke_: invalid byte: '$3'" >&2; exit 1 ;; esac
+ printf %s "$3" | dd of="$1" bs=1 seek="$2" count=1 conv=notrunc
+}
+
+# byte 56 of the partition entry is the first byte of its 72-byte name field
+gpt1_pte_name_offset_()
+{
+ local ss=$1
+ case $ss in *[^0-9]*) echo "$0: invalid sector size: $ss">&2; return 1;; esac
+ expr $ss \* 2 + 56
+ return 0
+}
+
+# Change the name of the first partition in the primary GPT table,
+# thus invalidating the PartitionEntryArrayCRC32 checksum.
+gpt_corrupt_primary_table_()
+{
+ case $# in 2) ;; *) echo "$0: expected 2 args, got $#" >&2; return 1;; esac
+ local dev=$1
+ local ss=$2
+ case $ss in *[^0-9]*) echo "$0: invalid sector size: $ss">&2; return 1;; esac
+
+ # get the first byte of the name
+ local orig_pte_name_byte=$(peek_ $dev $(gpt1_pte_name_offset_ $ss)) ||
return 1
+
+ local new_byte
+ test x"$orig_pte_name_byte" = xA && new_byte=B || new_byte=A
+
+ # Replace with a different byte
+ poke_ $dev $(gpt1_pte_name_offset_ $ss) "$new_byte" || return 1
+
+ printf %s "$orig_pte_name_byte"
+ return 0
+}
+
+gpt_restore_primary_table_()
+{
+ case $# in 3) ;; *) echo "$0: expected 2 args, got $#" >&2; return 1;; esac
+ local dev=$1
+ local ss=$2
+ case $ss in *[^0-9]*) echo "$0: invalid sector size: $ss">&2; return 1;; esac
+ local orig_byte=$3
+ poke_ $dev $(gpt1_pte_name_offset_ $ss) "$orig_byte" || return 1
+}
+
. $srcdir/t-lvm.sh
diff --git a/tests/t0280-gpt-corrupt.sh b/tests/t0280-gpt-corrupt.sh
index 5c48116..5bf38f4 100755
--- a/tests/t0280-gpt-corrupt.sh
+++ b/tests/t0280-gpt-corrupt.sh
@@ -24,22 +24,6 @@ fi
: ${srcdir=.}
. $srcdir/t-lib.sh
-peek()
-{
- case $# in 2) ;; *) echo "usage: peek FILE 0_BASED_OFFSET" >&2; exit 1;; esac
- case $2 in *[^0-9]*) echo "peek: invalid offset: $2"; exit 1 ;; esac
- dd if="$1" bs=1 skip="$2" count=1
-}
-
-poke()
-{
- case $# in 3) ;; *) echo "usage: poke FILE 0_BASED_OFFSET BYTE" >&2; exit 1;;
- esac
- case $2 in *[^0-9]*) echo "poke: invalid offset: $2"; exit 1 ;; esac
- case $3 in ?) ;; *) echo "poke: invalid byte: '$3'"; exit 1 ;; esac
- printf %s "$3" | dd of="$1" bs=1 seek="$2" count=1 conv=notrunc
-}
-
dev=loop-file
ss=$sector_size_
@@ -67,17 +51,7 @@ compare /dev/null empty || fail=1
# We're going to change the name of the first partition,
# thus invalidating the PartitionEntryArrayCRC32 checksum.
-
-# byte 56 of the partition entry is the first byte of its 72-byte name field
-pte_offset=$(expr $ss \* 2 + 56)
-
-# get the first byte of the name
-pte_byte=$(peek $dev $pte_offset) || fail=1
-
-test x"$pte_byte" = xA && new_byte=B || new_byte=A
-
-# Replace with a different byte
-poke $dev $pte_offset "$new_byte" || fail=1
+orig_byte=$(gpt_corrupt_primary_table_ $dev $ss) || fail=1
# printing the table must succeed, but with a scary diagnostic.
parted -s $dev print > err 2>&1 || fail=1
@@ -91,9 +65,8 @@ compare exp err || fail=1
# ----------------------------------------------------------
# Now, restore things, and corrupt the MyLBA in the alternate GUID table.
-orig_byte=s
# Restore original byte
-poke $dev $pte_offset "$orig_byte" || fail=1
+gpt_restore_primary_table_ $dev $ss "$orig_byte" || fail=1
# print the table
parted -s $dev print > out 2> err || fail=1
@@ -103,13 +76,13 @@ compare /dev/null err || fail=1
# $n_sectors, 8-byte field starting at offset 24.
alt_my_lba_offset=$(expr $n_sectors \* $ss - $ss + 24)
# get the first byte of MyLBA
-byte=$(peek $dev $alt_my_lba_offset) || fail=1
+byte=$(peek_ $dev $alt_my_lba_offset) || fail=1
# Perturb it.
test x"$byte" = xA && new_byte=B || new_byte=A
# Replace good byte with the bad one.
-poke $dev $alt_my_lba_offset "$new_byte" || fail=1
+poke_ $dev $alt_my_lba_offset "$new_byte" || fail=1
# attempting to set partition name must print a diagnostic
parted -m -s $dev name 1 foo > err 2>&1 || fail=1
--
1.7.0.rc2.170.gbc565
>From 432a33115c50005bbe96a09d55edc7d034715ec8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Wed, 10 Feb 2010 17:11:25 +0100
Subject: [PATCH 2/2] gpt: "read-only" operation could clobber the pMBR in
another way
A read-only operation like "parted $dev print" would overwrite $dev's
pMBR when exactly one of the primary and backup tables was corrupt.
* libparted/labels/gpt.c (gpt_read): Clear "write_back" in those
two cases. Hans De Goede spotted this bug by inspection.
* NEWS (Bug fixes): Mention it.
* tests/t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh: New test.
* tests/Makefile.am (TESTS): Add
t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh.
---
NEWS | 5 ++
libparted/labels/gpt.c | 2 +
tests/Makefile.am | 1 +
...gpt-print-with-corrupt-primary-clobbers-pmbr.sh | 56 ++++++++++++++++++++
4 files changed, 64 insertions(+), 0 deletions(-)
create mode 100755 tests/t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh
diff --git a/NEWS b/NEWS
index 96ea96c..28f87de 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,11 @@ GNU parted NEWS -*-
outline -*-
gpt: read-only operation could clobber MBR part of hybrid GPT+MBR table
[bug introduced in parted-2.1]
+ gpt: a read-only operation like "parted $dev print" would overwrite $dev's
+ protective MBR when exactly one of the primary and backup GPT tables was
+ found to be corrupt.
+ [bug introduced prior to parted-1.8.0]
+
"make install" no longer installs tests programs named disk and label
diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index ea96a3b..48d580e 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -984,6 +984,7 @@ gpt_read (PedDisk *disk)
goto error_free_gpt;
gpt = primary_gpt;
+ write_back = 0;
}
else /* !primary_gpt && backup_gpt */
{
@@ -996,6 +997,7 @@ gpt_read (PedDisk *disk)
goto error_free_gpt;
gpt = backup_gpt;
+ write_back = 0;
}
backup_gpt = NULL;
primary_gpt = NULL;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 38922f6..8008400 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -10,6 +10,7 @@ TESTS = \
t0201-gpt.sh \
t0202-gpt-pmbr.sh \
t0205-gpt-list-clobbers-pmbr.sh \
+ t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh \
t0220-gpt-msftres.sh \
t0250-gpt.sh \
t0280-gpt-corrupt.sh \
diff --git a/tests/t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh
b/tests/t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh
new file mode 100755
index 0000000..f47549e
--- /dev/null
+++ b/tests/t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+# Ensure that printing a GPT partition table does not modify the pMBR.
+# Much like t0205, but with the addition of a corrupt PTE in primary table,
+# "parted $device print" would modify $device.
+
+# Copyright (C) 2010 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/>.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ parted --version
+fi
+
+: ${srcdir=.}
+. $srcdir/t-lib.sh
+
+fail=0
+
+ss=$sector_size_
+n_sectors=400
+dev=dev-file
+
+dd if=/dev/null of=$dev bs=$ss seek=$n_sectors || fail=1
+parted -s $dev mklabel gpt || fail=1
+parted -s $dev mkpart p1 128s 255s || fail=1
+
+parted -m -s $dev u s p || fail=1
+
+# Write non-NUL bytes all over the MBR, so we're likely to see any change.
+# However, be careful to leave the type of the first partition, 0xEE,
+# as well as the final two magic bytes.
+printf '%0450d\xee%059d\x55\xaa' 0 0 | dd of=$dev count=1 conv=notrunc ||
fail=1
+
+dd if=$dev of=before count=1 || fail=1
+
+orig_byte=$(gpt_corrupt_primary_table_ $dev $ss) || fail=1
+
+parted -m -s $dev u s p || fail=1
+
+dd if=$dev of=after count=1 || fail=1
+
+cmp before after || fail=1
+
+Exit $fail
--
1.7.0.rc2.170.gbc565
_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel
