Many groups patch.
Subsystems (15)
Quoted 7 lines
Quoted 3 lines
This version drops the internal groups array (it's so often shared that it's not worth it, and the logic becomes a bit neater), and does vmalloc fallback in case someone has massive number of groups.
Why?
The vmalloc space is limited, and the code just gets uglier.
Tim was going to write a version that segments groups into smaller arrays. I reckon it was too difficult?
Quoted 2 lines
Have you been looking at glibc sources lately, or why do you believe that we should encourage insane usage?
We have some customers who run insane number of groups, with their own patches. This practice is popular in the Beowulf crowd for some reason. I should note this is not very mainstream.
-- Pete
Quoted 10 lines · Pete Zaitcev
Quoted 7 lines
Quoted 3 lines
This version drops the internal groups array (it's so often shared that it's not worth it, and the logic becomes a bit neater), and does vmalloc fallback in case someone has massive number of groups.
Why?
The vmalloc space is limited, and the code just gets uglier.
Tim was going to write a version that segments groups into smaller arrays. I reckon it was too difficult?
I posted it once or twice then got busy. It's an array of pages. Rusty has it, but didn't believe me when I said Linus wouldn't let vmalloc() fly.
Quoted 7 lines
Quoted 2 lines
Have you been looking at glibc sources lately, or why do you believe that we should encourage insane usage?
We have some customers who run insane number of groups, with their own patches. This practice is popular in the Beowulf crowd for some reason. I should note this is not very mainstream.
I'd be ok with the simplest "kmalloc or too bad" version, but our customers wouldn't.
My version uses a struct group_info which has an array of pages. The groups are sorted and bsearched, instead of linear. The perfomance is quite good. An older version against 2.6.0-test1 or something is attached. If this method will fly, I'll take some of Rusty's good ideas and finish this version of it..
Linus?
Quoted 1 line · Tim Hockin
My version uses a struct
group_infowhich has an array of pages. The groups
Woops, patch is here
===== arch/mips/kernel/sysirix.c 1.14 vs edited =====
arch/mips/kernel/sysirix.c +1 -1
@@ -368,7 +368,7 @@ retval =HZ ; goto out;case 4 : - retval =NGROUPS ; + retval =INT_MAX ; goto out;case 5 : retval =NR_OPEN ;
===== arch/s390/kernel/compat_linux.c 1.6 vs edited =====
arch/s390/kernel/compat_linux.c +18 -16
@@ -191,18 +191,18 @@asmlinkage long sys32_getgroups16 (int gidsetsize,u16 *grouplist) { -u16 groups[NGROUPS ]; -int i,j; +int i;if (gidsetsize <0 )return -EINVAL ; - i = current->ngroups; +if (!current->group_info) +return 0 ; + + i = current->group_info->ngroups;if (gidsetsize) {if (i > gidsetsize)return -EINVAL ; -for (j=0 ;j<i;j++) - groups[j] = current->groups[j]; -if (copy_to_user (grouplist, groups,sizeof (u16)*i)) +if (groups16_to_user (grouplist, current->group_info))return -EFAULT ; }return i;
@@ -210,19 +210,21 @@asmlinkage long sys32_setgroups16 (int gidsetsize,u16 *grouplist) { -u16 groups[NGROUPS ]; -int i; +struct group_info *new_info; +int retval;if (!capable (CAP_SETGID ))return -EPERM ; -if ((unsigned ) gidsetsize >NGROUPS ) -return -EINVAL ; -if (copy_from_user (groups, grouplist, gidsetsize *sizeof (u16))) -return -EFAULT ; -for (i =0 ; i < gidsetsize ; i++) - current->groups[i] = (gid_t )groups[i]; - current->ngroups = gidsetsize; -return 0 ; + new_info =groups_alloc (gidsetsize); +if (!new_info) +return -ENOMEM ; + retval =groups16_from_user (new_info, grouplist); +if (retval) { +groups_free (new_info); +return retval; + } + +return set_group_info (new_info); }asmlinkage long sys32_getuid16 (void )
===== arch/sparc/kernel/sys_sunos.c 1.24 vs edited =====
arch/sparc/kernel/sys_sunos.c +1 -1
@@ -896,7 +896,7 @@ ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret =INT_MAX ;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
===== arch/sparc64/kernel/sys_sunos32.c 1.33 vs edited =====
arch/sparc64/kernel/sys_sunos32.c +1 -1
@@ -859,7 +859,7 @@ ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret =INT_MAX ;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
===== arch/sparc64/solaris/misc.c 1.14 vs edited =====
arch/sparc64/solaris/misc.c +1 -1
@@ -341,7 +341,7 @@ asmlinkage int solaris_sysconf(int id) {switch (id) { -case SOLARIS_CONFIG_NGROUPS :return NGROUPS_MAX ; +case SOLARIS_CONFIG_NGROUPS :return INT_MAX ;case SOLARIS_CONFIG_CHILD_MAX :return CHILD_MAX ;case SOLARIS_CONFIG_OPEN_FILES :return OPEN_MAX ;case SOLARIS_CONFIG_POSIX_VER :return 199309 ;
===== fs/nfsd/auth.c 1.2 vs edited =====
fs/nfsd/auth.c +7 -4
@@ -10,12 +10,15 @@#include <linux/sunrpc/svcauth.h> #include <linux/nfsd/nfsd.h> +extern asmlinkage long sys_setgroups (int gidsetsize,gid_t *grouplist); +#define CAP_NFSD_MASK (CAP_FS_MASK|CAP_TO_MASK(CAP_SYS_RESOURCE))void nfsd_setuser (struct svc_rqst *rqstp,struct svc_export *exp) {struct svc_cred *cred = &rqstp->rq_cred;int i; +gid_t groups[SVC_CRED_NGROUPS ];if (exp->ex_flags &NFSEXP_ALLSQUASH ) { cred->cr_uid = exp->ex_anon_uid;
@@ -26,7 +29,7 @@ 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,13 +42,13 @@ current->fsgid = cred->cr_gid; else current->fsgid = exp->ex_anon_gid; -for (i =0 ; i <NGROUPS ; i++) { +for (i =0 ; i <SVC_CRED_NGROUPS ; i++) {gid_t group = cred->cr_groups[i];if (group == (gid_t )NOGROUP )break ; - current->groups[i] = group; + groups[i] = group; } - current->ngroups = i; +sys_setgroups (i, groups);if ((cred->cr_uid)) {cap_t (current->cap_effective) &= ~CAP_NFSD_MASK ;
===== fs/nfsd/nfs4state.c 1.11 vs edited =====
fs/nfsd/nfs4state.c +1 -1
@@ -241,7 +241,7 @@ target->cr_uid = source->cr_uid; target->cr_gid = source->cr_gid; -for (i =0 ; i <NGROUPS ; i++) +for (i =0 ; i <SVC_CRED_NGROUPS ; i++) target->cr_groups[i] = source->cr_groups[i]; }
===== fs/proc/array.c 1.47 vs edited =====
fs/proc/array.c +5 -2
@@ -173,8 +173,11 @@ p->files ? p->files->max_fds : 0);task_unlock (p); -for (g =0 ; g < p->ngroups; g++) - buffer +=sprintf (buffer,"%d " , p->groups[g]); +if (p->group_info) { +for (g =0 ; g <min (p->group_info->ngroups,NGROUPS_SMALL ); g++) + buffer +=sprintf (buffer,"%d " , +GRP_AT (p->group_info,g)); + } buffer +=sprintf (buffer,"\n" );return buffer;
===== include/asm-alpha/param.h 1.2 vs edited =====
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 1.4 vs edited =====
include/asm-arm/param.h +0 -4
@@ -25,10 +25,6 @@# define HZ 100#endif -#ifndef NGROUPS -#define NGROUPS 32 -#endif -#ifndef NOGROUP #define NOGROUP (-1)#endif
===== include/asm-arm26/param.h 1.1 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.2 vs edited =====
include/asm-i386/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-ia64/param.h 1.3 vs edited =====
include/asm-ia64/param.h +0 -4
@@ -22,10 +22,6 @@#define EXEC_PAGESIZE 65536 -#ifndef NGROUPS -# define NGROUPS 32 -#endif -#ifndef NOGROUP # define NOGROUP (-1)#endif
===== include/asm-m68k/param.h 1.2 vs edited =====
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 1.1 vs edited =====
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 1.3 vs edited =====
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 1.2 vs edited =====
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 1.6 vs edited =====
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 1.2 vs edited =====
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 1.3 vs edited =====
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 1.2 vs edited =====
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 1.2 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.1 vs edited =====
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 1.3 vs edited =====
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.26 vs edited =====
include/linux/init_task.h +1 -0
@@ -87,6 +87,7 @@ .real_timer = { \ .function = it_real_fn \ }, \ + .group_info = NULL, \ .cap_effective = CAP_INIT_EFF_SET, \ .cap_inheritable = CAP_INIT_INH_SET, \ .cap_permitted = CAP_FULL_SET, \
===== include/linux/limits.h 1.3 vs edited =====
include/linux/limits.h +0 -1
@@ -3,7 +3,6 @@#define NR_OPEN 1024 -#define NGROUPS_MAX 32/* supplemental group IDs are available */ #define ARG_MAX 131072/* # bytes of args + environ for exec() */ #define CHILD_MAX 999/* no limit :-) */ #define OPEN_MAX 256/* # open files a process may have */
===== include/linux/sched.h 1.160 vs edited =====
include/linux/sched.h +18 -2
@@ -325,6 +325,23 @@struct io_context ;/* See blkdev.h */ void exit_io_context (void ); +#define NGROUPS_SMALL 32 +#define NGROUPS_BLOCK ((int)(EXEC_PAGESIZE / sizeof(gid_t))) +struct group_info { +int ngroups; +atomic_t refcount; +gid_t small_block[NGROUPS_SMALL ]; +int nblocks; +gid_t *blocks[0 ]; +}; +struct group_info *groups_alloc (int gidsetsize); +void groups_free (struct group_info *info); +int set_group_info (struct group_info *info); +/* access the group array through this */ +#define GRP_AT (info, index) \ + ((info)->blocks[(index)/NGROUPS_BLOCK][(index)%NGROUPS_BLOCK]) + +struct task_struct {volatile long state;/* -1 unrunnable, 0 runnable, >0 stopped */ struct thread_info *thread_info;
@@ -396,8 +413,7 @@/* process credentials */ uid_t uid,euid,suid,fsuid;gid_t gid,egid,sgid,fsgid; -int ngroups; -gid_t groups[NGROUPS ]; +struct group_info *group_info;kernel_cap_t cap_effective, cap_inheritable, cap_permitted; int keep_capabilities:1;struct user_struct *user;
===== include/linux/security.h 1.25 vs edited =====
include/linux/security.h +6 -7
@@ -551,9 +551,8 @@* Return 0 if permission is granted. * @task_setgroups: * Check permission before setting the supplementary group set of the -* current process to @grouplist. -* @gidsetsize contains the number of elements in @grouplist. -* @grouplist contains the array of gids. +* current process. +* @group_info contains the new group information. * Return 0 if permission is granted. * @task_setnice: * Check permission before setting the nice value of @p to @nice.
@@ -1097,7 +1096,7 @@int (*task_setpgid) (struct task_struct * p,pid_t pgid);int (*task_getpgid) (struct task_struct * p);int (*task_getsid) (struct task_struct * p); -int (*task_setgroups) (int gidsetsize,gid_t * grouplist); +int (*task_setgroups) (struct group_info *group_info);int (*task_setnice) (struct task_struct * p,int nice);int (*task_setrlimit) (unsigned int resource,struct rlimit * new_rlim); int (*task_setscheduler) (struct task_struct * p, int policy,
@@ -1647,9 +1646,9 @@return security_ops->task_getsid (p); } -static inline int security_task_setgroups (int gidsetsize,gid_t *grouplist) +static inline int security_task_setgroups (struct group_info *group_info) { -return security_ops->task_setgroups (gidsetsize, grouplist); +return security_ops->task_setgroups (group_info); }static inline int security_task_setnice (struct task_struct *p,int nice)
@@ -2275,7 +2274,7 @@return 0 ; } -static inline int security_task_setgroups (int gidsetsize,gid_t *grouplist) +static inline int security_task_setgroups (struct group_info *group_info) {return 0 ; }
===== include/linux/sunrpc/auth.h 1.9 vs edited =====
include/linux/sunrpc/auth.h +1 -2
@@ -28,8 +28,7 @@struct auth_cred {uid_t uid;gid_t gid; -int ngroups; -gid_t *groups; +struct group_info *group_info; };/*
===== include/linux/sunrpc/svcauth.h 1.9 vs edited =====
include/linux/sunrpc/svcauth.h +2 -1
@@ -16,10 +16,11 @@#include <linux/sunrpc/cache.h> #include <linux/hash.h> +#define SVC_CRED_NGROUPS 32struct 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 */
===== kernel/exit.c 1.111 vs edited =====
kernel/exit.c +3 -0
@@ -54,6 +54,9 @@BUG_ON (p->state <TASK_ZOMBIE ); +if (p->group_info &&atomic_dec_and_test (&p->group_info->refcount)) +groups_free (p->group_info); +atomic_dec (&p->user->processes);spin_lock (&p->proc_lock); proc_dentry =proc_pid_unhash (p);
===== kernel/fork.c 1.135 vs edited =====
kernel/fork.c +4 -0
@@ -885,6 +885,10 @@*/ clear_tsk_thread_flag (p,TIF_SYSCALL_TRACE ); +/* increment the groups ref count */ +if (p->group_info) +atomic_inc (&p->group_info->refcount); +/* Our parent execution domain becomes current domain These must match for thread signalling to apply */
===== kernel/sys.c 1.52 vs edited =====
kernel/sys.c +174 -35
251 lines not on this page · raw message
===== kernel/uid16.c 1.5 vs edited =====
kernel/uid16.c +84 -21
128 lines not on this page · raw message
===== net/sunrpc/auth.c 1.12 vs edited =====
net/sunrpc/auth.c +2 -4
@@ -249,8 +249,7 @@struct auth_cred acred = { .uid = current->fsuid, .gid = current->fsgid, - .ngroups = current->ngroups, - .groups = current->groups, + .group_info = current->group_info, };dprintk ("RPC: looking up %s cred\n" , auth->au_ops->au_name);
@@ -264,8 +263,7 @@struct auth_cred acred = { .uid = current->fsuid, .gid = current->fsgid, - .ngroups = current->ngroups, - .groups = current->groups, + .group_info = current->group_info, }; dprintk("RPC: %4d looking up %s cred\n",
===== net/sunrpc/auth_unix.c 1.11 vs edited =====
net/sunrpc/auth_unix.c +4 -4
@@ -82,7 +82,7 @@ cred->uc_gid = cred->uc_pgid =0 ; cred->uc_gids[0 ] =NOGROUP ; }else { -int groups = acred->ngroups; +int groups = acred->group_info ? acred->group_info->ngroups :0 ;if (groups >NFS_NGROUPS ) groups =NFS_NGROUPS ;
@@ -91,7 +91,7 @@ cred->uc_puid = current->uid; cred->uc_pgid = current->gid;for (i =0 ; i < groups; i++) - cred->uc_gids[i] = (gid_t ) acred->groups[i]; + cred->uc_gids[i] =GRP_AT (acred->group_info, i);if (i <NFS_NGROUPS ) cred->uc_gids[i] =NOGROUP ; }
@@ -126,11 +126,11 @@ || cred->uc_pgid != current->gid)return 0 ; - groups = acred->ngroups; + groups = acred->group_info ? acred->group_info->ngroups :0 ;if (groups >NFS_NGROUPS ) groups =NFS_NGROUPS ;for (i =0 ; i < groups ; i++) -if (cred->uc_gids[i] != (gid_t ) acred->groups[i]) +if (cred->uc_gids[i] !=GRP_AT (acred->group_info, i))return 0 ;return 1 ; }
===== net/sunrpc/svcauth_unix.c 1.20 vs edited =====
net/sunrpc/svcauth_unix.c +2 -2
@@ -434,11 +434,11 @@if (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 ) {
===== security/dummy.c 1.27 vs edited =====
security/dummy.c +1 -1
@@ -530,7 +530,7 @@return 0 ; } -static int dummy_task_setgroups (int gidsetsize,gid_t * grouplist) +static int dummy_task_setgroups (struct group_info *group_info) {return 0 ; }
Quoted 5 lines · Tim Hockin
Quoted 1 line · Tim Hockin
My version uses a struct
group_infowhich has an array of pages. The groupsWoops, patch is here
I'm definitely happier about this one.
Not that I'm any more thrilled about users using thousands of groups. But this looks a bit saner.
Linus
Quoted 7 lines · Linus Torvalds
Quoted 2 lines · Tim Hockin
Quoted 1 line
My version uses a struct
group_infowhich has an array of pages. The groupsI'm definitely happier about this one.
Not that I'm any more thrilled about users using thousands of groups. But this looks a bit saner.
OK, then. Here's a brushed up version against 2.6.0-test6. Linus, if you like this, I can get it into a public BK for you. If not, please indicate the issues.
Summary: Get rid of the NGROUPS hard limit.
This patch removes all fixed-size arrays which depend on NGROUPS, and replaces them with struct group_info, which is refcounted, and holds an array of pages in which to store groups. groups_alloc() and groups_free() are used to allocate and free struct group_info, and set_group_info is used to actually put a group_info into a task. Groups are sorted and b-searched for efficiency. Because groups are stored in a 2-D array, the GRP_AT() macro was added to allow simple 1-D style indexing.
This patch touches all the compat code in the 64-bit architectures. These files have a LOT of duplicated code from uid16.c. I did not try to reduce duplicated code, and instead followed suit. A proper cleanup of those architectures code-bases would be fun. Any sysconf() which used to return NGROUPS now returns INT_MAX - there is no hard limit.
This patch also touches nfsd by imposing a limit on the number of groups in an svc_cred struct.
This patch modifies /proc/pid/status to only display the first 32 groups.
This patch removes the NGROUPS define from all architectures as well as NGROUPS_MAX.
This patch changes the security API to check a struct group_info, rather than an array of gid_t.
This patch totally horks Intermezzo.
This was built and tested against 2.6.0-test6 (BK today) on an i386.
Summary: Get rid of the NGROUPS hard limit.
This patch removes all fixed-size arrays which depend on NGROUPS, and replaces them with struct group_info, which is refcounted, and holds an array of pages in which to store groups. groups_alloc() and groups_free() are used to allocate and free struct group_info, and set_group_info is used to actually put a group_info into a task. Groups are sorted and b-searched for efficiency. Because groups are stored in a 2-D array, the GRP_AT() macro was added to allow simple 1-D style indexing.
This patch touches all the compat code in the 64-bit architectures. These files have a LOT of duplicated code from uid16.c. I did not try to reduce duplicated code, and instead followed suit. A proper cleanup of those architectures code-bases would be fun. Any sysconf() which used to return NGROUPS now returns INT_MAX - there is no hard limit.
This patch also touches nfsd by imposing a limit on the number of groups in an svc_cred struct.
This patch modifies /proc/pid/status to only display the first 32 groups.
This patch removes the NGROUPS define from all architectures as well as NGROUPS_MAX.
This patch changes the security API to check a struct group_info, rather than an array of gid_t.
This patch totally horks Intermezzo.
This was built and tested against 2.6.0-test6 (BK today) on an i386.
===== arch/ia64/ia32/sys_ia32.c 1.78 vs edited =====
arch/ia64/ia32/sys_ia32.c +55 -28
@@ -2426,44 +2426,71 @@return sys_lseek (fd, offset, whence); } -extern asmlinkage long sys_getgroups (int gidsetsize,gid_t *grouplist); +static int +groups16_to_user (short *grouplist,struct group_info *info) +{ +int i; +for (i =0 ; i < info->ngroups; i++) { +short group; + group = (short )GRP_AT (info, i); +if (copy_to_user (grouplist+i, &group,sizeof (group))) +return -EFAULT ; + } +return 0 ; +} + +static int +groups16_from_user (struct group_info *info,short *grouplist) +{ +int i; +for (i =0 ; i < info->ngroups; i++) { +short group; +if (copy_from_user (&group, grouplist+i,sizeof (group))) +return -EFAULT ; +GRP_AT (info, i) = (gid_t )group; + } +return 0 ; +}asmlinkage long sys32_getgroups16 (int gidsetsize,short *grouplist) { -mm_segment_t old_fs =get_fs (); -gid_t gl[NGROUPS ]; -int ret, i; - -set_fs (KERNEL_DS ); - ret =sys_getgroups (gidsetsize, gl); -set_fs (old_fs); - -if (gidsetsize && ret >0 && ret <=NGROUPS ) -for (i =0 ; i < ret; i++, grouplist++) -if (put_user (gl[i], grouplist)) -return -EFAULT ; -return ret; -} +int i; -extern asmlinkage long sys_setgroups (int gidsetsize,gid_t *grouplist); +if (gidsetsize <0 ) +return -EINVAL ; + i = current->group_info->ngroups; +if (gidsetsize) { +if (i > gidsetsize) +return -EINVAL ; +if (groups16_to_user (grouplist, current->group_info)) +return -EFAULT ; + } +return i; +}asmlinkage long sys32_setgroups16 (int gidsetsize,short *grouplist) { -mm_segment_t old_fs =get_fs (); -gid_t gl[NGROUPS ]; -int ret, i; +struct group_info *new_info; +int retval; -if ((unsigned ) gidsetsize >NGROUPS ) -return -EINVAL ; -for (i =0 ; i < gidsetsize; i++, grouplist++) -if (get_user (gl[i], grouplist)) -return -EFAULT ; -set_fs (KERNEL_DS ); - ret =sys_setgroups (gidsetsize, gl); -set_fs (old_fs); -return ret; +if (!capable (CAP_SETGID )) +return -EPERM ; + new_info =groups_alloc (gidsetsize); +if (!new_info) +return -ENOMEM ; + retval =groups16_from_user (new_info, grouplist); +if (retval) { +groups_free (new_info); +return retval; + } + + retval =set_current_groups (new_info); +if (retval) +groups_free (new_info); + +return retval; }asmlinkage long
===== arch/mips/kernel/sysirix.c 1.18 vs edited =====
arch/mips/kernel/sysirix.c +1 -1
@@ -368,7 +368,7 @@ retval =HZ ; goto out;case 4 : - retval =NGROUPS ; + retval =INT_MAX ; goto out;case 5 : retval =NR_OPEN ;
===== arch/s390/kernel/compat_linux.c 1.9 vs edited =====
arch/s390/kernel/compat_linux.c +43 -16
82 lines not on this page · raw message
===== arch/sparc/kernel/sys_sunos.c 1.26 vs edited =====
arch/sparc/kernel/sys_sunos.c +1 -1
@@ -896,7 +896,7 @@ ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret =INT_MAX ;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
===== arch/sparc64/kernel/sys_sparc32.c 1.83 vs edited =====
arch/sparc64/kernel/sys_sparc32.c +43 -16
82 lines not on this page · raw message
===== arch/sparc64/kernel/sys_sunos32.c 1.35 vs edited =====
arch/sparc64/kernel/sys_sunos32.c +1 -1
@@ -859,7 +859,7 @@ ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret =INT_MAX ;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
===== arch/sparc64/solaris/misc.c 1.18 vs edited =====
arch/sparc64/solaris/misc.c +1 -1
@@ -341,7 +341,7 @@ asmlinkage int solaris_sysconf(int id) {switch (id) { -case SOLARIS_CONFIG_NGROUPS :return NGROUPS_MAX ; +case SOLARIS_CONFIG_NGROUPS :return INT_MAX ;case SOLARIS_CONFIG_CHILD_MAX :return CHILD_MAX ;case SOLARIS_CONFIG_OPEN_FILES :return OPEN_MAX ;case SOLARIS_CONFIG_POSIX_VER :return 199309 ;
===== fs/nfsd/auth.c 1.2 vs edited =====
fs/nfsd/auth.c +7 -4
@@ -10,12 +10,15 @@#include <linux/sunrpc/svcauth.h> #include <linux/nfsd/nfsd.h> +extern asmlinkage long sys_setgroups (int gidsetsize,gid_t *grouplist); +#define CAP_NFSD_MASK (CAP_FS_MASK|CAP_TO_MASK(CAP_SYS_RESOURCE))void nfsd_setuser (struct svc_rqst *rqstp,struct svc_export *exp) {struct svc_cred *cred = &rqstp->rq_cred;int i; +gid_t groups[SVC_CRED_NGROUPS ];if (exp->ex_flags &NFSEXP_ALLSQUASH ) { cred->cr_uid = exp->ex_anon_uid;
@@ -26,7 +29,7 @@ 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,13 +42,13 @@ current->fsgid = cred->cr_gid; else current->fsgid = exp->ex_anon_gid; -for (i =0 ; i <NGROUPS ; i++) { +for (i =0 ; i <SVC_CRED_NGROUPS ; i++) {gid_t group = cred->cr_groups[i];if (group == (gid_t )NOGROUP )break ; - current->groups[i] = group; + groups[i] = group; } - current->ngroups = i; +sys_setgroups (i, groups);if ((cred->cr_uid)) {cap_t (current->cap_effective) &= ~CAP_NFSD_MASK ;
===== fs/nfsd/nfs4state.c 1.20 vs edited =====
fs/nfsd/nfs4state.c +1 -1
@@ -244,7 +244,7 @@ target->cr_uid = source->cr_uid; target->cr_gid = source->cr_gid; -for (i =0 ; i <NGROUPS ; i++) +for (i =0 ; i <SVC_CRED_NGROUPS ; i++) target->cr_groups[i] = source->cr_groups[i]; }
===== fs/proc/array.c 1.52 vs edited =====
fs/proc/array.c +2 -2
@@ -176,8 +176,8 @@ p->files ? p->files->max_fds : 0);task_unlock (p); -for (g =0 ; g < p->ngroups; g++) - buffer +=sprintf (buffer,"%d " , p->groups[g]); +for (g =0 ; g <min (p->group_info->ngroups,NGROUPS_SMALL ); g++) + buffer +=sprintf (buffer,"%d " ,GRP_AT (p->group_info,g)); buffer +=sprintf (buffer,"\n" );return buffer;
===== include/asm-alpha/param.h 1.2 vs edited =====
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 1.5 vs edited =====
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 1.1 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.2 vs edited =====
include/asm-i386/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-ia64/param.h 1.4 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.3 vs edited =====
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 1.2 vs edited =====
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 1.6 vs edited =====
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 1.2 vs edited =====
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 1.3 vs edited =====
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 1.2 vs edited =====
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 1.2 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.1 vs edited =====
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 1.3 vs edited =====
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.27 vs edited =====
include/linux/init_task.h +3 -0
@@ -56,6 +56,8 @@ .siglock = SPIN_LOCK_UNLOCKED, \ } +extern struct group_info init_groups; +/* * INIT_TASK is used to set up the first task table, touch at * your own risk!. Base=0, limit=0x1fffff (=2MB)
@@ -87,6 +89,7 @@ .real_timer = { \ .function = it_real_fn \ }, \ + .group_info = &init_groups, \ .cap_effective = CAP_INIT_EFF_SET, \ .cap_inheritable = CAP_INIT_INH_SET, \ .cap_permitted = CAP_FULL_SET, \
===== include/linux/limits.h 1.3 vs edited =====
include/linux/limits.h +0 -1
@@ -3,7 +3,6 @@#define NR_OPEN 1024 -#define NGROUPS_MAX 32/* supplemental group IDs are available */ #define ARG_MAX 131072/* # bytes of args + environ for exec() */ #define CHILD_MAX 999/* no limit :-) */ #define OPEN_MAX 256/* # open files a process may have */
===== include/linux/sched.h 1.167 vs edited =====
include/linux/sched.h +27 -2
@@ -328,6 +328,32 @@struct io_context ;/* See blkdev.h */ void exit_io_context (void ); +#define NGROUPS_SMALL 32 +#define NGROUPS_BLOCK ((int)(EXEC_PAGESIZE / sizeof(gid_t))) +struct group_info { +int ngroups; +atomic_t usage; +gid_t small_block[NGROUPS_SMALL ]; +int nblocks; +gid_t *blocks[0 ]; +}; + +#define get_group_info (info) do { \ + atomic_inc(&(info)->usage); \ +} while (0); + +#define put_group_info (info) do { \ + if (atomic_dec_and_test(&(info)->usage)) \ + groups_free(info); \ +} while (0) + +struct group_info *groups_alloc (int gidsetsize); +void groups_free (struct group_info *info); +int set_current_groups (struct group_info *info); +/* access the groups "array" with this macro */ +#define GRP_AT (gi, i) ((gi)->blocks[(i)/NGROUPS_BLOCK][(i)%NGROUPS_BLOCK]) + +struct task_struct {volatile long state;/* -1 unrunnable, 0 runnable, >0 stopped */ struct thread_info *thread_info;
@@ -402,8 +428,7 @@/* process credentials */ uid_t uid,euid,suid,fsuid;gid_t gid,egid,sgid,fsgid; -int ngroups; -gid_t groups[NGROUPS ]; +struct group_info *group_info;kernel_cap_t cap_effective, cap_inheritable, cap_permitted; int keep_capabilities:1;struct user_struct *user;
===== include/linux/security.h 1.25 vs edited =====
include/linux/security.h +6 -7
@@ -551,9 +551,8 @@* Return 0 if permission is granted. * @task_setgroups: * Check permission before setting the supplementary group set of the -* current process to @grouplist. -* @gidsetsize contains the number of elements in @grouplist. -* @grouplist contains the array of gids. +* current process. +* @group_info contains the new group information. * Return 0 if permission is granted. * @task_setnice: * Check permission before setting the nice value of @p to @nice.
@@ -1097,7 +1096,7 @@int (*task_setpgid) (struct task_struct * p,pid_t pgid);int (*task_getpgid) (struct task_struct * p);int (*task_getsid) (struct task_struct * p); -int (*task_setgroups) (int gidsetsize,gid_t * grouplist); +int (*task_setgroups) (struct group_info *group_info);int (*task_setnice) (struct task_struct * p,int nice);int (*task_setrlimit) (unsigned int resource,struct rlimit * new_rlim); int (*task_setscheduler) (struct task_struct * p, int policy,
@@ -1647,9 +1646,9 @@return security_ops->task_getsid (p); } -static inline int security_task_setgroups (int gidsetsize,gid_t *grouplist) +static inline int security_task_setgroups (struct group_info *group_info) { -return security_ops->task_setgroups (gidsetsize, grouplist); +return security_ops->task_setgroups (group_info); }static inline int security_task_setnice (struct task_struct *p,int nice)
@@ -2275,7 +2274,7 @@return 0 ; } -static inline int security_task_setgroups (int gidsetsize,gid_t *grouplist) +static inline int security_task_setgroups (struct group_info *group_info) {return 0 ; }
===== include/linux/sunrpc/auth.h 1.9 vs edited =====
include/linux/sunrpc/auth.h +1 -2
@@ -28,8 +28,7 @@struct auth_cred {uid_t uid;gid_t gid; -int ngroups; -gid_t *groups; +struct group_info *group_info; };/*
===== include/linux/sunrpc/svcauth.h 1.9 vs edited =====
include/linux/sunrpc/svcauth.h +2 -1
@@ -16,10 +16,11 @@#include <linux/sunrpc/cache.h> #include <linux/hash.h> +#define SVC_CRED_NGROUPS 32struct 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 */
===== kernel/fork.c 1.142 vs edited =====
kernel/fork.c +3 -0
@@ -85,6 +85,7 @@security_task_free (tsk);free_uid (tsk->user); +put_group_info (tsk->group_info);free_task (tsk); }
@@ -817,6 +818,7 @@atomic_inc (&p->user->__count);atomic_inc (&p->user->processes); +get_group_info (p->group_info);/* * If multiple threads are within copy_process(), then this check
@@ -1061,6 +1063,7 @@ bad_fork_cleanup_put_domain:module_put (p->thread_info->exec_domain->module); bad_fork_cleanup_count: +put_group_info (p->group_info);atomic_dec (&p->user->processes);free_uid (p->user); bad_fork_free:
===== kernel/sys.c 1.61 vs edited =====
kernel/sys.c +170 -30
247 lines not on this page · raw message
===== kernel/uid16.c 1.5 vs edited =====
kernel/uid16.c +45 -21
89 lines not on this page · raw message
===== net/sunrpc/auth.c 1.12 vs edited =====
net/sunrpc/auth.c +2 -4
@@ -249,8 +249,7 @@struct auth_cred acred = { .uid = current->fsuid, .gid = current->fsgid, - .ngroups = current->ngroups, - .groups = current->groups, + .group_info = current->group_info, };dprintk ("RPC: looking up %s cred\n" , auth->au_ops->au_name);
@@ -264,8 +263,7 @@struct auth_cred acred = { .uid = current->fsuid, .gid = current->fsgid, - .ngroups = current->ngroups, - .groups = current->groups, + .group_info = current->group_info, }; dprintk("RPC: %4d looking up %s cred\n",
===== net/sunrpc/auth_unix.c 1.11 vs edited =====
net/sunrpc/auth_unix.c +4 -4
@@ -82,7 +82,7 @@ cred->uc_gid = cred->uc_pgid =0 ; cred->uc_gids[0 ] =NOGROUP ; }else { -int groups = acred->ngroups; +int groups = acred->group_info->ngroups;if (groups >NFS_NGROUPS ) groups =NFS_NGROUPS ;
@@ -91,7 +91,7 @@ cred->uc_puid = current->uid; cred->uc_pgid = current->gid;for (i =0 ; i < groups; i++) - cred->uc_gids[i] = (gid_t ) acred->groups[i]; + cred->uc_gids[i] =GRP_AT (acred->group_info, i);if (i <NFS_NGROUPS ) cred->uc_gids[i] =NOGROUP ; }
@@ -126,11 +126,11 @@ || cred->uc_pgid != current->gid)return 0 ; - groups = acred->ngroups; + groups = acred->group_info->ngroups;if (groups >NFS_NGROUPS ) groups =NFS_NGROUPS ;for (i =0 ; i < groups ; i++) -if (cred->uc_gids[i] != (gid_t ) acred->groups[i]) +if (cred->uc_gids[i] !=GRP_AT (acred->group_info, i))return 0 ;return 1 ; }
===== net/sunrpc/svcauth_unix.c 1.20 vs edited =====
net/sunrpc/svcauth_unix.c +2 -2
@@ -434,11 +434,11 @@if (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 ) {
===== security/dummy.c 1.27 vs edited =====
security/dummy.c +1 -1
@@ -530,7 +530,7 @@return 0 ; } -static int dummy_task_setgroups (int gidsetsize,gid_t * grouplist) +static int dummy_task_setgroups (struct group_info *group_info) {return 0 ; }
Quoted 4 lines · Tim Hockin
This patch touches all the compat code in the 64-bit architectures. These files have a LOT of duplicated code from
uid16.c. I did not try to reduce duplicated code, and instead followed suit.
Augh. It also makes code even uglier than it used to be:
...
+ u16 group;
+ if (copy_from_user(&group, grouplist+i, sizeof(group)))
+ return -EFAULT;
which is really a memcpy() of two bytes at a time. That's disgusting. It really should be
u16 group; if (nr > TASK_SIZE / sizeof(group)) return -EFAULT; if (!access_ok(grouplist, nr*sizeof(group)) return -EFAULT; ... if (__get_user(group, grouplist + i)) return -EFAULT; ...
which really is so common that it _really_ should be in kernel/uid16.c (or, actually create a new kernel/gid16.c file) rather than copied (incorrectly) to a lot of architectures. Then things like the above can be done right once, rather than merging this that does the nasty thing over and over.
Sorry to just complain all the time,
Linus
Quoted 7 lines · Linus Torvalds
Augh. It also makes code even uglier than it used to be:
...
+ u16 group;
+ if (copy_from_user(&group, grouplist+i, sizeof(group)))
+ return-EFAULT;
I can change it to do a copy_from_user one block at a time, if you prefer...
Quoted 9 lines
if (nr > TASK_SIZE / sizeof(group)) return -EFAULT; if (!access_ok(grouplist, nr*sizeof(group)) return -EFAULT; ... if (__get_user(group, grouplist + i)) return -EFAULT; ...
Or change it to this, which is the same 1-gid-at-a-time copy. This code is definitely SIMPLER than the 1-block-at-a-time copy. I'll go with that.
Quoted 5 lines
which really is so common that it _really_ should be in
kernel/uid16.c(or, actually create a newkernel/gid16.cfile) rather than copied (incorrectly) to a lot of architectures. Then things like the above can be done right once, rather than merging this that does the nasty thing over and over.
I'd love to put it in uid16.c, but uid16.c is not used by the 64-bit architectures. I remember proposing a simple fix at one point and being shot down. I'd love to fix that up, but it's a separate patch. If I fix it up as suggested above, will you take it with the promise that I'll find some way to do uid16 properly for the 64-bit arches and clean it up in a followup patch?
Quoted 1 line
Sorry to just complain all the time,
I'm just glad it's getting attention - I'm dying to take this off my todo list.
Tim
Quoted 3 lines · Tim Hockin
I'd love to put it in
uid16.c, butuid16.cis not used by the 64-bit architectures.
How about just putting it in "gid16.c" and then adding a CONFIG_GID16 config variable. Then architectures that want it (pretty much all, no?) can then obviously just do the
config GID16
bool
default y
in their Kconfig files and be happy. Add the obvious
obj-$(CONFIG_GID16) += gid16.o
to the kernel makefile and you're done. Looks surgically clean, and follows existing practice wrt uid16.
Ok?
Linus
Quoted 7 lines · Linus Torvalds
How about just putting it in "
gid16.c" and then adding aCONFIG_GID16config variable. Then architectures that want it (pretty much all, no?) can then obviously just do theconfig GID16
bool
default y
uid16 has this same code in it, which could be moved to gid16. The problem is that architectures that currently use CONFIG_UID16 expect to use old_gid_t and get all the highuid.h magic. The 64-bit architectures want to use stuff from uid16 (and by extension gid16) but they don't want it ALL the time, just in their compat code. Therefore, they don't have old_gid_t as 16 bits.
You end up with tons of duplicated code in the 64-bit architectures (most copies straight from uid16.c s/old_gid_t/short/).
You end up with places that include highuid.h and then undef functions and redefine them exactly the same s/old_gid_t/short/.
The gid16 way would work, but doesn't solve the real problem, which is all the duplicated code and differing types. Some use old_gid_t, some use short, some use u16.
What if I broke CONFIG_UID16 into CONFIG_UID16_SYSCALLS and CONFIG_OLDUID? UID16_SYSCALLS would make the build compile uid16.c. CONFIG_OLDUID would activate high2lowuid() and friends. It would involve copying some of highuid.h into uid16.c to make uid16.c always do high2lowuid(). But what types?
Your call - I'll make gid16.c if that's good enough for you. Or if you prefer, I'll do a separate patch in which we find a way to REALLY solve it.
Here's the prior fixups but not the gid16 duplicated code cleanups. If the rest of this patch meets approval, I'll be happy to go ahead on whatever you prefer for gid16 stuff.
o
Signature · 3 lines
Notice that as computers are becoming easier and easier to use, suddenly there's a big market for "Dummies" books. Cause and effect, or merely an ironic juxtaposition of unrelated facts?
forgot the patch again. Bad week for me.
===== arch/ia64/ia32/sys_ia32.c 1.78 vs edited =====
arch/ia64/ia32/sys_ia32.c +69 -28
@@ -2426,44 +2426,85 @@return sys_lseek (fd, offset, whence); } -extern asmlinkage long sys_getgroups (int gidsetsize,gid_t *grouplist); +static int +groups16_to_user (short *grouplist,struct group_info *info) +{ +int i; +short group; + +if (info->ngroups >TASK_SIZE /sizeof (group)) +return -EFAULT ; +if (!access_ok (grouplist, info->ngroups*sizeof (group))) +return -EFAULT ; + +for (i =0 ; i < info->ngroups; i++) { + group = (short )GROUP_AT (info, i); +if (__put_user (group, grouplist+i)) +return -EFAULT ; + } + +return 0 ; +} + +static int +groups16_from_user (struct group_info *info,short *grouplist) +{ +int i; +short group; + +if (info->ngroups >TASK_SIZE /sizeof (group)) +return -EFAULT ; +if (!access_ok (grouplist, info->ngroups*sizeof (group))) +return -EFAULT ; + +for (i =0 ; i < info->ngroups; i++) { +if (__get_user (group, grouplist+i)) +return -EFAULT ; +GROUP_AT (info, i) = (gid_t )group; + } + +return 0 ; +}asmlinkage long sys32_getgroups16 (int gidsetsize,short *grouplist) { -mm_segment_t old_fs =get_fs (); -gid_t gl[NGROUPS ]; -int ret, i; - -set_fs (KERNEL_DS ); - ret =sys_getgroups (gidsetsize, gl); -set_fs (old_fs); - -if (gidsetsize && ret >0 && ret <=NGROUPS ) -for (i =0 ; i < ret; i++, grouplist++) -if (put_user (gl[i], grouplist)) -return -EFAULT ; -return ret; -} +int i; -extern asmlinkage long sys_setgroups (int gidsetsize,gid_t *grouplist); +if (gidsetsize <0 ) +return -EINVAL ; + i = current->group_info->ngroups; +if (gidsetsize) { +if (i > gidsetsize) +return -EINVAL ; +if (groups16_to_user (grouplist, current->group_info)) +return -EFAULT ; + } +return i; +}asmlinkage long sys32_setgroups16 (int gidsetsize,short *grouplist) { -mm_segment_t old_fs =get_fs (); -gid_t gl[NGROUPS ]; -int ret, i; +struct group_info *new_info; +int retval; -if ((unsigned ) gidsetsize >NGROUPS ) -return -EINVAL ; -for (i =0 ; i < gidsetsize; i++, grouplist++) -if (get_user (gl[i], grouplist)) -return -EFAULT ; -set_fs (KERNEL_DS ); - ret =sys_setgroups (gidsetsize, gl); -set_fs (old_fs); -return ret; +if (!capable (CAP_SETGID )) +return -EPERM ; + new_info =groups_alloc (gidsetsize); +if (!new_info) +return -ENOMEM ; + retval =groups16_from_user (new_info, grouplist); +if (retval) { +groups_free (new_info); +return retval; + } + + retval =set_current_groups (new_info); +if (retval) +groups_free (new_info); + +return retval; }asmlinkage long
===== arch/mips/kernel/sysirix.c 1.18 vs edited =====
arch/mips/kernel/sysirix.c +1 -1
@@ -368,7 +368,7 @@ retval =HZ ; goto out;case 4 : - retval =NGROUPS ; + retval =INT_MAX ; goto out;case 5 : retval =NR_OPEN ;
===== arch/s390/kernel/compat_linux.c 1.9 vs edited =====
arch/s390/kernel/compat_linux.c +57 -16
96 lines not on this page · raw message
===== arch/sparc/kernel/sys_sunos.c 1.26 vs edited =====
arch/sparc/kernel/sys_sunos.c +1 -1
@@ -896,7 +896,7 @@ ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret =INT_MAX ;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
===== arch/sparc64/kernel/sys_sparc32.c 1.83 vs edited =====
arch/sparc64/kernel/sys_sparc32.c +57 -16
96 lines not on this page · raw message
===== arch/sparc64/kernel/sys_sunos32.c 1.35 vs edited =====
arch/sparc64/kernel/sys_sunos32.c +1 -1
@@ -859,7 +859,7 @@ ret =HZ ;break ;case _SC_NGROUPS_MAX: - ret =NGROUPS_MAX ; + ret =INT_MAX ;break ;case _SC_OPEN_MAX: ret =OPEN_MAX ;
===== arch/sparc64/solaris/misc.c 1.18 vs edited =====
arch/sparc64/solaris/misc.c +1 -1
@@ -341,7 +341,7 @@ asmlinkage int solaris_sysconf(int id) {switch (id) { -case SOLARIS_CONFIG_NGROUPS :return NGROUPS_MAX ; +case SOLARIS_CONFIG_NGROUPS :return INT_MAX ;case SOLARIS_CONFIG_CHILD_MAX :return CHILD_MAX ;case SOLARIS_CONFIG_OPEN_FILES :return OPEN_MAX ;case SOLARIS_CONFIG_POSIX_VER :return 199309 ;
===== fs/nfsd/auth.c 1.2 vs edited =====
fs/nfsd/auth.c +7 -4
@@ -10,12 +10,15 @@#include <linux/sunrpc/svcauth.h> #include <linux/nfsd/nfsd.h> +extern asmlinkage long sys_setgroups (int gidsetsize,gid_t *grouplist); +#define CAP_NFSD_MASK (CAP_FS_MASK|CAP_TO_MASK(CAP_SYS_RESOURCE))void nfsd_setuser (struct svc_rqst *rqstp,struct svc_export *exp) {struct svc_cred *cred = &rqstp->rq_cred;int i; +gid_t groups[SVC_CRED_NGROUPS ];if (exp->ex_flags &NFSEXP_ALLSQUASH ) { cred->cr_uid = exp->ex_anon_uid;
@@ -26,7 +29,7 @@ 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,13 +42,13 @@ current->fsgid = cred->cr_gid; else current->fsgid = exp->ex_anon_gid; -for (i =0 ; i <NGROUPS ; i++) { +for (i =0 ; i <SVC_CRED_NGROUPS ; i++) {gid_t group = cred->cr_groups[i];if (group == (gid_t )NOGROUP )break ; - current->groups[i] = group; + groups[i] = group; } - current->ngroups = i; +sys_setgroups (i, groups);if ((cred->cr_uid)) {cap_t (current->cap_effective) &= ~CAP_NFSD_MASK ;
===== fs/nfsd/nfs4state.c 1.20 vs edited =====
fs/nfsd/nfs4state.c +1 -1
@@ -244,7 +244,7 @@ target->cr_uid = source->cr_uid; target->cr_gid = source->cr_gid; -for (i =0 ; i <NGROUPS ; i++) +for (i =0 ; i <SVC_CRED_NGROUPS ; i++) target->cr_groups[i] = source->cr_groups[i]; }
===== fs/proc/array.c 1.52 vs edited =====
fs/proc/array.c +2 -2
@@ -176,8 +176,8 @@ p->files ? p->files->max_fds : 0);task_unlock (p); -for (g =0 ; g < p->ngroups; g++) - buffer +=sprintf (buffer,"%d " , p->groups[g]); +for (g =0 ; g <min (p->group_info->ngroups,NGROUPS_SMALL ); g++) + buffer +=sprintf (buffer,"%d " ,GROUP_AT (p->group_info,g)); buffer +=sprintf (buffer,"\n" );return buffer;
===== include/asm-alpha/param.h 1.2 vs edited =====
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 1.5 vs edited =====
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 1.1 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.2 vs edited =====
include/asm-i386/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-ia64/param.h 1.4 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.3 vs edited =====
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 1.2 vs edited =====
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 1.6 vs edited =====
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 1.2 vs edited =====
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 1.3 vs edited =====
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 1.2 vs edited =====
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 1.2 vs edited =====
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 1.2 vs edited =====
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 1.1 vs edited =====
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 1.1 vs edited =====
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 1.3 vs edited =====
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.27 vs edited =====
include/linux/init_task.h +3 -0
@@ -56,6 +56,8 @@ .siglock = SPIN_LOCK_UNLOCKED, \ } +extern struct group_info init_groups; +/* * INIT_TASK is used to set up the first task table, touch at * your own risk!. Base=0, limit=0x1fffff (=2MB)
@@ -87,6 +89,7 @@ .real_timer = { \ .function = it_real_fn \ }, \ + .group_info = &init_groups, \ .cap_effective = CAP_INIT_EFF_SET, \ .cap_inheritable = CAP_INIT_INH_SET, \ .cap_permitted = CAP_FULL_SET, \
===== include/linux/limits.h 1.3 vs edited =====
include/linux/limits.h +0 -1
@@ -3,7 +3,6 @@#define NR_OPEN 1024 -#define NGROUPS_MAX 32/* supplemental group IDs are available */ #define ARG_MAX 131072/* # bytes of args + environ for exec() */ #define CHILD_MAX 999/* no limit :-) */ #define OPEN_MAX 256/* # open files a process may have */
===== include/linux/sched.h 1.167 vs edited =====
include/linux/sched.h +27 -2
@@ -328,6 +328,32 @@struct io_context ;/* See blkdev.h */ void exit_io_context (void ); +#define NGROUPS_SMALL 32 +#define NGROUPS_BLOCK ((int)(EXEC_PAGESIZE / sizeof(gid_t))) +struct group_info { +int ngroups; +atomic_t usage; +gid_t small_block[NGROUPS_SMALL ]; +int nblocks; +gid_t *blocks[0 ]; +}; + +#define get_group_info (info) do { \ + atomic_inc(&(info)->usage); \ +} while (0); + +#define put_group_info (info) do { \ + if (atomic_dec_and_test(&(info)->usage)) \ + groups_free(info); \ +} while (0) + +struct group_info *groups_alloc (int gidsetsize); +void groups_free (struct group_info *info); +int set_current_groups (struct group_info *info); +/* access the groups "array" with this macro */ +#define GROUP_AT (gi, i) ((gi)->blocks[(i)/NGROUPS_BLOCK][(i)%NGROUPS_BLOCK]) + +struct task_struct {volatile long state;/* -1 unrunnable, 0 runnable, >0 stopped */ struct thread_info *thread_info;
@@ -402,8 +428,7 @@/* process credentials */ uid_t uid,euid,suid,fsuid;gid_t gid,egid,sgid,fsgid; -int ngroups; -gid_t groups[NGROUPS ]; +struct group_info *group_info;kernel_cap_t cap_effective, cap_inheritable, cap_permitted; int keep_capabilities:1;struct user_struct *user;
===== include/linux/security.h 1.25 vs edited =====
include/linux/security.h +6 -7
@@ -551,9 +551,8 @@* Return 0 if permission is granted. * @task_setgroups: * Check permission before setting the supplementary group set of the -* current process to @grouplist. -* @gidsetsize contains the number of elements in @grouplist. -* @grouplist contains the array of gids. +* current process. +* @group_info contains the new group information. * Return 0 if permission is granted. * @task_setnice: * Check permission before setting the nice value of @p to @nice.
@@ -1097,7 +1096,7 @@int (*task_setpgid) (struct task_struct * p,pid_t pgid);int (*task_getpgid) (struct task_struct * p);int (*task_getsid) (struct task_struct * p); -int (*task_setgroups) (int gidsetsize,gid_t * grouplist); +int (*task_setgroups) (struct group_info *group_info);int (*task_setnice) (struct task_struct * p,int nice);int (*task_setrlimit) (unsigned int resource,struct rlimit * new_rlim); int (*task_setscheduler) (struct task_struct * p, int policy,
@@ -1647,9 +1646,9 @@return security_ops->task_getsid (p); } -static inline int security_task_setgroups (int gidsetsize,gid_t *grouplist) +static inline int security_task_setgroups (struct group_info *group_info) { -return security_ops->task_setgroups (gidsetsize, grouplist); +return security_ops->task_setgroups (group_info); }static inline int security_task_setnice (struct task_struct *p,int nice)
@@ -2275,7 +2274,7 @@return 0 ; } -static inline int security_task_setgroups (int gidsetsize,gid_t *grouplist) +static inline int security_task_setgroups (struct group_info *group_info) {return 0 ; }
===== include/linux/sunrpc/auth.h 1.9 vs edited =====
include/linux/sunrpc/auth.h +1 -2
@@ -28,8 +28,7 @@struct auth_cred {uid_t uid;gid_t gid; -int ngroups; -gid_t *groups; +struct group_info *group_info; };/*
===== include/linux/sunrpc/svcauth.h 1.9 vs edited =====
include/linux/sunrpc/svcauth.h +2 -1
@@ -16,10 +16,11 @@#include <linux/sunrpc/cache.h> #include <linux/hash.h> +#define SVC_CRED_NGROUPS 32struct 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 */
===== kernel/fork.c 1.142 vs edited =====
kernel/fork.c +3 -0
@@ -85,6 +85,7 @@security_task_free (tsk);free_uid (tsk->user); +put_group_info (tsk->group_info);free_task (tsk); }
@@ -817,6 +818,7 @@atomic_inc (&p->user->__count);atomic_inc (&p->user->processes); +get_group_info (p->group_info);/* * If multiple threads are within copy_process(), then this check
@@ -1061,6 +1063,7 @@ bad_fork_cleanup_put_domain:module_put (p->thread_info->exec_domain->module); bad_fork_cleanup_count: +put_group_info (p->group_info);atomic_dec (&p->user->processes);free_uid (p->user); bad_fork_free:
===== kernel/sys.c 1.61 vs edited =====
kernel/sys.c +170 -30
247 lines not on this page · raw message
===== kernel/uid16.c 1.5 vs edited =====
kernel/uid16.c +59 -21
103 lines not on this page · raw message
===== net/sunrpc/auth.c 1.12 vs edited =====
net/sunrpc/auth.c +2 -4
@@ -249,8 +249,7 @@struct auth_cred acred = { .uid = current->fsuid, .gid = current->fsgid, - .ngroups = current->ngroups, - .groups = current->groups, + .group_info = current->group_info, };dprintk ("RPC: looking up %s cred\n" , auth->au_ops->au_name);
@@ -264,8 +263,7 @@struct auth_cred acred = { .uid = current->fsuid, .gid = current->fsgid, - .ngroups = current->ngroups, - .groups = current->groups, + .group_info = current->group_info, }; dprintk("RPC: %4d looking up %s cred\n",
===== net/sunrpc/auth_unix.c 1.11 vs edited =====
net/sunrpc/auth_unix.c +4 -4
@@ -82,7 +82,7 @@ cred->uc_gid = cred->uc_pgid =0 ; cred->uc_gids[0 ] =NOGROUP ; }else { -int groups = acred->ngroups; +int groups = acred->group_info->ngroups;if (groups >NFS_NGROUPS ) groups =NFS_NGROUPS ;
@@ -91,7 +91,7 @@ cred->uc_puid = current->uid; cred->uc_pgid = current->gid;for (i =0 ; i < groups; i++) - cred->uc_gids[i] = (gid_t ) acred->groups[i]; + cred->uc_gids[i] =GROUP_AT (acred->group_info, i);if (i <NFS_NGROUPS ) cred->uc_gids[i] =NOGROUP ; }
@@ -126,11 +126,11 @@ || cred->uc_pgid != current->gid)return 0 ; - groups = acred->ngroups; + groups = acred->group_info->ngroups;if (groups >NFS_NGROUPS ) groups =NFS_NGROUPS ;for (i =0 ; i < groups ; i++) -if (cred->uc_gids[i] != (gid_t ) acred->groups[i]) +if (cred->uc_gids[i] !=GROUP_AT (acred->group_info, i))return 0 ;return 1 ; }
===== net/sunrpc/svcauth_unix.c 1.20 vs edited =====
net/sunrpc/svcauth_unix.c +2 -2
@@ -434,11 +434,11 @@if (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 ) {
===== security/dummy.c 1.27 vs edited =====
security/dummy.c +1 -1
@@ -530,7 +530,7 @@return 0 ; } -static int dummy_task_setgroups (int gidsetsize,gid_t * grouplist) +static int dummy_task_setgroups (struct group_info *group_info) {return 0 ; }
IDEA: arch uid16 cleanup (was 'Many groups patch')
In reply to Linus TorvaldsQuoted 5 lines · Linus Torvalds
Quoted 3 lines · Tim Hockin
I'd love to put it in
uid16.c, butuid16.cis not used by the 64-bit architectures.
Quoted 7 lines
How about just putting it in "
gid16.c" and then adding aCONFIG_GID16config variable. Then architectures that want it (pretty much all, no?) can then obviously just do theconfig GID16
bool
default y
So I dug deeper into the problem, and I think it can be solved relatively painlessly.
First a few observations, based on grep:
* uid16_t is only used once: fs/smbfs/ioctl.c
* gid16_t is never used
* every arch defines uid16_t/gid16_t to unsigned short
* some arches define old_uid_t/old_gid_t the same as uid_t/gid_t, some don't
* ncpfs and smbfs use __kernel_old_uid_t
* old_uid_t and old_gid_t are only used in highuid.h and uid16.c
* every arch that defines UID16 defines old_uid_t and old_gid_t to ushort, except x86_64 (which I *think* is a bug)
So what I'm thinking is this:
1) convert uid16.c to use uid16_t and gid16_t, and NOT use highuid.h
2) build uid16.o iff CONFIG_UID16_SYSCALLS
3) anywhere that defines CONFIG_UID16 adds CONFIG_UID16_SYSCALLS
4) any 64-bit arch that wants uid16 stuff adds CONFIG_UID16_SYSCALLS
Now, the 16-bit forms of the syscalls are available to all the interested parties. Then we go through the arch stuff and remove all the duplicated uid16 stuff, where ever possible.
This will leave highuid.h unmolested, so all dependants of that will still work.
Here's the really simple patch, without removing any arch code, yet. What's the preferred way in Kconfig to identify the proper arrangement of this idea?
Thoughts? Can anyone (64bit arch people?) poke a hole in it?
Tim
===== arch/arm/Kconfig 1.38 vs edited =====
arch/arm/Kconfig +1 -0
@@ -48,6 +48,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/arm26/Kconfig 1.9 vs edited =====
arch/arm26/Kconfig +1 -0
@@ -36,6 +36,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/cris/Kconfig 1.15 vs edited =====
arch/cris/Kconfig +1 -0
@@ -11,6 +11,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/h8300/Kconfig 1.9 vs edited =====
arch/h8300/Kconfig +1 -0
@@ -19,6 +19,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/i386/Kconfig 1.86 vs edited =====
arch/i386/Kconfig +1 -0
@@ -23,6 +23,7 @@ config UID16 bool + select UID16_SYSCALLS default y config GENERIC_ISA_DMA
===== arch/ia64/Kconfig 1.46 vs edited =====
arch/ia64/Kconfig +1 -0
@@ -389,6 +389,7 @@ config COMPAT bool depends on IA32_SUPPORT + select UID16_SYSCALLS default y config PERFMON
===== arch/m68k/Kconfig 1.25 vs edited =====
arch/m68k/Kconfig +1 -0
@@ -12,6 +12,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/m68knommu/Kconfig 1.22 vs edited =====
arch/m68knommu/Kconfig +1 -0
@@ -15,6 +15,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/s390/Kconfig 1.18 vs edited =====
arch/s390/Kconfig +1 -0
@@ -139,6 +139,7 @@ config COMPAT bool depends on S390_SUPPORT + select UID16_SYSCALLS default y config BINFMT_ELF32
===== arch/sh/Kconfig 1.20 vs edited =====
arch/sh/Kconfig +1 -0
@@ -16,6 +16,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/sparc/Kconfig 1.22 vs edited =====
arch/sparc/Kconfig +1 -0
@@ -11,6 +11,7 @@ config UID16 bool + select UID16_SYSCALLS default y config HIGHMEM
===== arch/sparc64/Kconfig 1.36 vs edited =====
arch/sparc64/Kconfig +1 -0
@@ -357,6 +357,7 @@ config COMPAT bool depends on SPARC32_COMPAT + select UID16_SYSCALLS default y config BINFMT_ELF32
===== arch/um/Kconfig 1.12 vs edited =====
arch/um/Kconfig +1 -0
@@ -20,6 +20,7 @@ config UID16 bool + select UID16_SYSCALLS default y config RWSEM_GENERIC_SPINLOCK
===== arch/x86_64/Kconfig 1.32 vs edited =====
arch/x86_64/Kconfig +1 -0
@@ -383,6 +383,7 @@ config UID16 bool depends on IA32_EMULATION + select UID16_SYSCALLS default y endmenu
===== include/asm-x86_64/posix_types.h 1.4 vs edited =====
include/asm-x86_64/posix_types.h +2 -2
@@ -36,8 +36,8 @@int val[2 ]; } __kernel_fsid_t; -typedef __kernel_uid_t __kernel_old_uid_t ; -typedef __kernel_gid_t __kernel_old_gid_t ; +typedef unsigned short __kernel_old_uid_t ; +typedef unsigned short __kernel_old_gid_t ;typedef __kernel_uid_t __kernel_uid32_t ;typedef __kernel_gid_t __kernel_gid32_t ;
===== kernel/Makefile 1.35 vs edited =====
kernel/Makefile +1 -1
@@ -11,7 +11,7 @@ obj-$(CONFIG_FUTEX) += futex.o obj-$(CONFIG_GENERIC_ISA_DMA) += dma.o obj-$(CONFIG_SMP) += cpu.o -obj-$(CONFIG_UID16) += uid16.o +obj-$(CONFIG_UID16_SYSCALLS) += uid16.o obj-$(CONFIG_MODULES) += ksyms.o module.o obj-$(CONFIG_KALLSYMS) += kallsyms.o obj-$(CONFIG_PM) += power/
===== kernel/uid16.c 1.5 vs edited =====
kernel/uid16.c +28 -20
141 lines not on this page · raw message
In message <[email protected]> you write:
Quoted 8 lines · Tim Hockin
Quoted 4 lines
This patch touches all the compat code in the 64-bit architectures. These files have a LOT of duplicated code from
uid16.c. I did not try to reduce duplicated code, and instead followed suit.Augh. It also makes code even uglier than it used to be:
Sure. First step is to put this function in kernel/compat.c where it belongs. The identical function is already in kernel/uid16.c, but defining CONFIG_UID16 does not work for these platforms (which only want 16-bit uids for the 32-bit syscalls).
I discussed this with DaveM and David Mosberger last week.
Naturally, untested (damn obscure archs!), but ideally this changes nothing, just makes a groups patch simpler.
Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Name: Put 16-bit GID code in kernel/compat.c
Author: Rusty Russell
Status: Trivial
D: IA64, S390 and Sparc64 need 16-bit setgroups and getgroups calls,
D: but can't define CONFIG_UID16 since they only want 16-bit gid/uid
D: compatibility for 32-bit system calls. Centralize their versions
D: of setgroups16 and getgroups16 into kernel/compat.c, under a
D: NEED_COMPAT_GROUPS16 define.
arch/ia64/ia32/ia32_entry.S +2 -2
@@ -266,8 +266,8 @@ ia32_syscall_table: data8 compat_sys_getrusage data8 sys32_gettimeofday data8 sys32_settimeofday - data8 sys32_getgroups16 /* 80 */ - data8 sys32_setgroups16 + data8 compat_getgroups16 /* 80 */ + data8 compat_setgroups16 data8 sys32_old_select data8 sys_symlink data8 sys32_ni_syscall
arch/ia64/ia32/sys_ia32.c +0 -40
@@ -2426,46 +2426,6 @@ sys32_lseek (unsigned int fd, int offsetreturn sys_lseek (fd, offset, whence); } -extern asmlinkage long sys_getgroups (int gidsetsize,gid_t *grouplist); - -asmlinkage long -sys32_getgroups16 (int gidsetsize,short *grouplist) -{ -mm_segment_t old_fs =get_fs (); -gid_t gl[NGROUPS ]; -int ret, i; - -set_fs (KERNEL_DS ); - ret =sys_getgroups (gidsetsize, gl); -set_fs (old_fs); - -if (gidsetsize && ret >0 && ret <=NGROUPS ) -for (i =0 ; i < ret; i++, grouplist++) -if (put_user (gl[i], grouplist)) -return -EFAULT ; -return ret; -} - -extern asmlinkage long sys_setgroups (int gidsetsize,gid_t *grouplist); - -asmlinkage long -sys32_setgroups16 (int gidsetsize,short *grouplist) -{ -mm_segment_t old_fs =get_fs (); -gid_t gl[NGROUPS ]; -int ret, i; - -if ((unsigned ) gidsetsize >NGROUPS ) -return -EINVAL ; -for (i =0 ; i < gidsetsize; i++, grouplist++) -if (get_user (gl[i], grouplist)) -return -EFAULT ; -set_fs (KERNEL_DS ); - ret =sys_setgroups (gidsetsize, gl); -set_fs (old_fs); -return ret; -} -asmlinkage long sys32_truncate64 (unsigned int path,unsigned int len_lo,unsigned int len_hi) {
arch/s390/kernel/compat_linux.c +0 -36
@@ -189,42 +189,6 @@ asmlinkage long sys32_setfsgid16(u16 gidreturn sys_setfsgid ((gid_t )gid); } -asmlinkage long sys32_getgroups16 (int gidsetsize,u16 *grouplist) -{ -u16 groups[NGROUPS ]; -int i,j; - -if (gidsetsize <0 ) -return -EINVAL ; - i = current->ngroups; -if (gidsetsize) { -if (i > gidsetsize) -return -EINVAL ; -for (j=0 ;j<i;j++) - groups[j] = current->groups[j]; -if (copy_to_user (grouplist, groups,sizeof (u16)*i)) -return -EFAULT ; - } -return i; -} - -asmlinkage long sys32_setgroups16 (int gidsetsize,u16 *grouplist) -{ -u16 groups[NGROUPS ]; -int i; - -if (!capable (CAP_SETGID )) -return -EPERM ; -if ((unsigned ) gidsetsize >NGROUPS ) -return -EINVAL ; -if (copy_from_user (groups, grouplist, gidsetsize *sizeof (u16))) -return -EFAULT ; -for (i =0 ; i < gidsetsize ; i++) - current->groups[i] = (gid_t )groups[i]; - current->ngroups = gidsetsize; -return 0 ; -} -asmlinkage long sys32_getuid16 (void ) {return high2lowuid (current->uid);
arch/s390/kernel/compat_wrapper.S +2 -2
@@ -344,13 +344,13 @@ sys32_settimeofday_wrapper: sys32_getgroups16_wrapper: lgfr %r2,%r2 # int llgtr %r3,%r3 # __kernel_old_gid_emu31_t * - jg sys32_getgroups16 # branch to system call + jg compat_getgroups16 # branch to system call .globl sys32_setgroups16_wrapper sys32_setgroups16_wrapper: lgfr %r2,%r2 # int llgtr %r3,%r3 # __kernel_old_gid_emu31_t * - jg sys32_setgroups16 # branch to system call + jg compat_setgroups16 # branch to system call .globl sys32_symlink_wrapper sys32_symlink_wrapper:
arch/sparc64/kernel/sys_sparc32.c +0 -36
@@ -206,42 +206,6 @@ asmlinkage long sys32_setfsgid16(u16 gidreturn sys_setfsgid ((gid_t )gid); } -asmlinkage long sys32_getgroups16 (int gidsetsize,u16 *grouplist) -{ -u16 groups[NGROUPS ]; -int i,j; - -if (gidsetsize <0 ) -return -EINVAL ; - i = current->ngroups; -if (gidsetsize) { -if (i > gidsetsize) -return -EINVAL ; -for (j=0 ;j<i;j++) - groups[j] = current->groups[j]; -if (copy_to_user (grouplist, groups,sizeof (u16)*i)) -return -EFAULT ; - } -return i; -} - -asmlinkage long sys32_setgroups16 (int gidsetsize,u16 *grouplist) -{ -u16 groups[NGROUPS ]; -int i; - -if (!capable (CAP_SETGID )) -return -EPERM ; -if ((unsigned ) gidsetsize >NGROUPS ) -return -EINVAL ; -if (copy_from_user (groups, grouplist, gidsetsize *sizeof (u16))) -return -EFAULT ; -for (i =0 ; i < gidsetsize ; i++) - current->groups[i] = (gid_t )groups[i]; - current->ngroups = gidsetsize; -return 0 ; -} -asmlinkage long sys32_getuid16 (void ) {return high2lowuid (current->uid);
arch/sparc64/kernel/systbls.S +4 -4
@@ -34,8 +34,8 @@ sys_call_table32: /*60*/ .word sys_umask, sys_chroot, compat_sys_newfstat, sys_fstat64, sys_getpagesize .word sys_msync, sys_vfork, sys32_pread64, sys32_pwrite64, sys_geteuid /*70*/ .word sys_getegid, sys32_mmap, sys_setreuid, sys_munmap, sys_mprotect - .word sys_madvise, sys_vhangup, sys32_truncate64, sys_mincore, sys32_getgroups16 -/*80*/ .word sys32_setgroups16, sys_getpgrp, sys_setgroups, compat_sys_setitimer, sys32_ftruncate64 + .word sys_madvise, sys_vhangup, sys32_truncate64, sys_mincore, compat_getgroups16 +/*80*/ .word compat_setgroups16, sys_getpgrp, sys_setgroups, compat_sys_setitimer, sys32_ftruncate64 .word sys_swapon, compat_sys_getitimer, sys_setuid, sys_sethostname, sys_setgid /*90*/ .word sys_dup2, sys_setfsuid, compat_sys_fcntl, sys32_select, sys_setfsgid .word sys_fsync, sys_setpriority32, sys_nis_syscall, sys_nis_syscall, sys_nis_syscall
@@ -168,8 +168,8 @@ sunos_sys_table: .word sunos_nosys, sunos_sbrk, sunos_sstk .word sunos_mmap, sunos_vadvise, sys_munmap .word sys_mprotect, sys_madvise, sys_vhangup - .word sunos_nosys, sys_mincore, sys32_getgroups16 - .word sys32_setgroups16, sys_getpgrp, sunos_setpgrp + .word sunos_nosys, sys_mincore, compat_getgroups16 + .word compat_setgroups16, sys_getpgrp, sunos_setpgrp .word compat_sys_setitimer, sunos_nosys, sys_swapon .word compat_sys_getitimer, sys_gethostname, sys_sethostname .word sunos_getdtablesize, sys_dup2, sunos_nop
include/asm-ia64/compat.h +2 -0
@@ -136,4 +136,6 @@ compat_alloc_user_space (long len) return (void *) (((regs->r12 & 0xffffffff) & -16) - len); } +#define NEED_COMPAT_GROUPS16 +#endif /* _ASM_IA64_COMPAT_H */
include/asm-s390/compat.h +2 -0
@@ -134,4 +134,6 @@ static inline void *compat_alloc_user_sp return (void *) (stack - len); } +#define NEED_COMPAT_GROUPS16 +#endif /* _ASM_S390X_COMPAT_H */
include/asm-sparc64/compat.h +1 -0
@@ -132,4 +132,5 @@ static __inline__ void *compat_alloc_use return (void *) (usp - len); } +#define NEED_COMPAT_GROUPS16 #endif /* _ASM_SPARC64_COMPAT_H */
include/linux/compat.h +5 -0
@@ -90,5 +90,10 @@ struct compat_statfs64 {__u32 f_spare[5 ]; }; +#ifdef NEED_COMPAT_GROUPS16 +asmlinkage long compat_getgroups16 (int gidsetsize,short *grouplist); +asmlinkage long compat_setgroups16 (int gidsetsize,short *grouplist); +#endif /* NEED_COMPAT_GROUPS16 */ +#endif /* CONFIG_COMPAT */ #endif /* _LINUX_COMPAT_H */
kernel/compat.c +41 -0
44 lines not on this page · raw message
In message <[email protected]> you write:
Quoted 2 lines
Because groups are stored in a 2-D array, the
GRP_AT()macro was added to allow simple 1-D style indexing.
I respectfully disagree with this approach. I don't understand why Linus would think that 4 lines of code to fallback to vmalloc is "ugly", when doing your own array of pages means you have to change all the iterators and do magic when copying to and from userspace, since it's no longer a simple array.
Tim and I have had this discussion, but I would benifit from Linus' wisdom here.
Quoted 1 line
Groups are sorted and b-searched for efficiency.
That's where I feel Linus' comment about catering to stupidity comes in 8)
Quoted 1 line
This patch modifies
/proc/pid/statusto only display the first 32 groups.
Ouch, good catch, but this is broken in both our patches 8( I think we should bump the refcount before reading it to avoid accessing freed memory. Now, they have to be root to access that anyway, but it just makes me uncomfortable.
Quoted 2 lines
This patch removes the NGROUPS define from all architectures as well as
NGROUPS_MAX.
This will break glibc build (trivial to fix), but I agree: leaving NGROUPS_MAX is too likely to cause people to use it inside the kernel like intermezzo does.
Quoted 2 lines
This patch changes the security API to check a struct
group_info, rather than an array ofgid_t.
This is why I prefer the "fall back to vmalloc" approach.
Quoted 1 line
This patch totally horks Intermezzo.
That's a feature 8)
Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Quoted 4 lines
Quoted 1 line
Groups are sorted and b-searched for efficiency.
That's where I feel Linus' comment about catering to stupidity comes in 8)
I'm in this discussion for NFS groups only. The difficulty is that NFS needs to sort groups differently, LRU by usage (instead of numerical value). Therefore, two ways to index that data set are unavoidable. Two b-trees, perhaps.
-- Pete
Quoted 6 lines · Rusty Russell
Quoted 1 line
Augh. It also makes code even uglier than it used to be:
Sure. First step is to put this function in
kernel/compat.cwhere it belongs. The identical function is already inkernel/uid16.c, but definingCONFIG_UID16does not work for these platforms (which only want 16-bit uids for the 32-bit syscalls).
That works, I guess. See my other message today (sorry, I forgot to CC: you Rusty) with a different approach. It makes uid16.c actually always be used.
Quoted 4 lines · On Thu, 02 Oct 2003 12:09:19 +1000 Rusty Russell <[email protected]> wrote:
Sure. First step is to put this function in
kernel/compat.cwhere it belongs. The identical function is already inkernel/uid16.c, but definingCONFIG_UID16does not work for these platforms (which only want 16-bit uids for the 32-bit syscalls).
This is correct. To be more precise, we also need it for the compat ABI binary format handlers too (elf32, etc.)