[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [RFC] malloced getpw/grxxx/_r functions for bb
From: tito <farmatito () tiscali ! it>
Date: 2014-08-20 20:08:01
Message-ID: 201408202208.01334.farmatito () tiscali ! it
[Download RAW message or body]
On Monday 18 August 2014 21:50:44 tito wrote:
> On Tuesday 05 August 2014 20:16:04 Denys Vlasenko wrote:
> > On Mon, Aug 4, 2014 at 7:06 PM, Laszlo Papp <lpapp@kde.org> wrote:
> > > sudo busybox adduser
> > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> > >
> > > passwd: unknown user
> > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> > >
> > > Yet, the user is created in /etc/shadow:
> > >
> > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff:!:16286:0:99999:7:::
> > >
> > > This is at least one issue, but there is another here:
> > >
> > > sudo busybox deluser
> > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> > >
> > > deluser: unknown user
> > > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> > >
> >
> > Both issues come from the same location in codebase:
> > bb__pgsreader() parser drops lines which are longer than its buffer.
> >
> > Effectively, bbox ignores offending line in /etc/passwd.
> >
> > > Could you please look into this and potentially fix it? Thanks in advance.
> >
> > Anyone willing to rewrite getpwnam API to use variable-sized malloced buffer?
>
> Hi,
> i couldn 't resist as I was sure there was a lot to learn and so I've started
> to reinvent the wheel. At first I've tried to modify the original bb implementation
> but the code with its preprocessor black magic made it difficult so I've started
> from scratch.
> At the moment I have a proof of concept implementation for:
>
> getpwuid
> getgrgid
> getpwnam
> getgrnam
> getpwuid_r
> getgrgid_r
> getpwnam_r
>
> The code is ugly and probably bloated for bb so look at it at your own risk :-)
>
> Other functions still missing but used in bb:
> fgetpwent_r
> fgetgrent_r
> getspnam_r
> setpwent
> endpwent
> setgrent
> endgrent
> getpwent_r
> getgrent_r
> initgroups
>
> What it does:
> 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 _NEEDED_ size and reused for later calls.
>
> 2) the fields in the passwd files are checked and if empty (in some cases) or \
> containing whitespace, etc. a error message about malformed line in file xxxxxx
> is printed so that the user knows instead of being silently ignored.
>
> 3) the _r functions use the user supplied buffers that are never reallocated
> but use mostly the same common functions.
>
> 4) there was something........
>
> Before going further some review and improvements (size reduction, obvious errors, \
> bugs and other horrors) by the list members is needed and also some hints if this \
> implementation makes sense at all or if I should forget about it.
>
> My biggest problem at the moment is a memory leak in handling the members in
> group->gr_mem and I don't see a obvious way to fix it.
>
> The proof of concept is developed out of tree in the pwdgrp.c file that contains \
> also a test program to grossly check if everything works. The relevant libbb \
> functions are copied over to the libbb.c file which is included automatically. You \
> can compile it simply by:
> gcc pwdgrp.c -o test -Wall
>
>
> valgrind of a test run shows:
>
> =7229== HEAP SUMMARY:
> ==7229== in use at exit: 272 bytes in 4 blocks
> ==7229== total heap usage: 22 allocs, 18 frees, 4,012 bytes allocated
> ==7229==
> ==7229== 68 bytes in 1 blocks are still reachable in loss record 1 of 4
> ==7229== at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229== by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229== by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229== by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229== by 0x8049584: convert_to_grp (in /home/tito/Desktop/test)
> ==7229== by 0x80498AB: my_getgr_common (in /home/tito/Desktop/test)
> ==7229== by 0x80498FE: my_getgrgid (in /home/tito/Desktop/test)
> ==7229== by 0x8049EBD: main (in /home/tito/Desktop/test)
> ==7229==
> ==7229== 68 bytes in 1 blocks are definitely lost in loss record 2 of 4
> ==7229== at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229== by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229== by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229== by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229== by 0x804953E: convert_to_grp (in /home/tito/Desktop/test)
> ==7229== by 0x80498AB: my_getgr_common (in /home/tito/Desktop/test)
> ==7229== by 0x80498CD: my_getgrnam (in /home/tito/Desktop/test)
> ==7229== by 0x8049D43: main (in /home/tito/Desktop/test)
> ==7229==
> ==7229== 68 bytes in 1 blocks are definitely lost in loss record 3 of 4
> ==7229== at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229== by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229== by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229== by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229== by 0x804953E: convert_to_grp (in /home/tito/Desktop/test)
> ==7229== by 0x804976C: getgr_common_r (in /home/tito/Desktop/test)
> ==7229== by 0x80497B9: my_getgrnam_r (in /home/tito/Desktop/test)
> ==7229== by 0x8049E0A: main (in /home/tito/Desktop/test)
> ==7229==
> ==7229== 68 bytes in 1 blocks are definitely lost in loss record 4 of 4
> ==7229== at 0x4028308: malloc (vg_replace_malloc.c:263)
> ==7229== by 0x402849F: realloc (vg_replace_malloc.c:632)
> ==7229== by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
> ==7229== by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
> ==7229== by 0x804953E: convert_to_grp (in /home/tito/Desktop/test)
> ==7229== by 0x804976C: getgr_common_r (in /home/tito/Desktop/test)
> ==7229== by 0x8049806: my_getgrgid_r (in /home/tito/Desktop/test)
> ==7229== by 0x8049F84: main (in /home/tito/Desktop/test)
> ==7229==
> ==7229== LEAK SUMMARY:
> ==7229== definitely lost: 204 bytes in 3 blocks
> ==7229== indirectly lost: 0 bytes in 0 blocks
> ==7229== possibly lost: 0 bytes in 0 blocks
> ==7229== still reachable: 68 bytes in 1 blocks
> ==7229== suppressed: 0 bytes in 0 blocks
>
>
> Ciao,
> Tito
>
>
>
>
>
Hi,
v 0.2 memory leaks reduced and better checking of leading/trailing whitespace and \
empty fields in records.
Ciao,
Tito
=9068== LEAK SUMMARY:
==9068== definitely lost: 28 bytes in 2 blocks
==9068== indirectly lost: 0 bytes in 0 blocks
==9068== possibly lost: 12 bytes in 1 blocks
==9068== still reachable: 0 bytes in 0 blocks
==9068== suppressed: 0 bytes in 0 blocks
==9068==
["pwdgrp.c" (text/x-csrc)]
/* Copyright (C) 2014 <farmatito@tiscali.it>
* version 0.2
* Licensed under GPLv2 or later, see file LICENSE in this source tree.
*/
#include "./libbb.c"
#define TEST 1
#define NOT_EMPTY 0
#define MAYBE_EMPTY 1
/* Starting from pos 1 */
#define _PASSWD_FIELDS 7
#define _GROUP_FIELDS 4
#define PWD_GRP_BUFSIZE 256
static char *pwd_buffer = NULL;
static char *grp_buffer = NULL;
static size_t pwd_buffer_size = 0;
static size_t grp_buffer_size = 0;
struct passwd *my_pw;
struct group *my_gr;
struct passwd my_pwd;
struct group my_grp;
static int tokenize(char *buffer, int ch)
{
char *p = buffer;
int num_fields = 1;
while (*p) {
if (*p == ch) {
*p = '\0';
num_fields++;
}
p++;
}
return num_fields;
}
static int get_pwd_grp_line(FILE *file, char *linebuf, int len)
{
int ch;
int idx = 0;
while ((ch = getc(file)) != EOF) {
if (ch == '\n')
ch = '\0';
linebuf[idx++] = (char) ch;
if (ch == '\0' )
return 0;
if (idx == len) {
/* return -size of buffer needed to hold the full line */
/* instead of ERANGE */
while ((ch = getc(file)) != EOF) {
idx++;
if (ch == '\n' || ch == '\0')
break;
}
return -idx;
}
}
if (idx > 0) {
/* Don't discard prior data on EOF */
linebuf[idx] = '\0';
return 0;
}
/* EOF and no data */
return ENOENT;
}
static char *get_nth_string_no_whitespace(char *buffer, int idx, int *error, int \
maybe_empty) {
char *s = (char*) nth_string(buffer, idx);
/* leading or trailing whitespace not allowed */
if (*s) {
if (s[0] == ' ' || last_char_is(s, ' ') != NULL)
*error = EINVAL;
} else {
/* Empty field not allowed ? */
if (maybe_empty == NOT_EMPTY)
*error = EINVAL;
}
return s;
}
/* 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 number of fields of the passwd file expected starting from 0.
* size_t len is for to getXXXXX_r variants to check that the buffer provided is big \
enough.
*/
static int parse_pwd_grp_common(const char *key, int field, char *buf, size_t len, \
char *filename, int n_fields) {
int error = 0;
char *s;
FILE *f = fopen_for_read(filename);
if (!f)
return errno;
while (1) {
if ((error = get_pwd_grp_line(f, buf, len)) != 0) {
/* We use error to pass the needed length to the caller */
/* as negative value instead of ERANGE */
break;
}
/* Skip empty lines, comment lines */
if (buf[0] != '\0' && buf[0] != '#') {
/* Check that there are the expected fields */
if (tokenize(buf, ':') == n_fields ) {
/* No leading or trailing whitespace or empty fields */
s = get_nth_string_no_whitespace(buf, field, &error, NOT_EMPTY);
if (error)
goto error_msg;
if (strcmp(key, s) == 0) {
/* Record found */
break;
}
} else {
error_msg:
bb_error_msg("malformed line in '%s'", filename);
}
}
}
fclose(f);
return error;
}
static int parse_pwd_file(const char *key, int field, char *buf, size_t len) {
return parse_pwd_grp_common(key, field, buf, len, _PATH_PASSWD, _PASSWD_FIELDS);
}
static int parse_grp_file(const char *key, int field, char *buf, size_t len) {
return parse_pwd_grp_common(key, field, buf, len, _PATH_GROUP, _GROUP_FIELDS);
}
static struct passwd *convert_to_pwd(char *buffer, struct passwd *result, int *error)
{
result->pw_name = get_nth_string_no_whitespace(buffer, 0, error, NOT_EMPTY); \
/* username */ result->pw_passwd = get_nth_string_no_whitespace(buffer, 1, \
error, MAYBE_EMPTY); /* user password */ result->pw_uid = \
bb_strtou(nth_string(buffer, 2), NULL, 10); /* user id */ \
if (errno)
*error = EINVAL;
result->pw_gid = bb_strtou(nth_string(buffer, 3), NULL, 10); \
/* group id */ if (errno)
*error = EINVAL;
result->pw_gecos = get_nth_string_no_whitespace(buffer, 4, error, MAYBE_EMPTY); \
/* real name */ result->pw_dir = get_nth_string_no_whitespace(buffer, 5, \
error, NOT_EMPTY); /* home directory */ result->pw_shell = \
get_nth_string_no_whitespace(buffer, 6, error, NOT_EMPTY); /* shell program */
if (*error) {
bb_error_msg("malformed line in '%s'", _PATH_PASSWD);
return NULL;
}
return result;
}
static struct group *convert_to_grp(char *buffer, struct group *result, int *error)
{
char *s;
char *m;
int i;
int n = 0;
int num_members;
char **members = NULL;
result->gr_name = get_nth_string_no_whitespace(buffer, 0, error, NOT_EMPTY); \
/* group name */ result->gr_passwd = get_nth_string_no_whitespace(buffer, 1, \
error, MAYBE_EMPTY); /* group password */ result->gr_gid = \
bb_strtou(nth_string(buffer, 2), NULL, 10); /* group id */ \
if (errno)
*error = EINVAL;
s = (char *) nth_string(buffer, 3);
if (*s ) { \
/* group members */ num_members = tokenize(s, ',');
members = xrealloc(members, sizeof (char*) * (num_members + 1));
/* Leaks a little memory */
for (n = i = 0; i < num_members; i++) {
m = get_nth_string_no_whitespace(s, i, error, NOT_EMPTY);
/* skip empty members or leading/trailing whitespace */
if (!*error) {
members[n] = m;
n++;
}
}
members[n] = NULL;
((struct group *) result)->gr_mem = members;
}
if (*error) {
bb_error_msg("malformed line in '%s'", _PATH_GROUP);
return NULL;
}
return result;
}
static void init_global_buffer(char **buffer, size_t *size)
{
if (*size == 0) {
*buffer = xmalloc(PWD_GRP_BUFSIZE);
*size = PWD_GRP_BUFSIZE;
}
}
static void grow_global_buffer(char **buffer, size_t *size, int new_size)
{
*buffer = xrealloc(*buffer, new_size);
*size = new_size;
}
static int 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_pwd_file(name, field, buf, buflen);
if (error == 0) {
*result = convert_to_pwd(buf, pwd, &error);
}
return (error < 0) ? ERANGE : error;
}
int my_getpwuid_r(uid_t uid, struct passwd *pwd, char *buf, size_t buflen, struct \
passwd **result) {
int error;
char *key = xasprintf("%u", uid);
error = getpw_common_r(key, 2, pwd, buf, buflen, result);
free(key);
return error;
}
int my_getpwnam_r(const char *name, struct passwd *pwd, char *buf, size_t buflen, \
struct passwd **result) {
return getpw_common_r(name, 0, pwd, buf, buflen, result);
}
static int 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_grp_file(name, field, buf, buflen);
if (error == 0) {
*result = convert_to_grp(buf, grp, &error);
}
return (error < 0) ? ERANGE : error;
}
int my_getgrnam_r(const char *name, struct group *grp, char *buf, size_t buflen, \
struct group **result) {
return getgr_common_r(name, 0, grp, buf, buflen, result);
}
int my_getgrgid_r(gid_t gid, struct group *grp, char *buf, size_t buflen, struct \
group **result)
{
int error;
char *key = xasprintf("%u", gid);
error = getgr_common_r(key, 2, grp, buf, buflen, result);
free(key);
return error;
}
static struct group *my_getgr_common(const char *name, int field)
{
int error;
init_global_buffer(&grp_buffer, &grp_buffer_size);
retry:
error = parse_grp_file(name, field, grp_buffer, grp_buffer_size);
if (error < 0) {
grow_global_buffer(&grp_buffer, &grp_buffer_size, abs(error));
goto retry;
}
if (error == 0) {
return convert_to_grp(grp_buffer, &my_grp, &error);
}
return NULL;
}
struct group *my_getgrnam(const char *name)
{
return my_getgr_common(name, 0);
}
struct group *my_getgrgid(gid_t gid)
{
struct group * g;
char *key = xasprintf("%u", gid);
g = my_getgr_common(key, 2);
free(key);
return g;
}
static struct passwd *my_getpw_common(const char *name, int field)
{
int error;
init_global_buffer(&pwd_buffer, &pwd_buffer_size);
retry:
error = parse_pwd_file(name, field, pwd_buffer, pwd_buffer_size);
if (error < 0) {
grow_global_buffer(&pwd_buffer, &pwd_buffer_size, abs(error));
goto retry;
}
if (error == 0) {
return convert_to_pwd(pwd_buffer, &my_pwd, &error);
}
return NULL;
}
struct passwd *my_getpwnam(const char *name)
{
return my_getpw_common(name, 0);
}
struct passwd *my_getpwuid(uid_t uid)
{
struct passwd *p;
char *key = xasprintf("%u", uid);
p = my_getpw_common(key, 2);
free(key);
return p;
}
#if TEST
int main(int argc, char **argv)
{
int ret = 0;
struct passwd *pw;
struct passwd pwd;
struct group *gr;
struct group grp;
char buffer[PWD_GRP_BUFSIZE];
ret = my_getpwnam_r("root", &pwd, buffer, 256, &pw);
if (ret == 0) {
printf("my_getpwnam_r:\nPWNAM %s\nPASS %s\nUID %d\nGID %d\nGECOS %s\nHOME %s\nSHELL \
%s\n\n",
pw->pw_name, pw->pw_passwd, pw->pw_uid, pw->pw_gid, pw->pw_gecos, pw->pw_dir, \
pw->pw_shell); } else {
printf("my_getpwnam_r: %s\n", strerror(ret));
}
pw = my_getpwnam("root");
if (pw) {
printf("my_getpwnam:\nPWNAM %s\nPASS %s\nUID %d\nGID %d\nGECOS %s\nHOME %s\nSHELL \
%s\n\n",
pw->pw_name, pw->pw_passwd, pw->pw_uid, pw->pw_gid, pw->pw_gecos, pw->pw_dir, \
pw->pw_shell); } else {
puts("my_getpwnam: not found");
}
ret = my_getpwuid_r(33, &pwd, buffer, 256, &pw);
if (ret == 0) {
printf("my_getpwuid_r:\nPWNAM %s\nPASS %s\nUID %d\nGID %d\nGECOS %s\nHOME %s\nSHELL \
%s\n\n",
pw->pw_name, pw->pw_passwd, pw->pw_uid, pw->pw_gid, pw->pw_gecos, pw->pw_dir, \
pw->pw_shell); } else {
printf("my_getpwuid_r: %s\n", strerror(ret));
}
pw = my_getpwuid(33);
if (pw) {
printf("my_getpwuid:\nPWNAM %s\nPASS %s\nUID %d\nGID %d\nGECOS %s\nHOME %s\nSHELL \
%s\n\n",
pw->pw_name, pw->pw_passwd, pw->pw_uid, pw->pw_gid, pw->pw_gecos, pw->pw_dir, \
pw->pw_shell); } else {
puts("my_getpwuid: not found");
}
gr = my_getgrnam("burning");
if (gr) {
printf("my_getgrnam:\nGRNAM %s\nPASS %s\nGID %d\nMEMBERS:",
gr->gr_name, gr->gr_passwd, gr->gr_gid);
while (*(gr->gr_mem) != NULL)
printf(" %s,", *(gr->gr_mem)++);
printf("\n\n");;
} else {
puts("my_getgrnam: not found");
}
ret = my_getgrnam_r("audio", &grp, buffer, 256, &gr);
if (ret == 0) {
printf("my_getgrnam_r:\nGRNAM %s\nPASS %s\nGID %d\nMEMBERS:",
gr->gr_name, gr->gr_passwd, gr->gr_gid);
while (*(gr->gr_mem) != NULL)
printf(" %s,", *(gr->gr_mem)++);
printf("\n\n");
} else {
printf("my_getgrnam_r: %s\n", strerror(ret));
}
gr = my_getgrgid(33);
if (gr) {
printf("my_getgrgid:\nGRNAM %s\nPASS %s\nGID %d\nMEMBERS:",
gr->gr_name, gr->gr_passwd, gr->gr_gid);
while (*(gr->gr_mem) != NULL)
printf(" %s,", *(gr->gr_mem)++);
printf("\n\n");;
} else {
puts("my_getgrgid: not found");
}
ret = my_getgrgid_r(44, &grp, buffer, 256, &gr);
if (ret == 0) {
printf("my_getgrgid_r:\nGRNAM %s\nPASS %s\nGID %d\nMEMBERS:",
gr->gr_name, gr->gr_passwd, gr->gr_gid);
while (*(gr->gr_mem) != NULL)
printf(" %s,", *(gr->gr_mem)++);
printf("\n\n");
} else {
printf("my_getgrgid_r: %s\n", strerror(ret));
}
/* Clean up to make valgrind happy */
free(pwd_buffer);
free(grp_buffer);
return 0;
}
#endif
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic