From: Mike Yuan <me@yhndnzj.com>
Date: Wed, 31 Jul 2024 21:45:16 +0000 (+0200)
Subject: core/exec-credential: do not use unlink_and_free for relative path under dfd
X-Git-Tag: v257-rc1~740^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F33918%2Fhead;p=thirdparty%2Fsystemd.git

core/exec-credential: do not use unlink_and_free for relative path under dfd
---

diff --git a/src/core/exec-credential.c b/src/core/exec-credential.c
index 695c820f9c8..7772a929f02 100644
--- a/src/core/exec-credential.c
+++ b/src/core/exec-credential.c
@@ -310,7 +310,7 @@ static int write_credential(
                 gid_t gid,
                 bool ownership_ok) {
 
-        _cleanup_(unlink_and_freep) char *tmp = NULL;
+        _cleanup_free_ char *tmp = NULL;
         _cleanup_close_ int fd = -EBADF;
         int r;
 
@@ -323,42 +323,43 @@ static int write_credential(
                 return r;
 
         fd = openat(dfd, tmp, O_CREAT|O_RDWR|O_CLOEXEC|O_EXCL|O_NOFOLLOW|O_NOCTTY, 0600);
-        if (fd < 0) {
-                tmp = mfree(tmp);
+        if (fd < 0)
                 return -errno;
-        }
 
         r = loop_write(fd, data, size);
         if (r < 0)
-                return r;
+                goto fail;
 
-        if (fchmod(fd, 0400) < 0) /* Take away "w" bit */
-                return -errno;
+        r = RET_NERRNO(fchmod(fd, 0400)); /* Take away "w" bit */
+        if (r < 0)
+                goto fail;
 
         if (uid_is_valid(uid) && uid != getuid()) {
                 r = fd_add_uid_acl_permission(fd, uid, ACL_READ);
                 if (r < 0) {
-                        if (!ERRNO_IS_NOT_SUPPORTED(r) && !ERRNO_IS_PRIVILEGE(r))
-                                return r;
-
-                        if (!ownership_ok) /* Ideally we use ACLs, since we can neatly express what we want
-                                            * to express: that the user gets read access and nothing
-                                            * else. But if the backing fs can't support that (e.g. ramfs)
-                                            * then we can use file ownership instead. But that's only safe if
-                                            * we can then re-mount the whole thing read-only, so that the
-                                            * user can no longer chmod() the file to gain write access. */
-                                return r;
-
-                        if (fchown(fd, uid, gid) < 0)
-                                return -errno;
+                        /* Ideally we use ACLs, since we can neatly express what we want to express:
+                         * the user gets read access and nothing else. But if the backing fs can't
+                         * support that (e.g. ramfs), then we can use file ownership instead. But that's
+                         * only safe if we can then re-mount the whole thing read-only, so that the user
+                         * can no longer chmod() the file to gain write access. */
+                        if (!ownership_ok || (!ERRNO_IS_NOT_SUPPORTED(r) && !ERRNO_IS_PRIVILEGE(r)))
+                                goto fail;
+
+                        r = RET_NERRNO(fchown(fd, uid, gid));
+                        if (r < 0)
+                                goto fail;
                 }
         }
 
-        if (renameat(dfd, tmp, dfd, id) < 0)
-                return -errno;
+        r = RET_NERRNO(renameat(dfd, tmp, dfd, id));
+        if (r < 0)
+                goto fail;
 
-        tmp = mfree(tmp);
         return 0;
+
+fail:
+        (void) unlinkat(dfd, tmp, /* flags = */ 0);
+        return r;
 }
 
 typedef enum CredentialSearchPath {