Hi,
after a few weeks looking intensively at this code to improve it, shrink it etc,
now i get sick just when I think about it, so i guess it is time to post my
results
to the attention of the list members for further improvement.
Attached you will find a drop in replacement for busybox/libpwdgrp/pwd_grp.c
for easy testing as a patch would be big and does not make sense at this stage.
What I've done so far?
/* Rewrite of some parts. Main differences are:
*
* 1) the buffer for getpwuid, getgrgid, getpwnam, getgrnam is dynamically
* allocated and reused by later calls. if ERANGE error pops up it is
* reallocated to the size of the longest line found in the passwd/group
* files and reused for later calls.
*
* 2) the passwd/group files:
* a) must contain the expected number of fields;
* b) some fields are not allowed to be empty (e.g. username, homedir,
shell);
* c) leading or trailing spaces in fields are not allowed;
* d) the string representing uid/gid must be convertible by strtoXX
functions;
* if the above explained conditions are not met a error message about bad
* record in file is printed so that the user knows about it.
* 3) the internal function for getgrouplist uses a dynamically allocated
* buffer and retries with a bigger one in case it is to small;
* 4) the _r functions use the user supplied buffers that are never reallocated
* but use mostly the same common code as the other functions.
* 5) at the moment only the functions really used by busybox code are
* implemented, if you need a particular missing function it should be
* easy to write it by using the internal common code.
*/
How was it tested ?
1) at first as standalone out of tree project, by copying the relevant bb stuff
to it and with simple use cases to see if it somehow works.
2) then with valgrind to catch all the memory errors and leaks that I was
so good at creating.
3) then in bb's tree where it compiles cleanly and a few test runs of
relevant apps (adduser, deluser, id etc) seem to work as expected.
4) I've tried also "make test" but it seems to me that there are not
test for this specific functionalities.
The size?
There is a little size increase (due to the added functionalities and checks?!?)
and I'm not able to shrink it more (I swear I've tried!), maybe others will be
able
to do more optimizations. Bloat-o-meter says:
scripts/bloat-o-meter ../busybox_original ../busybox_new
function old new delta
convert_to_struct - 424 +424
parse_common - 214 +214
getgrouplist_internal 185 312 +127
getpw_common_malloc - 77 +77
getgr_common_malloc - 77 +77
parse_file - 75 +75
getpw_common_r - 73 +73
getgr_common_r - 73 +73
tokenize - 71 +71
.rodata 142172 142211 +39
bb_internal_getpwent_r 102 131 +29
my_pwd - 28 +28
convert_to_pwd - 25 +25
convert_to_grp - 25 +25
warn_if_error - 18 +18
my_grp - 16 +16
pwd_buffer_size - 4 +4
pwd_buffer - 4 +4
my_pw - 4 +4
my_gr - 4 +4
grp_buffer_size - 4 +4
grp_buffer - 4 +4
gr_off 3 4 +1
sp_off 9 8 -1
ptr_to_statics 4 - -4
bb_internal_getpwuid 37 19 -18
bb_internal_getspnam_r 119 99 -20
bb_internal_getgrgid 43 19 -24
bb_internal_getpwnam 37 11 -26
get_S 30 - -30
bb_internal_getgrnam 43 11 -32
bb_internal_getpwuid_r 111 - -111
bb_internal_getgrgid_r 111 - -111
bb__parsepwent 112 - -112
bb_internal_getpwnam_r 119 - -119
bb_internal_getgrnam_r 119 - -119
bb__parsespent 124 - -124
bb__pgsreader 194 - -194
bb__parsegrent 246 - -246
------------------------------------------------------------------------------
(add/remove: 19/10 grow/shrink: 4/6 up/down: 1416/-1291) Total: 125 bytes
Is it bug free?
Of course _NOT_ !!!! but i fixed all the bugs I could find.
Your help is welcome to catch the remaining bugs.
What can you do?
Hint, improvements, critics are as always welcome.
Ciao,
Tito
/* vi: set sw=4 ts=4: */
/* Copyright (C) 2003 Manuel Novoa III
*
* Licensed under GPLv2 or later, see file LICENSE in this source tree.
*/
/* Nov 6, 2003 Initial version.
*
* NOTE: This implementation is quite strict about requiring all
* field seperators. It also does not allow leading whitespace
* except when processing the numeric fields. glibc is more
* lenient. See the various glibc difference comments below.
*
* TODO:
* Move to dynamic allocation of (currently statically allocated)
* buffers; especially for the group-related functions since
* large group member lists will cause error returns.
*/
/* Copyright (C) 2014 Tito Ragusa <farmat...@tiscali.it>
*
* Licensed under GPLv2 or later, see file LICENSE in this source tree.
*/
/* Rewrite of some parts. Main differences are:
*
* 1) the buffer for getpwuid, getgrgid, getpwnam, getgrnam is dynamically
* allocated and reused by later calls. if ERANGE error pops up it is
* reallocated to the size of the longest line found in the passwd/group
* files and reused for later calls.
*
* 2) the passwd/group files:
* a) must contain the expected number of fields;
* b) some fields are not allowed to be empty (e.g. username, homedir, shell);
* c) leading or trailing spaces in fields are not allowed;
* d) the string representing uid/gid must be convertible by strtoXX functions;
* if the above explained conditions are not met a error message about bad
* record in file is printed so that the user knows about it.
* 3) the internal function for getgrouplist uses a dynamically allocated
* buffer and retries with a bigger one in case it is to small;
* 4) the _r functions use the user supplied buffers that are never reallocated
* but use mostly the same common code as the other functions.
* 5) at the moment only the functions really used by busybox code are
* implemented, if you need a particular missing function it should be
* easy to write it by using the internal common code.
*/
#include "libbb.h"
/* for the fields in passwd/group/shadow files */
#define NOT_EMPTY 0
#define MAYBE_EMPTY 1
/* Fields in passwd file starting from pos 1 */
#define _PASSWD_FIELDS 7
/* S = string not empty, s = string maybe empty, */
/* I = uid,gid, l = long maybe empty, m = members */
static const char *pw_def = "SsIIsSS";
/* Fields in group file starting from pos 1 */
#define _GROUP_FIELDS 4
static const char *gr_def = "SsIm";
/* Fields in shadow file starting from pos 1 */
#define _SHADOW_FIELDS 9
#if ENABLE_USE_BB_SHADOW
/* But we parse only 8 as last is reserved*/
static const char *sp_def = "Ssllllll";
#endif
/* Initial buffer size */
#define PWD_GRP_BUFSIZE 256
/* for getpw_common_malloc */
static char *pwd_buffer = NULL;
static size_t pwd_buffer_size = PWD_GRP_BUFSIZE;
struct passwd *my_pw;
struct passwd my_pwd;
/* for getgr_common_malloc */
static char *grp_buffer = NULL;
static size_t grp_buffer_size = PWD_GRP_BUFSIZE;
struct group *my_gr;
struct group my_grp;
/* for setpwent, getpwent_r, endpwent */
static FILE *pwf = NULL;
/* for setgrent, getgrent_r, endgrent */
static FILE *grf = NULL;
/* TODO: We do no file locking, but we do
* in libbb/update_passwd.c, so maybe the same code
* could be reused here */
#define LOCK ((void) 0)
#define UNLOCK ((void) 0)
/**********************************************************************/
/* Internal functions */
/**********************************************************************/
/* Divide the passwd/group/shadow record in fields
* by substituting the given delimeter
* e.g. ':' or ',' with '\0'.
* Returns the number of fields found.
* Checks for leading or trailing spaces in fields.
*/
static int FAST_FUNC tokenize(char *buffer, int ch, int *error)
{
char *p = buffer;
int num_fields = 1;
if (*p == ' ')
*error = EINVAL;
while (*p) {
if (*p == ch) {
*p = '\0';
num_fields++;
if (*(p - 1) == ' ' || *(p + 1) == ' ')
*error = EINVAL;
}
p++;
}
return num_fields;
}
/* Fill the buffer linebuf up to len with
* a line of a passwd/group file.
* Returns 0 on success, ENOENT on EOF and
* the length of the passwd/group record
* as negative integer if bigger than len.
* CAVEAT: if a OS has negative errno
* this will break!!!
*/
static int FAST_FUNC get_record_line(FILE *file, char *linebuf, int len)
{
int ch;
int idx = 0;
while ((ch = getc(file)) != EOF) {
if (idx < len) {
linebuf[idx] = (char) ch;
}
idx++;
if (ch == '\n' || ch == '\0')
break;
}
if (idx > len) {
/* return -size of buffer needed to hold the */
/* full line instead of ERANGE.*/
return -idx;
}
/* terminate string and remove trailing '\n' if any.*/
linebuf[idx - 1] = '\0';
/* return ENONT only on EOF and no data */
return (idx > 0) ? 0 : ENOENT;
}
static void FAST_FUNC warn_if_error(int error, const char *filename)
{
if (error > 0) {
bb_error_msg("'%s': bad record", filename);
}
}
/* Returns 0 on success and matching line broken up in fields by '\0' in buf or error.
* We require all separators ':' to be there and warn and skip the line if they are not.
* int n_fields is the expected number of fields of the passwd/group file starting from 1.
* size_t len is used to check that the buffer provided is big enough, if not we use the
* return value to pass the needed lenght as negative integer to the caller.
*/
static int FAST_FUNC parse_common(FILE *f, const char *key, int field,
char *buf, size_t len, const char *filename, int n_fields)
{
int error = 0;
const char *s;
while (1) {
if ((error = get_record_line(f, buf, len)) != 0) {
/* We use error to pass the needed length to the */
/* caller as negative value instead of ERANGE */
return error;
}
/* Skip empty lines, comment lines */
if (buf[0] != '\0' && buf[0] != '#') {
/* Check that there are the expected fields */
if (tokenize(buf, ':', &error) == n_fields) {
if (key) {
s = nth_string(buf, field);
/* Empty fields not allowed */
if (!*s) {
error = EINVAL;
break;
}
if (strcmp(key, s) == 0) {
/* Record found */
break;
}
} else {
/* Sequential read - return record */
break;
}
} else {
error = EINVAL;
}
}
}
warn_if_error(error, filename);
return error;
}
static int FAST_FUNC parse_file(const char *filename, int n_fields, const char *key, int field, char *buf, size_t len)
{
int ret;
FILE *f = fopen_for_read(filename);
ret = errno;
if (f) {
ret = parse_common(f, key, field, buf, len, filename, n_fields);
fclose(f);
}
return ret;
}
/* Convert passwd/group/shadow file record in buffer to a struct */
static void * FAST_FUNC convert_to_struct(const char *def, const unsigned char *off,
char *buffer, void *result, int *error, const char *filename)
{
int idx;
char **members = NULL;
char *m;
for (idx = 0; def[idx] != '\0'; idx++) {
if (def[idx] == 'S' || def[idx] == 's') {
m = (char *) nth_string(buffer, idx);
*(char **)((char *)result + off[idx]) = m;
if (!*m && (def[idx] == 'S')) {
*error = EINVAL;
}
}
if (def[idx] == 'I') {
*(int *)((int) result + off[idx]) = bb_strtou(nth_string(buffer, idx), NULL, 10);
if (errno) {
*error = EINVAL;
}
}
#if ENABLE_USE_BB_SHADOW
if (def[idx] == 'l') {
char *endptr;
long ret;
/* Reuse char *m */
m = (char *) nth_string(buffer, idx);
ret = bb_strtol(m, &endptr, 10);
if (endptr == m) {
ret = -1L;
} else if (errno) {
*error = EINVAL;
}
*(long *)((long) result + off[idx]) = ret;
}
#endif
if (def[idx] == 'm') {
int i = 0;
int num_members = 0;
char *s = (char *) nth_string(buffer, idx);
/* members with leading or trailing spaces set error to EINVAL */
num_members = tokenize(s, ',', error);
members = xmalloc(sizeof (char *) * (num_members + 1));
if (*s) {
/* Leaks a little memory */
while (num_members--) {
/* Reuse char *m */
m = (char *) nth_string(s, i);
if (!*m) {
*error = EINVAL;
}
members[i++] = m;
}
}
members[i] = NULL;
*(char ***)((char *)result + off[idx]) = members;
}
}
warn_if_error(*error, filename);
if (*error) {
free(members);
}
return (*error) ? NULL : result;
}
static int FAST_FUNC fix_error_code(int error)
{
return (error < 0) ? ERANGE : error;
}
#if ENABLE_USE_BB_SHADOW
static const unsigned char sp_off[] ALIGN1 = {
offsetof(struct spwd, sp_namp), /* 1 - Login name */
offsetof(struct spwd, sp_pwdp), /* 2 - Encrypted password */
offsetof(struct spwd, sp_lstchg), /* 2 - not a char ptr */
offsetof(struct spwd, sp_min), /* 3 - not a char ptr */
offsetof(struct spwd, sp_max), /* 4 - not a char ptr */
offsetof(struct spwd, sp_warn), /* 5 - not a char ptr */
offsetof(struct spwd, sp_inact), /* 6 - not a char ptr */
offsetof(struct spwd, sp_expire) /* 7 - not a char ptr */
// ,offsetof(struct spwd, sp_flag) /* 8 - Reserved */
};
static struct spwd * FAST_FUNC convert_to_shadow(char *buffer, struct spwd *result, int *error)
{
return convert_to_struct(sp_def, sp_off, buffer, result, error, _PATH_SHADOW);
}
int getspnam_r(const char *name, struct spwd *spbuf, char *buf, size_t buflen, struct spwd **spbufp)
{
int error;
*spbufp = NULL;
error = parse_file(_PATH_SHADOW, _SHADOW_FIELDS, name, 0, buf, buflen);
if (error == 0) {
*spbufp = convert_to_shadow(buf, spbuf, &error);
}
return fix_error_code(error);
}
#endif /* ENABLE_USE_BB_SHADOW */
static const unsigned char pw_off[] ALIGN1 = {
offsetof(struct passwd, pw_name), /* 0 */
offsetof(struct passwd, pw_passwd), /* 1 */
offsetof(struct passwd, pw_uid), /* 2 - not a char ptr */
offsetof(struct passwd, pw_gid), /* 3 - not a char ptr */
offsetof(struct passwd, pw_gecos), /* 4 */
offsetof(struct passwd, pw_dir), /* 5 */
offsetof(struct passwd, pw_shell) /* 6 */
};
static struct passwd * FAST_FUNC convert_to_pwd(char *buffer, struct passwd *result, int *error)
{
return convert_to_struct(pw_def, pw_off, buffer, result, error, _PATH_PASSWD);
}
static const unsigned char gr_off[] ALIGN1 = {
offsetof(struct group, gr_name), /* 0 */
offsetof(struct group, gr_passwd), /* 1 */
offsetof(struct group, gr_gid), /* 2 - not a char ptr */
offsetof(struct group, gr_mem) /* 3 - char ** ptr */
};
static struct group * FAST_FUNC convert_to_grp(char *buffer, struct group *result, int *error)
{
return convert_to_struct(gr_def, gr_off, buffer, result, error, _PATH_GROUP);
}
/* Common code for getpwxxx_r functions. */
static int FAST_FUNC getpw_common_r(const char *name, int field, struct passwd *pwd,
char *buf, size_t buflen, struct passwd **result)
{
int error;
*result = NULL;
error = parse_file(_PATH_PASSWD, _PASSWD_FIELDS, name, field, buf, buflen);
if (error == 0) {
*result = convert_to_pwd(buf, pwd, &error);
}
return error;
}
/* Common code for getgrxxx_r functions. */
static int FAST_FUNC getgr_common_r(const char *name, int field, struct group *grp,
char *buf, size_t buflen, struct group **result)
{
int error;
*result = NULL;
error = parse_file(_PATH_GROUP, _GROUP_FIELDS, name, field, buf, buflen);
if (error == 0) {
*result = convert_to_grp(buf, grp, &error);
}
return error;
}
/* Common code for getpwxxx functions. */
static struct passwd * FAST_FUNC getpw_common_malloc(const char *name, int field)
{
int error = 0;
retry:
pwd_buffer = xrealloc(pwd_buffer, pwd_buffer_size * sizeof( char));
error = getpw_common_r(name, field, &my_pwd, pwd_buffer, pwd_buffer_size, &my_pw);
if (error < 0) {
pwd_buffer_size = abs(error);
goto retry;
}
return my_pw;
}
/* Common code for getgrxxx functions. */
static struct group * FAST_FUNC getgr_common_malloc(const char *name, int field)
{
int error = 0;
retry:
grp_buffer = xrealloc(grp_buffer, grp_buffer_size * sizeof( char));
error = getgr_common_r(name, field, &my_grp, grp_buffer, grp_buffer_size, &my_gr);
if (error < 0) {
grp_buffer_size = abs(error);
goto retry;
}
return my_gr;
}
/**********************************************************************/
int getpwent_r(struct passwd *pwbuf, char *buf, size_t buflen, struct passwd **pwbufp)
{
int error;
LOCK;
*pwbufp = NULL;
if (!pwf) {
pwf = fopen_for_read(_PATH_PASSWD);
if (!pwf) {
return errno;
}
close_on_exec_on(fileno(pwf));
}
error = parse_common(pwf, NULL, -1, buf, buflen, _PATH_PASSWD, _PASSWD_FIELDS);
if (error == 0) {
*pwbufp = convert_to_pwd(buf, pwbuf, &error);
}
UNLOCK;
return fix_error_code(error);
}
int getgrent_r(struct group *gbuf, char *buf, size_t buflen, struct group **gbufp)
{
int error;
LOCK;
*gbufp = NULL;
if (!grf) {
grf = fopen_for_read(_PATH_GROUP);
if (!grf) {
return errno;
}
close_on_exec_on(fileno(grf));
}
error = parse_common(grf, NULL, -1, buf, buflen, _PATH_GROUP, _GROUP_FIELDS);
if (error == 0) {
*gbufp = convert_to_grp(buf, gbuf, &error);
}
UNLOCK;
return fix_error_code(error);
}
int getpwnam_r(const char *name, struct passwd *pwd, char *buf, size_t buflen, struct passwd **result)
{
return fix_error_code(getpw_common_r(name, 0, pwd, buf, buflen, result));
}
int getgrnam_r(const char *name, struct group *grp, char *buf, size_t buflen, struct group **result)
{
return fix_error_code(getgr_common_r(name, 0, grp, buf, buflen, result));
}
int getpwuid_r(uid_t uid, struct passwd *pwd, char *buf, size_t buflen, struct passwd **result)
{
return fix_error_code(getpw_common_r(utoa(uid), 2, pwd, buf, buflen, result));
}
int getgrgid_r(gid_t gid, struct group *grp, char *buf, size_t buflen, struct group **result)
{
return fix_error_code(getgr_common_r(utoa(gid), 2, grp, buf, buflen, result));
}
struct passwd *getpwnam(const char *name)
{
return getpw_common_malloc(name, 0);
}
struct group *getgrnam(const char *name)
{
return getgr_common_malloc(name, 0);
}
struct passwd *getpwuid(uid_t uid)
{
return getpw_common_malloc(utoa(uid), 2);
}
struct group *getgrgid(gid_t gid)
{
return getgr_common_malloc(utoa(gid), 2);
}
void endpwent(void)
{
LOCK;
if (pwf) {
fclose(pwf);
pwf = NULL;
}
UNLOCK;
}
void setpwent(void)
{
LOCK;
if (pwf) {
rewind(pwf);
}
UNLOCK;
}
void endgrent(void)
{
LOCK;
if (grf) {
fclose(grf);
grf = NULL;
}
UNLOCK;
}
void setgrent(void)
{
LOCK;
if (grf) {
rewind(grf);
}
UNLOCK;
}
static gid_t * FAST_FUNC getgrouplist_internal(int *ngroups_ptr, const char *user, gid_t gid)
{
FILE *f;
struct group *gr;
struct group grp;
gid_t *group_list;
int ngroups;
char * buffer = NULL;
size_t buflen = PWD_GRP_BUFSIZE;
int ret = 0;
retry:
group_list = NULL;
ngroups = 1;
f = fopen_for_read(_PATH_GROUP);
if (f) {
group_list = xrealloc(group_list, 1 * sizeof(gid_t));
*group_list = gid;
buffer = xrealloc(buffer, buflen * sizeof( char));
while ((ret = parse_common(f, NULL, -1, buffer, buflen, _PATH_GROUP, _GROUP_FIELDS)) == 0) {
/* convert_to_grp warns on error so ret is unused */
gr = convert_to_grp(buffer, &grp, &ret);
if (gr != NULL) {
if (gr->gr_gid != gid) {
/* reuse int ret */
for (/*ret = 0*/; gr->gr_mem[ret] != NULL; ret++) {
if (strcmp(gr->gr_mem[ret], user) == 0) {
group_list = xrealloc(group_list, (ngroups + 1) * sizeof(gid_t));
group_list[ngroups++] = gr->gr_gid;
}
}
}
/* our gr->gr_mem is malloced, free it to reduce memory leaks */
free(gr->gr_mem);
}
}
fclose(f);
}
if (ret < 0) {
/* buffer was to small, retry */
buflen = abs(ret);
free(group_list);
goto retry;
}
free(buffer);
*ngroups_ptr = ngroups;
return group_list;
}
int initgroups(const char *user, gid_t gid)
{
int ngroups;
gid_t *group_list = getgrouplist_internal(&ngroups, user, gid);
ngroups = setgroups(ngroups, group_list);
free(group_list);
return ngroups;
}
int getgrouplist(const char *user, gid_t gid, gid_t *groups, int *ngroups)
{
int ngroups_old = *ngroups;
gid_t *group_list = getgrouplist_internal(ngroups, user, gid);
if (*ngroups <= ngroups_old) {
ngroups_old = *ngroups;
memcpy(groups, group_list, ngroups_old * sizeof(groups[0]));
} else {
ngroups_old = -1;
}
free(group_list);
return ngroups_old;
}
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
