fixed obscure mkdir, mkfifo, mknod permissions problem

[Paul Eggert]

In a few obscure instances mkdir, mkfifo, and mknod create files with
incorrect permissions.  For example:

   $ umask 027
   $ mkdir -m =+x a
   $ ls -ld a
   d--x--x--x  2 eggert eggert 4096 Apr 22 17:03 a

The permissions are wrong here.  They should be d--x--x---.
I verified that Solaris 8 conforms to POSIX, so it's pretty clear that
this is a bug.

I installed the following patch.

I see no need for the second argument for mode_compile any more; all
programs that I know of assume that it's MODE_MASK_ALL, with the
exception of "install" where the value of that argument is immaterial
since "install" runs with umask(0).  So I'm inclined to remove the
second argument, since I think its presence contributed to this
permissions bug.  I'll ask about this on bug-gnulib first, though.

2005-04-22  Paul Eggert  <address@hidden>

        * NEWS: Fix bug with "mkdir -m =+x dir"; the umask was being ignored
        when the "+x" was being evaluated.
        * mkdir.c (main): Compile mode with MODE_MASK_ALL and initial umask.
        * mkfifo.c (main): Likewise.
        * mknod.c (main): Likewise.
        * tests/mkdir/perm: Test for the above bug.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.276
diff -p -u -r1.276 NEWS
--- NEWS        9 Apr 2005 04:51:11 -0000       1.276
+++ NEWS        22 Apr 2005 23:50:46 -0000
@@ -44,6 +44,9 @@ GNU coreutils NEWS                      
   ls now refuses to generate time stamps containing more than 1000 bytes, to
   foil potential denial-of-service attacks on hosts with very large stacks.
 
+  "mkdir -m =+x dir" no longer ignores the umask when evaluating "+x",
+  and similarly for mkfifo and mknod.
+
   "pr -D FORMAT" now accepts the same formats that "date +FORMAT" does.
 
   test now detects integer overflow when evaluating large integers,
Index: src/mkdir.c
===================================================================
RCS file: /fetish/cu/src/mkdir.c,v
retrieving revision 1.91
diff -p -u -r1.91 mkdir.c
--- src/mkdir.c 21 Sep 2004 22:26:42 -0000      1.91
+++ src/mkdir.c 22 Apr 2005 23:50:53 -0000
@@ -82,7 +82,7 @@ int
 main (int argc, char **argv)
 {
   mode_t newmode;
-  mode_t parent_mode;
+  mode_t parent_mode IF_LINT (= 0);
   const char *specified_mode = NULL;
   const char *verbose_fmt_string = NULL;
   int exit_status = EXIT_SUCCESS;
@@ -125,16 +125,9 @@ main (int argc, char **argv)
     }
 
   newmode = S_IRWXUGO;
-  {
-    mode_t umask_value = umask (0);
-    umask (umask_value);               /* Restore the old value. */
-    parent_mode = (newmode & (~ umask_value)) | S_IWUSR | S_IXUSR;
-  }
-
   if (specified_mode)
     {
-      struct mode_change *change = mode_compile (specified_mode, 0);
-      newmode &= ~ umask (0);
+      struct mode_change *change = mode_compile (specified_mode, 
MODE_MASK_ALL);
       if (change == MODE_INVALID)
        error (EXIT_FAILURE, 0, _("invalid mode %s"), quote (specified_mode));
       else if (change == MODE_MEMORY_EXHAUSTED)
@@ -142,6 +135,14 @@ main (int argc, char **argv)
       newmode = mode_adjust (newmode, change);
     }
 
+  if (specified_mode || create_parents)
+    {
+      mode_t umask_value = umask (0);
+      if (! specified_mode)
+       umask (umask_value);
+      parent_mode = (S_IRWXUGO & ~umask_value) | (S_IWUSR | S_IXUSR);
+    }
+
   for (; optind < argc; ++optind)
     {
       bool ok;
Index: src/mkfifo.c
===================================================================
RCS file: /fetish/cu/src/mkfifo.c,v
retrieving revision 1.73
diff -p -u -r1.73 mkfifo.c
--- src/mkfifo.c        21 Sep 2004 22:26:42 -0000      1.73
+++ src/mkfifo.c        22 Apr 2005 23:50:53 -0000
@@ -116,13 +116,13 @@ main (int argc, char **argv)
   newmode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
   if (specified_mode)
     {
-      newmode &= ~ umask (0);
-      change = mode_compile (specified_mode, 0);
+      change = mode_compile (specified_mode, MODE_MASK_ALL);
       if (change == MODE_INVALID)
        error (EXIT_FAILURE, 0, _("invalid mode"));
       else if (change == MODE_MEMORY_EXHAUSTED)
        xalloc_die ();
       newmode = mode_adjust (newmode, change);
+      umask (0);
     }
 
   for (; optind < argc; ++optind)
Index: src/mknod.c
===================================================================
RCS file: /fetish/cu/src/mknod.c,v
retrieving revision 1.84
diff -p -u -r1.84 mknod.c
--- src/mknod.c 21 Sep 2004 22:26:42 -0000      1.84
+++ src/mknod.c 22 Apr 2005 23:50:53 -0000
@@ -121,13 +121,13 @@ main (int argc, char **argv)
   newmode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
   if (specified_mode)
     {
-      newmode &= ~ umask (0);
-      change = mode_compile (specified_mode, 0);
+      change = mode_compile (specified_mode, MODE_MASK_ALL);
       if (change == MODE_INVALID)
        error (EXIT_FAILURE, 0, _("invalid mode"));
       else if (change == MODE_MEMORY_EXHAUSTED)
        xalloc_die ();
       newmode = mode_adjust (newmode, change);
+      umask (0);
     }
 
   /* If the number of arguments is 0 or 1,
Index: tests/mkdir/perm
===================================================================
RCS file: /fetish/cu/tests/mkdir/perm,v
retrieving revision 1.15
diff -p -u -r1.15 perm
--- tests/mkdir/perm    23 Jun 2004 15:07:04 -0000      1.15
+++ tests/mkdir/perm    22 Apr 2005 23:50:55 -0000
@@ -41,6 +41,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 :
+    027  :   -m =+x   : drwxr-x--- : d--x--x--- :
     -    :   -        : last       : last       :
     '
 for p in empty -p; do