Remainder :-)

[Denys Vlasenko]

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