diff --git a/src/basic/path-util.c b/src/basic/path-util.c index b442eebb87..40a819d47d 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -519,17 +519,17 @@ char* path_extend_internal(char **x, ...) { va_list ap; bool slash; - /* Joins all listed strings until the sentinel and places a "/" between them unless the strings end/begin - * already with one so that it is unnecessary. Note that slashes which are already duplicate won't be - * removed. The string returned is hence always equal to or longer than the sum of the lengths of each - * individual string. + /* Joins all listed strings until the sentinel and places a "/" between them unless the strings + * end/begin already with one so that it is unnecessary. Note that slashes which are already + * duplicate won't be removed. The string returned is hence always equal to or longer than the sum of + * the lengths of the individual strings. * * The first argument may be an already allocated string that is extended via realloc() if * non-NULL. path_extend() and path_join() are macro wrappers around this function, making use of the * first parameter to distinguish the two operations. * - * Note: any listed empty string is simply skipped. This can be useful for concatenating strings of which some - * are optional. + * Note: any listed empty string is simply skipped. This can be useful for concatenating strings of + * which some are optional. * * Examples: * diff --git a/src/basic/user-util.c b/src/basic/user-util.c index e2857b3102..6c5041230a 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -19,6 +19,7 @@ #include "fileio.h" #include "format-util.h" #include "macro.h" +#include "mkdir.h" #include "parse-util.h" #include "path-util.h" #include "path-util.h" @@ -670,7 +671,6 @@ int reset_uid_gid(void) { } int take_etc_passwd_lock(const char *root) { - struct flock flock = { .l_type = F_WRLCK, .l_whence = SEEK_SET, @@ -678,35 +678,27 @@ int take_etc_passwd_lock(const char *root) { .l_len = 0, }; - const char *path; - int fd, r; - - /* This is roughly the same as lckpwdf(), but not as awful. We - * don't want to use alarm() and signals, hence we implement - * our own trivial version of this. + /* This is roughly the same as lckpwdf(), but not as awful. We don't want to use alarm() and signals, + * hence we implement our own trivial version of this. * - * Note that shadow-utils also takes per-database locks in - * addition to lckpwdf(). However, we don't given that they - * are redundant as they invoke lckpwdf() first and keep - * it during everything they do. The per-database locks are - * awfully racy, and thus we just won't do them. */ + * Note that shadow-utils also takes per-database locks in addition to lckpwdf(). However, we don't, + * given that they are redundant: they invoke lckpwdf() first and keep it during everything they do. + * The per-database locks are awfully racy, and thus we just won't do them. */ - if (root) - path = prefix_roota(root, ETC_PASSWD_LOCK_PATH); - else - path = ETC_PASSWD_LOCK_PATH; + _cleanup_free_ char *path = path_join(root, ETC_PASSWD_LOCK_PATH); + if (!path) + return log_oom_debug(); - fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0600); + (void) mkdir_parents(path, 0755); + + _cleanup_close_ int fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0600); if (fd < 0) return log_debug_errno(errno, "Cannot open %s: %m", path); - r = fcntl(fd, F_SETLKW, &flock); - if (r < 0) { - safe_close(fd); + if (fcntl(fd, F_SETLKW, &flock) < 0) return log_debug_errno(errno, "Locking %s failed: %m", path); - } - return fd; + return TAKE_FD(fd); } bool valid_user_group_name(const char *u, ValidUserFlags flags) { diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 0ed2d1b31d..ce15758901 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -4606,7 +4606,7 @@ int config_parse_exec_directories( if (r == -ENOMEM) return log_oom(); if (r <= 0) { - log_syntax(unit, LOG_WARNING, filename, line, r ?: SYNTHETIC_ERRNO(EINVAL), + log_syntax(unit, LOG_WARNING, filename, line, r, "Invalid syntax in %s=, ignoring: %s", lvalue, tuple); return 0; } diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 45bb386da2..9e79f84691 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -904,8 +904,6 @@ static int process_root_account(void) { return 0; } - (void) mkdir_parents(etc_passwd, 0755); - lock = take_etc_passwd_lock(arg_root); if (lock < 0) return log_error_errno(lock, "Failed to take a lock on %s: %m", etc_passwd); diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 6e197b964d..3c438a38f1 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -76,17 +76,20 @@ typedef struct Item { gid_t gid; uid_t uid; - bool gid_set:1; + char *filename; + unsigned line; + + bool gid_set; /* When set the group with the specified GID must exist * and the check if a UID clashes with the GID is skipped. */ - bool id_set_strict:1; + bool id_set_strict; - bool uid_set:1; + bool uid_set; - bool todo_user:1; - bool todo_group:1; + bool todo_user; + bool todo_group; } Item; static char *arg_root = NULL; @@ -132,6 +135,14 @@ static int errno_is_not_exists(int code) { return IN_SET(code, 0, ENOENT, ESRCH, EBADF, EPERM); } +/* Note: the lifetime of the compound literal is the immediately surrounding block, + * see C11 §6.5.2.5, and + * https://stackoverflow.com/questions/34880638/compound-literal-lifetime-and-if-blocks */ +#define FORMAT_UID(is_set, uid) \ + ((is_set) ? snprintf_ok((char[DECIMAL_STR_MAX(uid_t)]){}, DECIMAL_STR_MAX(uid_t), UID_FMT, uid) : "(unset)") +#define FORMAT_GID(is_set, gid) \ + ((is_set) ? snprintf_ok((char[DECIMAL_STR_MAX(gid_t)]){}, DECIMAL_STR_MAX(gid_t), GID_FMT, gid) : "(unset)") + static void maybe_emit_login_defs_warning(void) { if (!login_defs_need_warning) return; @@ -1412,12 +1423,33 @@ static Item* item_free(Item *i) { free(i->description); free(i->home); free(i->shell); + free(i->filename); return mfree(i); } DEFINE_TRIVIAL_CLEANUP_FUNC(Item*, item_free); DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_hash_ops, char, string_hash_func, string_compare_func, Item, item_free); +static Item* item_new(ItemType type, const char *name, const char *filename, unsigned line) { + assert(name); + assert(!!filename == (line > 0)); + + _cleanup_(item_freep) Item *new = new(Item, 1); + if (!new) + return NULL; + + *new = (Item) { + .type = type, + .line = line, + }; + + if (free_and_strdup(&new->name, name) < 0 || + free_and_strdup(&new->filename, filename) < 0) + return NULL; + + return TAKE_PTR(new); +} + static int add_implicit(void) { char *g, **l; int r; @@ -1426,17 +1458,11 @@ static int add_implicit(void) { ORDERED_HASHMAP_FOREACH_KEY(l, g, members) { STRV_FOREACH(m, l) if (!ordered_hashmap_get(users, *m)) { - _cleanup_(item_freep) Item *j = NULL; - - j = new0(Item, 1); + _cleanup_(item_freep) Item *j = + item_new(ADD_USER, *m, /* filename= */ NULL, /* line= */ 0); if (!j) return log_oom(); - j->type = ADD_USER; - j->name = strdup(*m); - if (!j->name) - return log_oom(); - r = ordered_hashmap_ensure_put(&users, &item_hash_ops, j->name, j); if (r == -ENOMEM) return log_oom(); @@ -1449,17 +1475,11 @@ static int add_implicit(void) { if (!(ordered_hashmap_get(users, g) || ordered_hashmap_get(groups, g))) { - _cleanup_(item_freep) Item *j = NULL; - - j = new0(Item, 1); + _cleanup_(item_freep) Item *j = + item_new(ADD_GROUP, g, /* filename= */ NULL, /* line= */ 0); if (!j) return log_oom(); - j->type = ADD_GROUP; - j->name = strdup(g); - if (!j->name) - return log_oom(); - r = ordered_hashmap_ensure_put(&groups, &item_hash_ops, j->name, j); if (r == -ENOMEM) return log_oom(); @@ -1480,36 +1500,63 @@ static int item_equivalent(Item *a, Item *b) { assert(a); assert(b); - if (a->type != b->type) + if (a->type != b->type) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because types differ"); return false; + } - if (!streq_ptr(a->name, b->name)) + if (!streq_ptr(a->name, b->name)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because names differ ('%s' vs. '%s')", + a->name, b->name); return false; + } /* Paths were simplified previously, so we can use streq. */ - if (!streq_ptr(a->uid_path, b->uid_path)) + if (!streq_ptr(a->uid_path, b->uid_path)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because UID paths differ (%s vs. %s)", + a->uid_path ?: "(unset)", b->uid_path ?: "(unset)"); return false; + } - if (!streq_ptr(a->gid_path, b->gid_path)) + if (!streq_ptr(a->gid_path, b->gid_path)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because GID paths differ (%s vs. %s)", + a->gid_path ?: "(unset)", b->gid_path ?: "(unset)"); return false; + } - if (!streq_ptr(a->description, b->description)) + if (!streq_ptr(a->description, b->description)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because descriptions differ ('%s' vs. '%s')", + strempty(a->description), strempty(b->description)); return false; + } - if (a->uid_set != b->uid_set) + if ((a->uid_set != b->uid_set) || + (a->uid_set && a->uid != b->uid)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because UIDs differ (%s vs. %s)", + FORMAT_UID(a->uid_set, a->uid), FORMAT_UID(b->uid_set, b->uid)); return false; + } - if (a->uid_set && a->uid != b->uid) + if ((a->gid_set != b->gid_set) || + (a->gid_set && a->gid != b->gid)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because GIDs differ (%s vs. %s)", + FORMAT_GID(a->gid_set, a->gid), FORMAT_GID(b->gid_set, b->gid)); return false; + } - if (a->gid_set != b->gid_set) - return false; - - if (a->gid_set && a->gid != b->gid) - return false; - - if (!streq_ptr(a->home, b->home)) + if (!streq_ptr(a->home, b->home)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because home directories differ ('%s' vs. '%s')", + strempty(a->description), strempty(b->description)); return false; + } /* Check if the two paths refer to the same file. * If the paths are equal (after normalization), it's obviously the same file. @@ -1540,8 +1587,12 @@ static int item_equivalent(Item *a, Item *b) { return ERRNO_IS_RESOURCE(r) ? r : false; } - if (!path_equal(pa, pb)) + if (!path_equal(pa, pb)) { + log_syntax(NULL, LOG_DEBUG, a->filename, a->line, 0, + "Item not equivalent because shells differ ('%s' vs. '%s')", + pa, pb); return false; + } } return true; @@ -1720,15 +1771,14 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (r < 0) return log_oom(); - i = new0(Item, 1); + i = item_new(ADD_USER, resolved_name, fname, line); if (!i) return log_oom(); if (resolved_id) { - if (path_is_absolute(resolved_id)) { - i->uid_path = TAKE_PTR(resolved_id); - path_simplify(i->uid_path); - } else { + if (path_is_absolute(resolved_id)) + i->uid_path = path_simplify(TAKE_PTR(resolved_id)); + else { _cleanup_free_ char *uid = NULL, *gid = NULL; if (split_pair(resolved_id, ":", &uid, &gid) == 0) { r = parse_gid(gid, &i->gid); @@ -1776,15 +1826,14 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (r < 0) return log_oom(); - i = new0(Item, 1); + i = item_new(ADD_GROUP, resolved_name, fname, line); if (!i) return log_oom(); if (resolved_id) { - if (path_is_absolute(resolved_id)) { - i->gid_path = TAKE_PTR(resolved_id); - path_simplify(i->gid_path); - } else { + if (path_is_absolute(resolved_id)) + i->gid_path = path_simplify(TAKE_PTR(resolved_id)); + else { r = parse_gid(resolved_id, &i->gid); if (r < 0) return log_syntax(NULL, LOG_ERR, fname, line, r, @@ -1798,22 +1847,28 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { break; default: - return -EBADMSG; + assert_not_reached(); } - i->type = action[0]; - i->name = TAKE_PTR(resolved_name); - existing = ordered_hashmap_get(h, i->name); if (existing) { /* Two functionally-equivalent items are fine */ - r = item_equivalent(existing, i); + r = item_equivalent(i, existing); if (r < 0) return r; - if (r == 0) - log_syntax(NULL, LOG_WARNING, fname, line, SYNTHETIC_ERRNO(EUCLEAN), - "Conflict with earlier configuration for %s '%s', ignoring line.", - item_type_to_string(i->type), i->name); + if (r == 0) { + if (existing->filename) + log_syntax(NULL, LOG_WARNING, fname, line, 0, + "Conflict with earlier configuration for %s '%s' in %s:%u, ignoring line.", + item_type_to_string(i->type), + i->name, + existing->filename, existing->line); + else + log_syntax(NULL, LOG_WARNING, fname, line, 0, + "Conflict with earlier configuration for %s '%s', ignoring line.", + item_type_to_string(i->type), + i->name); + } return 0; } diff --git a/test/test-sysusers.sh.in b/test/test-sysusers.sh.in index 950dc297d8..fab37960bc 100755 --- a/test/test-sysusers.sh.in +++ b/test/test-sysusers.sh.in @@ -99,6 +99,14 @@ $SYSUSERS --root=$TESTDIR \ compare $SOURCE/inline "(--inline --replace=…)" +echo "*** Testing --inline with no /etc" +rm -rf $TESTDIR/etc +$SYSUSERS --root=$TESTDIR --inline \ + "u u1 222 - - /bin/zsh" \ + "g g1 111" + +compare $SOURCE/inline "(--inline)" + rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* cat >$TESTDIR/etc/login.defs <