missing_fcntl: Introduce O_ACCMODE_STRICT

On musl, O_ACCMODE is defined as (03|O_SEARCH), unlike glibc which
defines it as (O_RDONLY|O_WRONLY|O_RDWR). Additionally, O_SEARCH is
simply defined as O_PATH.

This causes problems for systemd on musl, as it changes the
behaviour of open_mkdir_at_full() to return -EINVAL if O_PATH is
included in flags due to the fact that O_ACCMODE includes O_SEARCH
(i.e. O_PATH). Consequently, this makes the test-fs-util test fail.

Upstream musl seems content with this behaviour and doesn't seem
interested in matching glibc's behaviour due to that defining it this
way allows for O_SEARCH to match POSIX better by allowing it to open
directories where read permission is missing. Apparently musl does some
emulation in other places to make this work more consistently as well.

Initially I took the approach of working around this by redefining
O_SEARCH as O_RDONLY if O_SEARCH == O_PATH. This fixes the test and is
the approach taken by both XZ[1] and Gzip[2][3], but was not taken as
redefining system headers potentially could be problematic.

Instead, introduce O_ACCMODE_STRICT which just is a copy of glibc's
O_ACCMODE and use it everywhere. This way we don't have to deal with
unusual definitions of O_ACCMODE from C standard libraries other than
glibc.

 [1]: https://git.tukaani.org/?p=xz.git;a=blob;f=src/xz/file_io.c;h=8c83269b13fa31284f7ea5f3627a1dfbce7d6e14;hb=HEAD#l72
 [2]: https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/fcntl.in.h
      (lines 380 and 396, commit d7f551b30f3f2a0fa57c1b10c12f4eea41a9b89e)
 [3]: https://lists.gnu.org/archive/html/bug-gzip/2025-01/msg00000.html

diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c
index a6a63454fca2f5ce9bbc86114ccfeb6bfe8e941f..5f17ed1792b6312e4ac20ec6859df6146b31dadf 100644 (file)
--- a/src/basic/fd-util.c
+++ b/src/basic/fd-util.c
@@ -1001,13 +1001,13 @@ int fd_verify_safe_flags_full(int fd, int extra_flags) {
         if (flags < 0)
                 return -errno;
 
-        unexpected_flags = flags & ~(O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE|extra_flags);
+        unexpected_flags = flags & ~(O_ACCMODE_STRICT|O_NOFOLLOW|RAW_O_LARGEFILE|extra_flags);
         if (unexpected_flags != 0)
                 return log_debug_errno(SYNTHETIC_ERRNO(EREMOTEIO),
                                        "Unexpected flags set for extrinsic fd: 0%o",
                                        (unsigned) unexpected_flags);
 
-        return flags & (O_ACCMODE | extra_flags); /* return the flags variable, but remove the noise */
+        return flags & (O_ACCMODE_STRICT | extra_flags); /* return the flags variable, but remove the noise */
 }
 
 int read_nr_open(void) {
@@ -1132,7 +1132,7 @@ int fds_are_same_mount(int fd1, int fd2) {
 }
 
 const char* accmode_to_string(int flags) {
-        switch (flags & O_ACCMODE) {
+        switch (flags & O_ACCMODE_STRICT) {
         case O_RDONLY:
                 return "ro";
         case O_WRONLY:

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 655f59a5480d8b96eda54b38d38d6fb1a9673665..2f16b6a5a7883e2ea03fb7633b829148f14bd2f7 100644 (file)
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -1036,7 +1036,7 @@ int open_mkdir_at_full(int dirfd, const char *path, int flags, XOpenFlags xopen_
 
         if (flags & ~(O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_EXCL|O_NOATIME|O_NOFOLLOW|O_PATH))
                 return -EINVAL;
-        if ((flags & O_ACCMODE) != O_RDONLY)
+        if ((flags & O_ACCMODE_STRICT) != O_RDONLY)
                 return -EINVAL;
 
         /* Note that O_DIRECTORY|O_NOFOLLOW is implied, but we allow specifying it anyway. The following

diff --git a/src/basic/missing_fcntl.h b/src/basic/missing_fcntl.h
index 3c3f3a445f56057659a4f77630291c94c259870a..b767186a4a63c761ea6c42f1fb5ece86faf11d81 100644 (file)
--- a/src/basic/missing_fcntl.h
+++ b/src/basic/missing_fcntl.h
@@ -43,3 +43,9 @@
 #ifndef AT_HANDLE_FID
 #define AT_HANDLE_FID AT_REMOVEDIR
 #endif
+
+/* On musl, O_ACCMODE is defined as (03|O_SEARCH), unlike glibc which defines it as
+ * (O_RDONLY|O_WRONLY|O_RDWR). Additionally, O_SEARCH is simply defined as O_PATH. This changes the behaviour
+ * of O_ACCMODE in certain situations, which we don't want. This definition is copied from glibc and works
+ * around the problems with musl's definition. */
+#define O_ACCMODE_STRICT (O_RDONLY|O_WRONLY|O_RDWR)

diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c
index ef129eb770d70cf782979d3d661fdd1aafe425d0..0bf6c836013b5e7eac8e9acbed702376bf7d73b7 100644 (file)
--- a/src/core/exec-invoke.c
+++ b/src/core/exec-invoke.c
@@ -267,7 +267,7 @@ static int acquire_path(const char *path, int flags, mode_t mode) {
 
         assert(path);
 
-        if (IN_SET(flags & O_ACCMODE, O_WRONLY, O_RDWR))
+        if (IN_SET(flags & O_ACCMODE_STRICT, O_WRONLY, O_RDWR))
                 flags |= O_CREAT;
 
         fd = open(path, flags|O_NOCTTY, mode);
@@ -291,9 +291,9 @@ static int acquire_path(const char *path, int flags, mode_t mode) {
         if (r < 0)
                 return r;
 
-        if ((flags & O_ACCMODE) == O_RDONLY)
+        if ((flags & O_ACCMODE_STRICT) == O_RDONLY)
                 r = shutdown(fd, SHUT_WR);
-        else if ((flags & O_ACCMODE) == O_WRONLY)
+        else if ((flags & O_ACCMODE_STRICT) == O_WRONLY)
                 r = shutdown(fd, SHUT_RD);
         else
                 r = 0;

diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c
index 30a5c716a835b1adf50c5be7407a37a44669e0c7..a0e618cf1dc2a36f95b2bda7a21ce67a0170abfa 100644 (file)
--- a/src/libsystemd/sd-journal/journal-file.c
+++ b/src/libsystemd/sd-journal/journal-file.c
@@ -98,7 +98,7 @@ DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
                 JournalFile, journal_file_close);
 
 static int mmap_prot_from_open_flags(int flags) {
-        switch (flags & O_ACCMODE) {
+        switch (flags & O_ACCMODE_STRICT) {
         case O_RDONLY:
                 return PROT_READ;
         case O_WRONLY:
@@ -4075,10 +4075,10 @@ int journal_file_open(
         assert(mmap_cache);
         assert(ret);
 
-        if (!IN_SET((open_flags & O_ACCMODE), O_RDONLY, O_RDWR))
+        if (!IN_SET((open_flags & O_ACCMODE_STRICT), O_RDONLY, O_RDWR))
                 return -EINVAL;
 
-        if ((open_flags & O_ACCMODE) == O_RDONLY && FLAGS_SET(open_flags, O_CREAT))
+        if ((open_flags & O_ACCMODE_STRICT) == O_RDONLY && FLAGS_SET(open_flags, O_CREAT))
                 return -EINVAL;
 
         if (fname && (open_flags & O_CREAT) && !endswith(fname, ".journal"))

diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h
index 669c0661562acd33508775773b1eac74669abe76..35f8f6ce6b4e573186ab0b1cb30a14a888534afd 100644 (file)
--- a/src/libsystemd/sd-journal/journal-file.h
+++ b/src/libsystemd/sd-journal/journal-file.h
@@ -1,7 +1,6 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
-#include <fcntl.h>
 #include <inttypes.h>
 #include <sys/uio.h>
 
@@ -15,6 +14,7 @@
 #include "compress.h"
 #include "hashmap.h"
 #include "journal-def.h"
+#include "missing_fcntl.h"
 #include "mmap-cache.h"
 #include "sparse-endian.h"
 #include "time-util.h"
@@ -391,5 +391,5 @@ static inline uint32_t COMPRESSION_TO_HEADER_INCOMPATIBLE_FLAG(Compression c) {
 
 static inline bool journal_file_writable(JournalFile *f) {
         assert(f);
-        return (f->open_flags & O_ACCMODE) != O_RDONLY;
+        return (f->open_flags & O_ACCMODE_STRICT) != O_RDONLY;
 }

diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 126020acf165692a003566e9cc6cab4ad154701d..5e374ae91e880c98a55605ba49b91c00588fb471 100644 (file)
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -536,7 +536,7 @@ static int method_set_tty(sd_bus_message *message, void *userdata, sd_bus_error
         flags = fcntl(fd, F_GETFL, 0);
         if (flags < 0)
                 return -errno;
-        if ((flags & O_ACCMODE) != O_RDWR)
+        if ((flags & O_ACCMODE_STRICT) != O_RDWR)
                 return -EACCES;
         if (FLAGS_SET(flags, O_PATH))
                 return -ENOTTY;

diff --git a/src/mountfsd/mountwork.c b/src/mountfsd/mountwork.c
index a693419eec7235238045726bd1635716d0e07546..9567146fdc16f6e11027fb09a77b7c08c246a8db 100644 (file)
--- a/src/mountfsd/mountwork.c
+++ b/src/mountfsd/mountwork.c
@@ -99,7 +99,7 @@ static int validate_image_fd(int fd, MountImageParameters *p) {
         if (fl < 0)
                 return log_debug_errno(fl, "Image file descriptor has unsafe flags set: %m");
 
-        switch (fl & O_ACCMODE) {
+        switch (fl & O_ACCMODE_STRICT) {
 
         case O_RDONLY:
                 p->read_only = true;

diff --git a/src/shared/data-fd-util.c b/src/shared/data-fd-util.c
index 6f4825d8c132ba8560b0456a746463f6665005d6..20d54f325d9ce02aa8c2fb723d34e0f4ad7a2c4b 100644 (file)
--- a/src/shared/data-fd-util.c
+++ b/src/shared/data-fd-util.c
@@ -146,13 +146,13 @@ int memfd_clone_fd(int fd, const char *name, int mode) {
 
         assert(fd >= 0);
         assert(name);
-        assert(IN_SET(mode & O_ACCMODE, O_RDONLY, O_RDWR));
+        assert(IN_SET(mode & O_ACCMODE_STRICT, O_RDONLY, O_RDWR));
         assert((mode & ~(O_RDONLY|O_RDWR|O_CLOEXEC)) == 0);
 
         if (fstat(fd, &st) < 0)
                 return -errno;
 
-        ro = (mode & O_ACCMODE) == O_RDONLY;
+        ro = (mode & O_ACCMODE_STRICT) == O_RDONLY;
         exec = st.st_mode & 0111;
 
         mfd = memfd_create_wrapper(name,

diff --git a/src/shared/journal-file-util.c b/src/shared/journal-file-util.c
index b352ede275a6b2b33dadb43ff24b53346c77bd52..edd5056ad9495c2a83eb8826a45043667ce59c72 100644 (file)
--- a/src/shared/journal-file-util.c
+++ b/src/shared/journal-file-util.c
@@ -504,7 +504,7 @@ int journal_file_open_reliably(
                     -EIDRM))            /* File has been deleted */
                 return r;
 
-        if ((open_flags & O_ACCMODE) == O_RDONLY)
+        if ((open_flags & O_ACCMODE_STRICT) == O_RDONLY)
                 return r;
 
         if (!(open_flags & O_CREAT))
@@ -519,7 +519,7 @@ int journal_file_open_reliably(
         /* The file is corrupted. Try opening it read-only as the template before rotating to inherit its
          * sequence number and ID. */
         r = journal_file_open(-EBADF, fname,
-                              (open_flags & ~(O_ACCMODE|O_CREAT|O_EXCL)) | O_RDONLY,
+                              (open_flags & ~(O_ACCMODE_STRICT|O_CREAT|O_EXCL)) | O_RDONLY,
                               file_flags, 0, compress_threshold_bytes, NULL,
                               mmap_cache, /* template = */ NULL, &old_file);
         if (r < 0)

diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c
index 4a6e0046dfc2796a65bc0bc01fd8302ceaa43a4d..5f2fe03e2e6572eee1ae16b005f6dc208b5e8165 100644 (file)
--- a/src/shared/loop-util.c
+++ b/src/shared/loop-util.c
@@ -500,7 +500,7 @@ static int loop_device_make_internal(
                 .block_size = sector_size,
                 .info = {
                         /* Use the specified flags, but configure the read-only flag from the open flags, and force autoclear */
-                        .lo_flags = (loop_flags & ~LO_FLAGS_READ_ONLY) | ((open_flags & O_ACCMODE) == O_RDONLY ? LO_FLAGS_READ_ONLY : 0) | LO_FLAGS_AUTOCLEAR,
+                        .lo_flags = (loop_flags & ~LO_FLAGS_READ_ONLY) | ((open_flags & O_ACCMODE_STRICT) == O_RDONLY ? LO_FLAGS_READ_ONLY : 0) | LO_FLAGS_AUTOCLEAR,
                         .lo_offset = offset,
                         .lo_sizelimit = size == UINT64_MAX ? 0 : size,
                 },

diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index dbd175ad7044840fb0c29ac78d1a00214ac20486..67943b65ba77923c5b57f8eff57f63caa050670f 100644 (file)
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -1104,7 +1104,7 @@ TEST(fdopen_independent) {
         zero(buf);
         assert_se(fread(buf, 1, sizeof(buf), f) == strlen(TEST_TEXT));
         ASSERT_STREQ(buf, TEST_TEXT);
-        assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE) == O_RDONLY);
+        assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE_STRICT) == O_RDONLY);
         assert_se(FLAGS_SET(fcntl(fileno(f), F_GETFD), FD_CLOEXEC));
         f = safe_fclose(f);
 
@@ -1112,7 +1112,7 @@ TEST(fdopen_independent) {
         zero(buf);
         assert_se(fread(buf, 1, sizeof(buf), f) == strlen(TEST_TEXT));
         ASSERT_STREQ(buf, TEST_TEXT);
-        assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE) == O_RDONLY);
+        assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE_STRICT) == O_RDONLY);
         assert_se(!FLAGS_SET(fcntl(fileno(f), F_GETFD), FD_CLOEXEC));
         f = safe_fclose(f);
 
@@ -1120,7 +1120,7 @@ TEST(fdopen_independent) {
         zero(buf);
         assert_se(fread(buf, 1, sizeof(buf), f) == strlen(TEST_TEXT));
         ASSERT_STREQ(buf, TEST_TEXT);
-        assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE) == O_RDWR);
+        assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE_STRICT) == O_RDWR);
         assert_se(FLAGS_SET(fcntl(fileno(f), F_GETFD), FD_CLOEXEC));
         f = safe_fclose(f);
 }