Merge pull request #32747 from YHNdnzj/tmpfiles-ret-gather

tmpfiles: some cleanups/follow-up

1 files changed, 58 insertions, 60 deletions

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 722dff100a..3ade4ee32e 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -67,6 +67,7 @@
 #include "string-table.h"
 #include "string-util.h"
 #include "strv.h"
+#include "sysctl-util.h"
 #include "terminal-util.h"
 #include "umask-util.h"
 #include "user-util.h"
@@ -932,37 +933,35 @@ finish:
         return r;
 }
 
-static bool dangerous_hardlinks(void) {
-        _cleanup_free_ char *value = NULL;
+static bool hardlinks_protected(void) {
         static int cached = -1;
         int r;
 
         /* Check whether the fs.protected_hardlinks sysctl is on. If we can't determine it we assume its off,
-         * as that's what the upstream default is. */
+         * as that's what the kernel default is.
+         * Note that we ship 50-default.conf where it is enabled, but better be safe than sorry. */
 
         if (cached >= 0)
                 return cached;
 
-        r = read_one_line_file("/proc/sys/fs/protected_hardlinks", &value);
-        if (r < 0) {
-                log_debug_errno(r, "Failed to read fs.protected_hardlinks sysctl: %m");
-                return true;
-        }
+        _cleanup_free_ char *value = NULL;
 
-        r = parse_boolean(value);
+        r = sysctl_read("fs/protected_hardlinks", &value);
         if (r < 0) {
-                log_debug_errno(r, "Failed to parse fs.protected_hardlinks sysctl: %m");
-                return true;
+                log_debug_errno(r, "Failed to read fs.protected_hardlinks sysctl, assuming disabled: %m");
+                return false;
         }
 
-        cached = r == 0;
-        return cached;
+        cached = parse_boolean(value);
+        if (cached < 0)
+                log_debug_errno(cached, "Failed to parse fs.protected_hardlinks sysctl, assuming disabled: %m");
+        return cached > 0;
 }
 
 static bool hardlink_vulnerable(const struct stat *st) {
         assert(st);
 
-        return !S_ISDIR(st->st_mode) && st->st_nlink > 1 && dangerous_hardlinks();
+        return !S_ISDIR(st->st_mode) && st->st_nlink > 1 && !hardlinks_protected();
 }
 
 static mode_t process_mask_perms(mode_t mode, mode_t current) {
@@ -2512,12 +2511,13 @@ static int item_do(
                 fdaction_t action) {
 
         struct stat st;
-        int r = 0, q = 0;
+        int r;
 
         assert(c);
         assert(i);
-        assert(path);
         assert(fd >= 0);
+        assert(path);
+        assert(action);
 
         if (fstat(fd, &st) < 0) {
                 r = log_error_errno(errno, "fstat() on file failed: %m");
@@ -2534,37 +2534,35 @@ static int item_do(
                  * reading the directory content. */
                 d = opendir(FORMAT_PROC_FD_PATH(fd));
                 if (!d) {
-                        log_error_errno(errno, "Failed to opendir() '%s': %m", FORMAT_PROC_FD_PATH(fd));
-                        if (r == 0)
-                                r = -errno;
+                        RET_GATHER(r, log_error_errno(errno, "Failed to opendir() '%s': %m", FORMAT_PROC_FD_PATH(fd)));
                         goto finish;
                 }
 
-                FOREACH_DIRENT_ALL(de, d, q = -errno; goto finish) {
-                        int de_fd;
+                FOREACH_DIRENT_ALL(de, d, RET_GATHER(r, -errno); goto finish) {
+                        _cleanup_close_ int de_fd = -EBADF;
+                        _cleanup_free_ char *de_path = NULL;
 
                         if (dot_or_dot_dot(de->d_name))
                                 continue;
 
                         de_fd = openat(fd, de->d_name, O_NOFOLLOW|O_CLOEXEC|O_PATH);
                         if (de_fd < 0) {
-                                if (errno != -ENOENT)
-                                        q = log_error_errno(errno, "Failed to open file '%s': %m", de->d_name);
-                        } else {
-                                _cleanup_free_ char *de_path = NULL;
-
-                                de_path = path_join(path, de->d_name);
-                                if (!de_path)
-                                        q = log_oom();
-                                else
-                                        /* Pass ownership of dirent fd over */
-                                        q = item_do(c, i, de_fd, de_path, CREATION_EXISTING, action);
+                                if (errno != ENOENT)
+                                        RET_GATHER(r, log_error_errno(errno, "Failed to open file '%s': %m", de->d_name));
+                                continue;
+                        }
+
+                        de_path = path_join(path, de->d_name);
+                        if (!de_path) {
+                                r = log_oom();
+                                goto finish;
                         }
 
-                        if (q < 0 && r == 0)
-                                r = q;
+                        /* Pass ownership of dirent fd over */
+                        RET_GATHER(r, item_do(c, i, TAKE_FD(de_fd), de_path, CREATION_EXISTING, action));
                 }
         }
+
 finish:
         safe_close(fd);
         return r;
@@ -2574,21 +2572,22 @@ static int glob_item(Context *c, Item *i, action_t action) {
         _cleanup_globfree_ glob_t g = {
                 .gl_opendir = (void *(*)(const char *)) opendir_nomod,
         };
-        int r = 0, k;
+        int r;
 
         assert(c);
         assert(i);
+        assert(action);
 
-        k = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g);
-        if (k < 0 && k != -ENOENT)
-                return log_error_errno(k, "glob(%s) failed: %m", i->path);
+        r = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g);
+        if (r == -ENOENT)
+                return 0;
+        if (r < 0)
+                return log_error_errno(r, "Failed to glob '%s': %m", i->path);
 
-        STRV_FOREACH(fn, g.gl_pathv) {
+        r = 0;
+        STRV_FOREACH(fn, g.gl_pathv)
                 /* We pass CREATION_EXISTING here, since if we are globbing for it, it always has to exist */
-                k = action(c, i, *fn, CREATION_EXISTING);
-                if (k < 0 && r == 0)
-                        r = k;
-        }
+                RET_GATHER(r, action(c, i, *fn, CREATION_EXISTING));
 
         return r;
 }
@@ -2601,34 +2600,33 @@ static int glob_item_recursively(
         _cleanup_globfree_ glob_t g = {
                 .gl_opendir = (void *(*)(const char *)) opendir_nomod,
         };
-        int r = 0, k;
+        int r;
 
-        k = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g);
-        if (k < 0 && k != -ENOENT)
-                return log_error_errno(k, "glob(%s) failed: %m", i->path);
+        assert(c);
+        assert(i);
+        assert(action);
+
+        r = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g);
+        if (r == -ENOENT)
+                return 0;
+        if (r < 0)
+                return log_error_errno(r, "Failed to glob '%s': %m", i->path);
 
+        r = 0;
         STRV_FOREACH(fn, g.gl_pathv) {
                 _cleanup_close_ int fd = -EBADF;
 
-                /* Make sure we won't trigger/follow file object (such as
-                 * device nodes, automounts, ...) pointed out by 'fn' with
-                 * O_PATH. Note, when O_PATH is used, flags other than
+                /* Make sure we won't trigger/follow file object (such as device nodes, automounts, ...)
+                 * pointed out by 'fn' with O_PATH. Note, when O_PATH is used, flags other than
                  * O_CLOEXEC, O_DIRECTORY, and O_NOFOLLOW are ignored. */
 
                 fd = open(*fn, O_CLOEXEC|O_NOFOLLOW|O_PATH);
                 if (fd < 0) {
-                        log_error_errno(errno, "Opening '%s' failed: %m", *fn);
-                        if (r == 0)
-                                r = -errno;
+                        RET_GATHER(r, log_error_errno(errno, "Failed to open '%s': %m", *fn));
                         continue;
                 }
 
-                k = item_do(c, i, fd, *fn, CREATION_EXISTING, action);
-                if (k < 0 && r == 0)
-                        r = k;
-
-                /* we passed fd ownership to the previous call */
-                fd = -EBADF;
+                RET_GATHER(r, item_do(c, i, TAKE_FD(fd), *fn, CREATION_EXISTING, action));
         }
 
         return r;
@@ -2659,7 +2657,7 @@ static int rm_if_wrong_type_safe(
         r = fstatat_harder(parent_fd, name, &st, flags, REMOVE_CHMOD | REMOVE_CHMOD_RESTORE);
         if (r < 0) {
                 (void) fd_get_path(parent_fd, &parent_name);
-                return log_full_errno(r == -ENOENT? LOG_DEBUG : LOG_ERR, r,
+                return log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_ERR, r,
                                       "Failed to stat \"%s/%s\": %m", parent_name ?: "...", name);
         }