Re: [PATCH] install-info: Fix dir file mode being restricted

[Arsen Arsenović]

Hi,

Gavin Smith <gavinsmith0123@gmail.com> writes:

> It breaks for me too.  The tests have output like
>
> + ../../install-info/ginstall-info 
> ../../install-info/tests/ii-0001-input-info-file ii01-eiGaE2Zf
> chmod: Bad file descriptor
>
> The line in the program
>
>   if (fchmod (tempfile, 0666 & um) < 0)
>
> occurs after fclose is called on the FILE opened from the tempfile file
> descriptor, which may be part of the problem.  The other part of the
> code that is incorrect is that if the output is being written via a 
> pipe (to a compression program), the tempfile fd is also closed early.

Heh, apparently I never recompiled after fixing up that mask block not
to use a fixed umask, and to happen later.  I originally ran fchmod
(tmpfile, 0644) directly after mkstemp, which is why it worked.  I've
pushed the attached commits to master:

From cc690c732619832d40318adeadf0832bdefa330a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= <arsen@aarsen.me>
Date: Sun, 26 Feb 2023 15:09:38 +0100
Subject: [PATCH] install-info: Relax tempfile mode earlier

* install-info/install-info.c (output_dirfile): Reorder the
tempfile mode relaxation block directly under mkstemp, in order to
not accidentally operate on a closed file descriptor.
---
 ChangeLog                   |  7 +++++++
 install-info/install-info.c | 21 +++++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7c8995c31b..64f53ce952 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2023-02-26  Arsen Arsenović  <arsen@aarsen.me>
+
+       install-info: Relax tempfile mode earlier
+       * install-info/install-info.c (output_dirfile): Reorder the
+       tempfile mode relaxation block directly under mkstemp, in order to
+       not accidentally operate on a closed file descriptor.
+
 2023-02-26  Patrice Dumas  <pertusus@free.fr>
 
        * tp/Texinfo/Common.pm (register_label, get_label_element),
diff --git a/install-info/install-info.c b/install-info/install-info.c
index 9d8201c977..b3258866c7 100644
--- a/install-info/install-info.c
+++ b/install-info/install-info.c
@@ -929,6 +929,17 @@ output_dirfile (char *dirfile, int dir_nlines, struct 
line_data *dir_lines,
   char tempname[] = "infodirXXXXXX";
 
   tempfile = mkstemp (tempname);
+
+  /* Reset the mode that the file is set to.  */
+  mode_t um = umask (0022);
+  umask (um);
+  if (fchmod (tempfile, 0666 & um) < 0)
+    {
+      perror ("chmod");
+      remove (tempname);
+      exit (EXIT_FAILURE);
+    }
+
   if (compression_program)
     {
       char *command;
@@ -1053,16 +1064,6 @@ output_dirfile (char *dirfile, int dir_nlines, struct 
line_data *dir_lines,
   else
     fclose (output);
 
-  /* Reset the mode that the file is set to.  */
-  mode_t um = umask (0022);
-  umask (um);
-  if (fchmod (tempfile, 0666 & um) < 0)
-    {
-      perror ("chmod");
-      remove (tempname);
-      exit (EXIT_FAILURE);
-    }
-
   /* Update dir file atomically.  This stops the dir file being corrupted
      if install-info is interrupted. */
   if (rename (tempname, dirfile) == -1)
-- 
2.39.2

From 38b4f126da5f348338837ff82b26499862e5fadc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= <arsen@aarsen.me>
Date: Sun, 26 Feb 2023 15:14:20 +0100
Subject: [PATCH] install-info: Invert umask in mode computation

* install-info/install-info.c (output_dirfile): Invert umask
applied to tempfile (as is specified in the definition for
umasks).
---
 ChangeLog                   | 7 +++++++
 install-info/install-info.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 64f53ce952..dad365b2ed 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2023-02-26  Arsen Arsenović  <arsen@aarsen.me>
+
+       install-info: Invert umask in mode computation
+       * install-info/install-info.c (output_dirfile): Invert umask
+       applied to tempfile (as is specified in the definition for
+       umasks).
+
 2023-02-26  Arsen Arsenović  <arsen@aarsen.me>
 
        install-info: Relax tempfile mode earlier
diff --git a/install-info/install-info.c b/install-info/install-info.c
index b3258866c7..0899f8c683 100644
--- a/install-info/install-info.c
+++ b/install-info/install-info.c
@@ -933,7 +933,7 @@ output_dirfile (char *dirfile, int dir_nlines, struct 
line_data *dir_lines,
   /* Reset the mode that the file is set to.  */
   mode_t um = umask (0022);
   umask (um);
-  if (fchmod (tempfile, 0666 & um) < 0)
+  if (fchmod (tempfile, 0666 & ~um) < 0)
     {
       perror ("chmod");
       remove (tempname);
-- 
2.39.2

Apologies for the inconvenience.
-- 
Arsen Arsenović

signature.asc
Description: PGP signature