forked from mirrors/linux
		
	kernel: make groups_sort calling a responsibility group_info allocators
In testing, we found that nfsd threads may call set_groups in parallel for the same entry cached in auth.unix.gid, racing in the call of groups_sort, corrupting the groups for that entry and leading to permission denials for the client. This patch: - Make groups_sort globally visible. - Move the call to groups_sort to the modifiers of group_info - Remove the call to groups_sort from set_groups Link: http://lkml.kernel.org/r/20171211151420.18655-1-thiago.becker@gmail.com Signed-off-by: Thiago Rafael Becker <thiago.becker@gmail.com> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com> Reviewed-by: NeilBrown <neilb@suse.com> Acked-by: "J. Bruce Fields" <bfields@fieldses.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
		
							parent
							
								
									1f704fd0d1
								
							
						
					
					
						commit
						bdcf0a423e
					
				
					 8 changed files with 13 additions and 2 deletions
				
			
		| 
						 | 
				
			
			@ -263,6 +263,7 @@ COMPAT_SYSCALL_DEFINE2(s390_setgroups16, int, gidsetsize, u16 __user *, grouplis
 | 
			
		|||
		return retval;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	groups_sort(group_info);
 | 
			
		||||
	retval = set_current_groups(group_info);
 | 
			
		||||
	put_group_info(group_info);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -60,6 +60,9 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
 | 
			
		|||
				gi->gid[i] = exp->ex_anon_gid;
 | 
			
		||||
			else
 | 
			
		||||
				gi->gid[i] = rqgi->gid[i];
 | 
			
		||||
 | 
			
		||||
			/* Each thread allocates its own gi, no race */
 | 
			
		||||
			groups_sort(gi);
 | 
			
		||||
		}
 | 
			
		||||
	} else {
 | 
			
		||||
		gi = get_group_info(rqgi);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -83,6 +83,7 @@ extern int set_current_groups(struct group_info *);
 | 
			
		|||
extern void set_groups(struct cred *, struct group_info *);
 | 
			
		||||
extern int groups_search(const struct group_info *, kgid_t);
 | 
			
		||||
extern bool may_setgroups(void);
 | 
			
		||||
extern void groups_sort(struct group_info *);
 | 
			
		||||
 | 
			
		||||
/*
 | 
			
		||||
 * The security context of a task
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -86,11 +86,12 @@ static int gid_cmp(const void *_a, const void *_b)
 | 
			
		|||
	return gid_gt(a, b) - gid_lt(a, b);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static void groups_sort(struct group_info *group_info)
 | 
			
		||||
void groups_sort(struct group_info *group_info)
 | 
			
		||||
{
 | 
			
		||||
	sort(group_info->gid, group_info->ngroups, sizeof(*group_info->gid),
 | 
			
		||||
	     gid_cmp, NULL);
 | 
			
		||||
}
 | 
			
		||||
EXPORT_SYMBOL(groups_sort);
 | 
			
		||||
 | 
			
		||||
/* a simple bsearch */
 | 
			
		||||
int groups_search(const struct group_info *group_info, kgid_t grp)
 | 
			
		||||
| 
						 | 
				
			
			@ -122,7 +123,6 @@ int groups_search(const struct group_info *group_info, kgid_t grp)
 | 
			
		|||
void set_groups(struct cred *new, struct group_info *group_info)
 | 
			
		||||
{
 | 
			
		||||
	put_group_info(new->group_info);
 | 
			
		||||
	groups_sort(group_info);
 | 
			
		||||
	get_group_info(group_info);
 | 
			
		||||
	new->group_info = group_info;
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -206,6 +206,7 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist)
 | 
			
		|||
		return retval;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	groups_sort(group_info);
 | 
			
		||||
	retval = set_current_groups(group_info);
 | 
			
		||||
	put_group_info(group_info);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -192,6 +192,7 @@ SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t __user *, grouplist)
 | 
			
		|||
		return retval;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	groups_sort(group_info);
 | 
			
		||||
	retval = set_current_groups(group_info);
 | 
			
		||||
	put_group_info(group_info);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -231,6 +231,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
 | 
			
		|||
			goto out_free_groups;
 | 
			
		||||
		creds->cr_group_info->gid[i] = kgid;
 | 
			
		||||
	}
 | 
			
		||||
	groups_sort(creds->cr_group_info);
 | 
			
		||||
 | 
			
		||||
	return 0;
 | 
			
		||||
out_free_groups:
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -481,6 +481,7 @@ static int rsc_parse(struct cache_detail *cd,
 | 
			
		|||
				goto out;
 | 
			
		||||
			rsci.cred.cr_group_info->gid[i] = kgid;
 | 
			
		||||
		}
 | 
			
		||||
		groups_sort(rsci.cred.cr_group_info);
 | 
			
		||||
 | 
			
		||||
		/* mech name */
 | 
			
		||||
		len = qword_get(&mesg, buf, mlen);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -520,6 +520,7 @@ static int unix_gid_parse(struct cache_detail *cd,
 | 
			
		|||
		ug.gi->gid[i] = kgid;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	groups_sort(ug.gi);
 | 
			
		||||
	ugp = unix_gid_lookup(cd, uid);
 | 
			
		||||
	if (ugp) {
 | 
			
		||||
		struct cache_head *ch;
 | 
			
		||||
| 
						 | 
				
			
			@ -819,6 +820,7 @@ svcauth_unix_accept(struct svc_rqst *rqstp, __be32 *authp)
 | 
			
		|||
		kgid_t kgid = make_kgid(&init_user_ns, svc_getnl(argv));
 | 
			
		||||
		cred->cr_group_info->gid[i] = kgid;
 | 
			
		||||
	}
 | 
			
		||||
	groups_sort(cred->cr_group_info);
 | 
			
		||||
	if (svc_getu32(argv) != htonl(RPC_AUTH_NULL) || svc_getu32(argv) != 0) {
 | 
			
		||||
		*authp = rpc_autherr_badverf;
 | 
			
		||||
		return SVC_DENIED;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue