Package: parted;
Reported by: Phillip Susi <psusi <at> ubuntu.com>
Date: Mon, 30 Sep 2013 02:20:02 UTC
Severity: normal
Tags: patch
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: bug-parted <at> gnu.org Subject: [PATCH] Refactor device canonicalization Date: Sun, 29 Sep 2013 22:18:55 -0400
Previous attempts to use the correct name for device-mapper devices on
linux simply bypassed canonicalizing the given name if it started with
"/dev/mapper". This gave the correct name ( as opposed to following the
symlink in /dev/mapper to /dev/dm-X ), but only when the given name was
the /dev/mapper name. The wrong name would still be used when given some
other symlink, such as /dev/disk/by-id. This was worked around in linux.c
by having linux_partition_get_path generate the full canonical path name
to the partition, while parted still reported the symlink as the name
of the disk.
This patch adds an arch specific get_normal_path method that, if
implemented, is used to find the canonical device path instead of the
canonicalize_file_name function from gnulib. The linux arch implements
this by generating the correct /dev/mapper name and no longer has to
special case linux_partition_get_path to generate the base name.
The original t3000-symlink test was incorrect because it relied on the
hard coded "/dev/mapper" prefix so it has been replaced with
t6004-dm-symlink.sh instead, which creates a real dm device and a symlink
to it to verify the real name is used when the user gives the symlink.
t6000-dm.sh was also relying on a private /dev/mapper directory, which
is unworkable.
---
include/parted/device.in.h | 1 +
libparted/arch/linux.c | 52 +++++++--------
libparted/device.c | 6 +-
libparted/tests/Makefile.am | 5 +-
libparted/tests/symlink.c | 135 ---------------------------------------
libparted/tests/t3000-symlink.sh | 27 --------
tests/Makefile.am | 1 +
tests/t6000-dm.sh | 2 +-
tests/t6004-dm-symlink.sh | 68 ++++++++++++++++++++
9 files changed, 103 insertions(+), 194 deletions(-)
diff --git a/include/parted/device.in.h b/include/parted/device.in.h
index 7c06a66..d243fcc 100644
--- a/include/parted/device.in.h
+++ b/include/parted/device.in.h
@@ -119,6 +119,7 @@ struct _PedDeviceArchOps {
/* These functions are optional */
PedAlignment *(*get_minimum_alignment)(const PedDevice *dev);
PedAlignment *(*get_optimum_alignment)(const PedDevice *dev);
+ char *(*get_normal_path)(const char *path);
};
#include <parted/constraint.h>
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index d3a03d6..c2a6184 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2286,24 +2286,29 @@ zasprintf (const char *format, ...)
}
static char *
-dm_canonical_path (PedDevice const *dev)
+linux_get_normal_path (const char *path)
{
- LinuxSpecific const *arch_specific = LINUX_SPECIFIC (dev);
-
- /* Get map name from devicemapper */
- struct dm_task *task = dm_task_create (DM_DEVICE_INFO);
- if (!task)
- goto err;
- if (!dm_task_set_major_minor (task, arch_specific->major,
- arch_specific->minor, 0))
- goto err;
- if (!dm_task_run(task))
- goto err;
- char *dev_name = zasprintf ("/dev/mapper/%s", dm_task_get_name (task));
- if (dev_name == NULL)
+ struct stat st;
+ if (stat(path, &st))
goto err;
- dm_task_destroy (task);
- return dev_name;
+ if (!S_ISBLK(st.st_mode))
+ return canonicalize_file_name(path);
+ if (_is_dm_major(major(st.st_rdev))) {
+ /* Get map name from devicemapper */
+ struct dm_task *task = dm_task_create (DM_DEVICE_INFO);
+ if (!task)
+ goto err;
+ if (!dm_task_set_major_minor (task, major(st.st_rdev),
+ minor(st.st_rdev), 0))
+ goto err;
+ if (!dm_task_run(task))
+ goto err;
+ char *dev_name = zasprintf ("/dev/mapper/%s", dm_task_get_name (task));
+ if (dev_name == NULL)
+ goto err;
+ dm_task_destroy (task);
+ return dev_name;
+ } else return canonicalize_file_name(path);
err:
return NULL;
}
@@ -2311,27 +2316,23 @@ err:
static char*
_device_get_part_path (PedDevice const *dev, int num)
{
- char *devpath = (dev->type == PED_DEVICE_DM
- ? dm_canonical_path (dev) : dev->path);
- size_t path_len = strlen (devpath);
+ size_t path_len = strlen (dev->path);
char *result;
/* Check for devfs-style /disc => /partN transformation
unconditionally; the system might be using udev with devfs rules,
and if not the test is harmless. */
- if (5 < path_len && !strcmp (devpath + path_len - 5, "/disc")) {
+ if (5 < path_len && !strcmp (dev->path + path_len - 5, "/disc")) {
/* replace /disc with /part%d */
result = zasprintf ("%.*s/part%d",
- (int) (path_len - 5), devpath, num);
+ (int) (path_len - 5), dev->path, num);
} else {
char const *p = (dev->type == PED_DEVICE_DAC960
|| dev->type == PED_DEVICE_CPQARRAY
|| dev->type == PED_DEVICE_ATARAID
- || isdigit (devpath[path_len - 1])
+ || isdigit (dev->path[path_len - 1])
? "p" : "");
- result = zasprintf ("%s%s%d", devpath, p, num);
+ result = zasprintf ("%s%s%d", dev->path, p, num);
}
- if (dev->type == PED_DEVICE_DM)
- free (devpath);
return result;
}
@@ -2997,6 +2998,7 @@ static PedDeviceArchOps linux_dev_ops = {
get_minimum_alignment: linux_get_minimum_alignment,
get_optimum_alignment: linux_get_optimum_alignment,
#endif
+ get_normal_path: linux_get_normal_path,
};
PedDiskArchOps linux_disk_ops = {
diff --git a/libparted/device.c b/libparted/device.c
index 738b320..9b67454 100644
--- a/libparted/device.c
+++ b/libparted/device.c
@@ -152,9 +152,9 @@ ped_device_get (const char* path)
char* normal_path = NULL;
PED_ASSERT (path != NULL);
- /* Don't canonicalize /dev/mapper paths, see tests/symlink.c */
- if (strncmp (path, "/dev/mapper/", 12))
- normal_path = canonicalize_file_name (path);
+ if (ped_architecture->dev_ops->get_normal_path)
+ normal_path = ped_architecture->dev_ops->get_normal_path(path);
+ else normal_path = canonicalize_file_name (path);
if (!normal_path)
/* Well, maybe it is just that the file does not exist.
* Try it anyway. */
diff --git a/libparted/tests/Makefile.am b/libparted/tests/Makefile.am
index bfa5790..4e99ed1 100644
--- a/libparted/tests/Makefile.am
+++ b/libparted/tests/Makefile.am
@@ -3,9 +3,9 @@
#
# This file may be modified and/or distributed without restriction.
-TESTS = t1000-label.sh t2000-disk.sh t2100-zerolen.sh t3000-symlink.sh
+TESTS = t1000-label.sh t2000-disk.sh t2100-zerolen.sh
EXTRA_DIST = $(TESTS)
-check_PROGRAMS = label disk zerolen symlink
+check_PROGRAMS = label disk zerolen
AM_CFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS)
LDADD = \
@@ -21,7 +21,6 @@ AM_CPPFLAGS = \
label_SOURCES = common.h common.c label.c
disk_SOURCES = common.h common.c disk.c
zerolen_SOURCES = common.h common.c zerolen.c
-symlink_SOURCES = common.h common.c symlink.c
# Arrange to symlink to tests/init.sh.
CLEANFILES = init.sh
diff --git a/libparted/tests/symlink.c b/libparted/tests/symlink.c
deleted file mode 100644
index 52e99ca..0000000
--- a/libparted/tests/symlink.c
+++ /dev/null
@@ -1,135 +0,0 @@
-/* Sometimes libparted operates on device mapper files, with a path of
- /dev/mapper/foo. With newer lvm versions /dev/mapper/foo is a symlink to
- /dev/dm-#. However some storage administration programs (anaconda for
- example) may do the following:
- 1) Create a ped_device for /dev/mapper/foo
- 2) ped_get_device resolves the symlink to /dev/dm-#, and the path
- in the PedDevice struct points to /dev/dm-#
- 3) The program does some things to lvm, causing the symlink to
- point to a different /dev/dm-# node
- 4) The program does something with the PedDevice, which results
- in an operation on the wrong device
-
- Newer libparted versions do not suffer from this problem, as they
- do not canonicalize device names under /dev/mapper. This test checks
- for this bug. */
-
-#include <config.h>
-#include <unistd.h>
-
-#include <check.h>
-
-#include <parted/parted.h>
-
-#include "common.h"
-#include "progname.h"
-
-static char *temporary_disk;
-
-static void
-create_disk (void)
-{
- temporary_disk = _create_disk (4096 * 1024);
- fail_if (temporary_disk == NULL, "Failed to create temporary disk");
-}
-
-static void
-destroy_disk (void)
-{
- unlink (temporary_disk);
- free (temporary_disk);
-}
-
-START_TEST (test_symlink)
-{
- char cwd[256], ln[256] = "/dev/mapper/parted-test-XXXXXX";
-
- if (!getcwd (cwd, sizeof cwd)) {
- fail ("Could not get cwd");
- return;
- }
-
- /* Create a symlink under /dev/mapper to our
- temporary disk */
- int tmp_fd = mkstemp (ln);
- if (tmp_fd == -1) {
- fail ("Could not create tempfile");
- return;
- }
-
- /* There is a temp file vulnerability symlink attack possibility
- here, but as /dev/mapper is root owned this is a non issue */
- close (tmp_fd);
- unlink (ln);
- char temp_disk_path[256];
- snprintf (temp_disk_path, sizeof temp_disk_path, "%s/%s", cwd,
- temporary_disk);
- int res = symlink (temp_disk_path, ln);
- if (res) {
- fail ("could not create symlink");
- return;
- }
-
- PedDevice *dev = ped_device_get (ln);
- if (dev == NULL)
- goto exit_unlink_ln;
-
- /* Create a second temporary_disk */
- char *temporary_disk2 = _create_disk (4096 * 1024);
- if (temporary_disk2 == NULL) {
- fail ("Failed to create 2nd temporary disk");
- goto exit_destroy_dev;
- }
-
- /* Remove first temporary disk, and make temporary_disk point to
- temporary_disk2 (for destroy_disk()). */
- unlink (temporary_disk);
- free (temporary_disk);
- temporary_disk = temporary_disk2;
-
- /* Update symlink to point to our new / second temporary disk */
- unlink (ln);
- snprintf (temp_disk_path, sizeof temp_disk_path, "%s/%s", cwd,
- temporary_disk);
- res = symlink (temp_disk_path, ln);
- if (res) {
- fail ("could not create 2nd symlink");
- goto exit_destroy_dev;
- }
-
- /* Do something to our PedDevice, if the symlink was resolved,
- instead of remembering the /dev/mapper/foo name, this will fail */
- ped_disk_clobber (dev);
-
-exit_destroy_dev:
- ped_device_destroy (dev);
-exit_unlink_ln:
- unlink (ln);
-}
-END_TEST
-
-int
-main (int argc, char **argv)
-{
- set_program_name (argv[0]);
- int number_failed;
- Suite* suite = suite_create ("Symlink");
- TCase* tcase_symlink = tcase_create ("/dev/mapper symlink");
-
- /* Fail when an exception is raised */
- ped_exception_set_handler (_test_exception_handler);
-
- tcase_add_checked_fixture (tcase_symlink, create_disk, destroy_disk);
- tcase_add_test (tcase_symlink, test_symlink);
- /* Disable timeout for this test */
- tcase_set_timeout (tcase_symlink, 0);
- suite_add_tcase (suite, tcase_symlink);
-
- SRunner* srunner = srunner_create (suite);
- srunner_run_all (srunner, CK_VERBOSE);
-
- number_failed = srunner_ntests_failed (srunner);
- srunner_free (srunner);
-
- return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
-}
diff --git a/libparted/tests/t3000-symlink.sh b/libparted/tests/t3000-symlink.sh
deleted file mode 100755
index 338e44a..0000000
--- a/libparted/tests/t3000-symlink.sh
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/bin/sh
-# run the /dev/mapper symlink test
-
-# Copyright (C) 2007-2013 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/>.
-
-. "${abs_top_srcdir=../..}/tests/init.sh"; path_prepend_ .
-
-. $abs_top_srcdir/tests/t-lib-helpers.sh
-# Need root privileges to create a symlink under /dev/mapper.
-require_root_
-
-symlink || fail=1
-
-Exit $fail
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 16ec5d2..c5472d2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -63,6 +63,7 @@ TESTS = \
t6001-psep.sh \
t6002-dm-busy.sh \
t6003-dm-hide.sh \
+ t6004-dm-symlink.sh \
t6100-mdraid-partitions.sh \
t7000-scripting.sh \
t8000-loop.sh \
diff --git a/tests/t6000-dm.sh b/tests/t6000-dm.sh
index c301dee..241f34b 100755
--- a/tests/t6000-dm.sh
+++ b/tests/t6000-dm.sh
@@ -65,7 +65,7 @@ for type in linear ; do
# setup: create a mapping
echo "$dmsetup_cmd" | dmsetup create "$type_kwd" || fail=1
- dev="$DM_DEV_DIR/mapper/$type_kwd"
+ dev="/dev/mapper/$type_kwd"
# Create msdos partition table
parted -s $dev mklabel msdos > out 2>&1 || fail=1
diff --git a/tests/t6004-dm-symlink.sh b/tests/t6004-dm-symlink.sh
new file mode 100644
index 0000000..0cb967b
--- /dev/null
+++ b/tests/t6004-dm-symlink.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+# ensure that parted canonicalizes symlinks to dm devices correctly
+
+# Copyright (C) 2008-2013 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_
+lvm_init_root_dir_
+
+test "x$ENABLE_DEVICE_MAPPER" = xyes \
+ || skip_ "no device-mapper support"
+
+# Device maps names - should be random to not conflict with existing ones on
+# the system
+linear_=plinear-$$
+
+d1=
+f1=
+ln1=
+cleanup_fn_() {
+ dmsetup remove $linear_
+ test -n "$d1" && losetup -d "$d1"
+ rm -f "$f1" "$ln1";
+}
+
+f1=$(pwd)/1; d1=$(loop_setup_ "$f1") \
+ || skip_ "is this partition mounted with 'nodev'?"
+
+dmsetup_cmd="0 1024 linear $d1 0"
+
+ # setup: create a mapping
+ echo "$dmsetup_cmd" | dmsetup create $linear_ || fail=1
+ dev="/dev/mapper/$linear_"
+ ln1=symlink
+ ln -s "$dev" symlink || fail=1
+
+ # Create msdos partition table
+ parted -s $dev mklabel msdos
+ parted -s symlink print > out 2>&1 || fail=1
+ # Create expected output file.
+ cat <<EOF >> exp || fail=1
+Model: Linux device-mapper (linear) (dm)
+Disk $dev: 524kB
+Sector size (logical/physical): 512B/512B
+Partition Table: msdos
+Disk Flags:
+
+Number Start End Size Type File system Flags
+
+EOF
+
+ compare exp out || fail=1
+
+Exit $fail
--
1.8.1.2
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.