bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

minor mkdir bug fixes


From: Jim Meyering
Subject: minor mkdir bug fixes
Date: Tue, 14 Jun 2005 11:14:44 +0200

There was a long-standing, mostly-theoretical bug in mkdir and mkdir-p.c.
It would cause trouble only when `mkdir -p' changed into some directory
but was unable to change back to an initially unsearchable directory.
In that case, the unfixed code would nonetheless continue processing
subsequent command line arguments.  And if one were a relative directory
name, it would try to create it in the wrong place.  Before this patch,
"mkdir -p /tmp/a/b u" would mistakenly create /tmp/a/u under those
circumstances.

As far as I can see, this bug is triggered only when running the command
from a chmod 0'd directory on a Linux system (or any other type of
system where getcwd works even when called from such a directory).
And since the only way to get into such a directory in the first place
is to cd into it before the permissions are restricted and then run e.g.,
`chmod 0 .', this isn't likely to happen to many actual users :)
See the new test script for details.


NEWS:

  "mkdir -p /tmp/a/b dir" no longer attempts to create the `.'-relative
  directory, dir (in /tmp/a), when, after creating /tmp/a/b, it is unable
  to return to its initial working directory.

ChangeLog
-----------------
2005-06-13  Jim Meyering  <address@hidden>

        * src/mkdir.c (main): Give a diagnostic for -- and skip -- each
        relative directory name after make_dir_parents fails to restore
        the working directory.  Before, `mkdir -p' could create directories
        in the wrong place in unusual circumstances.
        * src/install.c (main): Likewise.
        (install_file_in_file_parents): Update make_dir_parents caller.
        * tests/mkdir/p-3: New test for today's mkdir.c/mkdir-p.c bug fixes.
        * tests/mkdir/Makefile.am (TESTS): Add p-3.

lib/ChangeLog
-----------------
2005-06-14  Jim Meyering  <address@hidden>

        * mkdir-p.c (CLEANUP_CWD): Return *true*, not false when failing
        to restore initial working directory.

2005-06-13  Jim Meyering  <address@hidden>

        * mkdir-p.c (make_dir_parents): New parameter: different_working_dir,
        to tell caller if/when we change the working directory and are
        unable to return to the initial one.
        * mkdir-p.h (make_dir_parents): Update prototype.

2005-06-12  Jim Meyering  <address@hidden>

        * mkdir-p.c (CLEANUP_CWD): Change one more `return 1' to
        `return false'.  This fixes a bug introduced on 2004-07-30.





Index: src/mkdir.c
===================================================================
RCS file: /fetish/cu/src/mkdir.c,v
retrieving revision 1.97
retrieving revision 1.98
diff -u -p -u -r1.97 -r1.98
--- src/mkdir.c 2 Jun 2005 05:17:24 -0000       1.97
+++ src/mkdir.c 13 Jun 2005 10:17:36 -0000      1.98
@@ -87,6 +87,7 @@ main (int argc, char **argv)
   const char *verbose_fmt_string = NULL;
   int exit_status = EXIT_SUCCESS;
   int optc;
+  bool cwd_not_restored;
 
   initialize_main (&argc, &argv);
   program_name = argv[0];
@@ -145,15 +146,29 @@ main (int argc, char **argv)
        umask (umask_value);
     }
 
+  /* FIXME: when we assume C99, declare this here.  */
+  cwd_not_restored = false;
   for (; optind < argc; ++optind)
     {
       bool ok;
 
+      if (cwd_not_restored && IS_RELATIVE_FILE_NAME (argv[optind]))
+       {
+         error (0, 0, _("unable to create relative-named directory, %s,"
+                        " due to prior failure to restore working directory"),
+                quote (argv[optind]));
+         exit_status = EXIT_FAILURE;
+         continue;
+       }
+
       if (create_parents)
        {
+         bool different_cwd;
          char *dir = argv[optind];
          ok = make_dir_parents (dir, newmode, parent_mode,
-                                -1, -1, true, verbose_fmt_string);
+                                -1, -1, true, verbose_fmt_string,
+                                &different_cwd);
+         cwd_not_restored |= different_cwd;
        }
       else
        {
Index: src/install.c
===================================================================
RCS file: /fetish/cu/src/install.c,v
retrieving revision 1.179
retrieving revision 1.180
diff -u -p -u -r1.179 -r1.180
--- src/install.c       2 Jun 2005 05:13:29 -0000       1.179
+++ src/install.c       13 Jun 2005 10:19:18 -0000      1.180
@@ -357,11 +357,25 @@ main (int argc, char **argv)
   if (dir_arg)
     {
       int i;
+      bool cwd_not_restored = false;
       for (i = 0; i < n_files; i++)
        {
+         bool different_cwd;
+         if (cwd_not_restored && IS_RELATIVE_FILE_NAME (argv[optind]))
+           {
+             error (0, 0,
+                    _("unable to create relative-named directory, %s,"
+                      " due to prior failure to restore working directory"),
+                    quote (argv[optind]));
+             ok = false;;
+             continue;
+           }
+
          ok &=
            make_dir_parents (file[i], mode, mode, owner_id, group_id, false,
-                             (x.verbose ? _("creating directory %s") : NULL));
+                             (x.verbose ? _("creating directory %s") : NULL),
+                             &different_cwd);
+         cwd_not_restored |= different_cwd;
        }
     }
   else
@@ -409,9 +423,12 @@ install_file_in_file_parents (char const
         that this option is intended mainly to help installers when the
         distribution doesn't provide proper install rules.  */
       mode_t dir_mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
+      bool different_cwd;
       ok = make_dir_parents (dest_dir, dir_mode, dir_mode,
                             owner_id, group_id, true,
-                            (x->verbose ? _("creating directory %s") : NULL));
+                            (x->verbose ? _("creating directory %s") : NULL),
+                            &different_cwd);
+      /* Ignore different_cwd, since this function is called at most once.  */
     }
 
   free (dest_dir);

   
Index: lib/mkdir-p.c
===================================================================
RCS file: /fetish/cu/lib/mkdir-p.c,v
retrieving revision 1.2
retrieving revision 1.5
diff -u -p -r1.2 -r1.5
--- lib/mkdir-p.c       12 Jun 2005 19:56:37 -0000      1.2
+++ lib/mkdir-p.c       14 Jun 2005 08:55:37 -0000      1.5
@@ -63,7 +63,8 @@
                _("failed to return to initial working directory")); \
              free_cwd (&cwd);                          \
              errno = _saved_errno;                     \
-             return false;                             \
+             *different_working_dir = true;            \
+             return true;                              \
            }                                           \
          free_cwd (&cwd);                              \
        }                                               \
@@ -144,9 +145,13 @@ make_dir (char const *dir, char const *f
    with the name of the directory that was just made as an argument.
    If PRESERVE_EXISTING is true and ARG is an existing directory,
    then do not attempt to set its permissions and ownership.
-
-   Return true iff ARG exists as a directory with the proper
-   ownership and permissions when done.  */
+   Upon return, set *DIFFERENT_WORKING_DIR to true if this function
+   has changed the current working directory and is unable to restore
+   it to its initial state.
+
+   Return true iff ARG exists as a directory with the proper ownership
+   and permissions when done.  Note that this function returns true
+   even when it fails to return to the initial working directory.  */
 
 bool
 make_dir_parents (char const *arg,
@@ -155,10 +160,12 @@ make_dir_parents (char const *arg,
                  uid_t owner,
                  gid_t group,
                  bool preserve_existing,
-                 char const *verbose_fmt_string)
+                 char const *verbose_fmt_string,
+                 bool *different_working_dir)
 {
   struct stat stats;
   bool retval = true;
+  *different_working_dir = false;
 
   if (stat (arg, &stats) != 0)
     {
Index: lib/mkdir-p.c
===================================================================
RCS file: /fetish/cu/lib/mkdir-p.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -p -u -r1.1 -r1.2
--- lib/mkdir-p.c       2 Jun 2005 04:55:35 -0000       1.1
+++ lib/mkdir-p.c       12 Jun 2005 19:56:37 -0000      1.2
@@ -63,7 +63,7 @@
                _("failed to return to initial working directory")); \
              free_cwd (&cwd);                          \
              errno = _saved_errno;                     \
-             return 1;                                 \
+             return false;                             \
            }                                           \
          free_cwd (&cwd);                              \
        }                                               \




reply via email to

[Prev in Thread] Current Thread [Next in Thread]