[git commit] libpwdgrp: fix a memory leak in getXXnam (we did not save address of string buf)
Denys Vlasenko
vda.linux at googlemail.com
Sat Jan 3 14:54:04 UTC 2015
commit: http://git.busybox.net/busybox/commit/?id=8d547aca75f8b096976a472714241acd4328be46
branch: http://git.busybox.net/busybox/commit/?id=refs/heads/master
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 at googlemail.com>
---
libpwdgrp/pwd_grp.c | 131 +++++++++++++++++++++++++++++----------------------
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;
More information about the busybox-cvs
mailing list