Date: Fri, 8 Aug 2008 15:20:02 GMT From: ttw+bsd@cobbled.net To: freebsd-bugs@FreeBSD.org Subject: Re: kern/113398: [libc] initgroups fails rather than truncates if number of groups > NGROUPS_MAX meaning the user can no longer login Message-ID: <200808081520.m78FK2NL071149@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/113398; it has been noted by GNATS. From: ttw+bsd@cobbled.net To: bug-followup@FreeBSD.org, dean.povey@quest.com Cc: current@freebsd.org Subject: Re: kern/113398: [libc] initgroups fails rather than truncates if number of groups > NGROUPS_MAX meaning the user can no longer login Date: Fri, 08 Aug 2008 14:12:44 +0100 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 ceiling 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 compatability 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 application 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 NGROUPS_MAX/KERN_NGROUPS instead 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808081520.m78FK2NL071149>