From: Lennart Poettering Date: Thu, 4 Sep 2025 16:24:58 +0000 (+0200) Subject: tmpfile-util: introduce new CLEANUP_TMPFILE_AT() API X-Git-Tag: v259-rc1~549 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=444af9538f465f893c4d6bb5b4a7fad6c17b15a6;p=thirdparty%2Fsystemd.git tmpfile-util: introduce new CLEANUP_TMPFILE_AT() API This should allow us to get rid of a bunch of "fail:" labels, because we can clean up tmpfiles relative to some atfd this way. This only ports over a small number of potential users, but there's more work to be done. --- diff --git a/src/basic/env-file.c b/src/basic/env-file.c index 968cd756bdc..fda52d08ecc 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -650,14 +650,16 @@ int write_env_file(int dir_fd, const char *fname, char **headers, char **l, Writ } r = fopen_tmpfile_linkable_at(dir_fd, fname, O_WRONLY|O_CLOEXEC, &p, &f); - if (call_label_ops_post) - RET_GATHER(r, label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fname, /* created= */ !!f)); + int k = call_label_ops_post ? label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fname, /* created= */ !!f) : 0; if (r < 0) return r; + CLEANUP_TMPFILE_AT(dir_fd, p); + if (k < 0) + return k; r = fchmod_umask(fileno(f), 0644); if (r < 0) - goto fail; + return r; STRV_FOREACH(i, headers) { assert(isempty(*i) || startswith(*i, "#")); @@ -670,15 +672,11 @@ int write_env_file(int dir_fd, const char *fname, char **headers, char **l, Writ r = flink_tmpfile_at(f, dir_fd, p, fname, LINK_TMPFILE_REPLACE|LINK_TMPFILE_SYNC); if (r < 0) - goto fail; - - return 0; + return r; -fail: - if (p) - (void) unlinkat(dir_fd, p, 0); + p = mfree(p); /* disarm CLEANUP_TMPFILE_AT() */ - return r; + return 0; } int write_vconsole_conf(int dir_fd, const char *fname, char **l) { diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 97f92213da5..a2fb5c7649c 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -240,24 +240,28 @@ static int write_string_file_atomic_at( } r = fopen_temporary_at(dir_fd, fn, &f, &p); - if (call_label_ops_post) - /* If fopen_temporary_at() failed in the above, propagate the error code, and ignore failures - * in label_ops_post(). */ - RET_GATHER(r, label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fn, /* created= */ !!f)); + int k = call_label_ops_post ? label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fn, /* created= */ !!f) : 0; + /* If fopen_temporary_at() failed in the above, propagate the error code, and ignore failures in + * label_ops_post(). */ if (r < 0) - goto fail; + return r; + CLEANUP_TMPFILE_AT(dir_fd, p); + if (k < 0) + return k; r = write_string_stream_full(f, line, flags, ts); if (r < 0) - goto fail; + return r; r = fchmod_umask(fileno(f), mode); if (r < 0) - goto fail; + return r; r = RET_NERRNO(renameat(dir_fd, p, dir_fd, fn)); if (r < 0) - goto fail; + return r; + + p = mfree(p); /* disarm CLEANUP_TMPFILE_AT() */ if (FLAGS_SET(flags, WRITE_STRING_FILE_SYNC)) { /* Sync the rename, too */ @@ -267,11 +271,6 @@ static int write_string_file_atomic_at( } return 0; - -fail: - if (f) - (void) unlinkat(dir_fd, p, 0); - return r; } int write_string_file_full( diff --git a/src/basic/tmpfile-util.c b/src/basic/tmpfile-util.c index 6e2069de7cb..18a69367f13 100644 --- a/src/basic/tmpfile-util.c +++ b/src/basic/tmpfile-util.c @@ -436,3 +436,17 @@ int mkdtemp_open(const char *template, int flags, char **ret) { return fd; } + +void cleanup_tmpfile_data_done(struct cleanup_tmpfile_data *d) { + assert(d); + + if (!d->dir_fd || + *d->dir_fd < 0 || + !d->filename || + !*d->filename) + return; + + (void) unlinkat(*d->dir_fd, *d->filename, 0); + d->dir_fd = NULL; + d->filename = NULL; +} diff --git a/src/basic/tmpfile-util.h b/src/basic/tmpfile-util.h index b39680f078b..cbaae5dbb3d 100644 --- a/src/basic/tmpfile-util.h +++ b/src/basic/tmpfile-util.h @@ -48,3 +48,14 @@ static inline int flink_tmpfile(FILE *f, const char *path, const char *target, L int mkdtemp_malloc(const char *template, char **ret); int mkdtemp_open(const char *template, int flags, char **ret); + +/* A helper for removing link_tmpfile_at() via _cleanup_() */ +struct cleanup_tmpfile_data { + int *dir_fd; + char **filename; +}; +void cleanup_tmpfile_data_done(struct cleanup_tmpfile_data *d); +#define _CLEANUP_TMPFILE_AT(u, dir_fd, filename) \ + _unused_ _cleanup_(cleanup_tmpfile_data_done) struct cleanup_tmpfile_data UNIQ_T(cta, u) = { &(dir_fd), &(filename) } +#define CLEANUP_TMPFILE_AT(dir_fd, filename) \ + _CLEANUP_TMPFILE_AT(UNIQ, dir_fd, filename) diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c index c54349b9f7d..0d1a2da6511 100644 --- a/src/shared/creds-util.c +++ b/src/shared/creds-util.c @@ -393,6 +393,8 @@ static int make_credential_host_secret( if (fd < 0) return log_debug_errno(fd, "Failed to create temporary file for credential host secret: %m"); + CLEANUP_TMPFILE_AT(dfd, t); + r = chattr_secret(fd, 0); if (r < 0) log_debug_errno(r, "Failed to set file attributes for secrets file, ignoring: %m"); @@ -405,29 +407,22 @@ static int make_credential_host_secret( r = crypto_random_bytes(buf.data, sizeof(buf.data)); if (r < 0) - goto fail; + return r; r = loop_write(fd, &buf, sizeof(buf)); if (r < 0) - goto fail; - - if (fchmod(fd, 0400) < 0) { - r = -errno; - goto fail; - } + return r; - if (fsync(fd) < 0) { - r = -errno; - goto fail; - } + if (fchmod(fd, 0400) < 0) + return -errno; warn_not_encrypted(fd, flags, dirname, fn); r = link_tmpfile_at(fd, dfd, t, fn, LINK_TMPFILE_SYNC); - if (r < 0) { - log_debug_errno(r, "Failed to link host key into place: %m"); - goto fail; - } + if (r < 0) + return log_debug_errno(r, "Failed to link host key into place: %m"); + + t = mfree(t); /* disarm CLEANUP_ERASE() */ if (ret) { void *copy; @@ -440,12 +435,6 @@ static int make_credential_host_secret( } return 0; - -fail: - if (t && unlinkat(dfd, t, 0) < 0) - log_debug_errno(errno, "Failed to remove temporary credential key: %m"); - - return r; } int get_credential_host_secret(CredentialSecretFlags flags, struct iovec *ret) {