bug#45258: mkdir man page unclear in describing -m flag

2020-12-16 Thread Bernhard Voelker
On 12/16/20 2:34 PM, Pádraig Brady wrote:
> Insightful comments. I've updated the gotchas note with them:
> https://www.pixelbeat.org/docs/coreutils-gotchas.html#mkdir

Nice, and also better wording than mine, thanks!

Have a nice day,
Berny





bug#45258: mkdir man page unclear in describing -m flag

2020-12-16 Thread Pádraig Brady

On 16/12/2020 08:28, Bernhard Voelker wrote:

On 12/15/20 9:00 PM, Paul Eggert wrote:

Thanks for your bug report. I installed the attached patch; although it
doesn't use the exact wording you proposed, I hope it works well enough.


Thanks for clarifying.


+If the @option{-m} option is also given, it does not affect
+file permission bits of any newly-created parent directories.
+To control these bits, set the
  umask before invoking @command{mkdir}. [...]


Some further thoughts on this - maybe just for my reference:

One aspect of using -p is that the user doesn't want to get an error if
the target or any of its parent directories already exists.

If changing the umask before invoking mkdir is not that easy - maybe
because not called via a shell -, then an alternative to the above
umask method is to reference each of the target directories separately,
e.g.:
   $ mkdir -pm 0700  dir1  dir1/dir2  dir1/dir2/dir3

But it is important to know that 'mkdir' does not adjust the permission
bits of any of those already existing directories.

Therefore, if one does not want to get a failure for already existing
intermediate directories, and still wants their permission bits to get
adjusted, then one can use 'install' instead of 'mkdir' (still passing
each directory level as separate argument!):

   $ install -dm 0700  dir1  dir1/dir2  dir1/dir2/dir3


Insightful comments. I've updated the gotchas note with them:
https://www.pixelbeat.org/docs/coreutils-gotchas.html#mkdir

cheers,
Pádraig





bug#45258: mkdir man page unclear in describing -m flag

2020-12-16 Thread Bernhard Voelker
On 12/15/20 9:00 PM, Paul Eggert wrote:
> Thanks for your bug report. I installed the attached patch; although it
> doesn't use the exact wording you proposed, I hope it works well enough.

Thanks for clarifying.

> +If the @option{-m} option is also given, it does not affect
> +file permission bits of any newly-created parent directories.
> +To control these bits, set the
>  umask before invoking @command{mkdir}. [...]

Some further thoughts on this - maybe just for my reference:

One aspect of using -p is that the user doesn't want to get an error if
the target or any of its parent directories already exists.

If changing the umask before invoking mkdir is not that easy - maybe
because not called via a shell -, then an alternative to the above
umask method is to reference each of the target directories separately,
e.g.:
  $ mkdir -pm 0700  dir1  dir1/dir2  dir1/dir2/dir3

But it is important to know that 'mkdir' does not adjust the permission
bits of any of those already existing directories.

Therefore, if one does not want to get a failure for already existing
intermediate directories, and still wants their permission bits to get
adjusted, then one can use 'install' instead of 'mkdir' (still passing
each directory level as separate argument!):

  $ install -dm 0700  dir1  dir1/dir2  dir1/dir2/dir3

Have a nice day,
Berny





bug#45258: mkdir man page unclear in describing -m flag

2020-12-15 Thread Chris Colohan
Looks great, thank you so much!

Chris

On Tue, Dec 15, 2020 at 12:00 PM Paul Eggert  wrote:

> Thanks for your bug report. I installed the attached patch; although it
> doesn't use the exact wording you proposed, I hope it works well enough.
>


bug#45258: mkdir man page unclear in describing -m flag

2020-12-15 Thread Pádraig Brady

On 15/12/2020 20:00, Paul Eggert wrote:

Thanks for your bug report. I installed the attached patch; although it
doesn't use the exact wording you proposed, I hope it works well enough.


I agree with adding the clarification to the man page,
thanks for doing that.

I also added an entry to:
http://www.pixelbeat.org/docs/coreutils-gotchas.html#mkdir

cheers,
Pádraig





bug#45258: mkdir man page unclear in describing -m flag

2020-12-15 Thread Paul Eggert
Thanks for your bug report. I installed the attached patch; although it 
doesn't use the exact wording you proposed, I hope it works well enough.
>From 3ee0e25426a513c5da891ce6a370abed156a3b83 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 15 Dec 2020 11:52:19 -0800
Subject: [PATCH] doc: document mkdir -m -p better
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Chris Colohan wrote that the man page did not do enough to dispel
a common misunderstanding that “contributed to one of the scariest
outages Google has ever seen” (Bug#45258).
* doc/coreutils.texi (mkdir invocation):
* src/mkdir.c (usage): Document -m vs -p better.
---
 doc/coreutils.texi | 13 +
 src/mkdir.c|  3 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index df0655c20..44ce7d2e0 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10693,6 +10693,8 @@ Set the file permission bits of created directories to @var{mode},
 which uses the same syntax as
 in @command{chmod} and uses @samp{a=rwx} (read, write and execute allowed for
 everyone) for the point of the departure.  @xref{File permissions}.
+This option affects only directories given on the command line;
+it does not affect any parents that may be created via the @option{-p} option.
 
 Normally the directory has the desired file mode bits at the moment it
 is created.  As a GNU extension, @var{mode} may also mention
@@ -10708,15 +10710,18 @@ overridden in this way.
 @opindex --parents
 @cindex parent directories, creating
 Make any missing parent directories for each argument, setting their
-file permission bits to the umask modified by @samp{u+wx}.  Ignore
+file permission bits to @samp{=rwx,u+wx},
+that is, with the umask modified by @samp{u+wx}.  Ignore
 existing parent directories, and do not change their file permission
 bits.
 
-To set the file permission bits of any newly-created parent
-directories to a value that includes @samp{u+wx}, you can set the
+If the @option{-m} option is also given, it does not affect
+file permission bits of any newly-created parent directories.
+To control these bits, set the
 umask before invoking @command{mkdir}.  For example, if the shell
 command @samp{(umask u=rwx,go=rx; mkdir -p P/Q)} creates the parent
-@file{P} it sets the parent's permission bits to @samp{u=rwx,go=rx}.
+@file{P} it sets the parent's file permission bits to @samp{u=rwx,go=rx}.
+(The umask must include @samp{u=wx} for this method to work.)
 To set a parent's special mode bits as well, you can invoke
 @command{chmod} after @command{mkdir}.  @xref{Directory Setuid and
 Setgid}, for how the set-user-ID and set-group-ID bits of
diff --git a/src/mkdir.c b/src/mkdir.c
index 8f07d666e..1f4588f10 100644
--- a/src/mkdir.c
+++ b/src/mkdir.c
@@ -65,7 +65,8 @@ Create the DIRECTORY(ies), if they do not already exist.\n\
 
   fputs (_("\
   -m, --mode=MODE   set file mode (as in chmod), not a=rwx - umask\n\
-  -p, --parents no error if existing, make parent directories as needed\n\
+  -p, --parents no error if existing, make parent directories as needed,\n\
+with their file modes unaffected by any -m option.\n\
   -v, --verbose print a message for each created directory\n\
 "), stdout);
   fputs (_("\
-- 
2.27.0



bug#45258: mkdir man page unclear in describing -m flag

2020-12-15 Thread Chris Colohan
I like to present a challenge to my software engineer friends:  can you
tell me what this command does on Linux, if run in an empty directory?

mkdir -m 0755 -p ./usr/bin/foo

If they read the mkdir man page (
https://man7.org/linux/man-pages/man1/mkdir.1.html), they almost always say
the answer is:

- create the directory ./usr, with the mode 0755
- create the directory ./usr/bin, with the mode 0755
- create the directory ./usr/bin/foo, with the mode 0755

They are wrong.  (Side note -- this misunderstanding contributed to one of
the scariest outages Google has ever seen,
https://www.pdl.cmu.edu/SDI/2012/083012b.html).

What it actually does:

- create the directory ./usr, with the mode based on the umask
- create the directory ./usr/bin, with the mode based on the umask
- create the directory ./usr/bin/foo, with the mode 0755

I tried at the time to get the man page corrected, but I was told at the
time that nobody reads man pages, and the info page is correct, so it won't
be fixed.

I figured after almost 10 years, perhaps thinking has evolved.  Can we fix
the man page?

I have a suggested fix:  the current man page reads:

   -p, --parents
  no error if existing, make parent directories as needed

I can be updated to read:

   -p, --parents
  no error if existing, make parent directories as needed,
setting
  their file permission bits to the umask modified by ‘u+wx’.

I copied the new text from the info page.

Thanks!

Chris