From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Fri, 30 Jan 2004 13:12:56 -0800 From: Tim Hockin Subject: Re: 2.6.2-rc2-mm2 Message-ID: <20040130211256.GZ9155@sun.com> Reply-To: thockin@sun.com References: <20040130014108.09c964fd.akpm@osdl.org> <1075489136.5995.30.camel@moria.arnor.net> <200401302007.26333.thomas.schlichter@web.de> <1075490624.4272.7.camel@laptop.fenrus.com> <20040130114701.18aec4e8.akpm@osdl.org> <20040130201731.GY9155@sun.com> <20040130123301.70009427.akpm@osdl.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20040130123301.70009427.akpm@osdl.org> Sender: owner-linux-mm@kvack.org Return-Path: To: Andrew Morton Cc: arjanv@redhat.com, thomas.schlichter@web.de, thoffman@arnor.net, linux-kernel@vger.kernel.org, linux-mm@kvack.org List-ID: On Fri, Jan 30, 2004 at 12:33:01PM -0800, Andrew Morton wrote: > static long do_setgroups(int gidsetsize, gid_t __user *user_grouplist, > gid_t *kern_grouplist) > { > } > asmlinkage long sys_setgroups(int gidsetsize, gid_t __user *grouplist) > { > return do_setgroups(gidsetsize, grouplist, NULL); > } > > long kern_setgroups(int gidsetsize, gid_t *grouplist) > { > return do_setgroups(gidsetsize, NULL, grouplist); > } I guess that works. It saves a bit of duplicate code at the cost of said grubbiness. Is that really preferred over a parallel to sys_setgroups(): int kern_setgroups(int gidsetsize, gid_t *grouplist) or simpler: nfsd code: /* build up the array of SVC_CRED_NGROUPS */ group_info = groups_alloc(SVC_CRED_NGROUPS); /* error check */ /* copy local array into group_info */ retval = set_current_groups(group_info); /* error check */ The nfsd code does not need to check CAP_SETGID or > NGROUPS_MAX, really. Interestingly, nfsd_setuser returns void, so any error checking is moot. Bad news, there. set_current_groups() was extracted so that any place in kernel that needs to set the groups can do so properly. I suggest that I just clean it up as that, or add a kern_setgroups() that encapsulates the above. It will be about 12 lines of code. In fact, here is a rough cut (would need a coupel exported syms, too). The lack of any way to handle errors bothers me. printk and fail? yeesh. ===== fs/nfsd/auth.c 1.3 vs edited ===== --- 1.3/fs/nfsd/auth.c Thu Jan 29 13:40:50 2004 +++ edited/fs/nfsd/auth.c Fri Jan 30 13:11:21 2004 @@ -10,15 +10,14 @@ #include #include -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; + int i, j; gid_t groups[SVC_CRED_NGROUPS]; + struct group_info *group_info; if (exp->ex_flags & NFSEXP_ALLSQUASH) { cred->cr_uid = exp->ex_anon_uid; @@ -48,7 +47,12 @@ break; groups[i] = group; } - sys_setgroups(i, groups); + group_info = groups_alloc(i); + /* should be error checking, but we can't return ENOMEM! */ + for (j = 0; j < i; j++) + GROUP_AT(group_info, j) = groups[j]; + if (set_current_groups(group_info)) + put_group_info(group_info); if ((cred->cr_uid)) { cap_t(current->cap_effective) &= ~CAP_NFSD_MASK; -- Tim Hockin Sun Microsystems, Linux Software Engineering thockin@sun.com All opinions are my own, not Sun's -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: aart@kvack.org