kern/113398: [libc] initgroups fails rather than
truncates if number of groups > NGROUPS_MAX meaning the user can no
ttw+bsd at cobbled.net
ttw+bsd at cobbled.net
Fri Aug 8 15:18:38 UTC 2008
i'm using group accounts for restricting access to client's data and
have been paralysed by this limitation for some time. as such i've been
looking to implement a more complete solution to this problem. here are
my issues with the current patch
1. doesn't actually solve the problem, only increases the
2. increases kernel memory usage significantly as there are
a number of buffers initialised from these variables. i.e.
if i want to increase it to 65534 then i'm using an additional
128k in each ki_info (per thread?) and ucred (per process?)
as well as other knock on effects
3. the above increase occurs even if i don't actually need
the space and i cannot therefore be conservative about needs
without impacting usage
4. mangles the current ki_info size and thus breaks binary
5. other unseen impacts e.g. will xucred still work as it's
now too big (appears not as the IPC stuff defines a CMGROUPS_MAX
for it's own purposes)
6. still no explict handling of IPC/NFS limit within an
so i've done the following and would appreciate some feedback before i
go completing these changes and submitting a complete patch.
1. defined two group limits SYS_NGROUPS which is the system
limit, currently this is 65534 (minus one for egid) as most
'ngroup' are 'int' and IPC_NGROUPS which will be fixed at 16
2. statically defined NGROUPS as 16 for compatibility.
applications should use sysconf/syscall to get
3. removed NGROUPS_MAX to force the use of sysconf/syscall or
sensible use of application dependant defaults
4. defined KERN_NGROUPS as a changable sysctl variable and
defined the initial value as IPC_NGROUPS
5. renamed the 'cr_groups' array to be 'cr_ipc_groups' and
defined it's length at IPC_NGROUPS. then added a new pointer
after 'endcopy' with the name 'cr_groups' such that re-compiled
code will find the new group structure. when the number of
groups is less than IPC_GROUPS the 'cr_groups' simply points
to the 'cr_ipc_groups'.
6. altered the 'kinfo' initialisation to copy the 'ipc_groups'
instead. (may also use one of the 'kinfo' spare pointers to
copy the extended group)
7. removed KI_NGROUPS and defined ki_groups length as IPC_NGROUPS
8. removed CMGROUPS_MAX, using IPC_NGROUPS instead
here are my current issues and need for feed back.
1. will the common use of 'cr_ngroups' defeat any attempt to
increase the use beyond 16? it appears that "a lot"[tm] of
the code will 'MAX(cr_ngroups,NGROUPS)' correctly but i'm not
experienced enough to guess at the bigger picture
2. does the above juggling actually do anything productive;
i'm doing it to be conservative (i.e. keeping kinfo stuct
static) but will it actually save anything; considering the
alteration to the 'cred' stuct my guess is no. i was thinking
to add the extended groups using the spare pointers in kinfo
but again, with this much change does it actually save anything?
3. i'm thinking that keeping the current group list and morphing
it into an ipc_group list so that we can manage the 'clipping'
of the group list in a consistent manner. e.g. add another
sysctl variable to clip the ipc_group list to just the egid
or do "smart shuffling" of the visible ipc_groups but again,
i'm less than confident this will positively impact the
situation. is the definition of an 'ipc_groups' list of any
long term benefit?
4. as far as i'm concerned the use of NGROUPS at compile time
is wrong as it implies a limit to the number of groups below
that of available memory constraints. i see no actual reason
to do that. i do understand the argument of "restricting
access via group usage" but unless we stick to 16 groups that
is an issue we have to confront on NFS anyway. am i failing
to understand NGROUPS fully?
in summary, i'm pretty happy with the changes but i'm not confident
that they are actually what's required. perhaps i'd be better just
redefining 'cr_groups' as a pointer and MALLOC'ing the space dynamically,
at least then the use of 'cr_ngroups' is consistent.
thanks for any feedback and apologies for the rambling considerations
of this message.
More information about the freebsd-current