diff options
| -rw-r--r-- | ChangeLog | 24 | ||||
| -rw-r--r-- | NEWS | 5 | ||||
| -rw-r--r-- | bootstrap.conf | 2 | ||||
| -rw-r--r-- | src/install.c | 67 | ||||
| -rw-r--r-- | src/mkdir.c | 61 | ||||
| -rwxr-xr-x | tests/install/basic-1 | 25 | ||||
| -rwxr-xr-x | tests/mkdir/p-3 | 13 |
7 files changed, 143 insertions, 54 deletions
@@ -1,7 +1,23 @@ -2006-09-16 Jim Meyering <jim@meyering.net> - - * Makefile.maint (sc_require_config_h, sc_prohibit_assert_without_use): - Discard stdout from the new use of grep. +2006-09-16 Paul Eggert <eggert@cs.ucla.edu> + + * NEWS: Document that mkdir -p and install -d now fork on occasion. + * bootstrap.conf (gnulib_modules): Add savewd. + * src/install.c: Include savewd.h. + (process_dir): New function. + (main, install_file_in_file_parents): Use it, along with the new + savewd module, to avoid some race conditions. + * src/mkdir.c: Include savewd.h. + (struct mkdir_options): New members make_ancestor_function, mode, + mode_bits. + (make_ancestor): Return 1 if the resulting directory is not readable. + (process_dir): New function. + (main): Use it, along with new savewd module, to avoid some + race conditions. Fill in new slots of struct mkdir_options, so + that callees get the values. + * tests/install/basic-1: Test for coreutils 5.97 bug that was + fixed in coreutils 6.0, and which should still be fixed with + this change. + * tests/mkdir/p-3: Likewise. 2006-09-15 Jim Meyering <jim@meyering.net> @@ -4,6 +4,11 @@ GNU coreutils NEWS -*- outline -*- ** Changes in behavior + mkdir -p and install -d (or -D) now use a method that forks a child + process if the working directory is unreadable and a later argument + uses a relative file name. This avoids some race conditions, but it + means you may need to kill two processes to stop these programs. + rm now rejects attempts to remove the root directory, e.g., `rm -fr /' now fails without removing anything. Likewise for any file name with a final `./' or `../' component. diff --git a/bootstrap.conf b/bootstrap.conf index eb10b4696..390d09365 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -55,7 +55,7 @@ gnulib_modules=" quote quotearg raise readlink readtokens readtokens0 readutmp realloc regex rename-dest-slash rmdir rmdir-errno rpmatch safe-read same - save-cwd savedir settime sha1 sig2str ssize_t stat-macros + save-cwd savedir savewd settime sha1 sig2str ssize_t stat-macros stat-time stdbool stdlib-safer stpcpy strcase strftime strpbrk strtoimax strtoumax strverscmp timespec tzset unicodeio unistd-safer unlink-busy unlinkdir unlocked-io diff --git a/src/install.c b/src/install.c index 78fa28e0b..9c5150fbc 100644 --- a/src/install.c +++ b/src/install.c @@ -35,6 +35,7 @@ #include "mkdir-p.h" #include "modechange.h" #include "quote.h" +#include "savewd.h" #include "stat-time.h" #include "utimens.h" #include "xstrtol.h" @@ -193,11 +194,23 @@ target_directory_operand (char const *file) return is_a_dir; } +/* Process a command-line file name, for the -d option. */ +static int +process_dir (char *dir, struct savewd *wd, void *options) +{ + return (make_dir_parents (dir, wd, + make_ancestor, options, + dir_mode, announce_mkdir, + dir_mode_bits, owner_id, group_id, false) + ? EXIT_SUCCESS + : EXIT_FAILURE); +} + int main (int argc, char **argv) { int optc; - bool ok = true; + int exit_status = EXIT_SUCCESS; const char *specified_mode = NULL; bool make_backups = false; char *backup_suffix_string; @@ -361,13 +374,7 @@ main (int argc, char **argv) get_ids (); if (dir_arg) - { - int i; - for (i = 0; i < n_files; i++) - ok &= make_dir_parents (file[i], make_ancestor, &x, - dir_mode, announce_mkdir, - dir_mode_bits, owner_id, group_id, false); - } + exit_status = savewd_process_files (n_files, file, process_dir, &x); else { /* FIXME: it's a little gross that this initialization is @@ -376,23 +383,22 @@ main (int argc, char **argv) if (!target_directory) { - if (mkdir_and_install) - ok = install_file_in_file_parents (file[0], file[1], &x); - else - ok = install_file_in_file (file[0], file[1], &x); + if (! (mkdir_and_install + ? install_file_in_file_parents (file[0], file[1], &x) + : install_file_in_file (file[0], file[1], &x))) + exit_status = EXIT_FAILURE; } else { int i; dest_info_init (&x); for (i = 0; i < n_files; i++) - { - ok &= install_file_in_dir (file[i], target_directory, &x); - } + if (! install_file_in_dir (file[i], target_directory, &x)) + exit_status = EXIT_FAILURE; } } - exit (ok ? EXIT_SUCCESS : EXIT_FAILURE); + exit (exit_status); } /* Copy file FROM onto file TO, creating any missing parent directories of TO. @@ -402,13 +408,36 @@ static bool install_file_in_file_parents (char const *from, char *to, struct cp_options *x) { - if (mkancesdirs (to, make_ancestor, x) != 0) + bool save_working_directory = + ! (IS_ABSOLUTE_FILE_NAME (from) && IS_ABSOLUTE_FILE_NAME (to)); + int status = EXIT_SUCCESS; + + struct savewd wd; + savewd_init (&wd); + if (! save_working_directory) + savewd_finish (&wd); + + if (mkancesdirs (to, &wd, make_ancestor, x) == -1) { error (0, errno, _("cannot create directory %s"), to); - return false; + status = EXIT_FAILURE; + } + + if (save_working_directory) + { + int restore_result = savewd_restore (&wd, status); + int restore_errno = errno; + savewd_finish (&wd); + if (EXIT_SUCCESS < restore_result) + return false; + if (restore_result < 0 && status == EXIT_SUCCESS) + { + error (0, restore_errno, _("cannot create directory %s"), to); + return false; + } } - return install_file_in_file (from, to, x); + return (status == EXIT_SUCCESS && install_file_in_file (from, to, x)); } /* Copy file FROM onto file TO and give TO the appropriate diff --git a/src/mkdir.c b/src/mkdir.c index 852bc3e15..b28a02ac0 100644 --- a/src/mkdir.c +++ b/src/mkdir.c @@ -28,6 +28,7 @@ #include "mkdir-p.h" #include "modechange.h" #include "quote.h" +#include "savewd.h" /* The official name of this program (e.g., no `g' prefix). */ #define PROGRAM_NAME "mkdir" @@ -75,12 +76,22 @@ Mandatory arguments to long options are mandatory for short options too.\n\ exit (status); } -/* Options for announce_mkdir and make_ancestor. */ +/* Options passed to subsidiary functions. */ struct mkdir_options { + /* Function to make an ancestor, or NULL if ancestors should not be + made. */ + int (*make_ancestor_function) (char const *, void *); + /* Mode for ancestor directory. */ mode_t ancestor_mode; + /* Mode for directory itself. */ + mode_t mode; + + /* File mode bits affected by MODE. */ + mode_t mode_bits; + /* If not null, format to use when reporting newly made directories. */ char const *created_directory_format; }; @@ -94,27 +105,44 @@ announce_mkdir (char const *dir, void *options) error (0, 0, o->created_directory_format, quote (dir)); } -/* Make ancestor directory DIR, with options OPTIONS. */ +/* Make ancestor directory DIR, with options OPTIONS. Return 0 if + successful and the resulting directory is readable, 1 if successful + but the resulting directory is not readable, -1 (setting errno) + otherwise. */ static int make_ancestor (char const *dir, void *options) { struct mkdir_options const *o = options; int r = mkdir (dir, o->ancestor_mode); if (r == 0) - announce_mkdir (dir, options); + { + r = ! (o->ancestor_mode & S_IRUSR); + announce_mkdir (dir, options); + } return r; } +/* Process a command-line file name. */ +static int +process_dir (char *dir, struct savewd *wd, void *options) +{ + struct mkdir_options const *o = options; + return (make_dir_parents (dir, wd, o->make_ancestor_function, options, + o->mode, announce_mkdir, + o->mode_bits, (uid_t) -1, (gid_t) -1, true) + ? EXIT_SUCCESS + : EXIT_FAILURE); +} + int main (int argc, char **argv) { - mode_t mode = S_IRWXUGO; - mode_t mode_bits = 0; - int (*make_ancestor_function) (char const *, void *) = NULL; const char *specified_mode = NULL; - int exit_status = EXIT_SUCCESS; int optc; struct mkdir_options options; + options.make_ancestor_function = NULL; + options.mode = S_IRWXUGO; + options.mode_bits = 0; options.created_directory_format = NULL; initialize_main (&argc, &argv); @@ -130,7 +158,7 @@ main (int argc, char **argv) switch (optc) { case 'p': - make_ancestor_function = make_ancestor; + options.make_ancestor_function = make_ancestor; break; case 'm': specified_mode = optarg; @@ -151,7 +179,7 @@ main (int argc, char **argv) usage (EXIT_FAILURE); } - if (make_ancestor_function || specified_mode) + if (options.make_ancestor_function || specified_mode) { mode_t umask_value = umask (0); @@ -163,19 +191,14 @@ main (int argc, char **argv) if (!change) error (EXIT_FAILURE, 0, _("invalid mode %s"), quote (specified_mode)); - mode = mode_adjust (S_IRWXUGO, true, umask_value, change, - &mode_bits); + options.mode = mode_adjust (S_IRWXUGO, true, umask_value, change, + &options.mode_bits); free (change); } else - mode &= ~umask_value; + options.mode = S_IRWXUGO & ~umask_value; } - for (; optind < argc; ++optind) - if (! make_dir_parents (argv[optind], make_ancestor_function, &options, - mode, announce_mkdir, - mode_bits, (uid_t) -1, (gid_t) -1, true)) - exit_status = EXIT_FAILURE; - - exit (exit_status); + exit (savewd_process_files (argc - optind, argv + optind, + process_dir, &options)); } diff --git a/tests/install/basic-1 b/tests/install/basic-1 index c41ea889b..bfddd6ef8 100755 --- a/tests/install/basic-1 +++ b/tests/install/basic-1 @@ -1,7 +1,7 @@ #! /bin/sh # Basic tests for "install". -# Copyright (C) 1998, 2000, 2001, 2002, 2004, 2005 Free Software +# Copyright (C) 1998, 2000, 2001, 2002, 2004, 2005, 2006 Free Software # Foundation, Inc. # This program is free software; you can redistribute it and/or modify @@ -95,16 +95,27 @@ test -d newdir3 || fail=1 # initial working directory ($abs) after creating the first argument, and # hence cannot do anything meaningful with the following relative-named dirs. abs=$pwd/$tmp -mkdir sub && cd sub -chmod 0 .; ginstall -d $abs/xx/yy rel/sub1 rel/sub2 2> /dev/null && fail=1 -chmod 755 $abs/sub +mkdir sub || fail=1 +(cd sub && chmod 0 . && ginstall -d $abs/xx/yy rel/sub1 rel/sub2 2> /dev/null) && fail=1 +chmod 755 sub # Ensure that the first argument-dir has been created. -test -d $abs/xx/yy || fail=1 +test -d xx/yy || fail=1 # Make sure that the `rel' directory was not created... -test -d $abs/sub/rel && fail=1 +test -d sub/rel && fail=1 # and make sure it was not created in the wrong place. -test -d $abs/xx/rel && fail=1 +test -d xx/rel && fail=1 + +# Test that we can install from an unreadable directory with an +# inaccessible parent. coreutils 5.97 fails this test. +mkdir -p sub1/d || fail=1 +(cd sub1/d && chmod a-rx .. && chmod a-r . && + ginstall -d $abs/xx/zz rel/a rel/b 2> /dev/null) || fail=1 +chmod 755 sub1 || fail=1 +chmod 755 sub1/d || fail=1 +test -d xx/zz || fail=1 +test -d sub1/d/rel/a || fail=1 +test -d sub1/d/rel/b || fail=1 (exit $fail); exit $fail diff --git a/tests/mkdir/p-3 b/tests/mkdir/p-3 index 3fab37827..59bf33781 100755 --- a/tests/mkdir/p-3 +++ b/tests/mkdir/p-3 @@ -37,7 +37,7 @@ mkdir -p $tmp || framework_failure=1 cd $tmp || framework_failure=1 mkdir no-access || framework_failure=1 mkdir no-acce2s || framework_failure=1 -mkdir no-acce3s || framework_failure=1 +mkdir -p no-acce3s/d || framework_failure=1 if test $framework_failure = 1; then echo "$0: failure in testing framework" 1>&2 @@ -45,13 +45,18 @@ if test $framework_failure = 1; then fi p=$pwd/$tmp -(cd no-access; chmod 0 . && mkdir -p $p/a/b u/v) 2> /dev/null && fail=1 +(cd no-access && chmod 0 . && mkdir -p $p/a/b u/v) 2> /dev/null && fail=1 test -d $p/a/b || fail=1 # Same as above, but with a following *absolute* name, it should succeed -(cd no-acce2s; chmod 0 . && mkdir -p $p/b/b $p/z) || fail=1 +(cd no-acce2s && chmod 0 . && mkdir -p $p/b/b $p/z) || fail=1 +test -d $p/b/b && test -d $p/z || fail=1 -test -d $p/z || fail=1 +# Same as above, but a trailing relative name in an unreadable directory +# whose parent is inaccessible. coreutils 5.97 fails this test. +(cd no-acce3s/d && chmod a-rx .. && chmod a-r . && mkdir -p a/b $p/b/c d/e && + test -d a/b && test -d d/e) || fail=1 +test -d $p/b/c || fail=1 b=`ls $p/a|tr -d '\n'` # With coreutils-5.3.0, this would fail with $b=bu. |