From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:36745 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751029AbeACW3T (ORCPT ); Wed, 3 Jan 2018 17:29:19 -0500 From: NeilBrown To: gregkh@linuxfoundation.org, thiago.becker@gmail.com, akpm@linux-foundation.org, bfields@fieldses.org, mawilcox@microsoft.com, schwidefsky@de.ibm.com, stable@vger.kernel.org, torvalds@linux-foundation.org, viro@zeniv.linux.org.uk Date: Thu, 04 Jan 2018 09:29:09 +1100 Cc: stable@vger.kernel.org Subject: [PATCH 4.9-stable backport] kernel: make groups_sort calling a responsibility group_info allocators In-Reply-To: <1513596995115252@kroah.com> References: <1513596995115252@kroah.com> Message-ID: <87fu7mefuy.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: stable-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable From: 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/20171211151420.18655-1-thiago.becker@gmail.c= om Signed-off-by: Thiago Rafael Becker Reviewed-by: Matthew Wilcox Reviewed-by: NeilBrown Acked-by: "J. Bruce Fields" Cc: Al Viro Cc: Martin Schwidefsky Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds =2D-- 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(-) diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linu= x.c index 0f9cd90c11af..f06a9a0063f1 100644 =2D-- a/arch/s390/kernel/compat_linux.c +++ b/arch/s390/kernel/compat_linux.c @@ -263,6 +263,7 @@ COMPAT_SYSCALL_DEFINE2(s390_setgroups16, int, gidsetsiz= e, u16 __user *, grouplis return retval; } =20 + groups_sort(group_info); retval =3D set_current_groups(group_info); put_group_info(group_info); =20 diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c index 62469c60be23..75f942ae5176 100644 =2D-- a/fs/nfsd/auth.c +++ b/fs/nfsd/auth.c @@ -59,6 +59,9 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_expor= t *exp) gi->gid[i] =3D exp->ex_anon_gid; else gi->gid[i] =3D rqgi->gid[i]; + + /* Each thread allocates its own gi, no race */ + groups_sort(gi); } } else { gi =3D get_group_info(rqgi); diff --git a/include/linux/cred.h b/include/linux/cred.h index f0e70a1bb3ac..cf1a5d0c4eb4 100644 =2D-- a/include/linux/cred.h +++ b/include/linux/cred.h @@ -82,6 +82,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 *); =20 /* * The security context of a task diff --git a/kernel/groups.c b/kernel/groups.c index 2fcadd66a8fd..94bde5210e3d 100644 =2D-- a/kernel/groups.c +++ b/kernel/groups.c @@ -77,7 +77,7 @@ static int groups_from_user(struct group_info *group_info, } =20 /* a simple Shell sort */ =2Dstatic void groups_sort(struct group_info *group_info) +void groups_sort(struct group_info *group_info) { int base, max, stride; int gidsetsize =3D group_info->ngroups; @@ -103,6 +103,7 @@ static void groups_sort(struct group_info *group_info) stride /=3D 3; } } +EXPORT_SYMBOL(groups_sort); =20 /* a simple bsearch */ int groups_search(const struct group_info *group_info, kgid_t grp) @@ -134,7 +135,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); =2D groups_sort(group_info); get_group_info(group_info); new->group_info =3D group_info; } @@ -218,6 +218,7 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __use= r *, grouplist) return retval; } =20 + groups_sort(group_info); retval =3D set_current_groups(group_info); put_group_info(group_info); =20 diff --git a/kernel/uid16.c b/kernel/uid16.c index cc40793464e3..dcffcce9d75e 100644 =2D-- a/kernel/uid16.c +++ b/kernel/uid16.c @@ -190,6 +190,7 @@ SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t= __user *, grouplist) return retval; } =20 + groups_sort(group_info); retval =3D set_current_groups(group_info); put_group_info(group_info); =20 diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rp= c_xdr.c index 25d9a9cf7b66..624c322af3ab 100644 =2D-- a/net/sunrpc/auth_gss/gss_rpc_xdr.c +++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c @@ -231,6 +231,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr, goto out_free_groups; creds->cr_group_info->gid[i] =3D kgid; } + groups_sort(creds->cr_group_info); =20 return 0; out_free_groups: diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcaut= h_gss.c index 153082598522..6a08bc451247 100644 =2D-- a/net/sunrpc/auth_gss/svcauth_gss.c +++ b/net/sunrpc/auth_gss/svcauth_gss.c @@ -481,6 +481,7 @@ static int rsc_parse(struct cache_detail *cd, goto out; rsci.cred.cr_group_info->gid[i] =3D kgid; } + groups_sort(rsci.cred.cr_group_info); =20 /* mech name */ len =3D qword_get(&mesg, buf, mlen); diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c index 64af4f034de6..738a243c68a2 100644 =2D-- a/net/sunrpc/svcauth_unix.c +++ b/net/sunrpc/svcauth_unix.c @@ -520,6 +520,7 @@ static int unix_gid_parse(struct cache_detail *cd, ug.gi->gid[i] =3D kgid; } =20 + groups_sort(ug.gi); ugp =3D unix_gid_lookup(cd, uid); if (ugp) { struct cache_head *ch; @@ -819,6 +820,7 @@ svcauth_unix_accept(struct svc_rqst *rqstp, __be32 *aut= hp) kgid_t kgid =3D make_kgid(&init_user_ns, svc_getnl(argv)); cred->cr_group_info->gid[i] =3D kgid; } + groups_sort(cred->cr_group_info); if (svc_getu32(argv) !=3D htonl(RPC_AUTH_NULL) || svc_getu32(argv) !=3D 0= ) { *authp =3D rpc_autherr_badverf; return SVC_DENIED; =2D-=20 2.14.0.rc0.dirty --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAlpNWTUACgkQOeye3VZi gbkOng//Z9qTAnXTE4HMQLYbpMBpdQqBRDsbc8X+/H+DQQksueiULdCn8yEWzGE+ ul+QJzvDbd7CvnK7gASDXK9TC0ygeBvls+GIZ8wfYchQhoiFJhI80sM4Sq1Andti BqmF3toP2qKgXk1LX6se9vUwOi424tfrZWE+95sXXY9MFUo7y6Pl4gqUciEaZp9Q qGhAnh7qFD/hPycl1Nd+vqXqdAcRG7OrEgW2QtQuuZJidDNfGop4tPiXorPfGeB5 DaKFAZautLTuogsN2ODd/t3bue5JKm4lkZmNRdDzDq+CzKInb/sBt3+lj1gfwSyw DAaFGRGvWZSXC/tZcyTusbXaeMegaeWrAFGyFZK58I/dWj9n85SsiNftvJw/1nuk q9n5kQW4HFmYsCSlCkcn6koWhwWMIMCxoKmggENYqkG/JtB6OA6wCohR0CwkgKyi eQ5wpGEmWVkgnrz+m0HaSK2rnS+xG21G3BF0n0s2AIdOHEDv6znIGMdqeCnzK8vp ym51PVyeLfhFyvSj5l+n/L4axJ82zMZXWit1AdIKNieKHMuXKJ8+BBktDXIVp/SE rClIaEnrCYocVFvc9xSmdH8Y8AeGOCNrxC4f6e9ij+BjlZW+kDBNbhmrcbLREPTI 45AKVXWrdfh5kMXNnaIDRSfcSCOgL7H4kGme4utMtUfxZ4/EJgM= =bmi1 -----END PGP SIGNATURE----- --=-=-=--