Commit bdcf0a42 authored by Thiago Rafael Becker's avatar Thiago Rafael Becker Committed by Linus Torvalds
Browse files

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: default avatarThiago Rafael Becker <thiago.becker@gmail.com>
Reviewed-by: default avatarMatthew Wilcox <mawilcox@microsoft.com>
Reviewed-by: default avatarNeilBrown <neilb@suse.com>
Acked-by: default avatar"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: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 1f704fd0

arch/s390/kernel/compat_linux.c

+1 −0
Original line number Diff line number Diff line
@@ -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);

fs/nfsd/auth.c

+3 −0
Original line number Diff line number Diff line
@@ -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);

include/linux/cred.h

+1 −0
Original line number Diff line number Diff line
@@ -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

kernel/groups.c

+3 −2
Original line number Diff line number Diff line
@@ -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);

kernel/uid16.c

+1 −0
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE