From 12620ca1fbb593e124c6db4e80d4bf6ed8757f58 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 21 Oct 2024 21:54:37 +0200 Subject: [PATCH 01/12] fs-util: remove misplaced RET_NERRNO() --- src/basic/fs-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 35cebfc849..5994ebd403 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1193,7 +1193,7 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ xopen_flags &= ~XO_LABEL; } - fd = RET_NERRNO(openat_report_new(dir_fd, path, open_flags, mode, &made_file)); + fd = openat_report_new(dir_fd, path, open_flags, mode, &made_file); if (fd < 0) { if (IN_SET(fd, /* We got ENOENT? then someone else immediately removed it after we From 652371a3c100a74999e21d9424538d3ab4cb9b97 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 21 Oct 2024 21:56:05 +0200 Subject: [PATCH 02/12] fs-util: always go through the unlink cleanup paths in xopenat_full() We didn't go through it at all if label_ops_post() failed. --- src/basic/fs-util.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 5994ebd403..bbf2fea3e6 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1186,7 +1186,7 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ if (FLAGS_SET(xopen_flags, XO_LABEL)) { r = label_ops_post(dir_fd, path); if (r < 0) - return r; + goto error; } open_flags &= ~(O_EXCL|O_CREAT); @@ -1206,10 +1206,8 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ -ENOTDIR)) return fd; - if (made_dir) - (void) unlinkat(dir_fd, path, AT_REMOVEDIR); - - return fd; + r = fd; + goto error; } if (FLAGS_SET(open_flags, O_CREAT) && FLAGS_SET(xopen_flags, XO_LABEL)) { From d49449c89b3bec486b16ddaec582a4de28bc3dea Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 21 Oct 2024 22:07:56 +0200 Subject: [PATCH 03/12] label: tweak LabelOps post() hook to take "created" boolean We have two distinct implementations of the post hook. 1. For SELinux we just reset the selinux label we told the kernel earlier to use for new inodes. 2. For SMACK we might apply an xattr to the specified file. The two calls are quite different: the first call we want to call in all cases (failure or success), the latter only if we actually managed to create an inode, in which case it is called on the inode. --- src/basic/fs-util.c | 4 ++-- src/basic/label.c | 4 ++-- src/basic/label.h | 6 ++++-- src/shared/selinux-util.c | 2 +- src/shared/smack-util.c | 5 ++++- src/test/test-label.c | 14 +++++++------- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index bbf2fea3e6..7ccd3f6750 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1184,7 +1184,7 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ made_dir = true; if (FLAGS_SET(xopen_flags, XO_LABEL)) { - r = label_ops_post(dir_fd, path); + r = label_ops_post(dir_fd, path, made_dir); if (r < 0) goto error; } @@ -1211,7 +1211,7 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ } if (FLAGS_SET(open_flags, O_CREAT) && FLAGS_SET(xopen_flags, XO_LABEL)) { - r = label_ops_post(dir_fd, path); + r = label_ops_post(dir_fd, path, made_file || made_dir); if (r < 0) goto error; } diff --git a/src/basic/label.c b/src/basic/label.c index 8b084a7c05..47d1967825 100644 --- a/src/basic/label.c +++ b/src/basic/label.c @@ -22,11 +22,11 @@ int label_ops_pre(int dir_fd, const char *path, mode_t mode) { return label_ops->pre(dir_fd, path, mode); } -int label_ops_post(int dir_fd, const char *path) { +int label_ops_post(int dir_fd, const char *path, bool created) { if (!label_ops || !label_ops->post) return 0; - return label_ops->post(dir_fd, path); + return label_ops->post(dir_fd, path, created); } void label_ops_reset(void) { diff --git a/src/basic/label.h b/src/basic/label.h index a070bf28cc..eb851bcb4f 100644 --- a/src/basic/label.h +++ b/src/basic/label.h @@ -1,15 +1,17 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include #include typedef struct LabelOps { int (*pre)(int dir_fd, const char *path, mode_t mode); - int (*post)(int dir_fd, const char *path); + int (*post)(int dir_fd, const char *path, bool created); } LabelOps; int label_ops_set(const LabelOps *label_ops); int label_ops_pre(int dir_fd, const char *path, mode_t mode); -int label_ops_post(int dir_fd, const char *path); +int label_ops_post(int dir_fd, const char *path, bool created); + void label_ops_reset(void); diff --git a/src/shared/selinux-util.c b/src/shared/selinux-util.c index e4f6e07e53..4d3b127d8e 100644 --- a/src/shared/selinux-util.c +++ b/src/shared/selinux-util.c @@ -64,7 +64,7 @@ static int mac_selinux_label_pre(int dir_fd, const char *path, mode_t mode) { return mac_selinux_create_file_prepare_at(dir_fd, path, mode); } -static int mac_selinux_label_post(int dir_fd, const char *path) { +static int mac_selinux_label_post(int dir_fd, const char *path, bool created) { mac_selinux_create_file_clear(); return 0; } diff --git a/src/shared/smack-util.c b/src/shared/smack-util.c index 1f88e724d0..d0a79b2635 100644 --- a/src/shared/smack-util.c +++ b/src/shared/smack-util.c @@ -294,7 +294,10 @@ static int mac_smack_label_pre(int dir_fd, const char *path, mode_t mode) { return 0; } -static int mac_smack_label_post(int dir_fd, const char *path) { +static int mac_smack_label_post(int dir_fd, const char *path, bool created) { + if (!created) + return 0; + return mac_smack_fix_full(dir_fd, path, NULL, 0); } diff --git a/src/test/test-label.c b/src/test/test-label.c index 9d7ac18ba9..06690ec86c 100644 --- a/src/test/test-label.c +++ b/src/test/test-label.c @@ -43,7 +43,7 @@ static int pre_labelling_func(int dir_fd, const char *path, mode_t mode) { return 0; } -static int post_labelling_func(int dir_fd, const char *path) { +static int post_labelling_func(int dir_fd, const char *path, bool created) { int r; /* assume label policies that restrict certain labels */ @@ -140,17 +140,17 @@ TEST(label_ops_post) { text1 = "Add initial texts to file for testing label operations to file1\n"; assert(labelling_op(fd, text1, "file1.txt", 0644) == 0); - assert_se(label_ops_post(fd, "file1.txt") == 0); + assert_se(label_ops_post(fd, "file1.txt", true) == 0); assert_se(strlen(text1) == (size_t)buf.st_size); text2 = "Add text2 data to file2\n"; assert(labelling_op(fd, text2, "file2.txt", 0644) == 0); - assert_se(label_ops_post(fd, "file2.txt") == 0); + assert_se(label_ops_post(fd, "file2.txt", true) == 0); assert_se(strlen(text2) == (size_t)buf.st_size); - assert_se(label_ops_post(fd, "file3.txt") == -ENOENT); - assert_se(label_ops_post(fd, "/abcd") == -ENOENT); - assert_se(label_ops_post(fd, "/restricted_directory") == -EACCES); - assert_se(label_ops_post(fd, "") == -EINVAL); + assert_se(label_ops_post(fd, "file3.txt", true) == -ENOENT); + assert_se(label_ops_post(fd, "/abcd", true) == -ENOENT); + assert_se(label_ops_post(fd, "/restricted_directory", true) == -EACCES); + assert_se(label_ops_post(fd, "", true) == -EINVAL); } DEFINE_TEST_MAIN(LOG_INFO) From da3d81cccd1f915e91a756b3d783e8bc3ecd0a5c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 21 Oct 2024 22:40:52 +0200 Subject: [PATCH 04/12] fs-util: don't second guess openat_report_new() return values If openat_report_new() fails, then 'made_file' will be false, as no file was created, hence there's no need to skip the unlinkat() explicitly early, given that we check for 'made_file' anyway in the error path. The extra error code checks are hence entirely redundant. --- src/basic/fs-util.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 7ccd3f6750..53206c3b37 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1195,17 +1195,6 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ fd = openat_report_new(dir_fd, path, open_flags, mode, &made_file); if (fd < 0) { - if (IN_SET(fd, - /* We got ENOENT? then someone else immediately removed it after we - * created it. In that case let's return immediately without unlinking - * anything, because there simply isn't anything to unlink anymore. */ - -ENOENT, - /* is a symlink? exists already → created by someone else, don't unlink */ - -ELOOP, - /* not a directory? exists already → created by someone else, don't unlink */ - -ENOTDIR)) - return fd; - r = fd; goto error; } From 64053bed083d24f2151d05951935d0804173e657 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 21 Oct 2024 22:43:18 +0200 Subject: [PATCH 05/12] fs-util: always call label post ops in xopenat_full(), in both success and error path For SELinux it is essential that we reset the file creation label both in the success and in the error path, hence do so. Moreover, when calling the label post ops do it if possible with the opened fd of the inode itself, rather than always going via its path, simply to reduce the attack surface. --- src/basic/fs-util.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 53206c3b37..1d0533e4f0 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1164,10 +1164,14 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ return fd_reopen(dir_fd, open_flags & ~O_NOFOLLOW); } + bool call_label_ops_post = false; + if (FLAGS_SET(open_flags, O_CREAT) && FLAGS_SET(xopen_flags, XO_LABEL)) { r = label_ops_pre(dir_fd, path, FLAGS_SET(open_flags, O_DIRECTORY) ? S_IFDIR : S_IFREG); if (r < 0) return r; + + call_label_ops_post = true; } if (FLAGS_SET(open_flags, O_DIRECTORY|O_CREAT)) { @@ -1183,14 +1187,7 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ else made_dir = true; - if (FLAGS_SET(xopen_flags, XO_LABEL)) { - r = label_ops_post(dir_fd, path, made_dir); - if (r < 0) - goto error; - } - open_flags &= ~(O_EXCL|O_CREAT); - xopen_flags &= ~XO_LABEL; } fd = openat_report_new(dir_fd, path, open_flags, mode, &made_file); @@ -1199,8 +1196,10 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ goto error; } - if (FLAGS_SET(open_flags, O_CREAT) && FLAGS_SET(xopen_flags, XO_LABEL)) { - r = label_ops_post(dir_fd, path, made_file || made_dir); + if (call_label_ops_post) { + call_label_ops_post = false; + + r = label_ops_post(fd, /* path= */ NULL, made_file || made_dir); if (r < 0) goto error; } @@ -1214,6 +1213,9 @@ int xopenat_full(int dir_fd, const char *path, int open_flags, XOpenFlags xopen_ return TAKE_FD(fd); error: + if (call_label_ops_post) + (void) label_ops_post(fd >= 0 ? fd : dir_fd, fd >= 0 ? NULL : path, made_dir || made_file); + if (made_dir || made_file) (void) unlinkat(dir_fd, path, made_dir ? AT_REMOVEDIR : 0); From 4946dd4197efee693432b3ddff56f30205d3bdfd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 21 Oct 2024 22:58:25 +0200 Subject: [PATCH 06/12] fs-util: tweak how openat_report_new() operates when O_CREAT is used on a dangling symlink One of the big mistakes of Linux is that when you create a file with open() and O_CREAT and the file already exists as dangling symlink that the symlink will be followed and the file created that it points to. This has resulted in many vulnerabilities, and triggered the creation of the O_MOFOLLOW flag, addressing the problem. O_NOFOLLOW is less than ideal in many ways, but in particular one: when actually creating a file it makes sense to set, because it is a problem to follow final symlinks in that case. But if the file is already existing, it actually does make sense to follow the symlinks. With openat_report_new() we distinguish these two cases anyway (the whole function exists only to distinguish the create and the exists-already case after all), hence let's do something about this: let's simply never create files "through symlinks". This can be implemented very easily: just pass O_NOFOLLOW to the 2nd openat() call, where we actually create files. And then basically remove 0dd82dab91eaac5e7b17bd5e9a1e07c6d2b78dca again, because we don't need to care anymore, we already will see ELOOP when we touch a symlink. Note that this change means that openat_report_new() will thus start to deviate from plain openat() behaviour in this one small detail: when actually creating files we will *never* follow the symlink. That should be a systematic improvement of security. Fixes: #34088 --- src/basic/fs-util.c | 46 ++++++++++++++--------------------------- src/test/test-fs-util.c | 3 +++ src/test/test-id128.c | 5 ++--- 3 files changed, 21 insertions(+), 33 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 1d0533e4f0..9397c4b384 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1082,17 +1082,20 @@ int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, b /* Just like openat(), but adds one thing: optionally returns whether we created the file anew or if * it already existed before. This is only relevant if O_CREAT is set without O_EXCL, and thus will - * shortcut to openat() otherwise */ - - if (!ret_newly_created) - return RET_NERRNO(openat(dirfd, pathname, flags, mode)); + * shortcut to openat() otherwise. + * + * Note that this routine is a bit more strict with symlinks than regular openat() is. If O_NOFOLLOW + * is not specified, then we'll follow the symlink when opening an existing file but we will *not* + * follow it when creating a new one (because that's a terrible UNIX misfeature and generally a + * security hole). */ if (!FLAGS_SET(flags, O_CREAT) || FLAGS_SET(flags, O_EXCL)) { fd = openat(dirfd, pathname, flags, mode); if (fd < 0) return -errno; - *ret_newly_created = FLAGS_SET(flags, O_CREAT); + if (ret_newly_created) + *ret_newly_created = FLAGS_SET(flags, O_CREAT); return fd; } @@ -1100,42 +1103,25 @@ int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, b /* First, attempt to open without O_CREAT/O_EXCL, i.e. open existing file */ fd = openat(dirfd, pathname, flags & ~(O_CREAT | O_EXCL), mode); if (fd >= 0) { - *ret_newly_created = false; + if (ret_newly_created) + *ret_newly_created = false; return fd; } if (errno != ENOENT) return -errno; - /* So the file didn't exist yet, hence create it with O_CREAT/O_EXCL. */ - fd = openat(dirfd, pathname, flags | O_CREAT | O_EXCL, mode); + /* So the file didn't exist yet, hence create it with O_CREAT/O_EXCL/O_NOFOLLOW. */ + fd = openat(dirfd, pathname, flags | O_CREAT | O_EXCL | O_NOFOLLOW, mode); if (fd >= 0) { - *ret_newly_created = true; + if (ret_newly_created) + *ret_newly_created = true; return fd; } if (errno != EEXIST) return -errno; - /* Hmm, so now we got EEXIST? This can indicate two things. First, if the path points to a - * dangling symlink, the first openat() will fail with ENOENT because the symlink is resolved - * and the second openat() will fail with EEXIST because symlinks are not followed when - * O_CREAT|O_EXCL is specified. Let's check for this explicitly and fall back to opening with - * just O_CREAT and assume we're the ones that created the file. */ - - struct stat st; - if (fstatat(dirfd, pathname, &st, AT_SYMLINK_NOFOLLOW) < 0) - return -errno; - - if (S_ISLNK(st.st_mode)) { - fd = openat(dirfd, pathname, flags | O_CREAT, mode); - if (fd < 0) - return -errno; - - *ret_newly_created = true; - return fd; - } - - /* If we're not operating on a symlink, someone might have created the file between the first - * and second call to openat(). Let's try again but with a limit so we don't spin forever. */ + /* Hmm, so now we got EEXIST? Then someone might have created the file between the first and + * second call to openat(). Let's try again but with a limit so we don't spin forever. */ if (--attempts == 0) /* Give up eventually, somebody is playing with us */ return -EEXIST; diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 3da3caf4ab..55202240d3 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -670,6 +670,9 @@ TEST(openat_report_new) { ASSERT_OK_ERRNO(symlinkat("target", tfd, "link")); fd = openat_report_new(tfd, "link", O_RDWR|O_CREAT, 0666, &b); + ASSERT_ERROR(fd, EEXIST); + + fd = openat_report_new(tfd, "target", O_RDWR|O_CREAT, 0666, &b); ASSERT_OK(fd); fd = safe_close(fd); ASSERT_TRUE(b); diff --git a/src/test/test-id128.c b/src/test/test-id128.c index a6ed640bd6..eb3bfec2bb 100644 --- a/src/test/test-id128.c +++ b/src/test/test-id128.c @@ -286,9 +286,8 @@ TEST(id128_at) { ASSERT_OK_ERRNO(unlinkat(tfd, "etc/machine-id", 0)); ASSERT_OK(id128_write_at(tfd, "etc2/machine-id", ID128_FORMAT_PLAIN, id)); ASSERT_OK_ERRNO(unlinkat(tfd, "etc/machine-id", 0)); - ASSERT_OK(id128_write_at(tfd, "etc/hoge-id", ID128_FORMAT_PLAIN, id)); - ASSERT_OK_ERRNO(unlinkat(tfd, "etc/machine-id", 0)); - ASSERT_OK(id128_write_at(tfd, "etc2/hoge-id", ID128_FORMAT_PLAIN, id)); + ASSERT_ERROR(id128_write_at(tfd, "etc/hoge-id", ID128_FORMAT_PLAIN, id), EEXIST); + ASSERT_OK(id128_write_at(tfd, "etc2/machine-id", ID128_FORMAT_PLAIN, id)); /* id128_read_at() */ i = SD_ID128_NULL; /* Not necessary in real code, but for testing that the id is really assigned. */ From 8eeb870971b86f1d7f39b3cb4cc18c8bf594320f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 22 Oct 2024 10:09:20 +0200 Subject: [PATCH 07/12] fileio: port write_string_file() to LabelOps, and thus add WRITE_STRING_FILE_LABEL flag Given that we have the LabelOps abstraction these days, we can teach write_string_file() to use it, which means we can get rid of fileio-label.[ch] as a separate concept. (The only reason that fileio-label.[ch] exists independently of fileio.[ch] was that the former linekd to libselinux potentially, and thus had to be in src/shared/ while the other always was in src/basic/. But the LabelOps vtable provides us with a nice work-around) --- src/basic/fileio.c | 77 +++++++++++++++++++++------ src/basic/fileio.h | 6 +-- src/core/unit-serialize.c | 1 - src/core/unit.c | 3 +- src/debug-generator/debug-generator.c | 4 +- src/hostname/hostnamed.c | 3 +- src/locale/localed-util.c | 1 - src/login/logind-dbus.c | 23 ++++---- src/shared/dropin.c | 4 +- src/shared/fileio-label.c | 21 ++------ src/shared/fileio-label.h | 8 --- src/timedate/timedated.c | 3 +- src/update-done/update-done.c | 4 +- 13 files changed, 84 insertions(+), 74 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index bf1603de0e..891fe065d1 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -18,6 +18,7 @@ #include "fileio.h" #include "fs-util.h" #include "hexdecoct.h" +#include "label.h" #include "log.h" #include "macro.h" #include "mkdir.h" @@ -230,22 +231,40 @@ static int write_string_file_atomic_at( /* Note that we'd really like to use O_TMPFILE here, but can't really, since we want replacement * semantics here, and O_TMPFILE can't offer that. i.e. rename() replaces but linkat() doesn't. */ + mode_t mode = write_string_file_flags_to_mode(flags); + + bool call_label_ops_post = false; + if (FLAGS_SET(flags, WRITE_STRING_FILE_LABEL)) { + r = label_ops_pre(dir_fd, fn, mode); + if (r < 0) + return r; + + call_label_ops_post = true; + } + r = fopen_temporary_at(dir_fd, fn, &f, &p); if (r < 0) - return r; + goto fail; + + if (call_label_ops_post) { + call_label_ops_post = false; + + r = label_ops_post(fileno(f), /* path= */ NULL, /* created= */ true); + if (r < 0) + goto fail; + } r = write_string_stream_full(f, line, flags, ts); if (r < 0) goto fail; - r = fchmod_umask(fileno(f), write_string_file_flags_to_mode(flags)); + r = fchmod_umask(fileno(f), mode); if (r < 0) goto fail; - if (renameat(dir_fd, p, dir_fd, fn) < 0) { - r = -errno; + r = RET_NERRNO(renameat(dir_fd, p, dir_fd, fn)); + if (r < 0) goto fail; - } if (FLAGS_SET(flags, WRITE_STRING_FILE_SYNC)) { /* Sync the rename, too */ @@ -257,7 +276,11 @@ static int write_string_file_atomic_at( return 0; fail: - (void) unlinkat(dir_fd, p, 0); + if (call_label_ops_post) + (void) label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fn, /* created= */ !!f); + + if (f) + (void) unlinkat(dir_fd, p, 0); return r; } @@ -268,9 +291,10 @@ int write_string_file_full( WriteStringFileFlags flags, const struct timespec *ts) { + bool call_label_ops_post = false; _cleanup_fclose_ FILE *f = NULL; _cleanup_close_ int fd = -EBADF; - int q, r; + int r; assert(fn); assert(line); @@ -292,8 +316,17 @@ int write_string_file_full( goto fail; return r; - } else - assert(!ts); + } + + mode_t mode = write_string_file_flags_to_mode(flags); + + if (FLAGS_SET(flags, WRITE_STRING_FILE_LABEL|WRITE_STRING_FILE_CREATE)) { + r = label_ops_pre(dir_fd, fn, mode); + if (r < 0) + goto fail; + + call_label_ops_post = true; + } /* We manually build our own version of fopen(..., "we") that works without O_CREAT and with O_NOFOLLOW if needed. */ fd = openat(dir_fd, fn, O_CLOEXEC|O_NOCTTY | @@ -301,12 +334,20 @@ int write_string_file_full( (FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0) | (FLAGS_SET(flags, WRITE_STRING_FILE_TRUNCATE) ? O_TRUNC : 0) | (FLAGS_SET(flags, WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL) ? O_RDWR : O_WRONLY), - write_string_file_flags_to_mode(flags)); + mode); if (fd < 0) { r = -errno; goto fail; } + if (call_label_ops_post) { + call_label_ops_post = false; + + r = label_ops_post(fd, /* path= */ NULL, /* created= */ true); + if (r < 0) + goto fail; + } + r = take_fdopen_unlocked(&fd, "w", &f); if (r < 0) goto fail; @@ -321,19 +362,21 @@ int write_string_file_full( return 0; fail: + if (call_label_ops_post) + (void) label_ops_post(fd >= 0 ? fd : dir_fd, fd >= 0 ? NULL : fn, /* created= */ true); + if (!(flags & WRITE_STRING_FILE_VERIFY_ON_FAILURE)) return r; f = safe_fclose(f); + fd = safe_close(fd); - /* OK, the operation failed, but let's see if the right - * contents in place already. If so, eat up the error. */ + /* OK, the operation failed, but let's see if the right contents in place already. If so, eat up the + * error. */ + if (verify_file(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) || (flags & WRITE_STRING_FILE_VERIFY_IGNORE_NEWLINE)) > 0) + return 0; - q = verify_file(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) || (flags & WRITE_STRING_FILE_VERIFY_IGNORE_NEWLINE)); - if (q <= 0) - return r; - - return 0; + return r; } int write_string_filef( diff --git a/src/basic/fileio.h b/src/basic/fileio.h index dc514e3ccc..8f54491c12 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -28,11 +28,7 @@ typedef enum { WRITE_STRING_FILE_MODE_0600 = 1 << 10, WRITE_STRING_FILE_MODE_0444 = 1 << 11, WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL = 1 << 12, - - /* And before you wonder, why write_string_file_atomic_label_ts() is a separate function instead of just one - more flag here: it's about linking: we don't want to pull -lselinux into all users of write_string_file() - and friends. */ - + WRITE_STRING_FILE_LABEL = 1 << 13, } WriteStringFileFlags; typedef enum { diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index f96bae19f9..ec1d3df32f 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -4,7 +4,6 @@ #include "bpf-socket-bind.h" #include "bus-util.h" #include "dbus.h" -#include "fileio-label.h" #include "fileio.h" #include "format-util.h" #include "parse-util.h" diff --git a/src/core/unit.c b/src/core/unit.c index 4384e3bfcb..64ce5967f4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -29,7 +29,6 @@ #include "exec-credential.h" #include "execute.h" #include "fd-util.h" -#include "fileio-label.h" #include "fileio.h" #include "format-util.h" #include "id128-util.h" @@ -4620,7 +4619,7 @@ int unit_write_setting(Unit *u, UnitWriteFlags flags, const char *name, const ch return r; } - r = write_string_file_atomic_label(q, wrapped); + r = write_string_file(q, wrapped, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL); if (r < 0) return r; diff --git a/src/debug-generator/debug-generator.c b/src/debug-generator/debug-generator.c index 98670a6c90..664d57d452 100644 --- a/src/debug-generator/debug-generator.c +++ b/src/debug-generator/debug-generator.c @@ -7,7 +7,7 @@ #include "dropin.h" #include "errno-util.h" #include "fd-util.h" -#include "fileio-label.h" +#include "fileio.h" #include "generator.h" #include "initrd-util.h" #include "parse-util.h" @@ -206,7 +206,7 @@ static int process_unit_credentials(const char *credentials_dir) { if (!p) return log_oom(); - r = write_string_file_at_label(AT_FDCWD, p, d, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_MKDIR_0755); + r = write_string_file(p, d, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_MKDIR_0755|WRITE_STRING_FILE_LABEL); if (r < 0) { log_warning_errno(r, "Failed to write unit file '%s' from credential '%s', ignoring: %m", unit, de->d_name); diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index df0ebe8daa..d2444dbf2d 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -19,7 +19,6 @@ #include "env-file-label.h" #include "env-file.h" #include "env-util.h" -#include "fileio-label.h" #include "fileio.h" #include "hostname-setup.h" #include "hostname-util.h" @@ -619,7 +618,7 @@ static int context_write_data_static_hostname(Context *c) { return 0; } - r = write_string_file_atomic_label("/etc/hostname", c->data[PROP_STATIC_HOSTNAME]); + r = write_string_file("/etc/hostname", c->data[PROP_STATIC_HOSTNAME], WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL); if (r < 0) return r; diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 56996596fe..6413288ea3 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -11,7 +11,6 @@ #include "env-file.h" #include "env-util.h" #include "fd-util.h" -#include "fileio-label.h" #include "fileio.h" #include "fs-util.h" #include "kbd-util.h" diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 35549e663a..9582fb47df 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -29,8 +29,8 @@ #include "escape.h" #include "event-util.h" #include "fd-util.h" -#include "fileio-label.h" #include "fileio.h" +#include "fileio-label.h" #include "format-util.h" #include "fs-util.h" #include "logind-action.h" @@ -1593,7 +1593,7 @@ static int trigger_device(Manager *m, sd_device *parent) { static int attach_device(Manager *m, const char *seat, const char *sysfs, sd_bus_error *error) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; - _cleanup_free_ char *rule = NULL, *file = NULL; + _cleanup_free_ char *file = NULL; const char *id_for_seat; int r; @@ -1614,11 +1614,10 @@ static int attach_device(Manager *m, const char *seat, const char *sysfs, sd_bus if (asprintf(&file, "/etc/udev/rules.d/72-seat-%s.rules", id_for_seat) < 0) return -ENOMEM; - if (asprintf(&rule, "TAG==\"seat\", ENV{ID_FOR_SEAT}==\"%s\", ENV{ID_SEAT}=\"%s\"", id_for_seat, seat) < 0) - return -ENOMEM; - - (void) mkdir_p_label("/etc/udev/rules.d", 0755); - r = write_string_file_atomic_label(file, rule); + r = write_string_filef( + file, + WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_MKDIR_0755|WRITE_STRING_FILE_LABEL, + "TAG==\"seat\", ENV{ID_FOR_SEAT}==\"%s\", ENV{ID_SEAT}=\"%s\"", id_for_seat, seat); if (r < 0) return r; @@ -3286,11 +3285,9 @@ static int method_set_reboot_to_boot_loader_menu( if (unlink("/run/systemd/reboot-to-boot-loader-menu") < 0 && errno != ENOENT) return -errno; } else { - char buf[DECIMAL_STR_MAX(uint64_t) + 1]; - - xsprintf(buf, "%" PRIu64, x); /* μs granularity */ - - r = write_string_file_atomic_label("/run/systemd/reboot-to-boot-loader-menu", buf); + r = write_string_filef("/run/systemd/reboot-to-boot-loader-menu", + WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL, + "%" PRIu64, x); /* μs granularity */ if (r < 0) return r; } @@ -3479,7 +3476,7 @@ static int method_set_reboot_to_boot_loader_entry( if (unlink("/run/systemd/reboot-to-boot-loader-entry") < 0 && errno != ENOENT) return -errno; } else { - r = write_string_file_atomic_label("/run/systemd/reboot-boot-to-loader-entry", v); + r = write_string_file("/run/systemd/reboot-boot-to-loader-entry", v, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL); if (r < 0) return r; } diff --git a/src/shared/dropin.c b/src/shared/dropin.c index 3e1eaa6623..2e285f8eb6 100644 --- a/src/shared/dropin.c +++ b/src/shared/dropin.c @@ -12,7 +12,7 @@ #include "dropin.h" #include "escape.h" #include "fd-util.h" -#include "fileio-label.h" +#include "fileio.h" #include "hashmap.h" #include "log.h" #include "macro.h" @@ -87,7 +87,7 @@ int write_drop_in( if (r < 0) return r; - return write_string_file_at_label(AT_FDCWD, p, data, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_MKDIR_0755); + return write_string_file(p, data, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_MKDIR_0755|WRITE_STRING_FILE_LABEL); } int write_drop_in_format( diff --git a/src/shared/fileio-label.c b/src/shared/fileio-label.c index 39de54fa25..69ea82cf03 100644 --- a/src/shared/fileio-label.c +++ b/src/shared/fileio-label.c @@ -6,20 +6,6 @@ #include "fileio.h" #include "selinux-util.h" -int write_string_file_full_label(int atfd, const char *fn, const char *line, WriteStringFileFlags flags, struct timespec *ts) { - int r; - - r = mac_selinux_create_file_prepare_at(atfd, fn, S_IFREG); - if (r < 0) - return r; - - r = write_string_file_full(atfd, fn, line, flags, ts); - - mac_selinux_create_file_clear(); - - return r; -} - int create_shutdown_run_nologin_or_warn(void) { int r; @@ -33,9 +19,10 @@ int create_shutdown_run_nologin_or_warn(void) { * 13 years later we stopped managing /etc/nologin, leaving it for the administrator to manage. */ - r = write_string_file_atomic_label("/run/nologin", - "System is going down. Unprivileged users are not permitted to log in anymore. " - "For technical details, see pam_nologin(8)."); + r = write_string_file("/run/nologin", + "System is going down. Unprivileged users are not permitted to log in anymore. " + "For technical details, see pam_nologin(8).", + WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL); if (r < 0) return log_error_errno(r, "Failed to create /run/nologin: %m"); diff --git a/src/shared/fileio-label.h b/src/shared/fileio-label.h index 5aa5c40224..9fbe0f42e5 100644 --- a/src/shared/fileio-label.h +++ b/src/shared/fileio-label.h @@ -9,12 +9,4 @@ #include "fileio.h" -int write_string_file_full_label(int atfd, const char *fn, const char *line, WriteStringFileFlags flags, struct timespec *ts); -static inline int write_string_file_at_label(int atfd, const char *fn, const char *line, WriteStringFileFlags flags) { - return write_string_file_full_label(atfd, fn, line, flags, /* ts= */ NULL); -} -static inline int write_string_file_atomic_label(const char *fn, const char *line) { - return write_string_file_at_label(AT_FDCWD, fn, line, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC); -} - int create_shutdown_run_nologin_or_warn(void); diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index ea99a77c85..c79bb864df 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -24,7 +24,6 @@ #include "constants.h" #include "daemon-util.h" #include "fd-util.h" -#include "fileio-label.h" #include "fileio.h" #include "fs-util.h" #include "hashmap.h" @@ -395,7 +394,7 @@ static int context_write_data_local_rtc(Context *c) { if (r < 0) return r; - return write_string_file_atomic_label("/etc/adjtime", w); + return write_string_file("/etc/adjtime", w, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL); } static int context_update_ntp_status(Context *c, sd_bus *bus, sd_bus_message *m) { diff --git a/src/update-done/update-done.c b/src/update-done/update-done.c index 436bae639b..277d2860b9 100644 --- a/src/update-done/update-done.c +++ b/src/update-done/update-done.c @@ -5,7 +5,7 @@ #include #include "alloc-util.h" -#include "fileio-label.h" +#include "fileio.h" #include "selinux-util.h" #include "time-util.h" @@ -29,7 +29,7 @@ static int apply_timestamp(const char *path, struct timespec *ts) { timespec_load_nsec(ts)) < 0) return log_oom(); - r = write_string_file_full_label(AT_FDCWD, path, message, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC, ts); + r = write_string_file_full(AT_FDCWD, path, message, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL, ts); if (r == -EROFS) log_debug_errno(r, "Cannot create \"%s\", file system is read-only.", path); else if (r < 0) From aec1262a2e0f7525ad5f755ec0d8f0c183e7aecf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 22 Oct 2024 10:19:03 +0200 Subject: [PATCH 08/12] fileio: port write_string_file_full() to openat_report_new() This brings two benefits: we will label the created file only if it is actually created, and we can correctly delete any file we create again on failure. --- src/basic/fileio.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 891fe065d1..aa1657c118 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -291,7 +291,7 @@ int write_string_file_full( WriteStringFileFlags flags, const struct timespec *ts) { - bool call_label_ops_post = false; + bool call_label_ops_post = false, made_file = false; _cleanup_fclose_ FILE *f = NULL; _cleanup_close_ int fd = -EBADF; int r; @@ -329,21 +329,23 @@ int write_string_file_full( } /* We manually build our own version of fopen(..., "we") that works without O_CREAT and with O_NOFOLLOW if needed. */ - fd = openat(dir_fd, fn, O_CLOEXEC|O_NOCTTY | - (FLAGS_SET(flags, WRITE_STRING_FILE_NOFOLLOW) ? O_NOFOLLOW : 0) | - (FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0) | - (FLAGS_SET(flags, WRITE_STRING_FILE_TRUNCATE) ? O_TRUNC : 0) | - (FLAGS_SET(flags, WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL) ? O_RDWR : O_WRONLY), - mode); + fd = openat_report_new( + dir_fd, fn, O_CLOEXEC | O_NOCTTY | + (FLAGS_SET(flags, WRITE_STRING_FILE_NOFOLLOW) ? O_NOFOLLOW : 0) | + (FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0) | + (FLAGS_SET(flags, WRITE_STRING_FILE_TRUNCATE) ? O_TRUNC : 0) | + (FLAGS_SET(flags, WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL) ? O_RDWR : O_WRONLY), + mode, + &made_file); if (fd < 0) { - r = -errno; + r = fd; goto fail; } if (call_label_ops_post) { call_label_ops_post = false; - r = label_ops_post(fd, /* path= */ NULL, /* created= */ true); + r = label_ops_post(fd, /* path= */ NULL, made_file); if (r < 0) goto fail; } @@ -363,7 +365,10 @@ int write_string_file_full( fail: if (call_label_ops_post) - (void) label_ops_post(fd >= 0 ? fd : dir_fd, fd >= 0 ? NULL : fn, /* created= */ true); + (void) label_ops_post(fd >= 0 ? fd : dir_fd, fd >= 0 ? NULL : fn, made_file); + + if (made_file) + (void) unlinkat(dir_fd, fn, 0); if (!(flags & WRITE_STRING_FILE_VERIFY_ON_FAILURE)) return r; From 3a7ae4ba62baf0ccccc90d4f95055609124677b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 22 Oct 2024 10:25:26 +0200 Subject: [PATCH 09/12] shared: get rid of fileio-label.[ch] Move the renaming function to reboot-util.h (since it writes out /run/nologin at shutdown), and let's get rid of fileio-label.[ch] now that it serves no purpose anymore. --- src/login/logind-dbus.c | 1 - src/shared/fileio-label.c | 30 ------------------------------ src/shared/fileio-label.h | 12 ------------ src/shared/meson.build | 1 - src/shared/reboot-util.c | 23 +++++++++++++++++++++++ src/shared/reboot-util.h | 4 ++++ src/user-sessions/user-sessions.c | 5 ++--- 7 files changed, 29 insertions(+), 47 deletions(-) delete mode 100644 src/shared/fileio-label.c delete mode 100644 src/shared/fileio-label.h diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 9582fb47df..5de882e42b 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -30,7 +30,6 @@ #include "event-util.h" #include "fd-util.h" #include "fileio.h" -#include "fileio-label.h" #include "format-util.h" #include "fs-util.h" #include "logind-action.h" diff --git a/src/shared/fileio-label.c b/src/shared/fileio-label.c deleted file mode 100644 index 69ea82cf03..0000000000 --- a/src/shared/fileio-label.c +++ /dev/null @@ -1,30 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ - -#include - -#include "fileio-label.h" -#include "fileio.h" -#include "selinux-util.h" - -int create_shutdown_run_nologin_or_warn(void) { - int r; - - /* This is used twice: once in systemd-user-sessions.service, in order to block logins when we - * actually go down, and once in systemd-logind.service when shutdowns are scheduled, and logins are - * to be turned off a bit in advance. We use the same wording of the message in both cases. - * - * Traditionally, there was only /etc/nologin, and we managed that. Then, in PAM 1.1 - * support for /run/nologin was added as alternative - * (https://github.com/linux-pam/linux-pam/commit/e9e593f6ddeaf975b7fe8446d184e6bc387d450b). - * 13 years later we stopped managing /etc/nologin, leaving it for the administrator to manage. - */ - - r = write_string_file("/run/nologin", - "System is going down. Unprivileged users are not permitted to log in anymore. " - "For technical details, see pam_nologin(8).", - WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL); - if (r < 0) - return log_error_errno(r, "Failed to create /run/nologin: %m"); - - return 0; -} diff --git a/src/shared/fileio-label.h b/src/shared/fileio-label.h deleted file mode 100644 index 9fbe0f42e5..0000000000 --- a/src/shared/fileio-label.h +++ /dev/null @@ -1,12 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -#pragma once - -#include - -/* These functions are split out of fileio.h (and not for example just flags to the functions they wrap) in order to - * optimize linking: This way, -lselinux is needed only for the callers of these functions that need selinux, but not - * for all */ - -#include "fileio.h" - -int create_shutdown_run_nologin_or_warn(void); diff --git a/src/shared/meson.build b/src/shared/meson.build index 1141efa453..af9ef74b32 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -72,7 +72,6 @@ shared_sources = files( 'extension-util.c', 'fdset.c', 'fido2-util.c', - 'fileio-label.c', 'find-esp.c', 'firewall-util-nft.c', 'firewall-util.c', diff --git a/src/shared/reboot-util.c b/src/shared/reboot-util.c index b1c1aa75e6..903e19efae 100644 --- a/src/shared/reboot-util.c +++ b/src/shared/reboot-util.c @@ -204,3 +204,26 @@ bool kexec_loaded(void) { return s[0] == '1'; } + +int create_shutdown_run_nologin_or_warn(void) { + int r; + + /* This is used twice: once in systemd-user-sessions.service, in order to block logins when we + * actually go down, and once in systemd-logind.service when shutdowns are scheduled, and logins are + * to be turned off a bit in advance. We use the same wording of the message in both cases. + * + * Traditionally, there was only /etc/nologin, and we managed that. Then, in PAM 1.1 + * support for /run/nologin was added as alternative + * (https://github.com/linux-pam/linux-pam/commit/e9e593f6ddeaf975b7fe8446d184e6bc387d450b). + * 13 years later we stopped managing /etc/nologin, leaving it for the administrator to manage. + */ + + r = write_string_file("/run/nologin", + "System is going down. Unprivileged users are not permitted to log in anymore. " + "For technical details, see pam_nologin(8).", + WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_LABEL); + if (r < 0) + return log_error_errno(r, "Failed to create /run/nologin: %m"); + + return 0; +} diff --git a/src/shared/reboot-util.h b/src/shared/reboot-util.h index 2ac478f784..92189bee88 100644 --- a/src/shared/reboot-util.h +++ b/src/shared/reboot-util.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include + bool reboot_parameter_is_valid(const char *parameter); int update_reboot_parameter_and_warn(const char *parameter, bool keep); @@ -16,3 +18,5 @@ int reboot_with_parameter(RebootFlags flags); bool shall_restore_state(void); bool kexec_loaded(void); + +int create_shutdown_run_nologin_or_warn(void); diff --git a/src/user-sessions/user-sessions.c b/src/user-sessions/user-sessions.c index 58054f89fb..7d84d6b540 100644 --- a/src/user-sessions/user-sessions.c +++ b/src/user-sessions/user-sessions.c @@ -6,11 +6,10 @@ #include #include -#include "fileio.h" -#include "fileio-label.h" #include "fs-util.h" -#include "main-func.h" #include "log.h" +#include "main-func.h" +#include "reboot-util.h" #include "selinux-util.h" #include "string-util.h" From 4e4ed4b64ddacc22d2cab6e0e889bfbfc9314435 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 22 Oct 2024 17:47:16 +0200 Subject: [PATCH 10/12] label: add missing assert() to label_ops_set() --- src/basic/label.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/basic/label.c b/src/basic/label.c index 47d1967825..18f6214fad 100644 --- a/src/basic/label.c +++ b/src/basic/label.c @@ -4,10 +4,13 @@ #include #include "label.h" +#include "macro.h" static const LabelOps *label_ops = NULL; int label_ops_set(const LabelOps *ops) { + assert(ops); + if (label_ops) return -EBUSY; From 4ffecbbbee17bf7a71b54758d4792615608df2ec Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 22 Oct 2024 17:47:22 +0200 Subject: [PATCH 11/12] label: move label_ops_reset() up a bit Let#s move it close to label_ops_set(), since it is somewhat symmetric to it. --- src/basic/label.c | 8 ++++---- src/basic/label.h | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/basic/label.c b/src/basic/label.c index 18f6214fad..6bae653188 100644 --- a/src/basic/label.c +++ b/src/basic/label.c @@ -18,6 +18,10 @@ int label_ops_set(const LabelOps *ops) { return 0; } +void label_ops_reset(void) { + label_ops = NULL; +} + int label_ops_pre(int dir_fd, const char *path, mode_t mode) { if (!label_ops || !label_ops->pre) return 0; @@ -31,7 +35,3 @@ int label_ops_post(int dir_fd, const char *path, bool created) { return label_ops->post(dir_fd, path, created); } - -void label_ops_reset(void) { - label_ops = NULL; -} diff --git a/src/basic/label.h b/src/basic/label.h index eb851bcb4f..d001307a4f 100644 --- a/src/basic/label.h +++ b/src/basic/label.h @@ -10,8 +10,7 @@ typedef struct LabelOps { } LabelOps; int label_ops_set(const LabelOps *label_ops); +void label_ops_reset(void); int label_ops_pre(int dir_fd, const char *path, mode_t mode); int label_ops_post(int dir_fd, const char *path, bool created); - -void label_ops_reset(void); From b9633ebb2a714ada7dc9747ec53af02c3627d05c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 22 Oct 2024 17:49:40 +0200 Subject: [PATCH 12/12] fs-util: move attempts counter in openat_report_new() into loop --- src/basic/fs-util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 9397c4b384..7c88e9fef7 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1077,7 +1077,6 @@ int open_mkdir_at_full(int dirfd, const char *path, int flags, XOpenFlags xopen_ } int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, bool *ret_newly_created) { - unsigned attempts = 7; int fd; /* Just like openat(), but adds one thing: optionally returns whether we created the file anew or if @@ -1099,7 +1098,7 @@ int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, b return fd; } - for (;;) { + for (unsigned attempts = 7;;) { /* First, attempt to open without O_CREAT/O_EXCL, i.e. open existing file */ fd = openat(dirfd, pathname, flags & ~(O_CREAT | O_EXCL), mode); if (fd >= 0) {