FAILED: patch "[PATCH] kernel: make groups_sort calling a responsibility group_info" failed to apply to 4.4-stable tree

PATCH In review [email protected] 3 participants 5 messages Dec 18, 2017 – Jan 5, 2018

Subsystems (6)

Networking Stable branch NFS client NFS server +2 more subsystems

lore

Share & tools

lore

[email protected]

author maintainer

2017-12-18 11:36 UTC #0 lore raw

The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From bdcf0a423ea1c40bbb40e7ee483b50fc8aa3d758 Mon Sep 17 00:00:00 2001

From: Thiago Rafael Becker <[email protected]>
Date: Thu, 14 Dec 2017 15:33:12 -0800
Subject: [PATCH] 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/[email protected] open here

Signed-off-by Thiago Rafael Becker <[email protected]>

Reviewed-by Matthew Wilcox <[email protected]>

Reviewed-by NeilBrown <[email protected]>

Acked-by "J. Bruce Fields" <[email protected]>

Cc Al Viro <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

arch/s390/kernel/compat_linux.c +1 -0

index f04db3779b34..59eea9c65d3e 100644
@@ -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

index 697f8ae7792d..f650e475d8f0 100644
@@ -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

index 099058e1178b..631286535d0f 100644
@@ -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

index e357bc800111..daae2f2dc6d4 100644
@@ -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

index ce74a4901d2b..ef1da2a5f9bd 100644
@@ -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);

net/sunrpc/auth_gss/gss_rpc_xdr.c +1 -0

index c4778cae58ef..444380f968f1 100644
@@ -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:

net/sunrpc/auth_gss/svcauth_gss.c +1 -0

index 5dd4e6c9fef2..26531193fce4 100644
@@ -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);

net/sunrpc/svcauth_unix.c +2 -0

index 740b67d5a733..af7f28fb8102 100644
@@ -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;

Thiago Rafael Becker

2017-12-19 01:03 UTC #1 lore raw

PATCH 1/1 kernel: make groups_sort calling a responsibility group_info allocators

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/[email protected] open here

(backported from bdcf0a423ea1c40bbb40e7ee483b50fc8aa3d758)

Signed-off-by Thiago Rafael Becker <[email protected]>

Reviewed-by Matthew Wilcox <[email protected]>

Reviewed-by NeilBrown <[email protected]>

Acked-by "J. Bruce Fields" <[email protected]>

Cc Al Viro <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

---
 arch/s390/kernel/compat_linux.c   | 1 +
 fs/nfsd/auth.c                    | 3 +++
 include/linux/cred.h              | 1 +
 kernel/groups.c                   | 5 +++--
 kernel/uid16.c                    | 1 +
 net/sunrpc/auth_gss/gss_rpc_xdr.c | 1 +
 net/sunrpc/auth_gss/svcauth_gss.c | 1 +
 net/sunrpc/svcauth_unix.c         | 2 ++
 8 files changed, 13 insertions(+), 2 deletions(-)

arch/s390/kernel/compat_linux.c +1 -0

index 437e611..0176ebc 100644
@@ -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

index 9d46a0b..a260060 100644
@@ -59,6 +59,9 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
 				GROUP_AT(gi, i) = exp->ex_anon_gid;
 			else
 				GROUP_AT(gi, i) = GROUP_AT(rqgi, i);
+
+			/* Each thread allocates its own gi, no race */
+			groups_sort(gi);
 		}
 	} else {
 		gi = get_group_info(rqgi);

include/linux/cred.h +1 -0

index 8d70e13..a0dcacb 100644
@@ -87,6 +87,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 *);
 
 /* access the groups "array" with this macro */
 #define GROUP_AT(gi, i) \

kernel/groups.c +3 -2

index 74d431d..5ea9847 100644
@@ -101,7 +101,7 @@ static int groups_from_user(struct group_info *group_info,
 }
 
 /* a simple Shell sort */
-static void groups_sort(struct group_info *group_info)
+void groups_sort(struct group_info *group_info)
 {
 	int base, max, stride;
 	int gidsetsize = group_info->ngroups;

@@ -128,6 +128,7 @@ static void groups_sort(struct group_info *group_info)
 		stride /= 3;
 	}
 }
+EXPORT_SYMBOL(groups_sort);
 
 /* a simple bsearch */
 int groups_search(const struct group_info *group_info, kgid_t grp)

@@ -159,7 +160,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;
 }

@@ -243,6 +243,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

index d58cc4d..651aaa5 100644
@@ -190,6 +190,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);

net/sunrpc/auth_gss/gss_rpc_xdr.c +1 -0

index eeeba5a..1486eaa 100644
@@ -231,6 +231,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
 			goto out_free_groups;
 		GROUP_AT(creds->cr_group_info, i) = kgid;
 	}
+	groups_sort(creds->cr_group_info);
 
 	return 0;
 out_free_groups:

net/sunrpc/auth_gss/svcauth_gss.c +1 -0

index 1095be9..52d92e9 100644
@@ -481,6 +481,7 @@ static int rsc_parse(struct cache_detail *cd,
 				goto out;
 			GROUP_AT(rsci.cred.cr_group_info, i) = kgid;
 		}
+		groups_sort(rsci.cred.cr_group_info);
 
 		/* mech name */
 		len = qword_get(&mesg, buf, mlen);

net/sunrpc/svcauth_unix.c +2 -0

index 621ca7b..98db171 100644
@@ -520,6 +520,7 @@ static int unix_gid_parse(struct cache_detail *cd,
 		GROUP_AT(ug.gi, i) = kgid;
 	}
 
+	groups_sort(ug.gi);
 	ugp = unix_gid_lookup(cd, uid);
 	if (ugp) {
 		struct cache_head *ch;

@@ -827,6 +828,7 @@ svcauth_unix_accept(struct svc_rqst *rqstp, __be32 *authp)
 		kgid_t kgid = make_kgid(&init_user_ns, svc_getnl(argv));
 		GROUP_AT(cred->cr_group_info, 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;

Signature · 1 line

2.9.5

Greg KH

author maintainer

2017-12-20 17:05 UTC #2 lore raw

Re: [PATCH] kernel: make groups_sort calling a responsibility group_info allocators

Quoted 21 lines · Thiago Rafael Becker

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/[email protected] open here
(backported from bdcf0a423ea1c40bbb40e7ee483b50fc8aa3d758)
Signed-off-by Thiago Rafael Becker <[email protected]>
Reviewed-by Matthew Wilcox <[email protected]>
Reviewed-by NeilBrown <[email protected]>
Acked-by "J. Bruce Fields" <[email protected]>
Cc Al Viro <[email protected]>
Cc Martin Schwidefsky <[email protected]>
Cc <[email protected]>
Signed-off-by Andrew Morton <[email protected]>
Signed-off-by Linus Torvalds <[email protected]>

Quoted 1 line

---

Any hint as to what kernel tree(s) this backport is for?

thanks,

greg k-h

NeilBrown

2018-01-03 22:21 UTC #3 lore raw

PATCH 1/1 kernel: make groups_sort calling a responsibility group_info allocators

In reply to [email protected]

From: Thiago Rafael Becker <[email protected]>

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/[email protected] open here

Signed-off-by Thiago Rafael Becker <[email protected]>

Reviewed-by Matthew Wilcox <[email protected]>

Reviewed-by NeilBrown <[email protected]>

Acked-by "J. Bruce Fields" <[email protected]>

Cc Al Viro <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

---
 arch/s390/kernel/compat_linux.c   | 1 +
 fs/nfsd/auth.c                    | 3 +++
 include/linux/cred.h              | 1 +
 kernel/groups.c                   | 5 +++--
 kernel/uid16.c                    | 1 +
 net/sunrpc/auth_gss/gss_rpc_xdr.c | 1 +
 net/sunrpc/auth_gss/svcauth_gss.c | 1 +
 net/sunrpc/svcauth_unix.c         | 2 ++
 8 files changed, 13 insertions(+), 2 deletions(-)

arch/s390/kernel/compat_linux.c +1 -0

index 437e61159279..0176ebc97bfd 100644
@@ -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

index 9d46a0bdd9f9..a260060042ad 100644
@@ -59,6 +59,9 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
 				GROUP_AT(gi, i) = exp->ex_anon_gid;
 			else
 				GROUP_AT(gi, i) = GROUP_AT(rqgi, i);
+
+			/* Each thread allocates its own gi, no race */
+			groups_sort(gi);
 		}
 	} else {
 		gi = get_group_info(rqgi);

include/linux/cred.h +1 -0

index 257db64562e5..9e120c92551b 100644
@@ -87,6 +87,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 *);
 
 /* access the groups "array" with this macro */
 #define GROUP_AT(gi, i) \

kernel/groups.c +3 -2

index 74d431d25251..5ea9847f172f 100644
@@ -101,7 +101,7 @@ static int groups_from_user(struct group_info *group_info,
 }
 
 /* a simple Shell sort */
-static void groups_sort(struct group_info *group_info)
+void groups_sort(struct group_info *group_info)
 {
 	int base, max, stride;
 	int gidsetsize = group_info->ngroups;

@@ -128,6 +128,7 @@ static void groups_sort(struct group_info *group_info)
 		stride /= 3;
 	}
 }
+EXPORT_SYMBOL(groups_sort);
 
 /* a simple bsearch */
 int groups_search(const struct group_info *group_info, kgid_t grp)

@@ -159,7 +160,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;
 }

@@ -243,6 +243,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

index d58cc4d8f0d1..651aaa5221ec 100644
@@ -190,6 +190,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);

net/sunrpc/auth_gss/gss_rpc_xdr.c +1 -0

index 2410d557ae39..89731c9023f0 100644
@@ -231,6 +231,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
 			goto out_free_groups;
 		GROUP_AT(creds->cr_group_info, i) = kgid;
 	}
+	groups_sort(creds->cr_group_info);
 
 	return 0;
 out_free_groups:

net/sunrpc/auth_gss/svcauth_gss.c +1 -0

index 033fec307528..036bbf2b44c1 100644
@@ -481,6 +481,7 @@ static int rsc_parse(struct cache_detail *cd,
 				goto out;
 			GROUP_AT(rsci.cred.cr_group_info, i) = kgid;
 		}
+		groups_sort(rsci.cred.cr_group_info);
 
 		/* mech name */
 		len = qword_get(&mesg, buf, mlen);

net/sunrpc/svcauth_unix.c +2 -0

index 621ca7b4a155..98db1715cb17 100644
@@ -520,6 +520,7 @@ static int unix_gid_parse(struct cache_detail *cd,
 		GROUP_AT(ug.gi, i) = kgid;
 	}
 
+	groups_sort(ug.gi);
 	ugp = unix_gid_lookup(cd, uid);
 	if (ugp) {
 		struct cache_head *ch;

@@ -827,6 +828,7 @@ svcauth_unix_accept(struct svc_rqst *rqstp, __be32 *authp)
 		kgid_t kgid = make_kgid(&init_user_ns, svc_getnl(argv));
 		GROUP_AT(cred->cr_group_info, 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;

Signature · 1 line

2.14.0.rc0.dirty

Greg KH

author maintainer

2018-01-05 22:10 UTC #4 lore raw

Re: [PATCH - 4.4-stable backport] kernel: make groups_sort calling a responsibility group_info allocators

Quoted 3 lines · NeilBrown

From: Thiago Rafael Becker <[email protected]>

Thanks for the two backports, now queued up.

greg k-h