bug#28875: 25.3.50; set-default-file-modes ignores execution bits

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Tino Calancha]

emacs -Q:
(let ((foo (make-temp-file "foo")) mode)
  (with-file-modes #o755 (write-region "" nil foo nil 0))
  ;; (set-file-modes foo #o755)
  (setq mode (nth 8 (file-attributes foo)))
  (delete-file foo) mode)
=> "-rw-------"

To set foo file permissions to "-rwxr-xr-x" I need uncomment
(set-file-modes foo #o755) above.
Why? Is for security reasons?


In GNU Emacs 25.3.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-09-20 built on calancha-pc
Repository revision: c3ff6712ad24fcf45874dc0665a8606e9b2208a4

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Andreas Schwab]

On Okt 17 2017, Tino Calancha <tino.calancha@gmail.com> wrote:

> emacs -Q:
> (let ((foo (make-temp-file "foo")) mode)
>   (with-file-modes #o755 (write-region "" nil foo nil 0))
>   ;; (set-file-modes foo #o755)
>   (setq mode (nth 8 (file-attributes foo)))
>   (delete-file foo) mode)
> => "-rw-------"
>
> To set foo file permissions to "-rwxr-xr-x" I need uncomment
> (set-file-modes foo #o755) above.
> Why? Is for security reasons?

make-temp-file already creates the file (with restrictive modes), so
with-file-modes has no effect (write-region does not change the mode of
existing files).  But write-region also never sets the x bits in the
first place, it uses #o666 as the base mode.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Tino Calancha]

On Tue, 17 Oct 2017, Andreas Schwab wrote:

> On Okt 17 2017, Tino Calancha <tino.calancha@gmail.com> wrote:
>
>> emacs -Q:
>> (let ((foo (make-temp-file "foo")) mode)
>>   (with-file-modes #o755 (write-region "" nil foo nil 0))
>>   ;; (set-file-modes foo #o755)
>>   (setq mode (nth 8 (file-attributes foo)))
>>   (delete-file foo) mode)
>> => "-rw-------"
>>
>> To set foo file permissions to "-rwxr-xr-x" I need uncomment
>> (set-file-modes foo #o755) above.
>> Why? Is for security reasons?
>
> make-temp-file already creates the file (with restrictive modes), so
> with-file-modes has no effect (write-region does not change the mode of
> existing files).  But write-region also never sets the x bits in the
> first place, it uses #o666 as the base mode.
>
Thank you for th enice explanation.  I understand now.
I think I was fooled by the docstring of `set-default-file-modes':
"Set the file permission bits for newly created files..."

I thought that setting will be used by the functions in my snippet.

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Tino Calancha]

Tino Calancha <tino.calancha@gmail.com> writes:

> On Tue, 17 Oct 2017, Andreas Schwab wrote:
>
>> On Okt 17 2017, Tino Calancha <tino.calancha@gmail.com> wrote:
>>
>>> emacs -Q:
>>> (let ((foo (make-temp-file "foo")) mode)
>>>   (with-file-modes #o755 (write-region "" nil foo nil 0))
>>>   ;; (set-file-modes foo #o755)
>>>   (setq mode (nth 8 (file-attributes foo)))
>>>   (delete-file foo) mode)
>>> => "-rw-------"
>>>
>>> To set foo file permissions to "-rwxr-xr-x" I need uncomment
>>> (set-file-modes foo #o755) above.
>>> Why? Is for security reasons?
>>
>> make-temp-file already creates the file (with restrictive modes), so
>> with-file-modes has no effect (write-region does not change the mode of
>> existing files).  But write-region also never sets the x bits in the
>> first place, it uses #o666 as the base mode.
>>
> I think I was fooled by the docstring of `set-default-file-modes':
> "Set the file permission bits for newly created files..."
Besides the behavior is mentioned in the manual, that is relevant
for the correct use of the function, so IMO it's worth to include it
the docstring as well.

--8<-----------------------------cut here---------------start------------->8---
commit d749ad1ca0375a938c0283155b56e737cc120640
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Fri Oct 20 11:58:17 2017 +0900

    * src/fileio.c (set-default-file-modes): Doc fix

diff --git a/src/fileio.c b/src/fileio.c
index b7df38c857..d707bfc1c6 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3125,7 +3125,11 @@ symbolic notation, like the `chmod' command from GNU Coreutils.  */)
 DEFUN ("set-default-file-modes", Fset_default_file_modes, Sset_default_file_modes, 1, 1, 0,
        doc: /* Set the file permission bits for newly created files.
 The argument MODE should be an integer; only the low 9 bits are used.
-This setting is inherited by subprocesses.  */)
+This setting is inherited by subprocesses.
+
+Note that some functions (e.g., `write-region') ignore the execution
+bits in MODE.  In that case you need first to create the file,
+and then set the permisions with `set-file-modes'.  */)
   (Lisp_Object mode)
 {
   mode_t oldrealmask, oldumask, newumask;

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.90 (build 6, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-10-20
Repository revision: ddd547fada112c603dae7a204fa0b141429f1927

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Andreas Schwab <schwab@suse.de>
> Date: Fri, 20 Oct 2017 12:10:18 +0900
> 
> >> make-temp-file already creates the file (with restrictive modes), so
> >> with-file-modes has no effect (write-region does not change the mode of
> >> existing files).  But write-region also never sets the x bits in the
> >> first place, it uses #o666 as the base mode.
> >>
> > I think I was fooled by the docstring of `set-default-file-modes':
> > "Set the file permission bits for newly created files..."
> Besides the behavior is mentioned in the manual, that is relevant
> for the correct use of the function, so IMO it's worth to include it
> the docstring as well.
> 
> --8<-----------------------------cut here---------------start------------->8---
> commit d749ad1ca0375a938c0283155b56e737cc120640
> Author: Tino Calancha <tino.calancha@gmail.com>
> Date:   Fri Oct 20 11:58:17 2017 +0900
> 
>     * src/fileio.c (set-default-file-modes): Doc fix
> 
> diff --git a/src/fileio.c b/src/fileio.c
> index b7df38c857..d707bfc1c6 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -3125,7 +3125,11 @@ symbolic notation, like the `chmod' command from GNU Coreutils.  */)
>  DEFUN ("set-default-file-modes", Fset_default_file_modes, Sset_default_file_modes, 1, 1, 0,
>         doc: /* Set the file permission bits for newly created files.
>  The argument MODE should be an integer; only the low 9 bits are used.
> -This setting is inherited by subprocesses.  */)
> +This setting is inherited by subprocesses.
> +
> +Note that some functions (e.g., `write-region') ignore the execution
> +bits in MODE.  In that case you need first to create the file,
> +and then set the permisions with `set-file-modes'.  */)

Why would someone assume that write-region could/should change the
mode bits of an already existing file?  It's entirely unreasonable for
write-region to do so, as the mode bits are determined when the file
is created, in this case by make-temp-file.

So I'm not sure why should we make this correction to the doc string.
What am I missing?

Thanks.

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Fri, 20 Oct 2017 16:31:46 +0900
> 
>          (with-file-modes ?\700
>                (if (file-exists-p
>                     (setq pidfile (format "/tmp/Mosaic.%d" pid)))
>                    (delete-file pidfile))
>                ;; https://debbugs.gnu.org/17428.  Use O_EXCL.
>                (write-region nil nil pidfile nil 'silent nil 'excl)))
> 
> Here the file doesn't exist, `write-region' create it.  Beside the
> (with-file-modes ?\700
> 
> the actual file permission for pidfile is: 600
> This was a bit sorprising for me; I need to read the manual to fully
> understand it.
> The docstring of with-file-modes links to set-default-file-modes, so
> maybe enough just t mention about this there.

I think you are missing what Andreas points out: that
set-default-file-modes works via 'umask', which can only remove bits
from the default permissions, it cannot add bits.  I'm okay with
describing this in more details, for those might not be aware of how
'umask' works.  That would be a different text, though, not the one
you suggested.

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Tino Calancha]

On 10/27/2017 10:54 PM, Eli Zaretskii wrote:
>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Fri, 20 Oct 2017 18:37:11 +0900
>>
>>> I think you are missing what Andreas points out: that
>>> set-default-file-modes works via 'umask', which can only remove bits
>>> from the default permissions, it cannot add bits.  I'm okay with
>>> describing this in more details, for those might not be aware of how
>>> 'umask' works.  That would be a different text, though, not the one
>>> you suggested.
>> You are right I don't know much about umask.
>> Probably most of the people using set-default-file-modes
> OK, I added some text to the docs to clarify this.

Thank you, it looks more clear to me know.
There is a duplicated word:
+This function works by setting the Emacs's file mode creation mask.
+Each bit that is set in the mask means that the corresponding bit
+in the the permissions of newly created files will be disabled.
        ^^^^

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Fri, 27 Oct 2017 23:02:28 +0900
> 
> There is a duplicated word:

Thanks, fixed.

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Tino Calancha]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Fri, 27 Oct 2017 23:02:28 +0900
>> 
>> There is a duplicated word:
>
> Thanks, fixed.
I am closing this bug report which was already handled.

bug#28875: 25.3.50; set-default-file-modes ignores execution bits

[Andreas Schwab]

On Okt 20 2017, Tino Calancha <tino.calancha@gmail.com> wrote:

> diff --git a/src/fileio.c b/src/fileio.c
> index b7df38c857..d707bfc1c6 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -3125,7 +3125,11 @@ symbolic notation, like the `chmod' command from GNU Coreutils.  */)
>  DEFUN ("set-default-file-modes", Fset_default_file_modes, Sset_default_file_modes, 1, 1, 0,
>         doc: /* Set the file permission bits for newly created files.
>  The argument MODE should be an integer; only the low 9 bits are used.
> -This setting is inherited by subprocesses.  */)
> +This setting is inherited by subprocesses.
> +
> +Note that some functions (e.g., `write-region') ignore the execution
> +bits in MODE.  In that case you need first to create the file,
> +and then set the permisions with `set-file-modes'.  */)

The umask cannot broaden the permissions, only restrict them.  The zero
bits specify the bits that are always cleared from the mode, the one
bits leave them unmodified.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."