[parted-devel] [PATCH 1/2] tests: factor utility functions into "library"
Hans de Goede
hdegoede at redhat.com
Wed Feb 10 19:21:48 UTC 2010
Hi,
Would it not be better to initialize write back to 0, and only set it to 1
when we actually want to do writeback ?
Also since the code paths for using the ok gpt in case the primary
or backup is corrupt, only warn without asking anything, how can
one cause the table to be fixed up without making any changes now ?
This is a parted problem I think no a libparted problem, as parted is
instand apply, there is no commit command afaik, I case we should add
one so issues like one of the 2 tables being corrupt can be fixed
by using the commit command.
Regards,
Hans
On 02/10/2010 06:36 PM, Jim Meyering wrote:
> 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<meyering at redhat.com>
> 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<meyering at redhat.com>
> 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
More information about the parted-devel
mailing list