tmpfiles: use fd_get_path() even less excessively

A follow-up for commit 9d874aec451b591401d9b14cf8743b9d179159b2.

This patch makes "path" parameter mandatory in fd_set_*() helpers removing the
need to use fd_get_path() when NULL was passed. The caller is supposed to pass
the fd anyway so assuming that it also knows the path should be safe.

Actually, the only case where this was useful (or used) was when we were
walking through directory trees (in item_do()). But even in those cases the
paths could be constructed trivially, which is still better than relying on
fd_get_path() (which is an ugly API).

A very succinct test case is also added for 'z/Z' operators so the code dealing
with recursive operators is tested minimally.

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 48c0fb6..b91b212 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -771,20 +771,11 @@ static bool hardlink_vulnerable(const struct stat *st) {
 }
 
 static int fd_set_perms(Item *i, int fd, const char *path, const struct stat *st) {
-        _cleanup_free_ char *buffer = NULL;
         struct stat stbuf;
-        int r;
 
         assert(i);
         assert(fd);
-
-        if (!path) {
-                r = fd_get_path(fd, &buffer);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to get path: %m");
-
-                path = buffer;
-        }
+        assert(path);
 
         if (!i->mode_set && !i->uid_set && !i->gid_set)
                 goto shortcut;
@@ -944,20 +935,11 @@ static int parse_xattrs_from_arg(Item *i) {
 
 static int fd_set_xattrs(Item *i, int fd, const char *path, const struct stat *st) {
         char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
-        _cleanup_free_ char *buffer = NULL;
         char **name, **value;
-        int r;
 
         assert(i);
         assert(fd);
-
-        if (!path) {
-                r = fd_get_path(fd, &buffer);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to get path: %m");
-
-                path = buffer;
-        }
+        assert(path);
 
         xsprintf(procfs_path, "/proc/self/fd/%i", fd);
 
@@ -1052,19 +1034,11 @@ static int fd_set_acls(Item *item, int fd, const char *path, const struct stat *
         int r = 0;
 #if HAVE_ACL
         char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
-        _cleanup_free_ char *buffer = NULL;
         struct stat stbuf;
 
         assert(item);
         assert(fd);
-
-        if (!path) {
-                r = fd_get_path(fd, &buffer);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to get path: %m");
-
-                path = buffer;
-        }
+        assert(path);
 
         if (!st) {
                 if (fstat(fd, &stbuf) < 0)
@@ -1221,22 +1195,17 @@ static int parse_attribute_from_arg(Item *item) {
 
 static int fd_set_attribute(Item *item, int fd, const char *path, const struct stat *st) {
         _cleanup_close_ int procfs_fd = -1;
-        _cleanup_free_ char *buffer = NULL;
         struct stat stbuf;
         unsigned f;
         int r;
 
+        assert(item);
+        assert(fd);
+        assert(path);
+
         if (!item->attribute_set || item->attribute_mask == 0)
                 return 0;
 
-        if (!path) {
-                r = fd_get_path(fd, &buffer);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to get path: %m");
-
-                path = buffer;
-        }
-
         if (!st) {
                 if (fstat(fd, &stbuf) < 0)
                         return log_error_errno(errno, "fstat(%s) failed: %m", path);
@@ -1823,6 +1792,7 @@ static int item_do(Item *i, int fd, const char *path, fdaction_t action) {
         int r = 0, q;
 
         assert(i);
+        assert(path);
         assert(fd >= 0);
 
         if (fstat(fd, &st) < 0) {
@@ -1860,9 +1830,16 @@ static int item_do(Item *i, int fd, const char *path, fdaction_t action) {
                         de_fd = openat(fd, de->d_name, O_NOFOLLOW|O_CLOEXEC|O_PATH);
                         if (de_fd < 0)
                                 q = log_error_errno(errno, "Failed to open() file '%s': %m", de->d_name);
-                        else
-                                /* Pass ownership of dirent fd over */
-                                q = item_do(i, de_fd, NULL, action);
+                        else {
+                                _cleanup_free_ char *de_path = NULL;
+
+                                de_path = path_join(NULL, path, de->d_name);
+                                if (!de_path)
+                                        q = log_oom();
+                                else
+                                        /* Pass ownership of dirent fd over */
+                                        q = item_do(i, de_fd, de_path, action);
+                        }
 
                         if (q < 0 && r == 0)
                                 r = q;

diff --git a/test/TEST-22-TMPFILES/test-05.sh b/test/TEST-22-TMPFILES/test-05.sh
new file mode 100755 (executable)
index 0000000..13c4ac8
--- /dev/null
+++ b/test/TEST-22-TMPFILES/test-05.sh
@@ -0,0 +1,45 @@
+#! /bin/bash
+
+set -e
+set -x
+
+rm -fr /tmp/{z,Z}
+mkdir  /tmp/{z,Z}
+
+#
+# 'z'
+#
+mkdir /tmp/z/d{1,2}
+touch /tmp/z/f1 /tmp/z/d1/f11 /tmp/z/d2/f21
+
+systemd-tmpfiles --create - <<EOF
+z     /tmp/z/f1    0755 daemon daemon - -
+z     /tmp/z/d1    0755 daemon daemon - -
+EOF
+
+test $(stat -c %U:%G /tmp/z/f1) = "daemon:daemon"
+test $(stat -c %U:%G /tmp/z/d1) = "daemon:daemon"
+test $(stat -c %U:%G /tmp/z/d1/f11) = "root:root"
+
+systemd-tmpfiles --create - <<EOF
+z     /tmp/z/d2/*    0755 daemon daemon - -
+EOF
+
+test $(stat -c %U:%G /tmp/z/d2/f21) = "daemon:daemon"
+
+#
+# 'Z'
+#
+mkdir /tmp/Z/d1 /tmp/Z/d1/d11
+touch /tmp/Z/f1 /tmp/Z/d1/f11 /tmp/Z/d1/d11/f111
+
+systemd-tmpfiles --create - <<EOF
+Z     /tmp/Z/f1    0755 daemon daemon - -
+Z     /tmp/Z/d1    0755 daemon daemon - -
+EOF
+
+test $(stat -c %U:%G /tmp/Z/f1) = "daemon:daemon"
+test $(stat -c %U:%G /tmp/Z/d1) = "daemon:daemon"
+test $(stat -c %U:%G /tmp/Z/d1/d11) = "daemon:daemon"
+test $(stat -c %U:%G /tmp/Z/d1/f11) = "daemon:daemon"
+test $(stat -c %U:%G /tmp/Z/d1/d11/f111) = "daemon:daemon"