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 823884e..6d938f6 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;