sysctl-controlled number of groups.
Subsystems (10)
PATCH 1/1 sysctl-controlled number of groups.
We have a client (using SAMBA) who has people in 190 groups. Since NT has hierarchical groups, this is not all that rare.
What do people think of this patch?
Name: Dynamic Allocation of Groups Array When Required: With Refcounting
Author: Rusty Russell
Status: Tested on 2.6.0-test5-bk5
Depends: Misc/qemu-page-offset.patch.gz
D: This patch allows the maximum number of groups can be varied using
D: sysctl. To do this, we use a separate group allocation if >
D: NGROUPS_INTERNAL, which we reference count (sharing being
D: v. v. common).
D:
D: Changes: D: 1) Remove the NGROUPS define from archs. D: 2) Fixup the few places which declare [NGROUPS] arrays on the stack. D: 3) The ia64, s390 and sparc64 ports have their own setgroups/getgroups D: implementations: unify them on the ia64 one, which calls the core D: functions. D: 4) Change the task_struct to have an [NGROUPS_INTERNAL] array and D: a pointer (not convinced this is worth it at all, might skip it D: and always use external). D: 5) Use a reference counted external array, fix up fork() to deal. D: 6) Introduce max_groups and use it instead of NGROUPS. D: 7) Add a sysctl to vary max_groups.
D:
D: This patch scars nfs: artificially restrict the groups there to
D: SVC_CRED_NGROUPS (32), which is probably wrong, but won't break if
D: they don't change the default.
D:
D: This patch breaks intermezzo: I'm not sure how they want to deal
D: with it.
arch/ia64/ia32/sys_ia32.c +20 -7
@@ -2432,17 +2432,24 @@ asmlinkage long sys32_getgroups16 (int gidsetsize, short *grouplist) { mm_segment_t old_fs = get_fs(); -gid_t gl[NGROUPS ]; +gid_t *gl;int ret, i; + gl =kmalloc (sizeof (gl[0 ]) * current->ngroups,GFP_KERNEL ); +if (!gl) +return -ENOMEM ; +set_fs (KERNEL_DS ); ret =sys_getgroups (gidsetsize, gl);set_fs (old_fs); -if (gidsetsize && ret >0 && ret <=NGROUPS ) +if (gidsetsize && ret >0 )for (i =0 ; i < ret; i++, grouplist++) -if (put_user (gl[i], grouplist)) -return -EFAULT ; +if (put_user (gl[i], grouplist)) { + ret = -EFAULT ; +break ; + } +kfree (gl);return ret; }
@@ -2452,17 +2459,23 @@ asmlinkage long sys32_setgroups16 (int gidsetsize, short *grouplist) { mm_segment_t old_fs = get_fs(); -gid_t gl[NGROUPS ]; +gid_t *gl;int ret, i; -if ((unsigned ) gidsetsize >NGROUPS ) +if ((unsigned ) gidsetsize > max_groups)return -EINVAL ; + gl =kmalloc (sizeof (gl[0 ]) * gidsetsize,GFP_KERNEL ); +if (!gl) +return -ENOMEM ;for (i =0 ; i < gidsetsize; i++, grouplist++) -if (get_user (gl[i], grouplist)) +if (get_user (gl[i], grouplist)) { +kfree (gl);return -EFAULT ; + }set_fs (KERNEL_DS ); ret =sys_setgroups (gidsetsize, gl);set_fs (old_fs); +kfree (gl);return ret; }
arch/mips/kernel/sysirix.c +1 -1
@@ -368,7 +368,7 @@ asmlinkage int irix_syssgi(struct pt_reg retval =HZ ; goto out;case 4 : - retval =NGROUPS ; + retval = max_groups; goto out;case 5 : retval =NR_OPEN ;
arch/s390/kernel/compat_linux.c +44 -27
84 lines not on this page · raw message
arch/sparc/kernel/sys_sunos.c +1 -1
@@ -896,7 +896,7 @@ extern asmlinkage long sunos_sysconf (in ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret = max_groups;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
arch/sparc64/kernel/sys_sparc32.c +44 -27
84 lines not on this page · raw message
arch/sparc64/kernel/sys_sunos32.c +1 -1
@@ -859,7 +859,7 @@ extern asmlinkage s32 sunos_sysconf (int ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret = max_groups;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
fs/nfsd/auth.c +4 -2
@@ -26,7 +26,7 @@ nfsd_setuser(struct svc_rqst *rqstp, str cred->cr_uid = exp->ex_anon_uid;if (!cred->cr_gid) cred->cr_gid = exp->ex_anon_gid; -for (i =0 ; i <NGROUPS ; i++) +for (i =0 ; i <SVC_CRED_NGROUPS ; i++)if (!cred->cr_groups[i]) cred->cr_groups[i] = exp->ex_anon_gid; }
@@ -39,7 +39,9 @@ nfsd_setuser(struct svc_rqst *rqstp, str current->fsgid = cred->cr_gid; else current->fsgid = exp->ex_anon_gid; -for (i =0 ; i <NGROUPS ; i++) { + +/* We can do this because cow_current_groups() was done at birth. */ +for (i =0 ; i <SVC_CRED_NGROUPS ; i++) {gid_t group = cred->cr_groups[i];if (group == (gid_t )NOGROUP )break ;
fs/nfsd/nfs4state.c +1 -1
@@ -244,7 +244,7 @@ copy_cred(struct svc_cred *target, struc target->cr_uid = source->cr_uid; target->cr_gid = source->cr_gid; -for (i =0 ; i <NGROUPS ; i++) +for (i =0 ; i <SVC_CREDS_NGROUPS ; i++) target->cr_groups[i] = source->cr_groups[i]; }
fs/nfsd/nfssvc.c +5 -0
@@ -182,6 +182,11 @@ nfsd(struct svc_rqst *rqstp)daemonize ("nfsd" ); current->rlim[RLIMIT_FSIZE ].rlim_cur =RLIM_INFINITY ; +if (cow_current_groups (SVC_CRED_NGROUPS ) <0 ) { +printk ("Unable to start nfsd thread: can't set groups\n" ); + goto out; + } +/* After daemonize() this kernel thread shares current->fs * with the init process. We need to create files with a * umask of 0 instead of init's umask. */
include/asm-alpha/param.h +0 -4
@@ -19,10 +19,6 @@#define EXEC_PAGESIZE 8192 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-arm/param.h +0 -4
@@ -26,10 +26,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-arm26/param.h +0 -4
@@ -22,10 +22,6 @@# define HZ 100#endif -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-cris/param.h +0 -4
@@ -14,10 +14,6 @@#define EXEC_PAGESIZE 8192 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-h8300/param.h +0 -4
@@ -14,10 +14,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-i386/param.h +0 -4
@@ -18,10 +18,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-ia64/param.h +0 -4
@@ -10,10 +10,6 @@#define EXEC_PAGESIZE 65536 -#ifndef NGROUPS -# define NGROUPS 32 -#endif -#ifndef NOGROUP # define NOGROUP (-1)#endif
include/asm-m68k/param.h +0 -4
@@ -13,10 +13,6 @@#define EXEC_PAGESIZE 8192 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-m68knommu/param.h +0 -4
@@ -44,10 +44,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-mips/param.h +0 -4
@@ -33,10 +33,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-parisc/param.h +0 -4
@@ -17,10 +17,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-ppc/param.h +0 -4
@@ -13,10 +13,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-ppc64/param.h +0 -4
@@ -20,10 +20,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-s390/param.h +0 -4
@@ -21,10 +21,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-sh/param.h +0 -4
@@ -17,10 +17,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-sparc/param.h +0 -4
@@ -14,10 +14,6 @@#define EXEC_PAGESIZE 8192/* Thanks for sun4's we carry baggage... */ -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-sparc64/param.h +0 -4
@@ -14,10 +14,6 @@#define EXEC_PAGESIZE 8192/* Thanks for sun4's we carry baggage... */ -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-um/param.h +0 -4
@@ -3,10 +3,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-v850/param.h +0 -4
@@ -18,10 +18,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/asm-x86_64/param.h +0 -4
@@ -13,10 +13,6 @@#define EXEC_PAGESIZE 4096 -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
include/linux/init_task.h +1 -0
@@ -108,6 +108,7 @@ .proc_lock = SPIN_LOCK_UNLOCKED, \ .switch_lock = SPIN_LOCK_UNLOCKED, \ .journal_info = NULL, \ + .groups = &(tsk).internal_groups, \ }
include/linux/sched.h +23 -1
@@ -328,6 +328,17 @@ struct k_itimer {struct io_context ;/* See blkdev.h */ void exit_io_context (void ); +/* Size is determined by task_struct's ngroups. */ +struct task_groups +{ +atomic_t usage; +gid_t groups[0 ]; +}; + +extern int max_groups; + +#define NGROUPS_INTERNAL 1/* For testing. */ +struct task_struct {volatile long state;/* -1 unrunnable, 0 runnable, >0 stopped */ struct thread_info *thread_info;
@@ -403,7 +414,8 @@ struct task_struct {uid_t uid,euid,suid,fsuid;gid_t gid,egid,sgid,fsgid;int ngroups; -gid_t groups[NGROUPS ]; +gid_t *groups;/* == internal_groups, or task_groups->groups */ +gid_t internal_groups[NGROUPS_INTERNAL ];kernel_cap_t cap_effective, cap_inheritable, cap_permitted; int keep_capabilities:1;struct user_struct *user;
@@ -602,6 +614,12 @@ extern int send_group_sigqueue(int, struextern int do_sigaction (int ,const struct k_sigaction *,struct k_sigaction *);extern int do_sigaltstack (const stack_t __user *,stack_t __user *,unsigned long ); +/* container_of doesn't like arrays. */ +static inline struct task_groups *task_groups (gid_t *groups) +{ +return (void *)groups - offsetof(struct task_groups , groups); +} +/* These can be the second arg to send_sig_info/send_group_sig_info. */ #define SEND_SIG_NOINFO ((struct siginfo *) 0)#define SEND_SIG_PRIV ((struct siginfo *) 1)
@@ -678,6 +696,10 @@ extern int do_execve(char *, char __userextern long do_fork (unsigned long ,unsigned long ,struct pt_regs *,unsigned long ,int __user *,int __user *);extern struct task_struct *copy_process (unsigned long ,unsigned long ,struct pt_regs *,unsigned long ,int __user *,int __user *); +/* Copy-on-write current groups to this size, if possible. 0 or -err. */ +extern int cow_current_groups (unsigned int ngroups); +extern void release_groups (gid_t *groups); +#ifdef CONFIG_SMP extern void wait_task_inactive (task_t * p);#else
include/linux/sunrpc/svcauth.h +3 -1
@@ -16,10 +16,12 @@#include <linux/sunrpc/cache.h> #include <linux/hash.h> +#define SVC_CRED_NGROUPS 32 +struct svc_cred {uid_t cr_uid;gid_t cr_gid; -gid_t cr_groups[NGROUPS ]; +gid_t cr_groups[SVC_CRED_NGROUPS ]; };struct svc_rqst ;/* forward decl */
include/linux/sysctl.h +1 -0
@@ -127,6 +127,7 @@ enumKERN_PANIC_ON_OOPS =57 ,/* int: whether we will panic on an oops */ KERN_HPPA_PWRSW =58 ,/* int: hppa soft-power enable */ KERN_HPPA_UNALIGNED =59 ,/* int: hppa unaligned-trap enable */ + KERN_MAX_GROUPS=60, /* int: setgroups limit */ };
kernel/fork.c +7 -0
@@ -73,6 +73,8 @@ static kmem_cache_t *task_struct_cachep;static void free_task (struct task_struct *tsk) { +if (unlikely (tsk->groups != tsk->internal_groups)) +release_groups (tsk->groups);free_thread_info (tsk->thread_info);free_task_struct (tsk); }
@@ -213,6 +215,11 @@ static struct task_struct *dup_task_stru tsk->thread_info = ti; ti->task = tsk; +if (unlikely (orig->groups != orig->internal_groups)) +atomic_inc (&task_groups (orig->groups)->usage); +else + tsk->groups = tsk->internal_groups; +/* One for us, one for whoever does the "release_task()" (usually parent) */ atomic_set (&tsk->usage,2 );return tsk;
kernel/sys.c +77 -7
110 lines not on this page · raw message
kernel/sysctl.c +8 -0
@@ -581,6 +581,14 @@ static ctl_table kern_table[] = { .mode = 0644, .proc_handler = &proc_dointvec, }, + { + .ctl_name = KERN_MAX_GROUPS, + .procname = "max_groups", + .data = &max_groups, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = &proc_dointvec, + }, { .ctl_name = 0 } };
kernel/uid16.c +27 -14
68 lines not on this page · raw message
net/sunrpc/svcauth_unix.c +2 -2
@@ -434,11 +434,11 @@ svcauth_unix_accept(struct svc_rqst *rqsif (slen >16 || (len -= (slen +2 )*4 ) <0 ) goto badcred;for (i =0 ; i < slen; i++) -if (i <NGROUPS ) +if (i <SVC_CRED_NGROUPS ) cred->cr_groups[i] =ntohl (svc_getu32 (argv));else svc_getu32 (argv); -if (i <NGROUPS ) +if (i <SVC_CRED_NGROUPS ) cred->cr_groups[i] =NOGROUP ;if (svc_getu32 (argv) !=RPC_AUTH_NULL ||svc_getu32 (argv) !=0 ) {
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 1 line
From the ia64-side, it looks mostly fine to me. Some minor things:
- Typo in first sentence of patch comment (can be -> to be ?)
- If I'm reading the patch right, there will be identical sys32_getgroups16() definitions in .../ia32/sys_ia32.c and and compat_linux.c; did you mean to name the latter compat_getgroups16()? (ditto for setgroups16 and s390 and sparc64, i think)
- I suspect removing NGROUPS from param.h will break glibc and/or user-level apps. param.h is one of those kernel files that are directly exposed to user-level; may want to keep NGROUPS inside an #ifndef __KERNEL__.
--david
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Rusty Russell
Rusty> We have a client (using SAMBA) who has people in 190 groups. Since NT Rusty> has hierarchical groups, this is not all that rare.
Rusty> What do people think of this patch?
Rusty> Name: Dynamic Allocation of Groups Array When Required: With Refcounting Rusty> Author: Rusty Russell
Rusty> Status: Tested on 2.6.0-test5-bk5
Rusty> Depends: Misc/qemu-page-offset.patch.gz
Rusty> D: This patch allows the maximum number of groups can be varied using Rusty> D: sysctl. To do this, we use a separate group allocation if > Rusty> D: NGROUPS_INTERNAL, which we reference count (sharing being Rusty> D: v. v. common).
Rusty> D:
Rusty> D: Changes:
Rusty> D: 1) Remove the NGROUPS define from archs.
Rusty> D: 2) Fixup the few places which declare [NGROUPS] arrays on the stack. Rusty> D: 3) The ia64, s390 and sparc64 ports have their own setgroups/getgroups
Rusty> D: implementations: unify them on the ia64 one, which calls the core
Rusty> D: functions.
Rusty> D: 4) Change the task_struct to have an [NGROUPS_INTERNAL] array and
Rusty> D: a pointer (not convinced this is worth it at all, might skip it
Rusty> D: and always use external).
Rusty> D: 5) Use a reference counted external array, fix up fork() to deal. Rusty> D: 6) Introduce max_groups and use it instead of NGROUPS. Rusty> D: 7) Add a sysctl to vary max_groups.
Rusty> D:
Rusty> D: This patch scars nfs: artificially restrict the groups there to Rusty> D: SVC_CRED_NGROUPS (32), which is probably wrong, but won't break if Rusty> D: they don't change the default.
Rusty> D:
Rusty> D: This patch breaks intermezzo: I'm not sure how they want to deal Rusty> D: with it.
In message <[email protected]> you write:
Quoted 3 lines
Quoted 1 line
From the ia64-side, it looks mostly fine to me. Some minor things:
- Typo in first sentence of patch comment (can be -> to be ?)
Thanks, fixed.
Quoted 4 lines
- If I'm reading the patch right, there will be identical
sys32_getgroups16()definitions in.../ia32/sys_ia32.cand andcompat_linux.c; did you mean to name the lattercompat_getgroups16()? (ditto for setgroups16 and s390 and sparc64, i think)
You're not reading it right: I didn't change the names, but since these (your) versions call the normal ones, I added the extern decl.
Quoted 3 lines
- I suspect removing NGROUPS from
param.hwill break glibc and/or user-level apps.param.his one of those kernel files that are directly exposed to user-level; may want to keep NGROUPS inside an #ifndef__KERNEL__.
AFAICT, they should use NGROUPS_MAX from limits.h, which I left.
Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 4 lines · Rusty Russell
We have a client (using SAMBA) who has people in 190 groups. Since NT has hierarchical groups, this is not all that rare.
What do people think of this patch?
Excuse me but JESUS FUCKING CHRIST. I've been sending that patch since last year. when I get in the office tomorrow, I'll look at this further, and compare it to mine. check list archives - my most recent retry was a few months back, but it was done. No sysctl - I didn't think it was needed at all.
In message <[email protected]> you write:
Quoted 8 lines · Rusty Russell
Quoted 4 lines
We have a client (using SAMBA) who has people in 190 groups. Since NT has hierarchical groups, this is not all that rare.
What do people think of this patch?
Excuse me but JESUS FUCKING CHRIST. I've been sending that patch since last year.
Yes, I saw this (google sent me to a much older version when I looked, but I just found your more recent one).
I like the fact that you have no internal array, but I like the fact that I preserve the raw groups pointer. You go furthur and try to do better than linear search, too, I was aiming for minimalism.
Quoted 1 line
No sysctl - I didn't think it was needed at all.
Perhaps, but I was being cautious. With the default setting, it's identical to the current situation.
Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 4 lines · Rusty Russell
We have a client (using SAMBA) who has people in 190 groups. Since NT has hierarchical groups, this is not all that rare.
What do people think of this patch?
.... FEATURECREEPER .....
(sorry couldn't resist ;)
Quoted 3 lines · Rusty Russell
We have a client (using SAMBA) who has people in 190 groups. Since NT has hierarchical groups, this is not all that rare.
What do people think of this patch?
I like the idea of raising the microscopic limit. BTW, was the Author:
supposed to be Tim Hockin? ISTR his writing this or something almost identical to it. Also, I'm not sure if the sysctl is worth anything.
-- wli
In message <[email protected]> you write:
Quoted 8 lines · Rusty Russell
Quoted 3 lines
We have a client (using SAMBA) who has people in 190 groups. Since NT has hierarchical groups, this is not all that rare.
What do people think of this patch?I like the idea of raising the microscopic limit. BTW, was the Author:
supposed to be Tim Hockin? ISTR his writing this or something almost identical to it. Also, I'm not sure if the sysctl is worth anything.
Nope, it's mine. Tim's is v. similar, and I think a merge of the two would produce something fine. But as he says, it's fairly simple...
Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.