libpwdgrp: fix a memory leak in getXXnam (we did not save address of string buf)

function                                             old     new   delta
convert_to_struct                                    261     269      +8
const_sp_db                                           20      24      +4
const_pw_db                                           20      24      +4
const_gr_db                                           20      24      +4
tokenize                                             144     147      +3
parse_common                                         185     188      +3
get_S                                                 82      85      +3
bb_internal_getpwent_r                               188     185      -3
gr_off                                                 4       -      -4
getXXnam                                             171     165      -6
pw_off                                                 7       -      -7
getgrouplist_internal                                237     229      -8
getXXnam_r                                           215     207      -8
sp_off                                                 9       -      -9
------------------------------------------------------------------------------
(add/remove: 0/3 grow/shrink: 7/4 up/down: 29/-45)            Total: -16 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

1 files changed, 74 insertions, 57 deletions

diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
index 823884edc..6d938f621 100644
--- a/libpwdgrp/pwd_grp.c
+++ b/libpwdgrp/pwd_grp.c
@@ -32,69 +32,79 @@
 
 #include "libbb.h"
 
-/* S = string not empty, s = string maybe empty,
- * I = uid,gid, l = long maybe empty, m = members,
- * r = reserved
- */
-#define PW_DEF "SsIIsSS"
-#define GR_DEF "SsIm"
-#define SP_DEF "Ssllllllr"
-
-static const uint8_t pw_off[] ALIGN1 = {
-        offsetof(struct passwd, pw_name),       /* 0 S */
-        offsetof(struct passwd, pw_passwd),     /* 1 s */
-        offsetof(struct passwd, pw_uid),        /* 2 I */
-        offsetof(struct passwd, pw_gid),        /* 3 I */
-        offsetof(struct passwd, pw_gecos),      /* 4 s */
-        offsetof(struct passwd, pw_dir),        /* 5 S */
-        offsetof(struct passwd, pw_shell)       /* 6 S */
-};
-static const uint8_t gr_off[] ALIGN1 = {
-        offsetof(struct group, gr_name),        /* 0 S */
-        offsetof(struct group, gr_passwd),      /* 1 s */
-        offsetof(struct group, gr_gid),         /* 2 I */
-        offsetof(struct group, gr_mem)          /* 3 m (char **) */
-};
-#if ENABLE_USE_BB_SHADOW
-static const uint8_t sp_off[] ALIGN1 = {
-        offsetof(struct spwd, sp_namp),         /* 0 S Login name */
-        offsetof(struct spwd, sp_pwdp),         /* 1 s Encrypted password */
-        offsetof(struct spwd, sp_lstchg),       /* 2 l */
-        offsetof(struct spwd, sp_min),          /* 3 l */
-        offsetof(struct spwd, sp_max),          /* 4 l */
-        offsetof(struct spwd, sp_warn),         /* 5 l */
-        offsetof(struct spwd, sp_inact),        /* 6 l */
-        offsetof(struct spwd, sp_expire),       /* 7 l */
-        offsetof(struct spwd, sp_flag)          /* 8 r Reserved */
-};
-#endif
-
 struct const_passdb {
         const char *filename;
-        const uint8_t *off;
         const char def[10];
+        const uint8_t off[9];
         uint8_t numfields;
-        uint8_t size_of;
 };
 struct passdb {
         const char *filename;
-        const uint8_t *off;
         const char def[10];
+        const uint8_t off[9];
         uint8_t numfields;
-        uint8_t size_of;
         FILE *fp;
-        void *malloced;
+        char *malloced;
+        char struct_result[0
+                                | sizeof(struct passwd)
+                                | sizeof(struct group)
+        IF_USE_BB_SHADOW(       | sizeof(struct spwd)  )
+        /* bitwise OR above is poor man's max(a,b,c) */
+        ];
 };
 
-static const struct const_passdb const_pw_db = { _PATH_PASSWD, pw_off, PW_DEF, sizeof(PW_DEF)-1, sizeof(struct passwd) };
-static const struct const_passdb const_gr_db = { _PATH_GROUP , gr_off, GR_DEF, sizeof(GR_DEF)-1, sizeof(struct group) };
+/* S = string not empty, s = string maybe empty,
+ * I = uid,gid, l = long maybe empty, m = members,
+ * r = reserved
+ */
+#define PW_DEF "SsIIsSS"
+#define GR_DEF "SsIm"
+#define SP_DEF "Ssllllllr"
+
+static const struct const_passdb const_pw_db = {
+        _PATH_PASSWD, PW_DEF,
+        {
+                offsetof(struct passwd, pw_name),       /* 0 S */
+                offsetof(struct passwd, pw_passwd),     /* 1 s */
+                offsetof(struct passwd, pw_uid),        /* 2 I */
+                offsetof(struct passwd, pw_gid),        /* 3 I */
+                offsetof(struct passwd, pw_gecos),      /* 4 s */
+                offsetof(struct passwd, pw_dir),        /* 5 S */
+                offsetof(struct passwd, pw_shell)       /* 6 S */
+        },
+        sizeof(PW_DEF)-1
+};
+static const struct const_passdb const_gr_db = {
+        _PATH_GROUP, GR_DEF,
+        {
+                offsetof(struct group, gr_name),        /* 0 S */
+                offsetof(struct group, gr_passwd),      /* 1 s */
+                offsetof(struct group, gr_gid),         /* 2 I */
+                offsetof(struct group, gr_mem)          /* 3 m (char **) */
+        },
+        sizeof(GR_DEF)-1
+};
 #if ENABLE_USE_BB_SHADOW
-static const struct const_passdb const_sp_db = { _PATH_SHADOW, sp_off, SP_DEF, sizeof(SP_DEF)-1, sizeof(struct spwd) };
+static const struct const_passdb const_sp_db = {
+        _PATH_SHADOW, SP_DEF,
+        {
+                offsetof(struct spwd, sp_namp),         /* 0 S Login name */
+                offsetof(struct spwd, sp_pwdp),         /* 1 s Encrypted password */
+                offsetof(struct spwd, sp_lstchg),       /* 2 l */
+                offsetof(struct spwd, sp_min),          /* 3 l */
+                offsetof(struct spwd, sp_max),          /* 4 l */
+                offsetof(struct spwd, sp_warn),         /* 5 l */
+                offsetof(struct spwd, sp_inact),        /* 6 l */
+                offsetof(struct spwd, sp_expire),       /* 7 l */
+                offsetof(struct spwd, sp_flag)          /* 8 r Reserved */
+        },
+        sizeof(SP_DEF)-1
+};
 #endif
 
 /* We avoid having big global data. */
 struct statics {
-        /* It's ok to use same buffer (db[0].malloced) for getpwuid and getpwnam.
+        /* It's ok to use same buffer (db[0].struct_result) for getpwuid and getpwnam.
          * Manpage says:
          * "The return value may point to a static area, and may be overwritten
          * by subsequent calls to getpwent(), getpwnam(), or getpwuid()."
@@ -223,9 +233,15 @@ static char *parse_file(const char *filename,
 }
 
 /* Convert passwd/group/shadow file record in buffer to a struct */
-static void *convert_to_struct(const char *def, const unsigned char *off,
+static void *convert_to_struct(struct passdb *db,
                 char *buffer, void *result)
 {
+        const char *def = db->def;
+        const uint8_t *off = db->off;
+
+/* TODO? for consistency, zero out all fields */
+/*      memset(result, 0, size_of_result);*/
+
         for (;;) {
                 void *member = (char*)result + (*off++);
 
@@ -282,7 +298,8 @@ static void *convert_to_struct(const char *def,	const unsigned char *off,
 
 /****** getXXnam/id_r */
 
-static int FAST_FUNC getXXnam_r(const char *name, uintptr_t db_and_field_pos, char *buffer, size_t buflen,
+static int FAST_FUNC getXXnam_r(const char *name, uintptr_t db_and_field_pos,
+                char *buffer, size_t buflen,
                 void *result)
 {
         void *struct_buf = *(void**)result;
@@ -299,7 +316,7 @@ static int FAST_FUNC getXXnam_r(const char *name, uintptr_t db_and_field_pos, ch
                         errno = ERANGE;
                 } else {
                         memcpy(buffer, buf, size);
-                        *(void**)result = convert_to_struct(db->def, db->off, buffer, struct_buf);
+                        *(void**)result = convert_to_struct(db, buffer, struct_buf);
                 }
                 free(buf);
         }
@@ -310,8 +327,9 @@ static int FAST_FUNC getXXnam_r(const char *name, uintptr_t db_and_field_pos, ch
         return errno;
 }
 
-int FAST_FUNC getpwnam_r(const char *name, struct passwd *struct_buf, char *buffer, size_t buflen,
-                                struct passwd **result)
+int FAST_FUNC getpwnam_r(const char *name, struct passwd *struct_buf,
+                char *buffer, size_t buflen,
+                struct passwd **result)
 {
         /* Why the "store buffer address in result" trick?
          * This way, getXXnam_r has the same ABI signature as getpwnam_r,
@@ -357,7 +375,7 @@ static int FAST_FUNC getXXent_r(void *struct_buf, char *buffer, size_t buflen,
                         errno = ERANGE;
                 } else {
                         memcpy(buffer, buf, size);
-                        *(void**)result = convert_to_struct(db->def, db->off, buffer, struct_buf);
+                        *(void**)result = convert_to_struct(db, buffer, struct_buf);
                 }
                 free(buf);
         }
@@ -393,12 +411,11 @@ static void* FAST_FUNC getXXnam(const char *name, unsigned db_and_field_pos)
                 close_on_exec_on(fileno(db->fp));
         }
 
-        free(db->malloced);
-        db->malloced = NULL;
         buf = parse_common(db->fp, db->filename, db->numfields, name, db_and_field_pos & 3);
         if (buf) {
-                db->malloced = xzalloc(db->size_of);
-                result = convert_to_struct(db->def, db->off, buf, db->malloced);
+                free(db->malloced);
+                db->malloced = buf;
+                result = convert_to_struct(db, buf, db->struct_result);
         }
         return result;
 }
@@ -465,7 +482,7 @@ static gid_t* FAST_FUNC getgrouplist_internal(int *ngroups_ptr,
                 while ((buf = parse_common(fp, _PATH_GROUP, sizeof(GR_DEF)-1, NULL, 0)) != NULL) {
                         char **m;
                         struct group group;
-                        if (!convert_to_struct(GR_DEF, gr_off, buf, &group))
+                        if (!convert_to_struct(&S.db[1], buf, &group))
                                 goto next;
                         if (group.gr_gid == gid)
                                 goto next;