From 992e8f224b91cacc3d6589bea7882c7ab9c0911b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jul 2016 17:23:28 +0200 Subject: util-lib: rework /tmp and /var/tmp handling code Beef up the existing var_tmp() call, rename it to var_tmp_dir() and add a matching tmp_dir() call (the former looks for the place for /var/tmp, the latter for /tmp). Both calls check $TMPDIR, $TEMP, $TMP, following the algorithm Python3 uses. All dirs are validated before use. secure_getenv() is used in order to limite exposure in suid binaries. This also ports a couple of users over to these new APIs. The var_tmp() return parameter is changed from an allocated buffer the caller will own to a const string either pointing into environ[], or into a static const buffer. Given that environ[] is mostly considered constant (and this is exposed in the very well-known getenv() call), this should be OK behaviour and allows us to avoid memory allocations in most cases. Note that $TMPDIR and friends override both /var/tmp and /tmp usage if set. --- src/basic/fs-util.c | 98 ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 79 insertions(+), 19 deletions(-) (limited to 'src/basic/fs-util.c') diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index f0c6f3265e..ce87257bc1 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -496,34 +496,94 @@ int get_files_in_directory(const char *path, char ***list) { return n; } -int var_tmp(char **ret) { - const char *tmp_dir = NULL; - const char *env_tmp_dir = NULL; - char *c = NULL; - int r; +static int getenv_tmp_dir(const char **ret_path) { + const char *n; + int r, ret = 0; - assert(ret); + assert(ret_path); - env_tmp_dir = getenv("TMPDIR"); - if (env_tmp_dir != NULL) { - r = is_dir(env_tmp_dir, true); - if (r < 0 && r != -ENOENT) - return r; - if (r > 0) - tmp_dir = env_tmp_dir; + /* We use the same order of environment variables python uses in tempfile.gettempdir(): + * https://docs.python.org/3/library/tempfile.html#tempfile.gettempdir */ + FOREACH_STRING(n, "TMPDIR", "TEMP", "TMP") { + const char *e; + + e = secure_getenv(n); + if (!e) + continue; + if (!path_is_absolute(e)) { + r = -ENOTDIR; + goto next; + } + if (!path_is_safe(e)) { + r = -EPERM; + goto next; + } + + r = is_dir(e, true); + if (r < 0) + goto next; + if (r == 0) { + r = -ENOTDIR; + goto next; + } + + *ret_path = e; + return 1; + + next: + /* Remember first error, to make this more debuggable */ + if (ret >= 0) + ret = r; } - if (!tmp_dir) - tmp_dir = "/var/tmp"; + if (ret < 0) + return ret; - c = strdup(tmp_dir); - if (!c) - return -ENOMEM; - *ret = c; + *ret_path = NULL; + return ret; +} +static int tmp_dir_internal(const char *def, const char **ret) { + const char *e; + int r, k; + + assert(def); + assert(ret); + + r = getenv_tmp_dir(&e); + if (r > 0) { + *ret = e; + return 0; + } + + k = is_dir(def, true); + if (k == 0) + k = -ENOTDIR; + if (k < 0) + return r < 0 ? r : k; + + *ret = def; return 0; } +int var_tmp_dir(const char **ret) { + + /* Returns the location for "larger" temporary files, that is backed by physical storage if available, and thus + * even might survive a boot: /var/tmp. If $TMPDIR (or related environment variables) are set, its value is + * returned preferably however. Note that both this function and tmp_dir() below are affected by $TMPDIR, + * making it a variable that overrides all temporary file storage locations. */ + + return tmp_dir_internal("/var/tmp", ret); +} + +int tmp_dir(const char **ret) { + + /* Similar to var_tmp_dir() above, but returns the location for "smaller" temporary files, which is usually + * backed by an in-memory file system: /tmp. */ + + return tmp_dir_internal("/tmp", ret); +} + int inotify_add_watch_fd(int fd, int what, uint32_t mask) { char path[strlen("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1]; int r; -- cgit v1.2.3-54-g00ecf From d944dc9553009822deaddec76814f5642a6a8176 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 24 Sep 2016 12:41:30 +0200 Subject: namespace: chase symlinks for mounts to set up in userspace This adds logic to chase symlinks for all mount points that shall be created in a namespace environment in userspace, instead of leaving this to the kernel. This has the advantage that we can correctly handle absolute symlinks that shall be taken relative to a specific root directory. Moreover, we can properly handle mounts created on symlinked files or directories as we can merge their mounts as necessary. (This also drops the "done" flag in the namespace logic, which was never actually working, but was supposed to permit a partial rollback of the namespace logic, which however is only mildly useful as it wasn't clear in which case it would or would not be able to roll back.) Fixes: #3867 --- src/basic/fs-util.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++ src/basic/fs-util.h | 2 + src/core/namespace.c | 118 +++++++++++++++++++----------- src/test/test-fs-util.c | 96 ++++++++++++++++++++++++- src/test/test-ns.c | 10 ++- 5 files changed, 367 insertions(+), 46 deletions(-) (limited to 'src/basic/fs-util.c') diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index ce87257bc1..86d9ad7e36 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -597,3 +597,190 @@ int inotify_add_watch_fd(int fd, int what, uint32_t mask) { return r; } + +int chase_symlinks(const char *path, const char *_root, char **ret) { + _cleanup_free_ char *buffer = NULL, *done = NULL, *root = NULL; + _cleanup_close_ int fd = -1; + unsigned max_follow = 32; /* how many symlinks to follow before giving up and returning ELOOP */ + char *todo; + int r; + + assert(path); + + /* This is a lot like canonicalize_file_name(), but takes an additional "root" parameter, that allows following + * symlinks relative to a root directory, instead of the root of the host. + * + * Note that "root" matters only if we encounter an absolute symlink, it's unused otherwise. Most importantly + * this means the path parameter passed in is not prefixed by it. + * + * Algorithmically this operates on two path buffers: "done" are the components of the path we already + * processed and resolved symlinks, "." and ".." of. "todo" are the components of the path we still need to + * process. On each iteration, we move one component from "todo" to "done", processing it's special meaning + * each time. The "todo" path always starts with at least one slash, the "done" path always ends in no + * slash. We always keep an O_PATH fd to the component we are currently processing, thus keeping lookup races + * at a minimum. */ + + r = path_make_absolute_cwd(path, &buffer); + if (r < 0) + return r; + + if (_root) { + r = path_make_absolute_cwd(_root, &root); + if (r < 0) + return r; + } + + fd = open("/", O_CLOEXEC|O_NOFOLLOW|O_PATH); + if (fd < 0) + return -errno; + + todo = buffer; + for (;;) { + _cleanup_free_ char *first = NULL; + _cleanup_close_ int child = -1; + struct stat st; + size_t n, m; + + /* Determine length of first component in the path */ + n = strspn(todo, "/"); /* The slashes */ + m = n + strcspn(todo + n, "/"); /* The entire length of the component */ + + /* Extract the first component. */ + first = strndup(todo, m); + if (!first) + return -ENOMEM; + + todo += m; + + /* Just a single slash? Then we reached the end. */ + if (isempty(first) || path_equal(first, "/")) + break; + + /* Just a dot? Then let's eat this up. */ + if (path_equal(first, "/.")) + continue; + + /* Two dots? Then chop off the last bit of what we already found out. */ + if (path_equal(first, "/..")) { + _cleanup_free_ char *parent = NULL; + int fd_parent = -1; + + if (isempty(done) || path_equal(done, "/")) + return -EINVAL; + + parent = dirname_malloc(done); + if (!parent) + return -ENOMEM; + + /* Don't allow this to leave the root dir */ + if (root && + path_startswith(done, root) && + !path_startswith(parent, root)) + return -EINVAL; + + free(done); + done = parent; + parent = NULL; + + fd_parent = openat(fd, "..", O_CLOEXEC|O_NOFOLLOW|O_PATH); + if (fd_parent < 0) + return -errno; + + safe_close(fd); + fd = fd_parent; + + continue; + } + + /* Otherwise let's see what this is. */ + child = openat(fd, first + n, O_CLOEXEC|O_NOFOLLOW|O_PATH); + if (child < 0) + return -errno; + + if (fstat(child, &st) < 0) + return -errno; + + if (S_ISLNK(st.st_mode)) { + _cleanup_free_ char *destination = NULL; + + /* This is a symlink, in this case read the destination. But let's make sure we don't follow + * symlinks without bounds. */ + if (--max_follow <= 0) + return -ELOOP; + + r = readlinkat_malloc(fd, first + n, &destination); + if (r < 0) + return r; + if (isempty(destination)) + return -EINVAL; + + if (path_is_absolute(destination)) { + + /* An absolute destination. Start the loop from the beginning, but use the root + * directory as base. */ + + safe_close(fd); + fd = open(root ?: "/", O_CLOEXEC|O_NOFOLLOW|O_PATH); + if (fd < 0) + return -errno; + + free(buffer); + buffer = destination; + destination = NULL; + + todo = buffer; + free(done); + + /* Note that we do not revalidate the root, we take it as is. */ + if (isempty(root)) + done = NULL; + else { + done = strdup(root); + if (!done) + return -ENOMEM; + } + + } else { + char *joined; + + /* A relative destination. If so, this is what we'll prefix what's left to do with what + * we just read, and start the loop again, but remain in the current directory. */ + + joined = strjoin("/", destination, todo, NULL); + if (!joined) + return -ENOMEM; + + free(buffer); + todo = buffer = joined; + } + + continue; + } + + /* If this is not a symlink, then let's just add the name we read to what we already verified. */ + if (!done) { + done = first; + first = NULL; + } else { + if (!strextend(&done, first, NULL)) + return -ENOMEM; + } + + /* And iterate again, but go one directory further down. */ + safe_close(fd); + fd = child; + child = -1; + } + + if (!done) { + /* Special case, turn the empty string into "/", to indicate the root directory. */ + done = strdup("/"); + if (!done) + return -ENOMEM; + } + + *ret = done; + done = NULL; + + return 0; +} diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 2c3b9a1c74..31df47cf1e 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -77,3 +77,5 @@ union inotify_event_buffer { }; int inotify_add_watch_fd(int fd, int what, uint32_t mask); + +int chase_symlinks(const char *path, const char *_root, char **ret); diff --git a/src/core/namespace.c b/src/core/namespace.c index 356d3c8121..d3ab2e8e3e 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -29,6 +29,7 @@ #include "alloc-util.h" #include "dev-setup.h" #include "fd-util.h" +#include "fs-util.h" #include "loopback-setup.h" #include "missing.h" #include "mkdir.h" @@ -57,9 +58,9 @@ typedef enum MountMode { } MountMode; typedef struct BindMount { - const char *path; + const char *path; /* stack memory, doesn't need to be freed explicitly */ + char *chased; /* malloc()ed memory, needs to be freed */ MountMode mode; - bool done; bool ignore; } BindMount; @@ -71,7 +72,6 @@ static int append_mounts(BindMount **p, char **strv, MountMode mode) { STRV_FOREACH(i, strv) { (*p)->ignore = false; - (*p)->done = false; if ((mode == INACCESSIBLE || mode == READONLY || mode == READWRITE) && (*i)[0] == '-') { (*p)->ignore = true; @@ -360,11 +360,8 @@ static int apply_mount( * inaccessible path. */ (void) umount_recursive(m->path, 0); - if (lstat(m->path, &target) < 0) { - if (m->ignore && errno == ENOENT) - return 0; + if (lstat(m->path, &target) < 0) return log_debug_errno(errno, "Failed to lstat() %s to determine what to mount over it: %m", m->path); - } what = mode_to_inaccessible_node(target.st_mode); if (!what) { @@ -378,11 +375,8 @@ static int apply_mount( case READWRITE: r = path_is_mount_point(m->path, 0); - if (r < 0) { - if (m->ignore && errno == ENOENT) - return 0; + if (r < 0) return log_debug_errno(r, "Failed to determine whether %s is already a mount point: %m", m->path); - } if (r > 0) /* Nothing to do here, it is already a mount. We just later toggle the MS_RDONLY bit for the mount point if needed. */ return 0; @@ -407,12 +401,8 @@ static int apply_mount( assert(what); - if (mount(what, m->path, NULL, MS_BIND|MS_REC, NULL) < 0) { - if (m->ignore && errno == ENOENT) - return 0; - + if (mount(what, m->path, NULL, MS_BIND|MS_REC, NULL) < 0) return log_debug_errno(errno, "Failed to mount %s to %s: %m", what, m->path); - } log_debug("Successfully mounted %s to %s", what, m->path); return 0; @@ -435,12 +425,43 @@ static int make_read_only(BindMount *m, char **blacklist) { * already stays this way. This improves compatibility with container managers, where we won't attempt to undo * read-only mounts already applied. */ - if (m->ignore && r == -ENOENT) - return 0; - return r; } +static int chase_all_symlinks(const char *root_directory, BindMount *m, unsigned *n) { + BindMount *f, *t; + int r; + + assert(m); + assert(n); + + /* Since mount() will always follow symlinks and we need to take the different root directory into account we + * chase the symlinks on our own first. This call wil do so for all entries and remove all entries where we + * can't resolve the path, and which have been marked for such removal. */ + + for (f = m, t = m; f < m+*n; f++) { + + r = chase_symlinks(f->path, root_directory, &f->chased); + if (r == -ENOENT && f->ignore) /* Doesn't exist? Then remove it! */ + continue; + if (r < 0) + return log_debug_errno(r, "Failed to chase symlinks for %s: %m", f->path); + + if (path_equal(f->path, f->chased)) + f->chased = mfree(f->chased); + else { + log_debug("Chased %s → %s", f->path, f->chased); + f->path = f->chased; + } + + *t = *f; + t++; + } + + *n = t - m; + return 0; +} + int setup_namespace( const char* root_directory, char** read_write_paths, @@ -456,6 +477,7 @@ int setup_namespace( unsigned long mount_flags) { BindMount *m, *mounts = NULL; + bool make_slave = false; unsigned n; int r = 0; @@ -475,6 +497,9 @@ int setup_namespace( ((protect_system != PROTECT_SYSTEM_NO ? 3 : 0) + (protect_system == PROTECT_SYSTEM_FULL ? 1 : 0))); + if (root_directory || n > 0) + make_slave = true; + if (n > 0) { m = mounts = (BindMount *) alloca0(n * sizeof(BindMount)); r = append_mounts(&m, read_write_paths, READWRITE); @@ -596,6 +621,13 @@ int setup_namespace( assert(mounts + n == m); + /* Resolve symlinks manually first, as mount() will always follow them relative to the host's + * root. Moreover we want to suppress duplicates based on the resolved paths. This of course is a bit + * racy. */ + r = chase_all_symlinks(root_directory, mounts, &n); + if (r < 0) + goto finish; + qsort(mounts, n, sizeof(BindMount), mount_path_compare); drop_duplicates(mounts, &n); @@ -603,20 +635,26 @@ int setup_namespace( drop_nop(mounts, &n); } - if (unshare(CLONE_NEWNS) < 0) - return -errno; + if (unshare(CLONE_NEWNS) < 0) { + r = -errno; + goto finish; + } - if (n > 0 || root_directory) { + if (make_slave) { /* Remount / as SLAVE so that nothing now mounted in the namespace shows up in the parent */ - if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL) < 0) - return -errno; + if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL) < 0) { + r = -errno; + goto finish; + } } if (root_directory) { /* Turn directory into bind mount */ - if (mount(root_directory, root_directory, NULL, MS_BIND|MS_REC, NULL) < 0) - return -errno; + if (mount(root_directory, root_directory, NULL, MS_BIND|MS_REC, NULL) < 0) { + r = -errno; + goto finish; + } } if (n > 0) { @@ -627,7 +665,7 @@ int setup_namespace( for (m = mounts; m < mounts + n; ++m) { r = apply_mount(m, tmp_dir, var_tmp_dir); if (r < 0) - goto fail; + goto finish; } /* Create a blacklist we can pass to bind_mount_recursive() */ @@ -640,34 +678,30 @@ int setup_namespace( for (m = mounts; m < mounts + n; ++m) { r = make_read_only(m, blacklist); if (r < 0) - goto fail; + goto finish; } } if (root_directory) { /* MS_MOVE does not work on MS_SHARED so the remount MS_SHARED will be done later */ r = mount_move_root(root_directory); - if (r < 0) /* at this point, we cannot rollback */ - return r; + if (r < 0) + goto finish; } /* Remount / as the desired mode. Not that this will not * reestablish propagation from our side to the host, since * what's disconnected is disconnected. */ - if (mount(NULL, "/", NULL, mount_flags | MS_REC, NULL) < 0) - return -errno; /* at this point, we cannot rollback */ + if (mount(NULL, "/", NULL, mount_flags | MS_REC, NULL) < 0) { + r = -errno; + goto finish; + } - return 0; + r = 0; -fail: - if (n > 0) { - for (m = mounts; m < mounts + n; ++m) { - if (!m->done) - continue; - - (void) umount2(m->path, MNT_DETACH); - } - } +finish: + for (m = mounts; m < mounts + n; m++) + free(m->chased); return r; } diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index b35a2ea2c8..53a3cdc663 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -20,16 +20,109 @@ #include #include "alloc-util.h" -#include "fileio.h" #include "fd-util.h" +#include "fileio.h" #include "fs-util.h" #include "macro.h" #include "mkdir.h" +#include "path-util.h" #include "rm-rf.h" #include "string-util.h" #include "strv.h" #include "util.h" +static void test_chase_symlinks(void) { + _cleanup_free_ char *result = NULL; + char temp[] = "/tmp/test-chase.XXXXXX"; + const char *top, *p, *q; + int r; + + assert_se(mkdtemp(temp)); + + top = strjoina(temp, "/top"); + assert_se(mkdir(top, 0700) >= 0); + + p = strjoina(top, "/dot"); + assert_se(symlink(".", p) >= 0); + + p = strjoina(top, "/dotdot"); + assert_se(symlink("..", p) >= 0); + + p = strjoina(top, "/dotdota"); + assert_se(symlink("../a", p) >= 0); + + p = strjoina(temp, "/a"); + assert_se(symlink("b", p) >= 0); + + p = strjoina(temp, "/b"); + assert_se(symlink("/usr", p) >= 0); + + p = strjoina(temp, "/start"); + assert_se(symlink("top/dot/dotdota", p) >= 0); + + r = chase_symlinks(p, NULL, &result); + assert_se(r >= 0); + assert_se(path_equal(result, "/usr")); + + result = mfree(result); + r = chase_symlinks(p, temp, &result); + assert_se(r == -ENOENT); + + q = strjoina(temp, "/usr"); + assert_se(mkdir(q, 0700) >= 0); + + r = chase_symlinks(p, temp, &result); + assert_se(r >= 0); + assert_se(path_equal(result, q)); + + p = strjoina(temp, "/slash"); + assert_se(symlink("/", p) >= 0); + + result = mfree(result); + r = chase_symlinks(p, NULL, &result); + assert_se(r >= 0); + assert_se(path_equal(result, "/")); + + result = mfree(result); + r = chase_symlinks(p, temp, &result); + assert_se(r >= 0); + assert_se(path_equal(result, temp)); + + p = strjoina(temp, "/slashslash"); + assert_se(symlink("///usr///", p) >= 0); + + result = mfree(result); + r = chase_symlinks(p, NULL, &result); + assert_se(r >= 0); + assert_se(path_equal(result, "/usr")); + + result = mfree(result); + r = chase_symlinks(p, temp, &result); + assert_se(r >= 0); + assert_se(path_equal(result, q)); + + result = mfree(result); + r = chase_symlinks("/etc/./.././", NULL, &result); + assert_se(r >= 0); + assert_se(path_equal(result, "/")); + + result = mfree(result); + r = chase_symlinks("/etc/./.././", "/etc", &result); + assert_se(r == -EINVAL); + + result = mfree(result); + r = chase_symlinks("/etc/machine-id/foo", NULL, &result); + assert_se(r == -ENOTDIR); + + result = mfree(result); + p = strjoina(temp, "/recursive-symlink"); + assert_se(symlink("recursive-symlink", p) >= 0); + r = chase_symlinks(p, NULL, &result); + assert_se(r == -ELOOP); + + assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); +} + static void test_unlink_noerrno(void) { char name[] = "/tmp/test-close_nointr.XXXXXX"; int fd; @@ -144,6 +237,7 @@ int main(int argc, char *argv[]) { test_readlink_and_make_absolute(); test_get_files_in_directory(); test_var_tmp(); + test_chase_symlinks(); return 0; } diff --git a/src/test/test-ns.c b/src/test/test-ns.c index 03a24620af..c4d4da6d05 100644 --- a/src/test/test-ns.c +++ b/src/test/test-ns.c @@ -26,14 +26,18 @@ int main(int argc, char *argv[]) { const char * const writable[] = { "/home", - "/home/lennart/projects/foobar", /* this should be masked automatically */ + "-/home/lennart/projects/foobar", /* this should be masked automatically */ NULL }; const char * const readonly[] = { - "/", - "/usr", + /* "/", */ + /* "/usr", */ "/boot", + "/lib", + "/usr/lib", + "-/lib64", + "-/usr/lib64", NULL }; -- cgit v1.2.3-54-g00ecf From 3b319885c4febb5f7ea9b5ab31c3395548ed6886 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 16 Oct 2016 19:23:35 -0400 Subject: tree-wide: introduce free_and_replace helper It's a common pattern, so add a helper for it. A macro is necessary because a function that takes a pointer to a pointer would be type specific, similarly to cleanup functions. Seems better to use a macro. --- coccinelle/free_and_replace.cocci | 15 +++++++++++++++ src/basic/alloc-util.h | 8 ++++++++ src/basic/fs-util.c | 8 ++------ src/basic/path-util.c | 4 +--- src/core/load-fragment.c | 10 ++-------- src/core/mount.c | 14 +++----------- src/core/service.c | 4 +--- src/journal-remote/journal-upload.c | 4 +--- src/shared/install.c | 10 ++-------- 9 files changed, 35 insertions(+), 42 deletions(-) create mode 100644 coccinelle/free_and_replace.cocci (limited to 'src/basic/fs-util.c') diff --git a/coccinelle/free_and_replace.cocci b/coccinelle/free_and_replace.cocci new file mode 100644 index 0000000000..9dcdbf4d42 --- /dev/null +++ b/coccinelle/free_and_replace.cocci @@ -0,0 +1,15 @@ +@@ +expression p, q; +@@ +- free(p); +- p = q; +- q = NULL; +- return 0; ++ return free_and_replace(p, q); +@@ +expression p, q; +@@ +- free(p); +- p = q; +- q = NULL; ++ free_and_replace(p, q); diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index ceeee519b7..a44dd473c1 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -43,6 +43,14 @@ static inline void *mfree(void *memory) { return NULL; } +#define free_and_replace(a, b) \ + ({ \ + free(a); \ + (a) = (b); \ + (b) = NULL; \ + 0; \ + }) + void* memdup(const void *p, size_t l) _alloc_(2); static inline void freep(void *p) { diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 86d9ad7e36..48952a1c26 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -678,9 +678,7 @@ int chase_symlinks(const char *path, const char *_root, char **ret) { !path_startswith(parent, root)) return -EINVAL; - free(done); - done = parent; - parent = NULL; + free_and_replace(done, parent); fd_parent = openat(fd, "..", O_CLOEXEC|O_NOFOLLOW|O_PATH); if (fd_parent < 0) @@ -724,9 +722,7 @@ int chase_symlinks(const char *path, const char *_root, char **ret) { if (fd < 0) return -errno; - free(buffer); - buffer = destination; - destination = NULL; + free_and_replace(buffer, destination); todo = buffer; free(done); diff --git a/src/basic/path-util.c b/src/basic/path-util.c index a76963aa9f..0f5b20cf05 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -288,9 +288,7 @@ char **path_strv_resolve(char **l, const char *prefix) { } else { /* canonicalized path goes outside of * prefix, keep the original path instead */ - free(u); - u = orig; - orig = NULL; + free_and_replace(u, orig); } } else free(t); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 06c156a623..8cc7a8e765 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1591,11 +1591,7 @@ int config_parse_fdname( return 0; } - free(s->fdname); - s->fdname = p; - p = NULL; - - return 0; + return free_and_replace(s->fdname, p); } int config_parse_service_sockets( @@ -2052,9 +2048,7 @@ int config_parse_working_directory( return 0; } - free(c->working_directory); - c->working_directory = k; - k = NULL; + free_and_replace(c->working_directory, k); c->working_directory_home = false; } diff --git a/src/core/mount.c b/src/core/mount.c index 15619dffe3..da480001e1 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1484,17 +1484,9 @@ static int mount_setup_unit( MOUNT(u)->from_proc_self_mountinfo = true; - free(p->what); - p->what = w; - w = NULL; - - free(p->options); - p->options = o; - o = NULL; - - free(p->fstype); - p->fstype = f; - f = NULL; + free_and_replace(p->what, w); + free_and_replace(p->options, o); + free_and_replace(p->fstype, f); if (load_extras) { r = mount_add_extras(MOUNT(u)); diff --git a/src/core/service.c b/src/core/service.c index 63045ede55..c949de9cbe 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3088,9 +3088,7 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) if (!streq_ptr(s->status_text, t)) { - free(s->status_text); - s->status_text = t; - t = NULL; + free_and_replace(s->status_text, t); notify_dbus = true; } diff --git a/src/journal-remote/journal-upload.c b/src/journal-remote/journal-upload.c index c0f967ab94..61190ff83c 100644 --- a/src/journal-remote/journal-upload.c +++ b/src/journal-remote/journal-upload.c @@ -527,9 +527,7 @@ static int perform_upload(Uploader *u) { log_debug("Upload finished successfully with code %ld: %s", status, strna(u->answer)); - free(u->last_cursor); - u->last_cursor = u->current_cursor; - u->current_cursor = NULL; + free_and_replace(u->last_cursor, u->current_cursor); return update_cursor_state(u); } diff --git a/src/shared/install.c b/src/shared/install.c index c1ef62b337..f70b3e3dd5 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1107,11 +1107,7 @@ static int config_parse_default_instance( if (!unit_instance_is_valid(printed)) return -EINVAL; - free(i->default_instance); - i->default_instance = printed; - printed = NULL; - - return 0; + return free_and_replace(i->default_instance, printed); } static int unit_file_load( @@ -1357,9 +1353,7 @@ static int install_info_follow( if (!streq(basename(i->symlink_target), i->name)) return -EXDEV; - free(i->path); - i->path = i->symlink_target; - i->symlink_target = NULL; + free_and_replace(i->path, i->symlink_target); i->type = _UNIT_FILE_TYPE_INVALID; return unit_file_load_or_readlink(c, i, i->path, root_dir, flags); -- cgit v1.2.3-54-g00ecf