From 241947d1b43556cb2b859fd36aae7352f3b1c8fb Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 18 Jul 2020 14:06:19 +0200 Subject: [PATCH] shared/offline-passwd: look at /usr/lib/{passwd,group} too This changes the code to allow looking at multiple files with different prefixes, but uses "/etc" and "/usr/lib". rpm-ostree uses /usr/lib/{passwd,group} with nss-altfiles. I see no harm in simply trying both paths on all systems. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1857530. A minor memory leak is fixed: hashmap_put() returns -EEXIST is the key is present *and* and the value is different. It return 0 if the value is the same. Thus, we would leak the user/group name if it was specified multiple times with the same uid/gid. I opted to remove the warning message completely: with multiple files it is reasonable to have the same name defined more than once. But even with one file the warning is dubious: all tools that read those files deal correctly with duplicate entries and we are not writing a linter. --- src/shared/offline-passwd.c | 154 ++++++++++++++++++++++-------------- 1 file changed, 95 insertions(+), 59 deletions(-) diff --git a/src/shared/offline-passwd.c b/src/shared/offline-passwd.c index 8ac5431c66d..3f8220d9ac5 100644 --- a/src/shared/offline-passwd.c +++ b/src/shared/offline-passwd.c @@ -7,34 +7,43 @@ DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(uid_gid_hash_ops, char, string_hash_func, string_compare_func, free); -int name_to_uid_offline( - const char *root, - const char *user, - uid_t *ret_uid, - Hashmap **cache) { +static int open_passwd_file(const char *root, const char *fname, FILE **ret_file) { + const char *p = prefix_roota(root, fname); + if (!p) + return -ENOMEM; - void *found; + FILE *f = fopen(p, "re"); + if (!f) + return -errno; + + log_debug("Reading %s entries from %s...", basename(fname), p); + + *ret_file = f; + return 0; +} + +static int populate_uid_cache(const char *root, Hashmap **ret) { + _cleanup_(hashmap_freep) Hashmap *cache = NULL; int r; - assert(user); - assert(ret_uid); - assert(cache); + cache = hashmap_new(&uid_gid_hash_ops); + if (!cache) + return -ENOMEM; - if (!*cache) { - _cleanup_(hashmap_freep) Hashmap *uid_by_name = NULL; - _cleanup_fclose_ FILE *f = NULL; - struct passwd *pw; - const char *passwd_path; + /* The directory list is harcoded here: /etc is the standard, and rpm-ostree uses /usr/lib. This + * could be made configurable, but I don't see the point right now. */ - passwd_path = prefix_roota(root, "/etc/passwd"); - f = fopen(passwd_path, "re"); - if (!f) - return errno == ENOENT ? -ESRCH : -errno; + const char *fname; + FOREACH_STRING(fname, "/etc/passwd", "/usr/lib/passwd") { + _cleanup_fclose_ FILE *f = NULL; - uid_by_name = hashmap_new(&uid_gid_hash_ops); - if (!uid_by_name) - return -ENOMEM; + r = open_passwd_file(root, fname, &f); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + struct passwd *pw; while ((r = fgetpwent_sane(f, &pw)) > 0) { _cleanup_free_ char *n = NULL; @@ -42,18 +51,75 @@ int name_to_uid_offline( if (!n) return -ENOMEM; - r = hashmap_put(uid_by_name, n, UID_TO_PTR(pw->pw_uid)); - if (r == -EEXIST) { - log_warning_errno(r, "Duplicate entry in %s for %s: %m", passwd_path, pw->pw_name); + r = hashmap_put(cache, n, UID_TO_PTR(pw->pw_uid)); + if (IN_SET(r, 0 -EEXIST)) continue; - } if (r < 0) return r; + TAKE_PTR(n); + } + } + + *ret = TAKE_PTR(cache); + return 0; +} + +static int populate_gid_cache(const char *root, Hashmap **ret) { + _cleanup_(hashmap_freep) Hashmap *cache = NULL; + int r; + + cache = hashmap_new(&uid_gid_hash_ops); + if (!cache) + return -ENOMEM; + + const char *fname; + FOREACH_STRING(fname, "/etc/group", "/usr/lib/group") { + _cleanup_fclose_ FILE *f = NULL; + r = open_passwd_file(root, fname, &f); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + + struct group *gr; + while ((r = fgetgrent_sane(f, &gr)) > 0) { + _cleanup_free_ char *n = NULL; + + n = strdup(gr->gr_name); + if (!n) + return -ENOMEM; + + r = hashmap_put(cache, n, GID_TO_PTR(gr->gr_gid)); + if (IN_SET(r, 0, -EEXIST)) + continue; + if (r < 0) + return r; TAKE_PTR(n); } + } + + *ret = TAKE_PTR(cache); + return 0; +} + +int name_to_uid_offline( + const char *root, + const char *user, + uid_t *ret_uid, + Hashmap **cache) { + + void *found; + int r; - *cache = TAKE_PTR(uid_by_name); + assert(user); + assert(ret_uid); + assert(cache); + + if (!*cache) { + r = populate_uid_cache(root, cache); + if (r < 0) + return r; } found = hashmap_get(*cache, user); @@ -78,39 +144,9 @@ int name_to_gid_offline( assert(cache); if (!*cache) { - _cleanup_(hashmap_freep) Hashmap *gid_by_name = NULL; - _cleanup_fclose_ FILE *f = NULL; - struct group *gr; - const char *group_path; - - group_path = prefix_roota(root, "/etc/group"); - f = fopen(group_path, "re"); - if (!f) - return errno == ENOENT ? -ESRCH : -errno; - - gid_by_name = hashmap_new(&uid_gid_hash_ops); - if (!gid_by_name) - return -ENOMEM; - - while ((r = fgetgrent_sane(f, &gr)) > 0) { - _cleanup_free_ char *n = NULL; - - n = strdup(gr->gr_name); - if (!n) - return -ENOMEM; - - r = hashmap_put(gid_by_name, n, GID_TO_PTR(gr->gr_gid)); - if (r == -EEXIST) { - log_warning_errno(r, "Duplicate entry in %s for %s: %m", group_path, gr->gr_name); - continue; - } - if (r < 0) - return r; - - TAKE_PTR(n); - } - - *cache = TAKE_PTR(gid_by_name); + r = populate_gid_cache(root, cache); + if (r < 0) + return r; } found = hashmap_get(*cache, group); -- 2.39.2