mkdir: fix bug when -m's more generous than umask

Problem reported by David McCall (Bug#45886).
I introduced this problem when fixing Bug#14371.
* NEWS: Mention the fix.
* src/mkdir.c (struct mkdir_options): New members umask_ancestor,
umask_self, replacing umask_value.
(make_ancestor): Use them when temporarily adjusting umask.
(main): Set them, and set the umask to umask_self instead
of leaving it alone.
* tests/mkdir/perm.sh (tests): Add test case for bug.

diff --git a/NEWS b/NEWS
index c2474fee35a2a95f4c64d75403ced6d59daf42b1..a6ba96450d778b8b68b9969df8e499cc64b434a4 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   ls no longer crashes when printing the SELinux context for unstatable files.
   [bug introduced in coreutils-6.9.91]
 
+  mkdir -m no longer mishandles modes more generous than the umask.
+  [bug introduced in coreutils-8.22]
+
   nl now handles single character --section-delimiter arguments,
   by assuming a second ':' character has been specified, as specified by POSIX.
   [This bug was present in "the beginning".]

diff --git a/src/mkdir.c b/src/mkdir.c
index eccc9d38294764e40e57ecbde3754a234d488b4e..b266cee8cbcc29128135331baa7a493bf8125f7a 100644 (file)
--- a/src/mkdir.c
+++ b/src/mkdir.c
@@ -89,8 +89,11 @@ struct mkdir_options
      made.  */
   int (*make_ancestor_function) (char const *, char const *, void *);
 
-  /* Umask value in effect.  */
-  mode_t umask_value;
+  /* Umask value for when making an ancestor.  */
+  mode_t umask_ancestor;
+
+  /* Umask value for when making the directory itself.  */
+  mode_t umask_self;
 
   /* Mode for directory itself.  */
   mode_t mode;
@@ -130,20 +133,18 @@ make_ancestor (char const *dir, char const *component, void *options)
     error (0, errno, _("failed to set default creation context for %s"),
            quoteaf (dir));
 
-  mode_t user_wx = S_IWUSR | S_IXUSR;
-  bool self_denying_umask = (o->umask_value & user_wx) != 0;
-  if (self_denying_umask)
-    umask (o->umask_value & ~user_wx);
+  if (o->umask_ancestor != o->umask_self)
+    umask (o->umask_ancestor);
   int r = mkdir (component, S_IRWXUGO);
-  if (self_denying_umask)
+  if (o->umask_ancestor != o->umask_self)
     {
       int mkdir_errno = errno;
-      umask (o->umask_value);
+      umask (o->umask_self);
       errno = mkdir_errno;
     }
   if (r == 0)
     {
-      r = (o->umask_value & S_IRUSR) != 0;
+      r = (o->umask_ancestor & S_IRUSR) != 0;
       announce_mkdir (dir, options);
     }
   return r;
@@ -282,8 +283,7 @@ main (int argc, char **argv)
   if (options.make_ancestor_function || specified_mode)
     {
       mode_t umask_value = umask (0);
-      umask (umask_value);
-      options.umask_value = umask_value;
+      options.umask_ancestor = umask_value & ~(S_IWUSR | S_IXUSR);
 
       if (specified_mode)
         {
@@ -293,10 +293,16 @@ main (int argc, char **argv)
                  quote (specified_mode));
           options.mode = mode_adjust (S_IRWXUGO, true, umask_value, change,
                                       &options.mode_bits);
+          options.umask_self = umask_value & ~options.mode;
           free (change);
         }
       else
-        options.mode = S_IRWXUGO;
+        {
+          options.mode = S_IRWXUGO;
+          options.umask_self = umask_value;
+        }
+
+      umask (options.umask_self);
     }
 
   return savewd_process_files (argc - optind, argv + optind,

diff --git a/tests/mkdir/perm.sh b/tests/mkdir/perm.sh
index 4d36f19b5ea1da9908ad81f28e2542c5b7732ae6..083a47733dae98c58fc7719d982c0226e9e51a48 100755 (executable)
--- a/tests/mkdir/perm.sh
+++ b/tests/mkdir/perm.sh
@@ -35,6 +35,7 @@ tests='
     050  :   -m 312   : drwx-w-rwx : d-wx--x-w- :
     160  :   empty    : drwx--xrwx : drw---xrwx :
     160  :   -m 743   : drwx--xrwx : drwxr---wx :
+    022  :   -m o-w   : drwxr-xr-x : drwxrwxr-x :
     027  :   -m =+x   : drwxr-x--- : d--x--x--- :
     027  :   -m =+X   : drwxr-x--- : d--x--x--- :
     -    :   -        : last       : last       :