Subject: bug#45886: closed (Re: bug#45886: mkdir -m argument does not work correctly, applies incorrect permissions)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 15 Jan 2021 11:04:02 +0000

Your bug report

#45886: mkdir -m argument does not work correctly, applies incorrect permissions

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
45886: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=45886
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#45886: mkdir -m argument does not work correctly, applies incorrect permissions
From: Paul Eggert <[email-masked]>
To: Davin McCall <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Fri, 15 Jan 2021 03:03:03 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for the bug report. I reproduced the problem and installed the 
attached patch to fix it.
>From b8375c422ffe0e018cbb4cad187d1e909195d263 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Fri, 15 Jan 2021 02:57:59 -0800
Subject: [PATCH] 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.
---
 NEWS                |  3 +++
 src/mkdir.c         | 30 ++++++++++++++++++------------
 tests/mkdir/perm.sh |  1 +
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index c2474fee3..a6ba96450 100644
--- 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 eccc9d382..b266cee8c 100644
--- 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 4d36f19b5..083a47733 100755
--- 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       :
-- 
2.27.0

Subject: mkdir -m argument does not work correctly, applies incorrect permissions
From: Davin McCall <[email-masked]>
To: [email-masked]
Date: Fri, 15 Jan 2021 17:45:29 +1000
List-Id: <debbugs-submit.debbugs.gnu.org>

Coreutils 8.32 (and possibly earlier) "mkdir -m <mode> ..." appears to
ignore parts of the mode specification. POSIX suggests that with -m
specified, the mode is "relative" to a=rwx. That implies a mode of "o-w"
should yield a mode of "rwxrwxr-x", and a mode of "o+w" should yield
"rwxrwxrwx". Instead:

$ mkdir -m o-w t1
$ mkdir -m o+w t2
$ ls -l
total 8
drwxr-xr-x 2 davmac users 4096 Jan 15 17:27 t1
drwxrwxrwx 2 davmac users 4096 Jan 15 17:27 t2
$

So, the "o+w" or "o-w" selects the "other" permissions but also the
"group" permissions. This also happens with "g+w" and "g-w" (i.e. both
"group" and "other" permissions get set or cleared). This implies it's
not just a case of the initial mode not matching POSIX, but somehow "o"
and "g" are being applied incorrectly.

Setting POSIXLY_CORRECT makes no difference.