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 <lasse.col...@tukaani.org>
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
