tmpfiles: skip redundant label writes to avoid unnecessary timestamp changes

When systemd-tmpfiles processes a 'z' (relabel) entry, fd_set_perms()
unconditionally calls label_fix_full() even when mode, owner, and group
already match. This causes setfilecon_raw() (SELinux) or xsetxattr() (SMACK)
to write the security label even if it is already correct, which on some
kernels updates the file's timestamps unnecessarily.

Fix this by comparing the current label with the desired label before
writing, and skipping the write when they already match. This is consistent
with how fd_set_perms() already skips chmod/chown when the values are
unchanged.

diff --git a/src/basic/xattr-util.c b/src/basic/xattr-util.c
index e77d0bc8e84efe4a5677cf2572442db400a4fde8..fb886f6ae82c56a547bab11ccc7181a02d97e237 100644 (file)
--- a/src/basic/xattr-util.c
+++ b/src/basic/xattr-util.c
@@ -301,6 +301,18 @@ int xsetxattr_full(
         if (size == SIZE_MAX)
                 size = strlen(value);
 
+        /* Skip the write if the xattr already has the correct value, to avoid
+         * unnecessary timestamp changes on the file. Only do this for plain
+         * replace mode (xattr_flags == 0) — XATTR_CREATE callers expect
+         * -EEXIST when the xattr already exists. */
+        _cleanup_free_ char *old_value = NULL;
+        size_t old_size;
+
+        if (xattr_flags == 0 &&
+            getxattr_at_malloc(fd, path, name, at_flags, &old_value, &old_size) >= 0 &&
+            memcmp_nn(old_value, old_size, value, size) == 0)
+                return 0;
+
         if (have_xattrat && !isempty(path)) {
                 struct xattr_args args = {
                         .value = PTR_TO_UINT64(value),

diff --git a/src/shared/selinux-util.c b/src/shared/selinux-util.c
index 1049044f87bbd2f4ca69e8e9c5eeb568ceef1d39..6b8e318a4c8d0f17b425d9bd14ef61a6365a774b 100644 (file)
--- a/src/shared/selinux-util.c
+++ b/src/shared/selinux-util.c
@@ -360,6 +360,19 @@ void mac_selinux_disable_logging(void) {
 }
 
 #if HAVE_SELINUX
+static int setfilecon_idempotent(int fd, const char *context) {
+        _cleanup_freecon_ char *oldcon = NULL;
+
+        assert(fd >= 0);
+        assert(context);
+
+        /* Read current context via /proc/self/fd/ so this works for O_PATH fds too */
+        if (sym_getfilecon_raw(FORMAT_PROC_FD_PATH(fd), &oldcon) >= 0 && streq_ptr(context, oldcon))
+                return 0; /* Already correct */
+
+        return RET_NERRNO(sym_setfilecon_raw(FORMAT_PROC_FD_PATH(fd), context));
+}
+
 static int selinux_fix_fd(
                 int fd,
                 const char *label_path,
@@ -389,25 +402,19 @@ static int selinux_fix_fd(
                 return log_selinux_enforcing_errno(errno, "Unable to lookup intended SELinux security context of %s: %m", label_path);
         }
 
-        r = RET_NERRNO(sym_setfilecon_raw(FORMAT_PROC_FD_PATH(fd), fcon));
-        if (r < 0) {
-                /* If the FS doesn't support labels, then exit without warning */
-                if (ERRNO_IS_NOT_SUPPORTED(r))
-                        return 0;
-
-                /* It the FS is read-only and we were told to ignore failures caused by that, suppress error */
-                if (r == -EROFS && (flags & LABEL_IGNORE_EROFS))
-                        return 0;
+        r = setfilecon_idempotent(fd, fcon);
+        if (r >= 0)
+                return 0;
 
-                /* If the old label is identical to the new one, suppress any kind of error */
-                _cleanup_freecon_ char *oldcon = NULL;
-                if (sym_getfilecon_raw(FORMAT_PROC_FD_PATH(fd), &oldcon) >= 0 && streq_ptr(fcon, oldcon))
-                        return 0;
+        /* If the FS doesn't support labels, then exit without warning */
+        if (ERRNO_IS_NOT_SUPPORTED(r))
+                return 0;
 
-                return log_selinux_enforcing_errno(r, "Unable to fix SELinux security context of %s: %m", label_path);
-        }
+        /* If the FS is read-only and we were told to ignore failures caused by that, suppress error */
+        if (r == -EROFS && (flags & LABEL_IGNORE_EROFS))
+                return 0;
 
-        return 0;
+        return log_selinux_enforcing_errno(r, "Unable to fix SELinux security context of %s: %m", label_path);
 }
 #endif
 
@@ -495,8 +502,9 @@ int mac_selinux_apply_fd(int fd, const char *path, const char *label) {
 
         assert(label);
 
-        if (sym_setfilecon_raw(FORMAT_PROC_FD_PATH(fd), label) < 0)
-                return log_selinux_enforcing_errno(errno, "Failed to set SELinux security context %s on path %s: %m", label, strna(path));
+        r = setfilecon_idempotent(fd, label);
+        if (r < 0)
+                return log_selinux_enforcing_errno(r, "Failed to set SELinux security context %s on path %s: %m", label, strna(path));
 #endif
         return 0;
 }

diff --git a/src/shared/smack-util.c b/src/shared/smack-util.c
index 6faec02a9f8eeafcb7a26b35a34685bf4a988484..e1dbf8686edda12f5966bdd03517cccdf51a627c 100644 (file)
--- a/src/shared/smack-util.c
+++ b/src/shared/smack-util.c
@@ -149,16 +149,8 @@ static int smack_fix_fd(
                                                                     to ignore failures caused by that,
                                                                     suppress error */
                 return 0;
-        if (r < 0) {
-                /* If the old label is identical to the new one, suppress any kind of error */
-                _cleanup_free_ char *old_label = NULL;
-
-                if (fgetxattr_malloc(fd, "security.SMACK64", &old_label, /* ret_size= */ NULL) >= 0 &&
-                    streq(old_label, label))
-                        return 0;
-
+        if (r < 0)
                 return log_debug_errno(r, "Unable to fix SMACK label of '%s': %m", label_path);
-        }
 
         return 0;
 }