On Sunday 14 September 2008 21:35:57 you wrote:
> On Sunday 14 September 2008 19:16, Tito wrote:
> > Hi,
> > sadly there was a bug in the previous patch:
> >
> > n = getgroups(0, 0);
> > groups = (gid_t *)xmalloc(sizeof(gid_t) * n);
> > getgroups(n, (gid_t *)groups);
> >
> > This code works only for the current process but doesn't accept
> > an username so for example:
> >
> > ./busybox id -G john
> >
> > returns the groups of the owner of the running process and
> > not of user john. I wonder how could i've been so blind...
> > The current patch fixes this bug and does some shrinkage
> > so that the correct behaviour is not so exepensive in terms
> > of binary size. Bloat-o-meter says:
> >
> > scripts/bloat-o-meter busybox_old busybox_unstripped
> > function old new delta
> > print_group_list - 155 +155
> > print_list_helper - 81 +81
> > .rodata 119213 119214 +1
> > id_main 494 315 -179
> > ------------------------------------------------------------------------------
> > (add/remove: 2/0 grow/shrink: 1/1 up/down: 237/-179) Total: 58
> > bytes
> >
> > The easiest way to implement it would have been to use:
> >
> > int getgrouplist(const char *user, gid_t group, gid_t *groups, int
> > *ngroups);
> >
> > but man says: This function is non-standard; it appears on most BSDs.
> > (needs _BSD_SOURCE to be defined) and busybox's libpwdgrp
> > doesn't know about it.
> > So I reinvented the wheel........
>
> Please implement it as getgrouplist() in libpwdgrp,
> and then use getgrouplist() in id.c
>
> I hope that most libc's have getgrouplist() as this seems to be
> the only semi-standard way to retrieve this information -
> I used it in my micro-nscd too. POSIX simply forgot to add
> a function which does this.
>
> Also, svn has some changes in id.c, please rediff against svn.
> Thanks
> --
> vda
>
Hi Denys,
rediffed against current svn and tested.
Please apply if you like it.
I will think about adding getgrouplist to libpwdgrp
later as i don't know if my skills are enough
to touch this voodoo stuff.
IMHO for id this gid_t *list stuff is useless
as we don't need to store the id's but just
retrieve them and print them on thr fly....
maybe for other applets it could be useful....
Ciao,
Tito
PS:: added one more bugfix to patch V4.
--- coreutils/id_orig.c 2008-09-14 21:53:23.000000000 +0200
+++ coreutils/id.c 2008-09-14 22:06:27.000000000 +0200
@@ -38,20 +38,57 @@
return status;
}
+static short print_list_helper(const char *fmt, gid_t gid, const char* prefix, unsigned flags)
+{
+ if (flags) /* -G */
+ printf(fmt, (flags & NAME_NOT_NUMBER) ? bb_getgrgid(NULL, -1, gid) : itoa(gid));
+ else /* Default mode */
+ return printf_full(gid, bb_getgrgid(NULL, 0, gid), prefix);
+ return EXIT_SUCCESS;
+}
+
+static short print_group_list(uid_t uid, gid_t gid, unsigned flags)
+{
+ struct group *g;
+ char *line;
+ short status = EXIT_SUCCESS;
+ const char *user = bb_getpwuid(NULL, -1, uid);
+ FILE *group_file = xfopen_for_read(bb_path_group_file);
+
+ status |= print_list_helper("%s", gid, " groups=", flags);
+
+ while ((line = xmalloc_fgetline(group_file)) != NULL) {
+ /* Get the group name token and obtain a struct group rather than try to parse the whole line */
+ g = getgrnam(strtok(line, ":"));
+ /* Walk through the group members */
+ while (g->gr_gid != gid && *(g->gr_mem)) {
+ /* Are we a member of this group? */
+ if (!strcmp(user, *(g->gr_mem)))
+ status |= print_list_helper(" %s", g->gr_gid, ",", flags);
+ (g->gr_mem)++;
+ }
+ /* Freed automagically on exit */
+ if (ENABLE_FEATURE_CLEAN_UP)
+ free(line);
+ }
+ /* Closed automagically on exit */
+ if (ENABLE_FEATURE_CLEAN_UP)
+ fclose(group_file);
+ return status;
+}
+
int id_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int id_main(int argc UNUSED_PARAM, char **argv)
{
struct passwd *p;
uid_t uid;
gid_t gid;
- gid_t *groups;
- int n;
unsigned flags;
short status;
#if ENABLE_SELINUX
security_context_t scontext;
#endif
- /* Don't allow -n -r -nr -ug -rug -nug -rnug */
+ /* 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"));
@@ -72,18 +109,10 @@
/* in this case PRINT_REAL is the same */
}
- n = getgroups(0, NULL);
- groups = xmalloc(sizeof(groups[0]) * n);
- getgroups(n, groups);
-
if (flags & JUST_ALL_GROUPS) {
- while (n--) {
- if (flags & NAME_NOT_NUMBER)
- printf("%s", bb_getgrgid(NULL, 0, *groups++));
- else
- printf("%u", (unsigned) *groups++);
- bb_putchar((n > 0) ? ' ' : '\n');
- }
+ /* Ignore return value, exits on failure */
+ print_group_list(uid, gid, flags);
+ bb_putchar('\n');
/* exit */
fflush_stdout_and_exit(EXIT_SUCCESS);
}
@@ -94,12 +123,7 @@
/* bb_getXXXid(-1) exits on failure, puts cannot segfault */
puts((flags & JUST_USER) ? bb_getpwuid(NULL, -1, uid) : bb_getgrgid(NULL, -1, gid));
} else {
- if (flags & JUST_USER) {
- printf("%u\n", (unsigned)uid);
- }
- if (flags & JUST_GROUP) {
- printf("%u\n", (unsigned)gid);
- }
+ printf("%u\n", (flags & JUST_USER) ? uid : gid);
}
#if ENABLE_SELINUX
@@ -123,16 +147,7 @@
/* bb_getpwuid(0) doesn't exit on failure (returns NULL) */
status = printf_full(uid, bb_getpwuid(NULL, 0, uid), "uid=");
status |= printf_full(gid, bb_getgrgid(NULL, 0, gid), " gid=");
- {
- const char *msg = " groups=";
- while (n--) {
- status |= printf_full(*groups, bb_getgrgid(NULL, 0, *groups), msg);
- msg = ",";
- groups++;
- }
- }
- /* we leak groups vector... */
-
+ status |= print_group_list(uid, gid, flags);
#if ENABLE_SELINUX
if (is_selinux_enabled()) {
security_context_t mysid;
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
