[Parted-commits] GNU Parted Official Repository: Changes to 'master'
Jim Meyering
meyering at alioth.debian.org
Tue Apr 6 14:57:42 UTC 2010
NEWS | 5 +
libparted/device.c | 6 +
libparted/tests/Makefile.am | 5 -
libparted/tests/symlink.c | 135 +++++++++++++++++++++++++++++++++++++++
libparted/tests/t3000-symlink.sh | 29 ++++++++
5 files changed, 176 insertions(+), 4 deletions(-)
New commits:
commit 85506df997edba889cf8b475df3b854f4887e9bb
Author: Hans de Goede <hdegoede at redhat.com>
Date: Tue Apr 6 15:57:17 2010 +0200
libparted: add test for /dev/mapper symlink issue
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.
* libparted/tests/symlink.c: New test which tests for this issue.
* libparted/tests/t3000-symlink.sh: New file.
* libparted/tests/Makefile.am: Include the new files. Run the new test.
diff --git a/libparted/tests/Makefile.am b/libparted/tests/Makefile.am
index 2f63741..b433c83 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
+TESTS = t1000-label.sh t2000-disk.sh t3000-symlink.sh
EXTRA_DIST = $(TESTS)
-check_PROGRAMS = label disk
+check_PROGRAMS = label disk symlink
AM_CFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS)
LDADD = \
@@ -19,6 +19,7 @@ AM_CPPFLAGS = \
label_SOURCES = common.h common.c label.c
disk_SOURCES = common.h common.c disk.c
+symlink_SOURCES = common.h common.c symlink.c
MAINTAINERCLEANFILES = Makefile.in
diff --git a/libparted/tests/symlink.c b/libparted/tests/symlink.c
new file mode 100644
index 0000000..52e99ca
--- /dev/null
+++ b/libparted/tests/symlink.c
@@ -0,0 +1,135 @@
+/* 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
new file mode 100755
index 0000000..9c64833
--- /dev/null
+++ b/libparted/tests/t3000-symlink.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# Copyright (C) 2007-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/>.
+
+test_description='run the /dev/mapper symlink test'
+
+# Need root privileges to create a symlink under /dev/mapper.
+privileges_required_=1
+
+: ${top_srcdir=../..}
+. "$top_srcdir/tests/test-lib.sh"
+
+test_expect_success \
+ 'run the actual tests' 'symlink'
+
+test_done
commit c1eb485b9fd8919e18f192d678bc52b0488e6ee0
Author: Hans de Goede <hdegoede at redhat.com>
Date: Tue Apr 6 15:57:18 2010 +0200
libparted: don't canonicalize /dev/mapper paths
Besides fixing the issue displayed by libparted/tests/symlink.c,
this has the added advantage that the output of parted p on one of these
devices now says:
"Disk /dev/mapper/BigVol2-lv_iscsi_disk2: 34.4GB"
Which is a lot more user friendly then the output before this patch:
"Disk /dev/dm-6: 34.4GB"
* libparted/device.c (ped_device_get): Don't canonicalize names
that start with "/dev/mapper/".
* NEWS (Bug fixes): Mention it.
Thanks to Ales Kozumplik for the analysis.
Details in <http://bugzilla.redhat.com/577824>.
diff --git a/NEWS b/NEWS
index 1c7ad4a..2471b8b 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,11 @@ GNU parted NEWS -*- outline -*-
** Bug fixes
+ When libparted deferenced a /dev/mapper/foo symlink, it would keep the
+ resulting /dev/dm-N name and sometimes use it later, even though it
+ had since become stale and invalid. It no longer stores the result
+ of dereferencing a /dev/mapper symlink.
+
libparted's msdos_partition_is_flag_available function now always reports
that the "hidden" flag is not available for an extended partition.
Similarly, msdos_partition_get_flag(p,PED_PARTITION_HIDDEN) always returns 0
diff --git a/libparted/device.c b/libparted/device.c
index 0f36a03..64da978 100644
--- a/libparted/device.c
+++ b/libparted/device.c
@@ -139,10 +139,12 @@ PedDevice*
ped_device_get (const char* path)
{
PedDevice* walk;
- char* normal_path;
+ char* normal_path = NULL;
PED_ASSERT (path != NULL, return NULL);
- normal_path = canonicalize_file_name (path);
+ /* Don't canonicalize /dev/mapper paths, see tests/symlink.c */
+ if (strncmp (path, "/dev/mapper/", 12))
+ normal_path = canonicalize_file_name (path);
if (!normal_path)
/* Well, maybe it is just that the file does not exist.
* Try it anyway. */
More information about the Parted-commits
mailing list