Subject: gzip --synchronous doesn't work with musl
From: Lasse Collin <[email-masked]>
To: [email-masked]
Date: Mon, 6 Jan 2025 15:29:22 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

On Linux/musl:

    $ echo foo > foo
    $ gzip --synchronous foo

    gzip: foo.gz: Bad file descriptor

From gzip.c:

      dfd = open (dfname, O_SEARCH | O_DIRECTORY);

...

        if ((synchronous
             && ((0 <= dfd && fdatasync (dfd) != 0 && errno != EINVAL)
                 || (fsync (ofd) != 0 && errno != EINVAL)))
            || close (ofd) != 0)
          write_error ();

In musl, O_SEARCH maps to Linux-specific O_PATH which doesn't allow
fsync or fdatasync. Gnulib's fcntl module's docs already warn about a
similar situation with fchmod.

As far as I understand this, O_SEARCH in POSIX is only meant for openat
and such APIs. Then it makes sense that O_SEARCH becomes O_PATH because
O_PATH can open a directory that lacks read permission. This way openat
can be used even for such directories.

Maybe there is no way to sync a directory that only has write and search
permissions. Luckily such a combination is rare.

A short fix is using O_RDONLY when --synchronous has been specified:

      dfd = open (dfname, (synchronous ? O_RDONLY : O_SEARCH)
                          | O_DIRECTORY);

I also tried opening a separate file descriptor for syncing (see the
attachment) but I think the above change is better. However, both have
a weakness that if the directory cannot be opened, --synchronous will
silently skip syncing the directory. It would be safer to show an error
if the directory cannot be opened when --synchronous has been specified.

-- 
Lasse Collin
From 833f8e731f8998c54ade71d263a96f8fd37d3cc0 Mon Sep 17 00:00:00 2001
From: Lasse Collin <[email-masked]>
Date: Mon, 6 Jan 2025 12:49:48 +0200
Subject: [PATCH] gzip: Fix --synchronous when O_SEARCH != O_RDONLY

On Linux/musl:

    $ echo foo > foo
    $ gzip --synchronous foo

    gzip: foo.gz: Bad file descriptor

O_SEARCH maps to Linux-specific O_PATH. The resulting file descriptor
doesn't allow fsync. O_SEARCH is only meant for openat and such APIs.

When --synchronous has been specified, open a separate file descriptor
with O_RDONLY when O_SEARCH != O_RDONLY.

Add a FIXME about lack of error checking. If opening the directory fails,
fdatasync on the directory is silently skipped which makes --synchronous
not properly synchronous.
---
 gzip.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/gzip.c b/gzip.c
index a33841a..b0687ec 100644
--- a/gzip.c
+++ b/gzip.c
@@ -215,6 +215,7 @@ static struct stat istat;         /* status for input file */
 int  ifd;                  /* input file descriptor */
 int  ofd;                  /* output file descriptor */
 static int dfd = -1;       /* output directory file descriptor */
+static int dfd_sync = -1;  /* output directory file descriptor for fdatasync */
 unsigned insize;           /* valid bytes in inbuf */
 unsigned inptr;            /* index of next byte to be processed in inbuf */
 unsigned outcnt;           /* bytes in output buffer */
@@ -849,6 +850,24 @@ atdir_set (char const *dir, ptrdiff_t dirlen)
       memcpy (dfname, dir, dirlen);
       dfname[dirlen] = '\0';
       dfd = open (dfname, O_SEARCH | O_DIRECTORY);
+
+      if (synchronous)
+        {
+          if (O_SEARCH != O_RDONLY)
+            {
+              if (0 <= dfd_sync)
+                close (dfd_sync);
+              /* FIXME: This shouldn't ignore errors. Silently skipping
+                 the fdatasync call means that --synchronous does its job
+                 only partially.
+               */
+              dfd_sync = open (dfname, O_RDONLY | O_DIRECTORY);
+            }
+          else
+            {
+              dfd_sync = dfd;
+            }
+        }
     }
 
   return dfd;
@@ -1010,7 +1029,8 @@ treat_file (char *iname)
         copy_stat (&istat);
 
         if ((synchronous
-             && ((0 <= dfd && fdatasync (dfd) != 0 && errno != EINVAL)
+             && ((0 <= dfd_sync && fdatasync (dfd_sync) != 0
+                  && errno != EINVAL)
                  || (fsync (ofd) != 0 && errno != EINVAL)))
             || close (ofd) != 0)
           write_error ();
-- 
2.47.1