Jim Meyering wrote: > 27/249 wrote: >> Seems that attached patch fixed this bug. I tested some cases, and >> everything works fine for me, but I'm not sure if it can break >> something hidden, so, developers, please review this fix. >> >> On Sun, Feb 7, 2010 at 6:50 PM, 27/249 <i27...@gmail.com> wrote: >>> Yes, I forgot to add that this bug is still present in latest GIT tree >>> (checked 1 minute ago). >>> >>> On Sun, Feb 7, 2010 at 6:48 PM, 27/249 <i27...@gmail.com> wrote: >>>> Few weeks ago I described this bug in >>>> http://parted.alioth.debian.org/cgi-bin/trac.cgi/ticket/250 >>>> >>>> This bug is very serious for me, so, since I didn't see any motion in >>>> bugtracker, I wish to try to fix it myself. If you can help me in any >>>> way - it will be very good :) First of all, it will be very nice to >>>> know what piece of code can commit partition table to disk. > > Thanks a lot for the report. > That appears to be a serious bug. > > BTW, email to this mailing list will catch my attention, > while I rarely look at that tracker. > > Would you please see if the patch below solves your problem?
He confirmed (off-list) that it solved the problem. I have written a script to exercise the bug and a NEWS entry. Here's the patch I expect to push soon. >From c7cf12751993474238220593168dedfbbe18fc0a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Sun, 7 Feb 2010 20:31:11 +0100 Subject: [PATCH] gpt: read-only operation could clobber MBR part of hybrid GPT+MBR table * libparted/labels/gpt.c (gpt_read): Fix a bug introduced by me in commit 7f753b1b, "gpt: rewrite GPT header-reading code". Set write_back=0 in one more code path. * tests/Makefile.am (TESTS): Add t0205-gpt-list-clobbers-pmbr.sh. * tests/t0205-gpt-list-clobbers-pmbr.sh: New test. * NEWS (Bug fixes): Mention this. Reported by aix27249 in http://parted.alioth.debian.org/cgi-bin/trac.cgi/ticket/250 --- NEWS | 5 +++ libparted/labels/gpt.c | 10 ++++-- tests/Makefile.am | 1 + tests/t0205-gpt-list-clobbers-pmbr.sh | 59 +++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/t0205-gpt-list-clobbers-pmbr.sh diff --git a/NEWS b/NEWS index ea6caa1..7b4994d 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,11 @@ GNU parted NEWS -*- outline -*- * Noteworthy changes in release ?.? (????-??-??) [?] +** Bug fixes + + gpt: read-only operation could clobber MBR part of hybrid GPT+MBR table + [bug introduced in parted-2.1] + * Noteworthy changes in release 2.1 (2009-12-20) [stable] diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c index 9d9876c..ea96a3b 100644 --- a/libparted/labels/gpt.c +++ b/libparted/labels/gpt.c @@ -4,7 +4,7 @@ original version by Matt Domsch <matt_dom...@dell.com> Disclaimed into the Public Domain - Portions Copyright (C) 2001-2003, 2005-2009 Free Software Foundation, Inc. + Portions Copyright (C) 2001-2003, 2005-2010 Free Software Foundation, Inc. EFI GUID Partition Table handling Per Intel EFI Specification v1.02 @@ -932,9 +932,9 @@ gpt_read (PedDisk *disk) if (primary_gpt && backup_gpt) { /* Both are valid. */ +#ifndef DISCOVER_ONLY if (PED_LE64_TO_CPU (primary_gpt->AlternateLBA) < disk->dev->length - 1) { -#ifndef DISCOVER_ONLY switch (ped_exception_throw (PED_EXCEPTION_ERROR, (PED_EXCEPTION_FIX | PED_EXCEPTION_CANCEL @@ -954,8 +954,12 @@ gpt_read (PedDisk *disk) write_back = 0; break; } -#endif /* !DISCOVER_ONLY */ } + else + { + write_back = 0; + } +#endif /* !DISCOVER_ONLY */ gpt = primary_gpt; pth_free (backup_gpt); } diff --git a/tests/Makefile.am b/tests/Makefile.am index 7bfb22e..38922f6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -9,6 +9,7 @@ TESTS = \ t0200-gpt.sh \ t0201-gpt.sh \ t0202-gpt-pmbr.sh \ + t0205-gpt-list-clobbers-pmbr.sh \ t0220-gpt-msftres.sh \ t0250-gpt.sh \ t0280-gpt-corrupt.sh \ diff --git a/tests/t0205-gpt-list-clobbers-pmbr.sh b/tests/t0205-gpt-list-clobbers-pmbr.sh new file mode 100644 index 0000000..979a15e --- /dev/null +++ b/tests/t0205-gpt-list-clobbers-pmbr.sh @@ -0,0 +1,59 @@ +#!/bin/sh +# Ensure that printing a GPT partition table does not modify the pMBR. +# Due to a bug in parted-2.1, "parted /dev/... print" would do just that. +# Not a problem for most, but if you have a hybrid, e.g., gptsync'd +# GPT/MBR table, merely listing the table with Parted-2.1 would clobber +# the MBR part. + +# 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 101s 150s || fail=1 +parted -s $dev mkpart p2 151s 200s || fail=1 +parted -s $dev mkpart p3 201s 250s || 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 + +chmod a-w $dev +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.156.g2ac04 _______________________________________________ bug-parted mailing list bug-parted@gnu.org http://lists.gnu.org/mailman/listinfo/bug-parted
