]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
shared/offline-passwd: look at /usr/lib/{passwd,group} too 16512/head
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Sat, 18 Jul 2020 12:06:19 +0000 (14:06 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Sat, 18 Jul 2020 12:14:19 +0000 (14:14 +0200)
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

index 8ac5431c66da8545ec480c5c3c85d86e17803bb2..3f8220d9ac59eab55c26bc2af8375c3e1cf2f5e5 100644 (file)
@@ -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);