Skip to content

Commit b1236ce

Browse files
Newbyteeyuwata
authored andcommitted
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
1 parent 710653d commit b1236ce

File tree

12 files changed

+28
-22
lines changed

12 files changed

+28
-22
lines changed

src/basic/fd-util.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,13 +1001,13 @@ int fd_verify_safe_flags_full(int fd, int extra_flags) {
10011001
if (flags < 0)
10021002
return -errno;
10031003

1004-
unexpected_flags = flags & ~(O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE|extra_flags);
1004+
unexpected_flags = flags & ~(O_ACCMODE_STRICT|O_NOFOLLOW|RAW_O_LARGEFILE|extra_flags);
10051005
if (unexpected_flags != 0)
10061006
return log_debug_errno(SYNTHETIC_ERRNO(EREMOTEIO),
10071007
"Unexpected flags set for extrinsic fd: 0%o",
10081008
(unsigned) unexpected_flags);
10091009

1010-
return flags & (O_ACCMODE | extra_flags); /* return the flags variable, but remove the noise */
1010+
return flags & (O_ACCMODE_STRICT | extra_flags); /* return the flags variable, but remove the noise */
10111011
}
10121012

10131013
int read_nr_open(void) {
@@ -1132,7 +1132,7 @@ int fds_are_same_mount(int fd1, int fd2) {
11321132
}
11331133

11341134
const char* accmode_to_string(int flags) {
1135-
switch (flags & O_ACCMODE) {
1135+
switch (flags & O_ACCMODE_STRICT) {
11361136
case O_RDONLY:
11371137
return "ro";
11381138
case O_WRONLY:

src/basic/fs-util.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1036,7 +1036,7 @@ int open_mkdir_at_full(int dirfd, const char *path, int flags, XOpenFlags xopen_
10361036

10371037
if (flags & ~(O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_EXCL|O_NOATIME|O_NOFOLLOW|O_PATH))
10381038
return -EINVAL;
1039-
if ((flags & O_ACCMODE) != O_RDONLY)
1039+
if ((flags & O_ACCMODE_STRICT) != O_RDONLY)
10401040
return -EINVAL;
10411041

10421042
/* Note that O_DIRECTORY|O_NOFOLLOW is implied, but we allow specifying it anyway. The following

src/basic/missing_fcntl.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,9 @@
4343
#ifndef AT_HANDLE_FID
4444
#define AT_HANDLE_FID AT_REMOVEDIR
4545
#endif
46+
47+
/* On musl, O_ACCMODE is defined as (03|O_SEARCH), unlike glibc which defines it as
48+
* (O_RDONLY|O_WRONLY|O_RDWR). Additionally, O_SEARCH is simply defined as O_PATH. This changes the behaviour
49+
* of O_ACCMODE in certain situations, which we don't want. This definition is copied from glibc and works
50+
* around the problems with musl's definition. */
51+
#define O_ACCMODE_STRICT (O_RDONLY|O_WRONLY|O_RDWR)

src/core/exec-invoke.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ static int acquire_path(const char *path, int flags, mode_t mode) {
267267

268268
assert(path);
269269

270-
if (IN_SET(flags & O_ACCMODE, O_WRONLY, O_RDWR))
270+
if (IN_SET(flags & O_ACCMODE_STRICT, O_WRONLY, O_RDWR))
271271
flags |= O_CREAT;
272272

273273
fd = open(path, flags|O_NOCTTY, mode);
@@ -291,9 +291,9 @@ static int acquire_path(const char *path, int flags, mode_t mode) {
291291
if (r < 0)
292292
return r;
293293

294-
if ((flags & O_ACCMODE) == O_RDONLY)
294+
if ((flags & O_ACCMODE_STRICT) == O_RDONLY)
295295
r = shutdown(fd, SHUT_WR);
296-
else if ((flags & O_ACCMODE) == O_WRONLY)
296+
else if ((flags & O_ACCMODE_STRICT) == O_WRONLY)
297297
r = shutdown(fd, SHUT_RD);
298298
else
299299
r = 0;

src/libsystemd/sd-journal/journal-file.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
9898
JournalFile, journal_file_close);
9999

100100
static int mmap_prot_from_open_flags(int flags) {
101-
switch (flags & O_ACCMODE) {
101+
switch (flags & O_ACCMODE_STRICT) {
102102
case O_RDONLY:
103103
return PROT_READ;
104104
case O_WRONLY:
@@ -4075,10 +4075,10 @@ int journal_file_open(
40754075
assert(mmap_cache);
40764076
assert(ret);
40774077

4078-
if (!IN_SET((open_flags & O_ACCMODE), O_RDONLY, O_RDWR))
4078+
if (!IN_SET((open_flags & O_ACCMODE_STRICT), O_RDONLY, O_RDWR))
40794079
return -EINVAL;
40804080

4081-
if ((open_flags & O_ACCMODE) == O_RDONLY && FLAGS_SET(open_flags, O_CREAT))
4081+
if ((open_flags & O_ACCMODE_STRICT) == O_RDONLY && FLAGS_SET(open_flags, O_CREAT))
40824082
return -EINVAL;
40834083

40844084
if (fname && (open_flags & O_CREAT) && !endswith(fname, ".journal"))

src/libsystemd/sd-journal/journal-file.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* SPDX-License-Identifier: LGPL-2.1-or-later */
22
#pragma once
33

4-
#include <fcntl.h>
54
#include <inttypes.h>
65
#include <sys/uio.h>
76

@@ -15,6 +14,7 @@
1514
#include "compress.h"
1615
#include "hashmap.h"
1716
#include "journal-def.h"
17+
#include "missing_fcntl.h"
1818
#include "mmap-cache.h"
1919
#include "sparse-endian.h"
2020
#include "time-util.h"
@@ -391,5 +391,5 @@ static inline uint32_t COMPRESSION_TO_HEADER_INCOMPATIBLE_FLAG(Compression c) {
391391

392392
static inline bool journal_file_writable(JournalFile *f) {
393393
assert(f);
394-
return (f->open_flags & O_ACCMODE) != O_RDONLY;
394+
return (f->open_flags & O_ACCMODE_STRICT) != O_RDONLY;
395395
}

src/login/logind-session-dbus.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ static int method_set_tty(sd_bus_message *message, void *userdata, sd_bus_error
536536
flags = fcntl(fd, F_GETFL, 0);
537537
if (flags < 0)
538538
return -errno;
539-
if ((flags & O_ACCMODE) != O_RDWR)
539+
if ((flags & O_ACCMODE_STRICT) != O_RDWR)
540540
return -EACCES;
541541
if (FLAGS_SET(flags, O_PATH))
542542
return -ENOTTY;

src/mountfsd/mountwork.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static int validate_image_fd(int fd, MountImageParameters *p) {
9999
if (fl < 0)
100100
return log_debug_errno(fl, "Image file descriptor has unsafe flags set: %m");
101101

102-
switch (fl & O_ACCMODE) {
102+
switch (fl & O_ACCMODE_STRICT) {
103103

104104
case O_RDONLY:
105105
p->read_only = true;

src/shared/data-fd-util.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,13 @@ int memfd_clone_fd(int fd, const char *name, int mode) {
146146

147147
assert(fd >= 0);
148148
assert(name);
149-
assert(IN_SET(mode & O_ACCMODE, O_RDONLY, O_RDWR));
149+
assert(IN_SET(mode & O_ACCMODE_STRICT, O_RDONLY, O_RDWR));
150150
assert((mode & ~(O_RDONLY|O_RDWR|O_CLOEXEC)) == 0);
151151

152152
if (fstat(fd, &st) < 0)
153153
return -errno;
154154

155-
ro = (mode & O_ACCMODE) == O_RDONLY;
155+
ro = (mode & O_ACCMODE_STRICT) == O_RDONLY;
156156
exec = st.st_mode & 0111;
157157

158158
mfd = memfd_create_wrapper(name,

src/shared/journal-file-util.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ int journal_file_open_reliably(
504504
-EIDRM)) /* File has been deleted */
505505
return r;
506506

507-
if ((open_flags & O_ACCMODE) == O_RDONLY)
507+
if ((open_flags & O_ACCMODE_STRICT) == O_RDONLY)
508508
return r;
509509

510510
if (!(open_flags & O_CREAT))
@@ -519,7 +519,7 @@ int journal_file_open_reliably(
519519
/* The file is corrupted. Try opening it read-only as the template before rotating to inherit its
520520
* sequence number and ID. */
521521
r = journal_file_open(-EBADF, fname,
522-
(open_flags & ~(O_ACCMODE|O_CREAT|O_EXCL)) | O_RDONLY,
522+
(open_flags & ~(O_ACCMODE_STRICT|O_CREAT|O_EXCL)) | O_RDONLY,
523523
file_flags, 0, compress_threshold_bytes, NULL,
524524
mmap_cache, /* template = */ NULL, &old_file);
525525
if (r < 0)

0 commit comments

Comments
 (0)