On Saturday 27 September 2008 23:53:15 Tito wrote:
> On Saturday 27 September 2008 16:45:47 you wrote:
> > On Saturday 27 September 2008 16:00, Tito wrote:
> > > Hi Denys,
> > > maybe this patches slipped through as my cc to you get
> > > bounced by a spam filter that blacklisted my ip?
> >
> > I received them. sorry about this delay.
> >
> > > # [PATCH 1/3] add bb_getgrouplist_malloc to libbb Tito
> > > http://www.busybox.net/lists/busybox/2008-September/033078.html
> > > # [PATCH 2/3] port id to bb_getgrouplist_malloc + fixes Tito
> > > http://www.busybox.net/lists/busybox/2008-September/033079.html
> >
> > Let's look at these two patches together.
> >
> > Here:
> >
> > if (username) {
> > -#if HAVE_getgrouplist
> > - int m;
> > -#endif
> > p = getpwnam(username);
> > /* xuname2uid is needed because it exits on failure */
> > uid = xuname2uid(username);
> > gid = p->pw_gid; /* in this case PRINT_REAL is the same */
> > -
> > -#if HAVE_getgrouplist
> > - n = 16;
> > - groups = NULL;
> > - do {
> > - m = n;
> > - groups = xrealloc(groups, sizeof(groups[0]) * m);
> > - getgrouplist(username, gid, groups, &n); /* GNUism?
> > */
> > - } while (n > m);
> > -#endif
> > - } else {
> > -#if HAVE_getgrouplist
> > - n = getgroups(0, NULL);
> > - groups = xmalloc(sizeof(groups[0]) * n);
> > - getgroups(n, groups);
> > -#endif
> > }
> > + group_list = bb_getgrouplist_malloc(uid, gid, NULL);
> >
> > you replace getgrouplist() call with bb_getgrouplist_malloc().
> > bb_getgrouplist_malloc() iterates through the whole set of group records:
> >
> > + while((grp = getgrent())) {
> > ...
> > + while (*(grp->gr_mem)) {
> > ...
> > + }
> > + }
> >
> > This may be very expensive. I know for the fact than in big organizations
> > some groups are HUGE - I saw 500kbytes large "guests" group.
> > (nscd died horribly trying to digest such a large group.)
> >
> > In these cases, user db is not in /etc/XXX files. It is in LDAP etc.
> >
> > getgrouplist() may have a more clever way of retrieving this data -
> > instead of pulling megabytes from user datrabase by iterating through
> > all groups it may just directly query uer db "give me group list
> > of this user". Likely to be less than kilobyte of data.
> >
> > There is no way around it. If you want to make sure you have at least
> > a fleeting chance of not performing horribly, you must use libc interfaces
> > fro retrieving group lists, of which there is two known to me:
> > initgroups (standard, but useless in many ceses) and getgrouplist (GNUism).
> > Then, *if* your libc is well-written, it will hopefully do it efficiently.
> >
> > Secondly, make bloatcheck says:
> >
> > function old new delta
> > bb_getgrouplist_malloc - 192 +192
> > print_single - 106 +106
> > print_group_list - 94 +94
> > decode_format_string 824 839 +15
> > ...(deleted gcc-induced random jitter +/-9 bytes)...
> > printf_full 44 - -44
> > id_main 539 331 -208
> > ------------------------------------------------------------------------------
> > (add/remove: 3/1 grow/shrink: 4/4 up/down: 418/-259) Total: 159
> > bytes
> >
> > This does not seem to be a progress.
> >
> > > # [PATCH 3/3] "euid and egid handling" (forgot the subject here ;-)
> > > http://www.busybox.net/lists/busybox/2008-September/033080.html
> >
> > Patch 2 also mentions "+ fixes".
> > Can you send these fixes (dropping bb_getgrouplist_malloc() for now)?
> >
> > Thanks.
> > --
> > vda
> >
>
> Hi Denys,
> I reworked id taking into account your objections.
> I'm sending a drop in replacement for the moment as
> I would like to hear your opinion about it before producing a diff.
> The pros are:
>
> 1) the size increase is little:
> scripts/bloat-o-meter busybox_old busybox_unstripped
> function old new delta
> print_single - 129 +129
> print_group_list - 84 +84
> .rodata 119055 119046 -9
> printf_full 44 - -44
> id_main 539 393 -146
> ------------------------------------------------------------------------------
> (add/remove: 2/1 grow/shrink: 0/2 up/down: 213/-199) Total: 14 bytes
> (BTW: can't say why bb_getgrouplist_malloc doesn't show up in the list,
> maybe it is inlined?);
>
> 2) the code allows for easy adding of euid and egid handling;
> 3) the code is more readable as there are less #ifdefs;
> 4) it returns EXIT_ERROR if a gid to groupname translation fails while
> printing a group list;
> 5) it returns EXIT_ERROR if getgrouplist is not available and we try to print
> a group list;
> 6) adding support for the groups command should be easy.
>
> The code is tested and it seems to work as expected but
> hints, critics and improvements are welcome.
>
> Ciao,
> Tito
>
>
>
Hi,
new version that fixes a bug (that i introduced, not in svn).
Ciao,
Tito
/* vi: set sw=4 ts=4: */
/*
* Mini id implementation for busybox
*
* Copyright (C) 2000 by Randolph Chung <[EMAIL PROTECTED]>
*
* Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
*/
/* BB_AUDIT SUSv3 compliant. */
/* Hacked by Tito Ragusa (C) 2004 to handle usernames of whatever length and to
* be more similar to GNU id.
* -Z option support: by Yuichi Nakamura <[EMAIL PROTECTED]>
* Added -G option Tito Ragusa (C) 2008 for SUSv3.
*/
#include "libbb.h"
#define PRINT_REAL 1
#define NAME_NOT_NUMBER 2
#define JUST_USER 4
#define JUST_GROUP 8
#define JUST_ALL_GROUPS 16
#if ENABLE_SELINUX
#define JUST_CONTEXT 32
#endif
static int print_single(gid_t gid,
char* FAST_FUNC bb_getXXXid(char *name, int bufsize, long uid),
unsigned flags,
const char *prefix)
{
short status = EXIT_SUCCESS;
/* bb_getXXXid(-1) exits on failure */
const char *name = bb_getXXXid(NULL, (!flags) ? 0 : -1, gid);
if (prefix) {
printf("%s", prefix);
}
if (!flags || !(flags & NAME_NOT_NUMBER) || !name) {
printf("%u", gid);
}
if (!flags || flags & NAME_NOT_NUMBER) {
if (name) {
printf((!flags) ? "(%s)" : "%s", name);
} else {
status = EXIT_FAILURE;
}
}
return status;
}
#if (defined(__GLIBC__) && !defined(__UCLIBC__))
#define HAVE_getgrouplist 1
#elif ENABLE_USE_BB_PWD_GRP
#define HAVE_getgrouplist 1
#else
#define HAVE_getgrouplist 0
#endif
static int print_group_list(llist_t *grp_list, unsigned flags, const char *prefix)
{
#if HAVE_getgrouplist
short status = EXIT_SUCCESS;
gid_t *gid;
while (grp_list) {
gid = llist_pop(&grp_list);
status |= print_single(*gid, bb_getgrgid, flags, prefix);
prefix = NULL;
if (ENABLE_FEATURE_CLEAN_UP) {
free(gid);
}
if (grp_list) {
bb_putchar((flags) ? ' ' : ',');
}
}
if (ENABLE_FEATURE_CLEAN_UP) {
free(grp_list);
}
return status;
#else
/* Return an error if getgrouplist is not available and we try to print groups */
return EXIT_FAILURE;
#endif
}
static llist_t *bb_getgrouplist_malloc(uid_t ruid, gid_t rgid, gid_t *egid)
{
llist_t *group_list = NULL;
#if HAVE_getgrouplist
const char *username;
int n = 0;
int i;
gid_t *groups = NULL;
/* Exits failure */
username = bb_getpwuid(NULL, -1, ruid);
/* Get the number of groups we belong to */
getgrouplist(username, rgid, groups, &n);
/* malloc the needed memory */
groups = malloc(sizeof(gid_t) * n);
/* Get the gid list */
getgrouplist(username, rgid, groups, &n);
/* Create our linked list */
for (i = 0; i < n; i++) {
if (i == 1 && egid && rgid != *egid) {
/* Add egid at the second place if needed */
llist_add_to_end(&group_list, memcpy(xmalloc(sizeof(gid_t)), egid, sizeof(gid_t)));
}
llist_add_to_end(&group_list, memcpy(xmalloc(sizeof(gid_t)), &groups[i], sizeof(gid_t)));
}
if (ENABLE_FEATURE_CLEAN_UP)
free(groups);
#endif
/* Return NULL if getgrouplist is not available */
return group_list;
}
int id_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int id_main(int argc UNUSED_PARAM, char **argv)
{
const char *username;
struct passwd *p;
uid_t uid;
gid_t gid;
llist_t *group_list;
unsigned flags;
short status = EXIT_SUCCESS;
#if ENABLE_SELINUX
security_context_t scontext;
#endif
/* Don't allow -n -r -nr -ug -uG -gG -rug -nug -rnug */
/* Don't allow more than one username */
opt_complementary = "?1:u--g:g--u:G--u:u--G:g--G:G--g:r?ugG:n?ugG" USE_SELINUX(":u--Z:Z--u:g--Z:Z--g");
flags = getopt32(argv, "rnugG" USE_SELINUX("Z"));
username = argv[optind];
/* This values could be overwritten later */
uid = geteuid();
gid = getegid();
if (flags & PRINT_REAL) {
uid = getuid();
gid = getgid();
}
if (username) {
p = getpwnam(username);
/* xuname2uid is needed because it exits on failure */
uid = xuname2uid(username);
gid = p->pw_gid; /* in this case PRINT_REAL is the same */
}
group_list = bb_getgrouplist_malloc(uid, gid, NULL);
/* JUST_ALL_GROUPS and JUST_GROUP or JUST_USER are mutually exclusive */
if (flags & JUST_ALL_GROUPS) {
status = print_group_list(group_list, flags, NULL);
} else if (flags & (JUST_GROUP | JUST_USER USE_SELINUX(| JUST_CONTEXT))) {
/* JUST_GROUP and JUST_USER are mutually exclusive */
if (flags & JUST_USER) {
/* Ignore return value, bb_getXXXid(-1) exits on failure */
print_single(uid, bb_getpwuid, flags, NULL);
}
if (flags & JUST_GROUP) {
print_single(gid, bb_getgrgid, flags, NULL);
}
#if ENABLE_SELINUX
if (flags & JUST_CONTEXT) {
selinux_or_die();
if (username) {
bb_error_msg_and_die("user name can't be passed with -Z");
}
if (getcon(&scontext)) {
bb_error_msg_and_die("can't get process context");
}
puts(scontext);
}
#endif
} else {
/* Print full info like GNU id */
/* bb_getpwuid(0) doesn't exit on failure (returns NULL) */
status = print_single(uid, bb_getpwuid, flags, "uid=");
status |= print_single(gid, bb_getgrgid, flags, " gid=");
status |= print_group_list(group_list, flags, " groups=");
#if ENABLE_SELINUX
if (is_selinux_enabled()) {
security_context_t mysid;
getcon(&mysid);
printf(" context=%s", mysid ? mysid : "unknown");
if (mysid) /* TODO: maybe freecon(NULL) is harmless? */
freecon(mysid);
}
#endif
}
bb_putchar('\n');
fflush_stdout_and_exit(status);
}
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
