From 29206d4619843252c2e04f20dc03c246547600a2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jul 2016 12:37:28 +0200 Subject: core: add a concept of "dynamic" user ids, that are allocated as long as a service is running This adds a new boolean setting DynamicUser= to service files. If set, a new user will be allocated dynamically when the unit is started, and released when it is stopped. The user ID is allocated from the range 61184..65519. The user will not be added to /etc/passwd (but an NSS module to be added later should make it show up in getent passwd). For now, care should be taken that the service writes no files to disk, since this might result in files owned by UIDs that might get assigned dynamically to a different service later on. Later patches will tighten sandboxing in order to ensure that this cannot happen, except for a few selected directories. A simple way to test this is: systemd-run -p DynamicUser=1 /bin/sleep 99999 --- src/core/dynamic-user.c | 763 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 763 insertions(+) create mode 100644 src/core/dynamic-user.c (limited to 'src/core/dynamic-user.c') diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c new file mode 100644 index 0000000000..8035bee231 --- /dev/null +++ b/src/core/dynamic-user.c @@ -0,0 +1,763 @@ +/*** + This file is part of systemd. + + Copyright 2016 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include +#include +#include + +#include "dynamic-user.h" +#include "fd-util.h" +#include "fs-util.h" +#include "parse-util.h" +#include "random-util.h" +#include "stdio-util.h" +#include "string-util.h" +#include "user-util.h" +#include "fileio.h" + +/* Let's pick a UIDs within the 16bit range, so that we are compatible with containers using 16bit user namespacing. At + * least on Fedora normal users are allocated until UID 60000, hence do not allocate from below this. Also stay away + * from the upper end of the range as that is often used for overflow/nobody users. */ +#define UID_PICK_MIN ((uid_t) UINT32_C(0x0000EF00)) +#define UID_PICK_MAX ((uid_t) UINT32_C(0x0000FFEF)) + +/* Takes a value generated randomly or by hashing and turns it into a UID in the right range */ +#define UID_CLAMP_INTO_RANGE(rnd) (((uid_t) (rnd) % (UID_PICK_MAX - UID_PICK_MIN + 1)) + UID_PICK_MIN) + +static DynamicUser* dynamic_user_free(DynamicUser *d) { + if (!d) + return NULL; + + if (d->manager) + (void) hashmap_remove(d->manager->dynamic_users, d->name); + + safe_close_pair(d->storage_socket); + free(d); + + return NULL; +} + +static int dynamic_user_add(Manager *m, const char *name, int storage_socket[2], DynamicUser **ret) { + DynamicUser *d = NULL; + int r; + + assert(m); + assert(name); + assert(storage_socket); + + r = hashmap_ensure_allocated(&m->dynamic_users, &string_hash_ops); + if (r < 0) + return r; + + d = malloc0(offsetof(DynamicUser, name) + strlen(name) + 1); + if (!d) + return -ENOMEM; + + strcpy(d->name, name); + + d->storage_socket[0] = storage_socket[0]; + d->storage_socket[1] = storage_socket[1]; + + r = hashmap_put(m->dynamic_users, d->name, d); + if (r < 0) { + free(d); + return r; + } + + d->manager = m; + + if (ret) + *ret = d; + + return 0; +} + +int dynamic_user_acquire(Manager *m, const char *name, DynamicUser** ret) { + _cleanup_close_pair_ int storage_socket[2] = { -1, -1 }; + DynamicUser *d; + int r; + + assert(m); + assert(name); + + /* Return the DynamicUser structure for a specific user name. Note that this won't actually allocate a UID for + * it, but just prepare the data structure for it. The UID is allocated only on demand, when it's really + * needed, and in the child process we fork off, since allocation involves NSS checks which are not OK to do + * from PID 1. To allow the children and PID 1 share information about allocated UIDs we use an anonymous + * AF_UNIX/SOCK_DGRAM socket (called the "storage socket") that contains at most one datagram with the + * allocated UID number, plus an fd referencing the lock file for the UID + * (i.e. /run/systemd/dynamic-uid/$UID). Why involve the socket pair? So that PID 1 and all its children can + * share the same storage for the UID and lock fd, simply by inheriting the storage socket fds. The socket pair + * may exist in three different states: + * + * a) no datagram stored. This is the initial state. In this case the dynamic user was never realized. + * + * b) a datagram containing a UID stored, but no lock fd attached to it. In this case there was already a + * statically assigned UID by the same name, which we are reusing. + * + * c) a datagram containing a UID stored, and a lock fd is attached to it. In this case we allocated a dynamic + * UID and locked it in the file system, using the lock fd. + * + * As PID 1 and various children might access the socket pair simultaneously, and pop the datagram or push it + * back in any time, we also maintain a lock on the socket pair. Note one peculiarity regarding locking here: + * the UID lock on disk is protected via a BSD file lock (i.e. an fd-bound lock), so that the lock is kept in + * place as long as there's a reference to the fd open. The lock on the storage socket pair however is a POSIX + * file lock (i.e. a process-bound lock), as all users share the same fd of this (after all it is anonymous, + * nobody else could get any access to it except via our own fd) and we want to synchronize access between all + * processes that have access to it. */ + + d = hashmap_get(m->dynamic_users, name); + if (d) { + /* We already have a structure for the dynamic user, let's increase the ref count and reuse it */ + d->n_ref++; + *ret = d; + return 0; + } + + if (!valid_user_group_name_or_id(name)) + return -EINVAL; + + if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, storage_socket) < 0) + return -errno; + + r = dynamic_user_add(m, name, storage_socket, &d); + if (r < 0) + return r; + + storage_socket[0] = storage_socket[1] = -1; + + if (ret) { + d->n_ref++; + *ret = d; + } + + return 1; +} + +static int pick_uid(const char *name, uid_t *ret_uid) { + + static const uint8_t hash_key[] = { + 0x37, 0x53, 0x7e, 0x31, 0xcf, 0xce, 0x48, 0xf5, + 0x8a, 0xbb, 0x39, 0x57, 0x8d, 0xd9, 0xec, 0x59 + }; + + unsigned n_tries = 100; + uid_t candidate; + int r; + + /* A static user by this name does not exist yet. Let's find a free ID then, and use that. We start with a UID + * generated as hash from the user name. */ + candidate = UID_CLAMP_INTO_RANGE(siphash24(name, strlen(name), hash_key)); + + (void) mkdir("/run/systemd/dynamic-uid", 0755); + + for (;;) { + char lock_path[strlen("/run/systemd/dynamic-uid/") + DECIMAL_STR_MAX(uid_t) + 1]; + _cleanup_close_ int lock_fd = -1; + ssize_t l; + + if (--n_tries <= 0) /* Give up retrying eventually */ + return -EBUSY; + + if (candidate < UID_PICK_MIN || candidate > UID_PICK_MAX) + goto next; + + xsprintf(lock_path, "/run/systemd/dynamic-uid/" UID_FMT, candidate); + + for (;;) { + struct stat st; + + lock_fd = open(lock_path, O_CREAT|O_RDWR|O_NOFOLLOW|O_CLOEXEC|O_NOCTTY, 0600); + if (lock_fd < 0) + return -errno; + + r = flock(lock_fd, LOCK_EX|LOCK_NB); /* Try to get a BSD file lock on the UID lock file */ + if (r < 0) { + if (errno == EBUSY || errno == EAGAIN) + goto next; /* already in use */ + + return -errno; + } + + if (fstat(lock_fd, &st) < 0) + return -errno; + if (st.st_nlink > 0) + break; + + /* Oh, bummer, we got got the lock, but the file was unlinked between the time we opened it and + * got the lock. Close it, and try again. */ + lock_fd = safe_close(lock_fd); + } + + /* Some superficial check whether this UID/GID might already be taken by some static user */ + if (getpwuid(candidate) || getgrgid((gid_t) candidate)) { + (void) unlink(lock_path); + goto next; + } + + /* Let's store the user name in the lock file, so that we can use it for looking up the username for a UID */ + l = pwritev(lock_fd, + (struct iovec[2]) { + { .iov_base = (char*) name, .iov_len = strlen(name) }, + { .iov_base = (char[1]) { '\n' }, .iov_len = 1 } + }, 2, 0); + if (l < 0) { + (void) unlink(lock_path); + return -errno; + } + + (void) ftruncate(lock_fd, l); + + *ret_uid = candidate; + r = lock_fd; + lock_fd = -1; + + return r; + + next: + /* Pick another random UID, and see if that works for us. */ + random_bytes(&candidate, sizeof(candidate)); + candidate = UID_CLAMP_INTO_RANGE(candidate); + } +} + +static int dynamic_user_pop(DynamicUser *d, uid_t *ret_uid, int *ret_lock_fd) { + uid_t uid = UID_INVALID; + struct iovec iov = { + .iov_base = &uid, + .iov_len = sizeof(uid), + }; + union { + struct cmsghdr cmsghdr; + uint8_t buf[CMSG_SPACE(sizeof(int))]; + } control = {}; + struct msghdr mh = { + .msg_control = &control, + .msg_controllen = sizeof(control), + .msg_iov = &iov, + .msg_iovlen = 1, + }; + struct cmsghdr *cmsg; + + ssize_t k; + int lock_fd = -1; + + assert(d); + assert(ret_uid); + assert(ret_lock_fd); + + /* Read the UID and lock fd that is stored in the storage AF_UNIX socket. This should be called with the lock + * on the socket taken. */ + + k = recvmsg(d->storage_socket[0], &mh, MSG_DONTWAIT|MSG_NOSIGNAL|MSG_CMSG_CLOEXEC); + if (k < 0) + return -errno; + + cmsg = cmsg_find(&mh, SOL_SOCKET, SCM_RIGHTS, CMSG_LEN(sizeof(int))); + if (cmsg) + lock_fd = *(int*) CMSG_DATA(cmsg); + else + cmsg_close_all(&mh); /* just in case... */ + + *ret_uid = uid; + *ret_lock_fd = lock_fd; + + return 0; +} + +static int dynamic_user_push(DynamicUser *d, uid_t uid, int lock_fd) { + struct iovec iov = { + .iov_base = &uid, + .iov_len = sizeof(uid), + }; + union { + struct cmsghdr cmsghdr; + uint8_t buf[CMSG_SPACE(sizeof(int))]; + } control = {}; + struct msghdr mh = { + .msg_control = &control, + .msg_controllen = sizeof(control), + .msg_iov = &iov, + .msg_iovlen = 1, + }; + ssize_t k; + + assert(d); + + /* Store the UID and lock_fd in the storage socket. This should be called with the socket pair lock taken. */ + + if (lock_fd >= 0) { + struct cmsghdr *cmsg; + + cmsg = CMSG_FIRSTHDR(&mh); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + memcpy(CMSG_DATA(cmsg), &lock_fd, sizeof(int)); + + mh.msg_controllen = CMSG_SPACE(sizeof(int)); + } else { + mh.msg_control = NULL; + mh.msg_controllen = 0; + } + + k = sendmsg(d->storage_socket[1], &mh, MSG_DONTWAIT|MSG_NOSIGNAL); + if (k < 0) + return -errno; + + return 0; +} + +static void unlink_uid_lock(int lock_fd, uid_t uid) { + char lock_path[strlen("/run/systemd/dynamic-uid/") + DECIMAL_STR_MAX(uid_t) + 1]; + + if (lock_fd < 0) + return; + + xsprintf(lock_path, "/run/systemd/dynamic-uid/" UID_FMT, uid); + (void) unlink_noerrno(lock_path); +} + +int dynamic_user_realize(DynamicUser *d, uid_t *ret) { + + _cleanup_close_ int etc_passwd_lock_fd = -1, uid_lock_fd = -1; + uid_t uid = UID_INVALID; + int r; + + assert(d); + + /* Acquire a UID for the user name. This will allocate a UID for the user name if the user doesn't exist + * yet. If it already exists its existing UID/GID will be reused. */ + + if (lockf(d->storage_socket[0], F_LOCK, 0) < 0) + return -errno; + + r = dynamic_user_pop(d, &uid, &uid_lock_fd); + if (r < 0) { + int new_uid_lock_fd; + uid_t new_uid; + + if (r != -EAGAIN) + goto finish; + + /* OK, nothing stored yet, let's try to find something useful. While we are working on this release the + * lock however, so that nobody else blocks on our NSS lookups. */ + (void) lockf(d->storage_socket[0], F_ULOCK, 0); + + /* Let's see if a proper, static user or group by this name exists. Try to take the lock on + * /etc/passwd, if that fails with EROFS then /etc is read-only. In that case it's fine if we don't + * take the lock, given that users can't be added there anyway in this case. */ + etc_passwd_lock_fd = take_etc_passwd_lock(NULL); + if (etc_passwd_lock_fd < 0 && etc_passwd_lock_fd != -EROFS) + return etc_passwd_lock_fd; + + /* First, let's parse this as numeric UID */ + r = parse_uid(d->name, &uid); + if (r < 0) { + struct passwd *p; + struct group *g; + + /* OK, this is not a numeric UID. Let's see if there's a user by this name */ + p = getpwnam(d->name); + if (p) + uid = p->pw_uid; + + /* Let's see if there's a group by this name */ + g = getgrnam(d->name); + if (g) { + /* If the UID/GID of the user/group of the same don't match, refuse operation */ + if (uid != UID_INVALID && uid != (uid_t) g->gr_gid) + return -EILSEQ; + + uid = (uid_t) g->gr_gid; + } + } + + if (uid == UID_INVALID) { + /* No static UID assigned yet, excellent. Let's pick a new dynamic one, and lock it. */ + + uid_lock_fd = pick_uid(d->name, &uid); + if (uid_lock_fd < 0) + return uid_lock_fd; + } + + /* So, we found a working UID/lock combination. Let's see if we actually still need it. */ + if (lockf(d->storage_socket[0], F_LOCK, 0) < 0) { + unlink_uid_lock(uid_lock_fd, uid); + return -errno; + } + + r = dynamic_user_pop(d, &new_uid, &new_uid_lock_fd); + if (r < 0) { + if (r != -EAGAIN) { + /* OK, something bad happened, let's get rid of the bits we acquired. */ + unlink_uid_lock(uid_lock_fd, uid); + goto finish; + } + + /* Great! Nothing is stored here, still. Store our newly acquired data. */ + } else { + /* Hmm, so as it appears there's now something stored in the storage socket. Throw away what we + * acquired, and use what's stored now. */ + + unlink_uid_lock(uid_lock_fd, uid); + safe_close(uid_lock_fd); + + uid = new_uid; + uid_lock_fd = new_uid_lock_fd; + } + } + + /* If the UID/GID was already allocated dynamically, push the data we popped out back in. If it was already + * allocated statically, push the UID back too, but do not push the lock fd in. If we allocated the UID + * dynamically right here, push that in along with the lock fd for it. */ + r = dynamic_user_push(d, uid, uid_lock_fd); + if (r < 0) + goto finish; + + *ret = uid; + r = 0; + +finish: + (void) lockf(d->storage_socket[0], F_ULOCK, 0); + return r; +} + +int dynamic_user_current(DynamicUser *d, uid_t *ret) { + _cleanup_close_ int lock_fd = -1; + uid_t uid; + int r; + + assert(d); + assert(ret); + + /* Get the currently assigned UID for the user, if there's any. This simply pops the data from the storage socket, and pushes it back in right-away. */ + + if (lockf(d->storage_socket[0], F_LOCK, 0) < 0) + return -errno; + + r = dynamic_user_pop(d, &uid, &lock_fd); + if (r < 0) + goto finish; + + r = dynamic_user_push(d, uid, lock_fd); + if (r < 0) + goto finish; + + *ret = uid; + r = 0; + +finish: + (void) lockf(d->storage_socket[0], F_ULOCK, 0); + return r; +} + +DynamicUser* dynamic_user_ref(DynamicUser *d) { + if (!d) + return NULL; + + assert(d->n_ref > 0); + d->n_ref++; + + return d; +} + +DynamicUser* dynamic_user_unref(DynamicUser *d) { + if (!d) + return NULL; + + /* Note that this doesn't actually release any resources itself. If a dynamic user should be fully destroyed + * and its UID released, use dynamic_user_destroy() instead. NB: the dynamic user table may contain entries + * with no references, which is commonly the case right before a daemon reload. */ + + assert(d->n_ref > 0); + d->n_ref--; + + return NULL; +} + +static int dynamic_user_close(DynamicUser *d) { + _cleanup_close_ int lock_fd = -1; + uid_t uid; + int r; + + /* Release the user ID, by releasing the lock on it, and emptying the storage socket. After this the user is + * unrealized again, much like it was after it the DynamicUser object was first allocated. */ + + if (lockf(d->storage_socket[0], F_LOCK, 0) < 0) + return -errno; + + r = dynamic_user_pop(d, &uid, &lock_fd); + if (r == -EAGAIN) { + /* User wasn't realized yet, nothing to do. */ + r = 0; + goto finish; + } + if (r < 0) + goto finish; + + /* This dynamic user was realized and dynamically allocated. In this case, let's remove the lock file. */ + unlink_uid_lock(lock_fd, uid); + r = 1; + +finish: + (void) lockf(d->storage_socket[0], F_ULOCK, 0); + return r; +} + +DynamicUser* dynamic_user_destroy(DynamicUser *d) { + if (!d) + return NULL; + + /* Drop a reference to a DynamicUser object, and destroy the user completely if this was the last + * reference. This is called whenever a service is shut down and wants its dynamic UID gone. Note that + * dynamic_user_unref() is what is called whenever a service is simply freed, for example during a reload + * cycle, where the dynamic users should not be destroyed, but our datastructures should. */ + + dynamic_user_unref(d); + + if (d->n_ref > 0) + return NULL; + + (void) dynamic_user_close(d); + return dynamic_user_free(d); +} + +int dynamic_user_serialize(Manager *m, FILE *f, FDSet *fds) { + DynamicUser *d; + Iterator i; + + assert(m); + assert(f); + assert(fds); + + /* Dump the dynamic user database into the manager serialization, to deal with daemon reloads. */ + + HASHMAP_FOREACH(d, m->dynamic_users, i) { + int copy0, copy1; + + copy0 = fdset_put_dup(fds, d->storage_socket[0]); + if (copy0 < 0) + return copy0; + + copy1 = fdset_put_dup(fds, d->storage_socket[1]); + if (copy1 < 0) + return copy1; + + fprintf(f, "dynamic-user=%s %i %i\n", d->name, copy0, copy1); + } + + return 0; +} + +void dynamic_user_deserialize_one(Manager *m, const char *value, FDSet *fds) { + _cleanup_free_ char *name = NULL, *s0 = NULL, *s1 = NULL; + int r, fd0, fd1; + + assert(m); + assert(value); + assert(fds); + + /* Parse the serialization again, after a daemon reload */ + + r = extract_many_words(&value, NULL, 0, &name, &s0, &s1, NULL); + if (r != 3 || !isempty(value)) { + log_debug("Unable to parse dynamic user line."); + return; + } + + if (safe_atoi(s0, &fd0) < 0 || !fdset_contains(fds, fd0)) { + log_debug("Unable to process dynamic user fd specification."); + return; + } + + if (safe_atoi(s1, &fd1) < 0 || !fdset_contains(fds, fd1)) { + log_debug("Unable to process dynamic user fd specification."); + return; + } + + r = dynamic_user_add(m, name, (int[]) { fd0, fd1 }, NULL); + if (r < 0) { + log_debug_errno(r, "Failed to add dynamic user: %m"); + return; + } + + (void) fdset_remove(fds, fd0); + (void) fdset_remove(fds, fd1); +} + +void dynamic_user_vacuum(Manager *m, bool close_user) { + DynamicUser *d; + Iterator i; + + assert(m); + + /* Empty the dynamic user database, optionally cleaning up orphaned dynamic users, i.e. destroy and free users + * to which no reference exist. This is called after a daemon reload finished, in order to destroy users which + * might not be referenced anymore. */ + + HASHMAP_FOREACH(d, m->dynamic_users, i) { + if (d->n_ref > 0) + continue; + + if (close_user) { + log_debug("Removing orphaned dynamic user %s", d->name); + (void) dynamic_user_close(d); + } + + dynamic_user_free(d); + } +} + +int dynamic_user_lookup_uid(Manager *m, uid_t uid, char **ret) { + char lock_path[strlen("/run/systemd/dynamic-uid/") + DECIMAL_STR_MAX(uid_t) + 1]; + _cleanup_free_ char *user = NULL; + uid_t check_uid; + int r; + + assert(m); + assert(ret); + + /* A friendly way to translate a dynamic user's UID into a his name. */ + + if (uid < UID_PICK_MIN) + return -ESRCH; + if (uid > UID_PICK_MAX) + return -ESRCH; + + xsprintf(lock_path, "/run/systemd/dynamic-uid/" UID_FMT, uid); + r = read_one_line_file(lock_path, &user); + if (r == -ENOENT) + return -ESRCH; + if (r < 0) + return r; + + /* The lock file might be stale, hence let's verify the data before we return it */ + r = dynamic_user_lookup_name(m, user, &check_uid); + if (r < 0) + return r; + if (check_uid != uid) /* lock file doesn't match our own idea */ + return -ESRCH; + + *ret = user; + user = NULL; + + return 0; +} + +int dynamic_user_lookup_name(Manager *m, const char *name, uid_t *ret) { + DynamicUser *d; + int r; + + assert(m); + assert(name); + assert(ret); + + /* A friendly call for translating a dynamic user's name into its UID */ + + d = hashmap_get(m->dynamic_users, name); + if (!d) + return -ESRCH; + + r = dynamic_user_current(d, ret); + if (r == -EAGAIN) /* not realized yet? */ + return -ESRCH; + + return r; +} + +int dynamic_creds_acquire(DynamicCreds *creds, Manager *m, const char *user, const char *group) { + bool acquired = false; + int r; + + assert(creds); + assert(m); + + /* A DynamicUser object encapsulates an allocation of both a UID and a GID for a specific name. However, some + * services use different user and groups. For cases like that there's DynamicCreds containing a pair of user + * and group. This call allocates a pair. */ + + if (!creds->user && user) { + r = dynamic_user_acquire(m, user, &creds->user); + if (r < 0) + return r; + + acquired = true; + } + + if (!creds->group) { + + if (creds->user && (!group || streq_ptr(user, group))) + creds->group = dynamic_user_ref(creds->user); + else { + r = dynamic_user_acquire(m, group, &creds->group); + if (r < 0) { + if (acquired) + creds->user = dynamic_user_unref(creds->user); + return r; + } + } + } + + return 0; +} + +int dynamic_creds_realize(DynamicCreds *creds, uid_t *uid, gid_t *gid) { + uid_t u = UID_INVALID; + gid_t g = GID_INVALID; + int r; + + assert(creds); + assert(uid); + assert(gid); + + /* Realize both the referenced user and group */ + + if (creds->user) { + r = dynamic_user_realize(creds->user, &u); + if (r < 0) + return r; + } + + if (creds->group && creds->group != creds->user) { + r = dynamic_user_realize(creds->group, &g); + if (r < 0) + return r; + } else + g = u; + + *uid = u; + *gid = g; + + return 0; +} + +void dynamic_creds_unref(DynamicCreds *creds) { + assert(creds); + + creds->user = dynamic_user_unref(creds->user); + creds->group = dynamic_user_unref(creds->group); +} + +void dynamic_creds_destroy(DynamicCreds *creds) { + assert(creds); + + creds->user = dynamic_user_destroy(creds->user); + creds->group = dynamic_user_destroy(creds->group); +} -- cgit v1.2.3-54-g00ecf From fd63e712b2025d235ce4bfbb512fada10e2690b5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Aug 2016 12:28:51 +0200 Subject: core: bypass dynamic user lookups from dbus-daemon dbus-daemon does NSS name look-ups in order to enforce its bus policy. This might dead-lock if an NSS module use wants to use D-Bus for the look-up itself, like our nss-systemd does. Let's work around this by bypassing bus communication in the NSS module if we run inside of dbus-daemon. To make this work we keep a bit of extra state in /run/systemd/dynamic-uid/ so that we don't have to consult the bus, but can still resolve the names. Note that the normal codepath continues to be via the bus, so that resolving works from all mount namespaces and is subject to authentication, as before. This is a bit dirty, but not too dirty, as dbus daemon is kinda special anyway for PID 1. --- src/core/dynamic-user.c | 51 +++++++- src/core/execute.c | 15 ++- src/nss-systemd/nss-systemd.c | 262 ++++++++++++++++++++++++++++-------------- 3 files changed, 235 insertions(+), 93 deletions(-) (limited to 'src/core/dynamic-user.c') diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c index 8035bee231..185d0e5f00 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -150,6 +150,42 @@ int dynamic_user_acquire(Manager *m, const char *name, DynamicUser** ret) { return 1; } +static int make_uid_symlinks(uid_t uid, const char *name, bool b) { + + char path1[strlen("/run/systemd/dynamic-uid/direct:") + DECIMAL_STR_MAX(uid_t) + 1]; + const char *path2; + int r = 0; + + /* Add direct additional symlinks for direct lookups of dynamic UIDs and their names by userspace code. The + * only reason we have this is because dbus-daemon cannot use D-Bus for resolving users and groups (since it + * would be its own client then). We hence keep these world-readable symlinks in place, so that the + * unprivileged dbus user can read the mappings when it needs them via these symlinks instead of having to go + * via the bus. Ideally, we'd use the lock files we keep for this anyway, but we can't since we use BSD locks + * on them and as those may be taken by any user with read access we can't make them world-readable. */ + + xsprintf(path1, "/run/systemd/dynamic-uid/direct:" UID_FMT, uid); + if (unlink(path1) < 0) { + if (errno != ENOENT) + r = -errno; + } + if (b) { + if (symlink(name, path1) < 0) + r = -errno; + } + + path2 = strjoina("/run/systemd/dynamic-uid/direct:", name); + if (unlink(path2) < 0) { + if (errno != ENOENT) + r = -errno; + } + if (b) { + if (symlink(path1 + strlen("/run/systemd/dynamic-uid/direct:"), path2) < 0) + r = -errno; + } + + return r; +} + static int pick_uid(const char *name, uid_t *ret_uid) { static const uint8_t hash_key[] = { @@ -223,6 +259,7 @@ static int pick_uid(const char *name, uid_t *ret_uid) { } (void) ftruncate(lock_fd, l); + (void) make_uid_symlinks(candidate, name, true); /* also add direct lookup symlinks */ *ret_uid = candidate; r = lock_fd; @@ -324,14 +361,16 @@ static int dynamic_user_push(DynamicUser *d, uid_t uid, int lock_fd) { return 0; } -static void unlink_uid_lock(int lock_fd, uid_t uid) { +static void unlink_uid_lock(int lock_fd, uid_t uid, const char *name) { char lock_path[strlen("/run/systemd/dynamic-uid/") + DECIMAL_STR_MAX(uid_t) + 1]; if (lock_fd < 0) return; xsprintf(lock_path, "/run/systemd/dynamic-uid/" UID_FMT, uid); - (void) unlink_noerrno(lock_path); + (void) unlink(lock_path); + + (void) make_uid_symlinks(uid, name, false); /* remove direct lookup symlinks */ } int dynamic_user_realize(DynamicUser *d, uid_t *ret) { @@ -399,7 +438,7 @@ int dynamic_user_realize(DynamicUser *d, uid_t *ret) { /* So, we found a working UID/lock combination. Let's see if we actually still need it. */ if (lockf(d->storage_socket[0], F_LOCK, 0) < 0) { - unlink_uid_lock(uid_lock_fd, uid); + unlink_uid_lock(uid_lock_fd, uid, d->name); return -errno; } @@ -407,7 +446,7 @@ int dynamic_user_realize(DynamicUser *d, uid_t *ret) { if (r < 0) { if (r != -EAGAIN) { /* OK, something bad happened, let's get rid of the bits we acquired. */ - unlink_uid_lock(uid_lock_fd, uid); + unlink_uid_lock(uid_lock_fd, uid, d->name); goto finish; } @@ -416,7 +455,7 @@ int dynamic_user_realize(DynamicUser *d, uid_t *ret) { /* Hmm, so as it appears there's now something stored in the storage socket. Throw away what we * acquired, and use what's stored now. */ - unlink_uid_lock(uid_lock_fd, uid); + unlink_uid_lock(uid_lock_fd, uid, d->name); safe_close(uid_lock_fd); uid = new_uid; @@ -513,7 +552,7 @@ static int dynamic_user_close(DynamicUser *d) { goto finish; /* This dynamic user was realized and dynamically allocated. In this case, let's remove the lock file. */ - unlink_uid_lock(lock_fd, uid); + unlink_uid_lock(lock_fd, uid, d->name); r = 1; finish: diff --git a/src/core/execute.c b/src/core/execute.c index 4c786a2e33..0af8eb5a02 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -91,6 +91,7 @@ #include "selinux-util.h" #include "signal-util.h" #include "smack-util.h" +#include "special.h" #include "string-table.h" #include "string-util.h" #include "strv.h" @@ -1384,6 +1385,7 @@ static void do_idle_pipe_dance(int idle_pipe[4]) { } static int build_environment( + Unit *u, const ExecContext *c, const ExecParameters *p, unsigned n_fds, @@ -1401,7 +1403,7 @@ static int build_environment( assert(c); assert(ret); - our_env = new0(char*, 12); + our_env = new0(char*, 13); if (!our_env) return -ENOMEM; @@ -1436,6 +1438,16 @@ static int build_environment( our_env[n_env++] = x; } + /* If this is D-Bus, tell the nss-systemd module, since it relies on being able to use D-Bus look up dynamic + * users via PID 1, possibly dead-locking the dbus daemon. This way it will not use D-Bus to resolve names, but + * check the database directly. */ + if (unit_has_name(u, SPECIAL_DBUS_SERVICE)) { + x = strdup("SYSTEMD_NSS_BYPASS_BUS=1"); + if (!x) + return -ENOMEM; + our_env[n_env++] = x; + } + if (home) { x = strappend("HOME=", home); if (!x) @@ -2100,6 +2112,7 @@ static int exec_child( } r = build_environment( + unit, context, params, n_fds, diff --git a/src/nss-systemd/nss-systemd.c b/src/nss-systemd/nss-systemd.c index 7078c0c50c..17d04e958d 100644 --- a/src/nss-systemd/nss-systemd.c +++ b/src/nss-systemd/nss-systemd.c @@ -21,11 +21,14 @@ #include "sd-bus.h" +#include "alloc-util.h" #include "bus-common-errors.h" #include "env-util.h" +#include "fs-util.h" #include "macro.h" #include "nss-util.h" #include "signal-util.h" +#include "stdio-util.h" #include "string-util.h" #include "user-util.h" #include "util.h" @@ -75,15 +78,50 @@ static const struct group nobody_group = { NSS_GETPW_PROTOTYPES(systemd); NSS_GETGR_PROTOTYPES(systemd); +static int direct_lookup_name(const char *name, uid_t *ret) { + _cleanup_free_ char *s = NULL; + const char *path; + int r; + + assert(name); + + /* Normally, we go via the bus to resolve names. That has the benefit that it is available from any mount + * namespace and subject to proper authentication. However, there's one problem: if our module is called from + * dbus-daemon itself we really can't use D-Bus to communicate. In this case, resort to a client-side hack, + * and look for the dynamic names directly. This is pretty ugly, but breaks the cyclic dependency. */ + + path = strjoina("/run/systemd/dynamic-uid/direct:", name); + r = readlink_malloc(path, &s); + if (r < 0) + return r; + + return parse_uid(s, ret); +} + +static int direct_lookup_uid(uid_t uid, char **ret) { + char path[strlen("/run/systemd/dynamic-uid/direct:") + DECIMAL_STR_MAX(uid_t) + 1], *s; + int r; + + xsprintf(path, "/run/systemd/dynamic-uid/direct:" UID_FMT, uid); + + r = readlink_malloc(path, &s); + if (r < 0) + return r; + if (!valid_user_group_name(s)) { /* extra safety check */ + free(s); + return -EINVAL; + } + + *ret = s; + return 0; +} + enum nss_status _nss_systemd_getpwnam_r( const char *name, struct passwd *pwd, char *buffer, size_t buflen, int *errnop) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message* reply = NULL; - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; uint32_t translated; size_t l; int r; @@ -114,30 +152,45 @@ enum nss_status _nss_systemd_getpwnam_r( if (getenv_bool("SYSTEMD_NSS_DYNAMIC_BYPASS") > 0) goto not_found; - r = sd_bus_open_system(&bus); - if (r < 0) - goto fail; + if (getenv_bool("SYSTEMD_NSS_BYPASS_BUS") > 0) { - r = sd_bus_call_method(bus, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - "LookupDynamicUserByName", - &error, - &reply, - "s", - name); - if (r < 0) { - if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + /* Access the dynamic UID allocation directly if we are called from dbus-daemon, see above. */ + r = direct_lookup_name(name, (uid_t*) &translated); + if (r == -ENOENT) goto not_found; - - goto fail; + if (r < 0) + goto fail; + + } else { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message* reply = NULL; + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + + r = sd_bus_open_system(&bus); + if (r < 0) + goto fail; + + r = sd_bus_call_method(bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "LookupDynamicUserByName", + &error, + &reply, + "s", + name); + if (r < 0) { + if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + goto not_found; + + goto fail; + } + + r = sd_bus_message_read(reply, "u", &translated); + if (r < 0) + goto fail; } - r = sd_bus_message_read(reply, "u", &translated); - if (r < 0) - goto fail; - l = strlen(name); if (buflen < l+1) { *errnop = ENOMEM; @@ -175,6 +228,7 @@ enum nss_status _nss_systemd_getpwuid_r( _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message* reply = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + _cleanup_free_ char *direct = NULL; const char *translated; size_t l; int r; @@ -204,30 +258,42 @@ enum nss_status _nss_systemd_getpwuid_r( if (getenv_bool("SYSTEMD_NSS_DYNAMIC_BYPASS") > 0) goto not_found; - r = sd_bus_open_system(&bus); - if (r < 0) - goto fail; + if (getenv_bool("SYSTEMD_NSS_BYPASS_BUS") > 0) { - r = sd_bus_call_method(bus, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - "LookupDynamicUserByUID", - &error, - &reply, - "u", - (uint32_t) uid); - if (r < 0) { - if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + r = direct_lookup_uid(uid, &direct); + if (r == -ENOENT) goto not_found; - - goto fail; + if (r < 0) + goto fail; + + translated = direct; + + } else { + r = sd_bus_open_system(&bus); + if (r < 0) + goto fail; + + r = sd_bus_call_method(bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "LookupDynamicUserByUID", + &error, + &reply, + "u", + (uint32_t) uid); + if (r < 0) { + if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + goto not_found; + + goto fail; + } + + r = sd_bus_message_read(reply, "s", &translated); + if (r < 0) + goto fail; } - r = sd_bus_message_read(reply, "s", &translated); - if (r < 0) - goto fail; - l = strlen(translated) + 1; if (buflen < l) { *errnop = ENOMEM; @@ -262,9 +328,6 @@ enum nss_status _nss_systemd_getgrnam_r( char *buffer, size_t buflen, int *errnop) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message* reply = NULL; - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; uint32_t translated; size_t l; int r; @@ -294,30 +357,45 @@ enum nss_status _nss_systemd_getgrnam_r( if (getenv_bool("SYSTEMD_NSS_DYNAMIC_BYPASS") > 0) goto not_found; - r = sd_bus_open_system(&bus); - if (r < 0) - goto fail; + if (getenv_bool("SYSTEMD_NSS_BYPASS_BUS") > 0) { - r = sd_bus_call_method(bus, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - "LookupDynamicUserByName", - &error, - &reply, - "s", - name); - if (r < 0) { - if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + /* Access the dynamic GID allocation directly if we are called from dbus-daemon, see above. */ + r = direct_lookup_name(name, (uid_t*) &translated); + if (r == -ENOENT) goto not_found; - - goto fail; + if (r < 0) + goto fail; + } else { + + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message* reply = NULL; + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + + r = sd_bus_open_system(&bus); + if (r < 0) + goto fail; + + r = sd_bus_call_method(bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "LookupDynamicUserByName", + &error, + &reply, + "s", + name); + if (r < 0) { + if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + goto not_found; + + goto fail; + } + + r = sd_bus_message_read(reply, "u", &translated); + if (r < 0) + goto fail; } - r = sd_bus_message_read(reply, "u", &translated); - if (r < 0) - goto fail; - l = sizeof(char*) + strlen(name) + 1; if (buflen < l) { *errnop = ENOMEM; @@ -353,6 +431,7 @@ enum nss_status _nss_systemd_getgrgid_r( _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message* reply = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + _cleanup_free_ char *direct = NULL; const char *translated; size_t l; int r; @@ -382,30 +461,41 @@ enum nss_status _nss_systemd_getgrgid_r( if (getenv_bool("SYSTEMD_NSS_DYNAMIC_BYPASS") > 0) goto not_found; - r = sd_bus_open_system(&bus); - if (r < 0) - goto fail; + if (getenv_bool("SYSTEMD_NSS_BYPASS_BUS") > 0) { - r = sd_bus_call_method(bus, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - "LookupDynamicUserByUID", - &error, - &reply, - "u", - (uint32_t) gid); - if (r < 0) { - if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + r = direct_lookup_uid(gid, &direct); + if (r == -ENOENT) goto not_found; - - goto fail; + if (r < 0) + goto fail; + + translated = direct; + } else { + r = sd_bus_open_system(&bus); + if (r < 0) + goto fail; + + r = sd_bus_call_method(bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "LookupDynamicUserByUID", + &error, + &reply, + "u", + (uint32_t) gid); + if (r < 0) { + if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_DYNAMIC_USER)) + goto not_found; + + goto fail; + } + + r = sd_bus_message_read(reply, "s", &translated); + if (r < 0) + goto fail; } - r = sd_bus_message_read(reply, "s", &translated); - if (r < 0) - goto fail; - l = sizeof(char*) + strlen(translated) + 1; if (buflen < l) { *errnop = ENOMEM; -- cgit v1.2.3-54-g00ecf From 986a34a6834795f1a2d25bcffbe67debf9bbd6b2 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Thu, 18 Aug 2016 23:09:29 -0400 Subject: core/dynamic-users: warn when creation of symlinks for dynamic users fails Also return the first error, since it's most likely to be interesting. If unlink fails, symlink will usually return EEXIST. --- src/core/dynamic-user.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) (limited to 'src/core/dynamic-user.c') diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c index 185d0e5f00..53c1699b92 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -154,7 +154,7 @@ static int make_uid_symlinks(uid_t uid, const char *name, bool b) { char path1[strlen("/run/systemd/dynamic-uid/direct:") + DECIMAL_STR_MAX(uid_t) + 1]; const char *path2; - int r = 0; + int r = 0, k; /* Add direct additional symlinks for direct lookups of dynamic UIDs and their names by userspace code. The * only reason we have this is because dbus-daemon cannot use D-Bus for resolving users and groups (since it @@ -164,23 +164,26 @@ static int make_uid_symlinks(uid_t uid, const char *name, bool b) { * on them and as those may be taken by any user with read access we can't make them world-readable. */ xsprintf(path1, "/run/systemd/dynamic-uid/direct:" UID_FMT, uid); - if (unlink(path1) < 0) { - if (errno != ENOENT) - r = -errno; - } - if (b) { - if (symlink(name, path1) < 0) - r = -errno; + if (unlink(path1) < 0 && errno != ENOENT) + r = -errno; + + if (b && symlink(name, path1) < 0) { + k = log_warning_errno(errno, "Failed to symlink \"%s\": %m", path1); + if (r == 0) + r = k; } path2 = strjoina("/run/systemd/dynamic-uid/direct:", name); - if (unlink(path2) < 0) { - if (errno != ENOENT) - r = -errno; + if (unlink(path2) < 0 && errno != ENOENT) { + k = -errno; + if (r == 0) + r = k; } - if (b) { - if (symlink(path1 + strlen("/run/systemd/dynamic-uid/direct:"), path2) < 0) - r = -errno; + + if (b && symlink(path1 + strlen("/run/systemd/dynamic-uid/direct:"), path2) < 0) { + k = log_warning_errno(errno, "Failed to symlink \"%s\": %m", path2); + if (r == 0) + r = k; } return r; -- cgit v1.2.3-54-g00ecf From 61755fdae08fffd72229feb52af74a85a9e0c8f7 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Thu, 18 Aug 2016 23:19:10 -0400 Subject: journald: do not create split journals for dynamic users Dynamic users should be treated like system users, and their logs should end up in the main system journal. --- src/basic/user-util.h | 16 ++++++++++++++-- src/core/dynamic-user.c | 17 ++++------------- src/journal/journald-server.c | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) (limited to 'src/core/dynamic-user.c') diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 36f71fb004..f569363811 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -20,6 +20,7 @@ ***/ #include +#include #include #include @@ -57,8 +58,19 @@ int take_etc_passwd_lock(const char *root); #define UID_INVALID ((uid_t) -1) #define GID_INVALID ((gid_t) -1) -/* The following macros add 1 when converting things, since UID 0 is a - * valid UID, while the pointer NULL is special */ +/* Let's pick a UIDs within the 16bit range, so that we are compatible with containers using 16bit + * user namespacing. At least on Fedora normal users are allocated until UID 60000, hence do not + * allocate from below this. Also stay away from the upper end of the range as that is often used + * for overflow/nobody users. */ +#define DYNAMIC_UID_MIN ((uid_t) UINT32_C(0x0000EF00)) +#define DYNAMIC_UID_MAX ((uid_t) UINT32_C(0x0000FFEF)) + +static inline bool uid_is_dynamic(uid_t uid) { + return DYNAMIC_UID_MIN <= uid && uid <= DYNAMIC_UID_MAX; +} + +/* The following macros add 1 when converting things, since UID 0 is a valid UID, while the pointer + * NULL is special */ #define PTR_TO_UID(p) ((uid_t) (((uintptr_t) (p))-1)) #define UID_TO_PTR(u) ((void*) (((uintptr_t) (u))+1)) diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c index 53c1699b92..310aaa94e1 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -31,14 +31,8 @@ #include "user-util.h" #include "fileio.h" -/* Let's pick a UIDs within the 16bit range, so that we are compatible with containers using 16bit user namespacing. At - * least on Fedora normal users are allocated until UID 60000, hence do not allocate from below this. Also stay away - * from the upper end of the range as that is often used for overflow/nobody users. */ -#define UID_PICK_MIN ((uid_t) UINT32_C(0x0000EF00)) -#define UID_PICK_MAX ((uid_t) UINT32_C(0x0000FFEF)) - /* Takes a value generated randomly or by hashing and turns it into a UID in the right range */ -#define UID_CLAMP_INTO_RANGE(rnd) (((uid_t) (rnd) % (UID_PICK_MAX - UID_PICK_MIN + 1)) + UID_PICK_MIN) +#define UID_CLAMP_INTO_RANGE(rnd) (((uid_t) (rnd) % (DYNAMIC_UID_MAX - DYNAMIC_UID_MIN + 1)) + DYNAMIC_UID_MIN) static DynamicUser* dynamic_user_free(DynamicUser *d) { if (!d) @@ -214,7 +208,7 @@ static int pick_uid(const char *name, uid_t *ret_uid) { if (--n_tries <= 0) /* Give up retrying eventually */ return -EBUSY; - if (candidate < UID_PICK_MIN || candidate > UID_PICK_MAX) + if (!uid_is_dynamic(candidate)) goto next; xsprintf(lock_path, "/run/systemd/dynamic-uid/" UID_FMT, candidate); @@ -676,11 +670,8 @@ int dynamic_user_lookup_uid(Manager *m, uid_t uid, char **ret) { assert(m); assert(ret); - /* A friendly way to translate a dynamic user's UID into a his name. */ - - if (uid < UID_PICK_MIN) - return -ESRCH; - if (uid > UID_PICK_MAX) + /* A friendly way to translate a dynamic user's UID into a name. */ + if (!uid_is_dynamic(uid)) return -ESRCH; xsprintf(lock_path, "/run/systemd/dynamic-uid/" UID_FMT, uid); diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 443b2a4cd7..2a043a95b1 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -370,7 +370,7 @@ static JournalFile* find_journal(Server *s, uid_t uid) { if (s->runtime_journal) return s->runtime_journal; - if (uid <= SYSTEM_UID_MAX) + if (uid <= SYSTEM_UID_MAX || uid_is_dynamic(uid)) return s->system_journal; r = sd_id128_get_machine(&machine); -- cgit v1.2.3-54-g00ecf From 629ff674ac48653703440ab16998adb03391f986 Mon Sep 17 00:00:00 2001 From: Stefan Schweter Date: Sun, 2 Oct 2016 19:37:21 +0200 Subject: tree-wide: remove consecutive duplicate words in comments --- src/basic/escape.c | 2 +- src/basic/util.c | 2 +- src/core/dynamic-user.c | 2 +- src/core/execute.c | 2 +- src/hostname/hostnamectl.c | 2 +- src/login/logind-session.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) (limited to 'src/core/dynamic-user.c') diff --git a/src/basic/escape.c b/src/basic/escape.c index 01daf11ce7..4a1ec4505e 100644 --- a/src/basic/escape.c +++ b/src/basic/escape.c @@ -333,7 +333,7 @@ int cunescape_length_with_prefix(const char *s, size_t length, const char *prefi assert(remaining > 0); if (*f != '\\') { - /* A literal literal, copy verbatim */ + /* A literal, copy verbatim */ *(t++) = *f; continue; } diff --git a/src/basic/util.c b/src/basic/util.c index 9d66d28eb7..ec7939dc83 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -467,7 +467,7 @@ bool in_initrd(void) { * 2. the root file system must be a memory file system * * The second check is extra paranoia, since misdetecting an - * initrd can have bad bad consequences due the initrd + * initrd can have bad consequences due the initrd * emptying when transititioning to the main systemd. */ diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c index 310aaa94e1..1043da3eb7 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -233,7 +233,7 @@ static int pick_uid(const char *name, uid_t *ret_uid) { if (st.st_nlink > 0) break; - /* Oh, bummer, we got got the lock, but the file was unlinked between the time we opened it and + /* Oh, bummer, we got the lock, but the file was unlinked between the time we opened it and * got the lock. Close it, and try again. */ lock_fd = safe_close(lock_fd); } diff --git a/src/core/execute.c b/src/core/execute.c index 3da7ef3be6..82d8c978c1 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2207,7 +2207,7 @@ static int exec_child( return r; } - /* Note that we don't set $HOME or $SHELL if they are are not particularly enlightening anyway + /* Note that we don't set $HOME or $SHELL if they are not particularly enlightening anyway * (i.e. are "/" or "/bin/nologin"). */ } diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index 4795324667..07c57fb567 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -278,7 +278,7 @@ static int set_hostname(sd_bus *bus, char **args, unsigned n) { /* Now that we set the pretty hostname, let's clean up the parameter and use that as static * hostname. If the hostname was already valid as static hostname, this will only chop off the trailing * dot if there is one. If it was not valid, then it will be made fully valid by truncating, dropping - * multiple dots, and and dropping weird chars. Note that we clean the name up only if we also are + * multiple dots, and dropping weird chars. Note that we clean the name up only if we also are * supposed to set the pretty name. If the pretty name is not being set we assume the user knows what * he does and pass the name as-is. */ h = strdup(hostname); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index b6da237397..ba1bcc2630 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -611,7 +611,7 @@ static int session_stop_scope(Session *s, bool force) { return 0; /* Let's always abandon the scope first. This tells systemd that we are not interested anymore, and everything - * that is left in in the scope is "left-over". Informing systemd about this has the benefit that it will log + * that is left in the scope is "left-over". Informing systemd about this has the benefit that it will log * when killing any processes left after this point. */ r = manager_abandon_scope(s->manager, s->scope, &error); if (r < 0) -- cgit v1.2.3-54-g00ecf From 6b430fdb7c0c2c52ea69a7d56f23d739218b13d0 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 16 Oct 2016 18:28:30 -0400 Subject: tree-wide: use mfree more --- coccinelle/mfree_return.cocci | 6 ++++++ src/basic/bitmap.c | 6 ++---- src/basic/env-util.c | 3 +-- src/basic/prioq.c | 4 +--- src/basic/replace-var.c | 3 +-- src/basic/strbuf.c | 3 +-- src/basic/string-util.c | 6 ++---- src/basic/strv.c | 9 +++------ src/core/dynamic-user.c | 4 +--- src/core/execute.c | 4 +--- src/core/manager.c | 3 +-- src/core/transaction.c | 6 ++---- src/core/unit.c | 6 ++---- src/cryptsetup/cryptsetup-generator.c | 9 +++------ src/import/curl-util.c | 4 +--- src/import/export-raw.c | 4 +--- src/import/export-tar.c | 4 +--- src/import/import-raw.c | 4 +--- src/import/import-tar.c | 4 +--- src/import/importd.c | 6 ++---- src/import/pull-job.c | 4 +--- src/import/pull-raw.c | 4 +--- src/import/pull-tar.c | 4 +--- src/journal-remote/journal-remote-write.c | 10 +++------- src/journal/journal-file.c | 3 +-- src/journal/mmap-cache.c | 6 ++---- src/journal/sd-journal.c | 9 +++------ src/libsystemd-network/ndisc-router.c | 3 +-- src/libsystemd-network/sd-dhcp-client.c | 4 +--- src/libsystemd-network/sd-dhcp-lease.c | 4 +--- src/libsystemd-network/sd-dhcp-server.c | 4 +--- src/libsystemd-network/sd-dhcp6-client.c | 4 +--- src/libsystemd-network/sd-dhcp6-lease.c | 4 +--- src/libsystemd-network/sd-ipv4acd.c | 4 +--- src/libsystemd-network/sd-ipv4ll.c | 4 +--- src/libsystemd-network/sd-lldp.c | 4 +--- src/libsystemd-network/sd-ndisc.c | 4 +--- src/libsystemd/sd-bus/bus-slot.c | 4 +--- src/libsystemd/sd-bus/bus-track.c | 8 ++------ src/libudev/libudev-list.c | 14 ++++++-------- src/libudev/libudev-monitor.c | 3 +-- src/login/logind-button.c | 9 +++------ src/login/logind-device.c | 9 +++------ src/login/logind-inhibit.c | 9 +++------ src/login/logind-seat.c | 9 +++------ src/login/logind-session.c | 12 ++++-------- src/machine/machine.c | 4 +--- src/machine/operation.c | 3 +-- src/network/networkd-wait-online-link.c | 3 +-- src/nspawn/nspawn-settings.c | 4 +--- src/resolve/resolved-dns-query.c | 8 ++------ src/resolve/resolved-dns-rr.c | 10 +++------- src/resolve/resolved-dns-scope.c | 4 +--- src/resolve/resolved-dns-search-domain.c | 4 +--- src/resolve/resolved-dns-server.c | 3 +-- src/resolve/resolved-dns-stream.c | 4 +--- src/resolve/resolved-dns-transaction.c | 3 +-- src/resolve/resolved-link.c | 6 ++---- src/resolve/resolved-manager.c | 4 +--- src/shared/machine-image.c | 3 +-- src/shared/ptyfwd.c | 3 +-- src/timesync/timesyncd-server.c | 7 ++----- src/udev/udev-ctrl.c | 3 +-- src/udev/udev-rules.c | 3 +-- 64 files changed, 105 insertions(+), 228 deletions(-) create mode 100644 coccinelle/mfree_return.cocci (limited to 'src/core/dynamic-user.c') diff --git a/coccinelle/mfree_return.cocci b/coccinelle/mfree_return.cocci new file mode 100644 index 0000000000..8119fe07f2 --- /dev/null +++ b/coccinelle/mfree_return.cocci @@ -0,0 +1,6 @@ +@@ +expression p; +@@ +- free(p); +- return NULL; ++ return mfree(p); diff --git a/src/basic/bitmap.c b/src/basic/bitmap.c index f4b12fc261..f6212e6151 100644 --- a/src/basic/bitmap.c +++ b/src/basic/bitmap.c @@ -58,10 +58,8 @@ Bitmap *bitmap_copy(Bitmap *b) { return NULL; ret->bitmaps = newdup(uint64_t, b->bitmaps, b->n_bitmaps); - if (!ret->bitmaps) { - free(ret); - return NULL; - } + if (!ret->bitmaps) + return mfree(ret); ret->n_bitmaps = ret->bitmaps_allocated = b->n_bitmaps; return ret; diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 7f5fddb700..b74290d6fd 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -544,8 +544,7 @@ char *replace_env(const char *format, char **env) { return k; fail: - free(r); - return NULL; + return mfree(r); } char **replace_env_argv(char **argv, char **env) { diff --git a/src/basic/prioq.c b/src/basic/prioq.c index d2ec516d29..4570b8e4ba 100644 --- a/src/basic/prioq.c +++ b/src/basic/prioq.c @@ -62,9 +62,7 @@ Prioq* prioq_free(Prioq *q) { return NULL; free(q->items); - free(q); - - return NULL; + return mfree(q); } int prioq_ensure_allocated(Prioq **q, compare_func_t compare_func) { diff --git a/src/basic/replace-var.c b/src/basic/replace-var.c index 6a204b9ec3..0d21423a9c 100644 --- a/src/basic/replace-var.c +++ b/src/basic/replace-var.c @@ -107,6 +107,5 @@ char *replace_var(const char *text, char *(*lookup)(const char *variable, void*u return r; oom: - free(r); - return NULL; + return mfree(r); } diff --git a/src/basic/strbuf.c b/src/basic/strbuf.c index 4bef87d3c2..00aaf9e621 100644 --- a/src/basic/strbuf.c +++ b/src/basic/strbuf.c @@ -62,8 +62,7 @@ struct strbuf *strbuf_new(void) { err: free(str->buf); free(str->root); - free(str); - return NULL; + return mfree(str); } static void strbuf_node_cleanup(struct strbuf_node *node) { diff --git a/src/basic/string-util.c b/src/basic/string-util.c index dc7de5dab8..6b06e643c9 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -610,8 +610,7 @@ char *strreplace(const char *text, const char *old_string, const char *new_strin return r; oom: - free(r); - return NULL; + return mfree(r); } char *strip_tab_ansi(char **ibuf, size_t *_isz) { @@ -682,8 +681,7 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz) { if (ferror(f)) { fclose(f); - free(obuf); - return NULL; + return mfree(obuf); } fclose(f); diff --git a/src/basic/strv.c b/src/basic/strv.c index 34e464d253..0eec868eed 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -87,8 +87,7 @@ void strv_clear(char **l) { char **strv_free(char **l) { strv_clear(l); - free(l); - return NULL; + return mfree(l); } char **strv_free_erase(char **l) { @@ -426,8 +425,7 @@ char *strv_join_quoted(char **l) { return buf; oom: - free(buf); - return NULL; + return mfree(buf); } int strv_push(char ***l, char *value) { @@ -869,8 +867,7 @@ char ***strv_free_free(char ***l) { for (i = l; *i; i++) strv_free(*i); - free(l); - return NULL; + return mfree(l); } char **strv_skip(char **l, size_t n) { diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c index 1043da3eb7..e1846e1adb 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -42,9 +42,7 @@ static DynamicUser* dynamic_user_free(DynamicUser *d) { (void) hashmap_remove(d->manager->dynamic_users, d->name); safe_close_pair(d->storage_socket); - free(d); - - return NULL; + return mfree(d); } static int dynamic_user_add(Manager *m, const char *name, int storage_socket[2], DynamicUser **ret) { diff --git a/src/core/execute.c b/src/core/execute.c index 869522704a..59c2bd0dd2 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3740,9 +3740,7 @@ ExecRuntime *exec_runtime_unref(ExecRuntime *r) { free(r->tmp_dir); free(r->var_tmp_dir); safe_close_pair(r->netns_storage_socket); - free(r); - - return NULL; + return mfree(r); } int exec_runtime_serialize(Unit *u, ExecRuntime *rt, FILE *f, FDSet *fds) { diff --git a/src/core/manager.c b/src/core/manager.c index 3569249788..50aae0d1ba 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1119,8 +1119,7 @@ Manager* manager_free(Manager *m) { hashmap_free(m->uid_refs); hashmap_free(m->gid_refs); - free(m); - return NULL; + return mfree(m); } void manager_enumerate(Manager *m) { diff --git a/src/core/transaction.c b/src/core/transaction.c index 8370b864fb..e22e3b30c2 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -1085,10 +1085,8 @@ Transaction *transaction_new(bool irreversible) { return NULL; tr->jobs = hashmap_new(NULL); - if (!tr->jobs) { - free(tr); - return NULL; - } + if (!tr->jobs) + return mfree(tr); tr->irreversible = irreversible; diff --git a/src/core/unit.c b/src/core/unit.c index 67668bdc48..a6b8ecdcb7 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -88,10 +88,8 @@ Unit *unit_new(Manager *m, size_t size) { return NULL; u->names = set_new(&string_hash_ops); - if (!u->names) { - free(u); - return NULL; - } + if (!u->names) + return mfree(u); u->manager = m; u->type = _UNIT_TYPE_INVALID; diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index 8ac5ab730a..de0a3b6f9c 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -264,16 +264,13 @@ static crypto_device *get_crypto_device(const char *uuid) { d->keyfile = d->options = d->name = NULL; d->uuid = strdup(uuid); - if (!d->uuid) { - free(d); - return NULL; - } + if (!d->uuid) + return mfree(d); r = hashmap_put(arg_disks, d->uuid, d); if (r < 0) { free(d->uuid); - free(d); - return NULL; + return mfree(d); } } diff --git a/src/import/curl-util.c b/src/import/curl-util.c index 6990c47f48..734e1560e6 100644 --- a/src/import/curl-util.c +++ b/src/import/curl-util.c @@ -235,9 +235,7 @@ CurlGlue *curl_glue_unref(CurlGlue *g) { sd_event_source_unref(g->timer); sd_event_unref(g->event); - free(g); - - return NULL; + return mfree(g); } int curl_glue_new(CurlGlue **glue, sd_event *event) { diff --git a/src/import/export-raw.c b/src/import/export-raw.c index 6136b677dd..a3dbce1954 100644 --- a/src/import/export-raw.c +++ b/src/import/export-raw.c @@ -87,9 +87,7 @@ RawExport *raw_export_unref(RawExport *e) { free(e->buffer); free(e->path); - free(e); - - return NULL; + return mfree(e); } int raw_export_new( diff --git a/src/import/export-tar.c b/src/import/export-tar.c index d79c27f2d0..3bb6027431 100644 --- a/src/import/export-tar.c +++ b/src/import/export-tar.c @@ -91,9 +91,7 @@ TarExport *tar_export_unref(TarExport *e) { free(e->buffer); free(e->path); - free(e); - - return NULL; + return mfree(e); } int tar_export_new( diff --git a/src/import/import-raw.c b/src/import/import-raw.c index fd6b9f7703..29f3f896e5 100644 --- a/src/import/import-raw.c +++ b/src/import/import-raw.c @@ -100,9 +100,7 @@ RawImport* raw_import_unref(RawImport *i) { free(i->final_path); free(i->image_root); free(i->local); - free(i); - - return NULL; + return mfree(i); } int raw_import_new( diff --git a/src/import/import-tar.c b/src/import/import-tar.c index 8b81324fde..22f9b8c5ea 100644 --- a/src/import/import-tar.c +++ b/src/import/import-tar.c @@ -107,9 +107,7 @@ TarImport* tar_import_unref(TarImport *i) { free(i->final_path); free(i->image_root); free(i->local); - free(i); - - return NULL; + return mfree(i); } int tar_import_new( diff --git a/src/import/importd.c b/src/import/importd.c index 28b4302cb3..9d31a956a5 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -141,8 +141,7 @@ static Transfer *transfer_unref(Transfer *t) { safe_close(t->stdin_fd); safe_close(t->stdout_fd); - free(t); - return NULL; + return mfree(t); } DEFINE_TRIVIAL_CLEANUP_FUNC(Transfer*, transfer_unref); @@ -548,8 +547,7 @@ static Manager *manager_unref(Manager *m) { m->bus = sd_bus_flush_close_unref(m->bus); sd_event_unref(m->event); - free(m); - return NULL; + return mfree(m); } DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_unref); diff --git a/src/import/pull-job.c b/src/import/pull-job.c index 6bcf35ef4e..e550df2c57 100644 --- a/src/import/pull-job.c +++ b/src/import/pull-job.c @@ -50,9 +50,7 @@ PullJob* pull_job_unref(PullJob *j) { free(j->payload); free(j->checksum); - free(j); - - return NULL; + return mfree(j); } static void pull_job_finish(PullJob *j, int ret) { diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 8993402821..0cf410a5d9 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -110,9 +110,7 @@ RawPull* raw_pull_unref(RawPull *i) { free(i->settings_path); free(i->image_root); free(i->local); - free(i); - - return NULL; + return mfree(i); } int raw_pull_new( diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index 8c61c46f73..68e2397b02 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -114,9 +114,7 @@ TarPull* tar_pull_unref(TarPull *i) { free(i->settings_path); free(i->image_root); free(i->local); - free(i); - - return NULL; + return mfree(i); } int tar_pull_new( diff --git a/src/journal-remote/journal-remote-write.c b/src/journal-remote/journal-remote-write.c index 7bba52566e..8729372aa3 100644 --- a/src/journal-remote/journal-remote-write.c +++ b/src/journal-remote/journal-remote-write.c @@ -75,10 +75,8 @@ Writer* writer_new(RemoteServer *server) { memset(&w->metrics, 0xFF, sizeof(w->metrics)); w->mmap = mmap_cache_new(); - if (!w->mmap) { - free(w); - return NULL; - } + if (!w->mmap) + return mfree(w); w->n_ref = 1; w->server = server; @@ -103,9 +101,7 @@ Writer* writer_free(Writer *w) { if (w->mmap) mmap_cache_unref(w->mmap); - free(w); - - return NULL; + return mfree(w); } Writer* writer_unref(Writer *w) { diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 49199b269f..d3e0214731 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -394,8 +394,7 @@ JournalFile* journal_file_close(JournalFile *f) { gcry_md_close(f->hmac); #endif - free(f); - return NULL; + return mfree(f); } void journal_file_close_set(Set *s) { diff --git a/src/journal/mmap-cache.c b/src/journal/mmap-cache.c index 293d27053a..d91247b524 100644 --- a/src/journal/mmap-cache.c +++ b/src/journal/mmap-cache.c @@ -325,10 +325,8 @@ static FileDescriptor* fd_add(MMapCache *m, int fd) { f->fd = fd; r = hashmap_put(m->fds, FD_TO_PTR(fd), f); - if (r < 0) { - free(f); - return NULL; - } + if (r < 0) + return mfree(f); return f; } diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 98c8a47afe..f2f8546086 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -387,7 +387,7 @@ _public_ int sd_journal_add_disjunction(sd_journal *j) { } static char *match_make_string(Match *m) { - char *p, *r; + char *p = NULL, *r; Match *i; bool enclose = false; @@ -397,15 +397,12 @@ static char *match_make_string(Match *m) { if (m->type == MATCH_DISCRETE) return strndup(m->data, m->size); - p = NULL; LIST_FOREACH(matches, i, m->matches) { char *t, *k; t = match_make_string(i); - if (!t) { - free(p); - return NULL; - } + if (!t) + return mfree(p); if (p) { k = strjoin(p, m->type == MATCH_OR_TERM ? " OR " : " AND ", t, NULL); diff --git a/src/libsystemd-network/ndisc-router.c b/src/libsystemd-network/ndisc-router.c index d9950b638c..41ff2b353a 100644 --- a/src/libsystemd-network/ndisc-router.c +++ b/src/libsystemd-network/ndisc-router.c @@ -49,8 +49,7 @@ _public_ sd_ndisc_router* sd_ndisc_router_unref(sd_ndisc_router *rt) { if (rt->n_ref > 0) return NULL; - free(rt); - return NULL; + return mfree(rt); } sd_ndisc_router *ndisc_router_new(size_t raw_size) { diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 179e5950bd..5ccb23922c 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -1873,9 +1873,7 @@ sd_dhcp_client *sd_dhcp_client_unref(sd_dhcp_client *client) { free(client->req_opts); free(client->hostname); free(client->vendor_class_identifier); - free(client); - - return NULL; + return mfree(client); } int sd_dhcp_client_new(sd_dhcp_client **ret) { diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index ef50ed17a1..8387b185c0 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -282,9 +282,7 @@ sd_dhcp_lease *sd_dhcp_lease_unref(sd_dhcp_lease *lease) { free(lease->static_route); free(lease->client_id); free(lease->vendor_specific); - free(lease); - - return NULL; + return mfree(lease); } static int lease_parse_u32(const uint8_t *option, size_t len, uint32_t *ret, uint32_t min) { diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 11ee2e252e..f16314a37f 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -178,9 +178,7 @@ sd_dhcp_server *sd_dhcp_server_unref(sd_dhcp_server *server) { hashmap_free(server->leases_by_client_id); free(server->bound_leases); - free(server); - - return NULL; + return mfree(server); } int sd_dhcp_server_new(sd_dhcp_server **ret, int ifindex) { diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 463fde401c..e81215f7d7 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -1300,9 +1300,7 @@ sd_dhcp6_client *sd_dhcp6_client_unref(sd_dhcp6_client *client) { sd_dhcp6_client_detach_event(client); free(client->req_opts); - free(client); - - return NULL; + return mfree(client); } int sd_dhcp6_client_new(sd_dhcp6_client **ret) { diff --git a/src/libsystemd-network/sd-dhcp6-lease.c b/src/libsystemd-network/sd-dhcp6-lease.c index 5c10a6326a..ab59977a3f 100644 --- a/src/libsystemd-network/sd-dhcp6-lease.c +++ b/src/libsystemd-network/sd-dhcp6-lease.c @@ -389,9 +389,7 @@ sd_dhcp6_lease *sd_dhcp6_lease_unref(sd_dhcp6_lease *lease) { free(lease->ntp); lease->ntp_fqdn = strv_free(lease->ntp_fqdn); - free(lease); - - return NULL; + return mfree(lease); } int dhcp6_lease_new(sd_dhcp6_lease **ret) { diff --git a/src/libsystemd-network/sd-ipv4acd.c b/src/libsystemd-network/sd-ipv4acd.c index 662885840f..4dd343c101 100644 --- a/src/libsystemd-network/sd-ipv4acd.c +++ b/src/libsystemd-network/sd-ipv4acd.c @@ -135,9 +135,7 @@ sd_ipv4acd *sd_ipv4acd_unref(sd_ipv4acd *acd) { ipv4acd_reset(acd); sd_ipv4acd_detach_event(acd); - free(acd); - - return NULL; + return mfree(acd); } int sd_ipv4acd_new(sd_ipv4acd **ret) { diff --git a/src/libsystemd-network/sd-ipv4ll.c b/src/libsystemd-network/sd-ipv4ll.c index 5603a533a5..13209261f9 100644 --- a/src/libsystemd-network/sd-ipv4ll.c +++ b/src/libsystemd-network/sd-ipv4ll.c @@ -90,9 +90,7 @@ sd_ipv4ll *sd_ipv4ll_unref(sd_ipv4ll *ll) { return NULL; sd_ipv4acd_unref(ll->acd); - free(ll); - - return NULL; + return mfree(ll); } int sd_ipv4ll_new(sd_ipv4ll **ret) { diff --git a/src/libsystemd-network/sd-lldp.c b/src/libsystemd-network/sd-lldp.c index 0bd1e66aa0..0702241506 100644 --- a/src/libsystemd-network/sd-lldp.c +++ b/src/libsystemd-network/sd-lldp.c @@ -374,9 +374,7 @@ _public_ sd_lldp* sd_lldp_unref(sd_lldp *lldp) { hashmap_free(lldp->neighbor_by_id); prioq_free(lldp->neighbor_by_expiry); - free(lldp); - - return NULL; + return mfree(lldp); } _public_ int sd_lldp_new(sd_lldp **ret) { diff --git a/src/libsystemd-network/sd-ndisc.c b/src/libsystemd-network/sd-ndisc.c index 07b0d7f704..1d3be9b862 100644 --- a/src/libsystemd-network/sd-ndisc.c +++ b/src/libsystemd-network/sd-ndisc.c @@ -148,9 +148,7 @@ _public_ sd_ndisc *sd_ndisc_unref(sd_ndisc *nd) { ndisc_reset(nd); sd_ndisc_detach_event(nd); - free(nd); - - return NULL; + return mfree(nd); } _public_ int sd_ndisc_new(sd_ndisc **ret) { diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index 8e9074c7df..33590c31ac 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -212,9 +212,7 @@ _public_ sd_bus_slot* sd_bus_slot_unref(sd_bus_slot *slot) { bus_slot_disconnect(slot); free(slot->description); - free(slot); - - return NULL; + return mfree(slot); } _public_ sd_bus* sd_bus_slot_get_bus(sd_bus_slot *slot) { diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c index 00e93a215f..4acaf24793 100644 --- a/src/libsystemd/sd-bus/bus-track.c +++ b/src/libsystemd/sd-bus/bus-track.c @@ -74,9 +74,7 @@ static struct track_item* track_item_free(struct track_item *i) { sd_bus_slot_unref(i->slot); free(i->name); - free(i); - - return NULL; + return mfree(i); } DEFINE_TRIVIAL_CLEANUP_FUNC(struct track_item*, track_item_free); @@ -206,9 +204,7 @@ _public_ sd_bus_track* sd_bus_track_unref(sd_bus_track *track) { bus_track_remove_from_queue(track); hashmap_free(track->names); sd_bus_unref(track->bus); - free(track); - - return NULL; + return mfree(track); } static int on_name_owner_changed(sd_bus_message *message, void *userdata, sd_bus_error *error) { diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index da496ed456..0d51322a15 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -166,17 +166,16 @@ struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char * entry = new0(struct udev_list_entry, 1); if (entry == NULL) return NULL; + entry->name = strdup(name); - if (entry->name == NULL) { - free(entry); - return NULL; - } + if (entry->name == NULL) + return mfree(entry); + if (value != NULL) { entry->value = strdup(value); if (entry->value == NULL) { free(entry->name); - free(entry); - return NULL; + return mfree(entry); } } @@ -193,8 +192,7 @@ struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char * if (entries == NULL) { free(entry->name); free(entry->value); - free(entry); - return NULL; + return mfree(entry); } list->entries = entries; list->entries_max += add; diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c index 1f9d16c450..a1f2b33ad5 100644 --- a/src/libudev/libudev-monitor.c +++ b/src/libudev/libudev-monitor.c @@ -207,8 +207,7 @@ struct udev_monitor *udev_monitor_new_from_netlink_fd(struct udev *udev, const c udev_monitor->sock = socket(PF_NETLINK, SOCK_RAW|SOCK_CLOEXEC|SOCK_NONBLOCK, NETLINK_KOBJECT_UEVENT); if (udev_monitor->sock < 0) { log_debug_errno(errno, "error getting socket: %m"); - free(udev_monitor); - return NULL; + return mfree(udev_monitor); } } else { udev_monitor->bound = true; diff --git a/src/login/logind-button.c b/src/login/logind-button.c index baa6b7113c..90fb93bbaf 100644 --- a/src/login/logind-button.c +++ b/src/login/logind-button.c @@ -43,15 +43,12 @@ Button* button_new(Manager *m, const char *name) { return NULL; b->name = strdup(name); - if (!b->name) { - free(b); - return NULL; - } + if (!b->name) + return mfree(b); if (hashmap_put(m->buttons, b->name, b) < 0) { free(b->name); - free(b); - return NULL; + return mfree(b); } b->manager = m; diff --git a/src/login/logind-device.c b/src/login/logind-device.c index eb5edd1cd5..6537fa04bf 100644 --- a/src/login/logind-device.c +++ b/src/login/logind-device.c @@ -34,15 +34,12 @@ Device* device_new(Manager *m, const char *sysfs, bool master) { return NULL; d->sysfs = strdup(sysfs); - if (!d->sysfs) { - free(d); - return NULL; - } + if (!d->sysfs) + return mfree(d); if (hashmap_put(m->devices, d->sysfs, d) < 0) { free(d->sysfs); - free(d); - return NULL; + return mfree(d); } d->manager = m; diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index 6c78e0dddc..c93b24009b 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -45,17 +45,14 @@ Inhibitor* inhibitor_new(Manager *m, const char* id) { return NULL; i->state_file = strappend("/run/systemd/inhibit/", id); - if (!i->state_file) { - free(i); - return NULL; - } + if (!i->state_file) + return mfree(i); i->id = basename(i->state_file); if (hashmap_put(m->inhibitors, i->id, i) < 0) { free(i->state_file); - free(i); - return NULL; + return mfree(i); } i->manager = m; diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index b5192320e4..ecc7bd2e5b 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -48,18 +48,15 @@ Seat *seat_new(Manager *m, const char *id) { return NULL; s->state_file = strappend("/run/systemd/seats/", id); - if (!s->state_file) { - free(s); - return NULL; - } + if (!s->state_file) + return mfree(s); s->id = basename(s->state_file); s->manager = m; if (hashmap_put(m->seats, s->id, s) < 0) { free(s->state_file); - free(s); - return NULL; + return mfree(s); } return s; diff --git a/src/login/logind-session.c b/src/login/logind-session.c index ba1bcc2630..cbf035f706 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -62,16 +62,13 @@ Session* session_new(Manager *m, const char *id) { return NULL; s->state_file = strappend("/run/systemd/sessions/", id); - if (!s->state_file) { - free(s); - return NULL; - } + if (!s->state_file) + return mfree(s); s->devices = hashmap_new(&devt_hash_ops); if (!s->devices) { free(s->state_file); - free(s); - return NULL; + return mfree(s); } s->id = basename(s->state_file); @@ -79,8 +76,7 @@ Session* session_new(Manager *m, const char *id) { if (hashmap_put(m->sessions, s->id, s) < 0) { hashmap_free(s->devices); free(s->state_file); - free(s); - return NULL; + return mfree(s); } s->manager = m; diff --git a/src/machine/machine.c b/src/machine/machine.c index dd046d6563..a02b9d7575 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -80,9 +80,7 @@ Machine* machine_new(Manager *manager, MachineClass class, const char *name) { fail: free(m->state_file); free(m->name); - free(m); - - return NULL; + return mfree(m); } void machine_free(Machine *m) { diff --git a/src/machine/operation.c b/src/machine/operation.c index 2bf93cb493..c966d0d21c 100644 --- a/src/machine/operation.c +++ b/src/machine/operation.c @@ -147,6 +147,5 @@ Operation *operation_free(Operation *o) { if (o->machine) LIST_REMOVE(operations_by_machine, o->machine->operations, o); - free(o); - return NULL; + return mfree(o); } diff --git a/src/network/networkd-wait-online-link.c b/src/network/networkd-wait-online-link.c index 5727422e3d..e63ba07e90 100644 --- a/src/network/networkd-wait-online-link.c +++ b/src/network/networkd-wait-online-link.c @@ -77,8 +77,7 @@ Link *link_free(Link *l) { } free(l->ifname); - free(l); - return NULL; + return mfree(l); } int link_update_rtnl(Link *l, sd_netlink_message *m) { diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index 5f1522cfb6..09c8f070ba 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -101,9 +101,7 @@ Settings* settings_free(Settings *s) { expose_port_free_all(s->expose_ports); custom_mount_free_all(s->custom_mounts, s->n_custom_mounts); - free(s); - - return NULL; + return mfree(s); } bool settings_private_network(Settings *s) { diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 53be18efc6..e03db4d003 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -83,9 +83,7 @@ DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c) { if (c->scope) LIST_REMOVE(candidates_by_scope, c->scope->query_candidates, c); - free(c); - - return NULL; + return mfree(c); } static int dns_query_candidate_next_search_domain(DnsQueryCandidate *c) { @@ -421,9 +419,7 @@ DnsQuery *dns_query_free(DnsQuery *q) { q->manager->n_dns_queries--; } - free(q); - - return NULL; + return mfree(q); } int dns_query_new( diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 5687588a7d..87e4abec6e 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -73,10 +73,8 @@ DnsResourceKey* dns_resource_key_new_redirect(const DnsResourceKey *key, const D return dns_resource_key_ref((DnsResourceKey*) key); k = dns_resource_key_new_consume(key->class, key->type, destination); - if (!k) { - free(destination); - return NULL; - } + if (!k) + return mfree(destination); return k; } @@ -513,9 +511,7 @@ DnsResourceRecord* dns_resource_record_unref(DnsResourceRecord *rr) { } free(rr->to_string); - free(rr); - - return NULL; + return mfree(rr); } int dns_resource_record_new_reverse(DnsResourceRecord **ret, int family, const union in_addr_union *address, const char *hostname) { diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 03811ac8e7..8dbc7f623b 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -128,9 +128,7 @@ DnsScope* dns_scope_free(DnsScope *s) { dns_zone_flush(&s->zone); LIST_REMOVE(scopes, s->manager->dns_scopes, s); - free(s); - - return NULL; + return mfree(s); } DnsServer *dns_scope_get_dns_server(DnsScope *s) { diff --git a/src/resolve/resolved-dns-search-domain.c b/src/resolve/resolved-dns-search-domain.c index 732471027b..1386e6a17b 100644 --- a/src/resolve/resolved-dns-search-domain.c +++ b/src/resolve/resolved-dns-search-domain.c @@ -104,9 +104,7 @@ DnsSearchDomain* dns_search_domain_unref(DnsSearchDomain *d) { return NULL; free(d->name); - free(d); - - return NULL; + return mfree(d); } void dns_search_domain_unlink(DnsSearchDomain *d) { diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 97cc8c0e09..7282848e35 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -139,8 +139,7 @@ DnsServer* dns_server_unref(DnsServer *s) { return NULL; free(s->server_string); - free(s); - return NULL; + return mfree(s); } void dns_server_unlink(DnsServer *s) { diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c index dd0e0b90e3..878bae47dc 100644 --- a/src/resolve/resolved-dns-stream.c +++ b/src/resolve/resolved-dns-stream.c @@ -343,9 +343,7 @@ DnsStream *dns_stream_unref(DnsStream *s) { dns_packet_unref(s->write_packet); dns_packet_unref(s->read_packet); - free(s); - - return NULL; + return mfree(s); } DEFINE_TRIVIAL_CLEANUP_FUNC(DnsStream*, dns_stream_unref); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index d455b6b1fa..2fce44ec8b 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -134,8 +134,7 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { dns_answer_unref(t->validated_keys); dns_resource_key_unref(t->key); - free(t); - return NULL; + return mfree(t); } DEFINE_TRIVIAL_CLEANUP_FUNC(DnsTransaction*, dns_transaction_free); diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index ea4a007139..13e1f91192 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -101,8 +101,7 @@ Link *link_free(Link *l) { free(l->state_file); - free(l); - return NULL; + return mfree(l); } void link_allocate_scopes(Link *l) { @@ -698,8 +697,7 @@ LinkAddress *link_address_free(LinkAddress *a) { dns_resource_record_unref(a->llmnr_address_rr); dns_resource_record_unref(a->llmnr_ptr_rr); - free(a); - return NULL; + return mfree(a); } void link_address_add_rrs(LinkAddress *a, bool force_remove) { diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 40f08e8044..0954641c20 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -630,9 +630,7 @@ Manager *manager_free(Manager *m) { dns_trust_anchor_flush(&m->trust_anchor); manager_etc_hosts_flush(m); - free(m); - - return NULL; + return mfree(m); } int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) { diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c index 529d89ee2a..060f8d50c7 100644 --- a/src/shared/machine-image.c +++ b/src/shared/machine-image.c @@ -62,8 +62,7 @@ Image *image_unref(Image *i) { free(i->name); free(i->path); - free(i); - return NULL; + return mfree(i); } static char **image_settings_path(Image *image) { diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 24055e772b..293c6673fc 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -463,8 +463,7 @@ int pty_forward_new( PTYForward *pty_forward_free(PTYForward *f) { pty_forward_disconnect(f); - free(f); - return NULL; + return mfree(f); } int pty_forward_get_last_char(PTYForward *f, char *ch) { diff --git a/src/timesync/timesyncd-server.c b/src/timesync/timesyncd-server.c index 6bda86fe6e..57a7bf2c25 100644 --- a/src/timesync/timesyncd-server.c +++ b/src/timesync/timesyncd-server.c @@ -61,8 +61,7 @@ ServerAddress* server_address_free(ServerAddress *a) { manager_set_server_address(a->name->manager, NULL); } - free(a); - return NULL; + return mfree(a); } int server_name_new( @@ -137,9 +136,7 @@ ServerName *server_name_free(ServerName *n) { log_debug("Removed server %s.", n->string); free(n->string); - free(n); - - return NULL; + return mfree(n); } void server_name_flush_addresses(ServerName *n) { diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index f68a09d7a8..7717ac7924 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -211,8 +211,7 @@ struct udev_ctrl_connection *udev_ctrl_get_connection(struct udev_ctrl *uctrl) { err: if (conn->sock >= 0) close(conn->sock); - free(conn); - return NULL; + return mfree(conn); } struct udev_ctrl_connection *udev_ctrl_connection_ref(struct udev_ctrl_connection *conn) { diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 26fa52cf6c..7619c8371b 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1583,8 +1583,7 @@ struct udev_rules *udev_rules_unref(struct udev_rules *rules) { strbuf_cleanup(rules->strbuf); free(rules->uids); free(rules->gids); - free(rules); - return NULL; + return mfree(rules); } bool udev_rules_check_timestamp(struct udev_rules *rules) { -- cgit v1.2.3-54-g00ecf