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>
This commit is contained in:
Denys Vlasenko 2015-01-03 15:54:04 +01:00
parent 31d6734457
commit 8d547aca75

View File

@ -32,6 +32,27 @@
#include "libbb.h" #include "libbb.h"
struct const_passdb {
const char *filename;
const char def[10];
const uint8_t off[9];
uint8_t numfields;
};
struct passdb {
const char *filename;
const char def[10];
const uint8_t off[9];
uint8_t numfields;
FILE *fp;
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) */
];
};
/* S = string not empty, s = string maybe empty, /* S = string not empty, s = string maybe empty,
* I = uid,gid, l = long maybe empty, m = members, * I = uid,gid, l = long maybe empty, m = members,
* r = reserved * r = reserved
@ -40,7 +61,9 @@
#define GR_DEF "SsIm" #define GR_DEF "SsIm"
#define SP_DEF "Ssllllllr" #define SP_DEF "Ssllllllr"
static const uint8_t pw_off[] ALIGN1 = { static const struct const_passdb const_pw_db = {
_PATH_PASSWD, PW_DEF,
{
offsetof(struct passwd, pw_name), /* 0 S */ offsetof(struct passwd, pw_name), /* 0 S */
offsetof(struct passwd, pw_passwd), /* 1 s */ offsetof(struct passwd, pw_passwd), /* 1 s */
offsetof(struct passwd, pw_uid), /* 2 I */ offsetof(struct passwd, pw_uid), /* 2 I */
@ -48,15 +71,23 @@ static const uint8_t pw_off[] ALIGN1 = {
offsetof(struct passwd, pw_gecos), /* 4 s */ offsetof(struct passwd, pw_gecos), /* 4 s */
offsetof(struct passwd, pw_dir), /* 5 S */ offsetof(struct passwd, pw_dir), /* 5 S */
offsetof(struct passwd, pw_shell) /* 6 S */ offsetof(struct passwd, pw_shell) /* 6 S */
},
sizeof(PW_DEF)-1
}; };
static const uint8_t gr_off[] ALIGN1 = { static const struct const_passdb const_gr_db = {
_PATH_GROUP, GR_DEF,
{
offsetof(struct group, gr_name), /* 0 S */ offsetof(struct group, gr_name), /* 0 S */
offsetof(struct group, gr_passwd), /* 1 s */ offsetof(struct group, gr_passwd), /* 1 s */
offsetof(struct group, gr_gid), /* 2 I */ offsetof(struct group, gr_gid), /* 2 I */
offsetof(struct group, gr_mem) /* 3 m (char **) */ offsetof(struct group, gr_mem) /* 3 m (char **) */
},
sizeof(GR_DEF)-1
}; };
#if ENABLE_USE_BB_SHADOW #if ENABLE_USE_BB_SHADOW
static const uint8_t sp_off[] ALIGN1 = { 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_namp), /* 0 S Login name */
offsetof(struct spwd, sp_pwdp), /* 1 s Encrypted password */ offsetof(struct spwd, sp_pwdp), /* 1 s Encrypted password */
offsetof(struct spwd, sp_lstchg), /* 2 l */ offsetof(struct spwd, sp_lstchg), /* 2 l */
@ -66,35 +97,14 @@ static const uint8_t sp_off[] ALIGN1 = {
offsetof(struct spwd, sp_inact), /* 6 l */ offsetof(struct spwd, sp_inact), /* 6 l */
offsetof(struct spwd, sp_expire), /* 7 l */ offsetof(struct spwd, sp_expire), /* 7 l */
offsetof(struct spwd, sp_flag) /* 8 r Reserved */ offsetof(struct spwd, sp_flag) /* 8 r Reserved */
},
sizeof(SP_DEF)-1
}; };
#endif #endif
struct const_passdb {
const char *filename;
const uint8_t *off;
const char def[10];
uint8_t numfields;
uint8_t size_of;
};
struct passdb {
const char *filename;
const uint8_t *off;
const char def[10];
uint8_t numfields;
uint8_t size_of;
FILE *fp;
void *malloced;
};
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) };
#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) };
#endif
/* We avoid having big global data. */ /* We avoid having big global data. */
struct statics { 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: * Manpage says:
* "The return value may point to a static area, and may be overwritten * "The return value may point to a static area, and may be overwritten
* by subsequent calls to getpwent(), getpwnam(), or getpwuid()." * 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 */ /* 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) 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 (;;) { for (;;) {
void *member = (char*)result + (*off++); void *member = (char*)result + (*off++);
@ -282,7 +298,8 @@ static void *convert_to_struct(const char *def, const unsigned char *off,
/****** getXXnam/id_r */ /****** 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 *result)
{ {
void *struct_buf = *(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; errno = ERANGE;
} else { } else {
memcpy(buffer, buf, size); 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); free(buf);
} }
@ -310,7 +327,8 @@ static int FAST_FUNC getXXnam_r(const char *name, uintptr_t db_and_field_pos, ch
return errno; return errno;
} }
int FAST_FUNC getpwnam_r(const char *name, struct passwd *struct_buf, char *buffer, size_t buflen, int FAST_FUNC getpwnam_r(const char *name, struct passwd *struct_buf,
char *buffer, size_t buflen,
struct passwd **result) struct passwd **result)
{ {
/* Why the "store buffer address in result" trick? /* Why the "store buffer address in result" trick?
@ -357,7 +375,7 @@ static int FAST_FUNC getXXent_r(void *struct_buf, char *buffer, size_t buflen,
errno = ERANGE; errno = ERANGE;
} else { } else {
memcpy(buffer, buf, size); 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); 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)); 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); buf = parse_common(db->fp, db->filename, db->numfields, name, db_and_field_pos & 3);
if (buf) { if (buf) {
db->malloced = xzalloc(db->size_of); free(db->malloced);
result = convert_to_struct(db->def, db->off, buf, db->malloced); db->malloced = buf;
result = convert_to_struct(db, buf, db->struct_result);
} }
return 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) { while ((buf = parse_common(fp, _PATH_GROUP, sizeof(GR_DEF)-1, NULL, 0)) != NULL) {
char **m; char **m;
struct group group; struct group group;
if (!convert_to_struct(GR_DEF, gr_off, buf, &group)) if (!convert_to_struct(&S.db[1], buf, &group))
goto next; goto next;
if (group.gr_gid == gid) if (group.gr_gid == gid)
goto next; goto next;