Hi Jim,
On Thu, 2011-04-07 at 08:22 -0400, Jim Rees wrote:
> Sean Finney wrote:
>
> Previously, in auth_unix_gid, group lists were stored in an array of
> hard-coded length 100, and in the situation that the group lists for a
> particular call were too large, the array was swapped with a dynamically
> allocated/freed buffer. For environments where users are commonly in
> a large number of groups, this isn't an ideal approach.
>
> Instead, make the group list static scoped to the function, and
> use malloc/realloc to grow it on an as-needed basis.
>
> I would re-word this. The list isn't static, the list pointer is. I don't
> think you need to mention that, it's just confusing. "Use malloc/realloc to
> grow the list on an as-needed basis."
Okay. I was trying to just put a hint that there was a persistant
buffer that would hang around through multiple invocations, though
perhaps that's a little too much 'implementation details'. Totally fine
with modifying the description either way :)
>
> + groups = malloc(sizeof(gid_t)*INITIAL_MANAGED_GROUPS);
>
> Style nit: Put blanks around the "*".
Will do.
> I was going to suggest you first call getgrouplist with groups set to 0 so
> you can always alloc() the correct size list, but then I found this in the
> man page:
>
> BUGS
> In glibc versions before 2.3.3, the implementation of this function
> contains a buffer-overrun bug: it returns the complete list of groups
> for user in the array groups, even when the number of groups exceeds
> *ngroups.
Yuck. The function is also not in POSIX or any other standards, so
maybe it's worth discussing at a later point whether getgroups(2) would
be a better interface to use, even if it means an extra step or two.
Anyway, I will re-spin this patch with the two changes that you've
suggested and submit it tomorrow. Care to take a look at patch
2/2? :)
Sean
ÿô.nlj·Ÿ®‰†+%ŠË±é¥Šwÿº{.nlj·¥Š{±þwìíè jg¬±¨ ¶‰šŽŠÝjÿ¾«þG«é¸ ¢·¦j:+v‰¨Šwèm¶Ÿÿþø®w¥þŠà£¢·hšâÿ†Ù
