Brian C. Lane wrote: > From: "Brian C. Lane" <b...@redhat.com> > > * libparted/fs/hfs/probe.c (hfsplus_probe): Add a check on the > search value and reject it if it is negative. > (hfsx_probe): Same > (hfs_and_wrapper_probe): Same
Thanks again. I've adjusted your log and pushed that patch, along with a test and a NEWS update: >From 6bc7e1965078579e9855f851254d67b3d8165784 Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" <b...@redhat.com> Date: Fri, 7 Oct 2011 16:53:48 -0700 Subject: [PATCH 1/2] libparted: HFS/HFS+ probe: don't let a corrupt FS evoke failed assertion * libparted/fs/hfs/probe.c (hfsplus_probe): Add a check on the search value and reject it if it is negative. (hfsx_probe): Likewise. (hfs_and_wrapper_probe): Likewise. Reported by Flos Lonicerae in http://bugzilla.redhat.com/714758 --- libparted/fs/hfs/probe.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-) diff --git a/libparted/fs/hfs/probe.c b/libparted/fs/hfs/probe.c index 8c656cf..bf4d70b 100644 --- a/libparted/fs/hfs/probe.c +++ b/libparted/fs/hfs/probe.c @@ -82,7 +82,8 @@ hfs_and_wrapper_probe (PedGeometry* geom) + ((PedSector) PED_BE16_TO_CPU (mdb->total_blocks) * (PED_BE32_TO_CPU (mdb->block_size) / PED_SECTOR_SIZE_DEFAULT ))); max = search + (PED_BE32_TO_CPU (mdb->block_size) / PED_SECTOR_SIZE_DEFAULT); - if (!(geom_ret = ped_geometry_new (geom->dev, geom->start, search + 2))) + if ((search < 0) + || !(geom_ret = ped_geometry_new (geom->dev, geom->start, search + 2))) return NULL; for (; search < max; search++) { @@ -141,8 +142,9 @@ hfsplus_probe (PedGeometry* geom) - 2; search = max - 2 * ( PED_BE32_TO_CPU (vh->block_size) / PED_SECTOR_SIZE_DEFAULT ) + 2; - if (!(geom_ret = ped_geometry_new (geom->dev, geom->start, - search + 2))) + if ((search < 0) + || !(geom_ret = ped_geometry_new (geom->dev, geom->start, + search + 2))) return NULL; for (; search < max; search++) { @@ -156,8 +158,9 @@ hfsplus_probe (PedGeometry* geom) search = ((PedSector) PED_BE32_TO_CPU (vh->total_blocks) - 1) * ( PED_BE32_TO_CPU (vh->block_size) / PED_SECTOR_SIZE_DEFAULT ) - 1; - if (!ped_geometry_set (geom_ret, geom_ret->start, - search + 2) + if ((search < 0) + || !ped_geometry_set (geom_ret, geom_ret->start, + search + 2) || !ped_geometry_read (geom_ret, buf, search, 1) || vh->signature != PED_CPU_TO_BE16 (HFSP_SIGNATURE)) { ped_geometry_destroy (geom_ret); @@ -213,8 +216,9 @@ hfsx_probe (PedGeometry* geom) * ( PED_BE32_TO_CPU (vh->block_size) / PED_SECTOR_SIZE_DEFAULT ) - 2; search = max - ( PED_BE32_TO_CPU (vh->block_size) / PED_SECTOR_SIZE_DEFAULT ); - if (!(geom_ret = ped_geometry_new (geom->dev, geom->start, - search + 2))) + if ((search < 0) + || !(geom_ret = ped_geometry_new (geom->dev, geom->start, + search + 2))) return NULL; for (; search < max; search++) { if (!ped_geometry_set (geom_ret, geom_ret->start, -- 1.7.8.rc0.35.gee6df >From 723ca1f12f13ae108eba611d72de26591e1214ba Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Sat, 5 Nov 2011 20:49:18 +0100 Subject: [PATCH 2/2] tests: exercise and document the HFS-probe bug fix Simply zeroing out the total_blocks and block_size members of the on-disk _HfsMasterDirectoryBlock would provoke a failed assertion any time parted tried to probe that partition. * tests/t2500-probe-corrupt-hfs.sh: New script. * tests/Makefile.am (TESTS): Add it. * NEWS (Bug fixes): Mention this. --- .gitignore | 2 +- NEWS | 4 +++ tests/Makefile.am | 1 + tests/t2500-probe-corrupt-hfs.sh | 41 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletions(-) create mode 100755 tests/t2500-probe-corrupt-hfs.sh diff --git a/.gitignore b/.gitignore index 5937e5b..98005db 100644 --- a/.gitignore +++ b/.gitignore @@ -36,9 +36,9 @@ libparted.pc libparted/tests/disk libparted/tests/init.sh libparted/tests/label +libparted/tests/symlink libparted/tests/t*.sh.log libparted/tests/test-suite.log -libparted/tests/symlink libparted/tests/zerolen libtool m4 diff --git a/NEWS b/NEWS index c6e22f0..43b73f1 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,10 @@ GNU parted NEWS -*- outline -*- cause an MSDOS partition table to be mistakenly identified as pc98. [bug present since the beginning] + libparted no longer gets a failed assertion when probing a partition + with an HFS or HFS+ signature, but with invalid ->total_blocks and/or + ->block_size values. + ** Changes in behavior parted: mkpart command has changed semantics with regard to specifying end diff --git a/tests/Makefile.am b/tests/Makefile.am index 71787e5..5bc513d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,6 +35,7 @@ TESTS = \ t2300-dos-label-extended-bootcode.sh \ t2310-dos-extended-2-sector-min-offset.sh \ t2400-dos-hfs-partition-type.sh \ + t2500-probe-corrupt-hfs.sh \ t3200-type-change.sh \ t3300-palo-prep.sh \ t3310-flags.sh \ diff --git a/tests/t2500-probe-corrupt-hfs.sh b/tests/t2500-probe-corrupt-hfs.sh new file mode 100755 index 0000000..435af9f --- /dev/null +++ b/tests/t2500-probe-corrupt-hfs.sh @@ -0,0 +1,41 @@ +#!/bin/sh +# Do not misbehave when probing a corrupt HFS partition. + +# Copyright (C) 2011 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 +ss=$sector_size_ + +N=3M +dev=loop-file +# create a file large enough to hold a GPT partition table +dd if=/dev/null of=$dev bs=1 seek=$N || framework_failure + +parted -s "$dev" mklabel gpt mkpart p1 1MiB 2MiB > out 2>&1 || fail=1 +compare out /dev/null || fail=1 + +parted -s "$dev" u s p || fail=1 + +# Poke an HFS+ signature into place +printf '\x48\x2b' | dd of=$dev seek=$((2048+2)) conv=notrunc || fail=1 + +# Or, if starting from a valid HFS/HFS+ file system, poke these: +# offset 18 total_blocks=0(16b) +# offset 20 vh->block_size=0(32b) + +parted -s "$dev" u s p || fail=1 + +Exit $fail -- 1.7.8.rc0.35.gee6df