Date: Fri, 3 Feb 2012 03:05:41 +0000 (UTC) From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r230928 - stable/9/sys/fs/nfs Message-ID: <201202030305.q1335fSN067243@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rmacklem Date: Fri Feb 3 03:05:41 2012 New Revision: 230928 URL: http://svn.freebsd.org/changeset/base/230928 Log: MFC: r230345 Martin Cracauer reported a problem to freebsd-current@ under the subject "Data corruption over NFS in -current". During investigation of this, I came across an ugly bogusity in the new NFS client where it replaced the cr_uid with the one used for the mount. This was done so that "system operations" like the NFSv4 Renew would be performed as the user that did the mount. However, if any other thread shares the credential with the one doing this operation, it could do an RPC (or just about anything else) as the wrong cr_uid. This patch fixes the above, by using the mount credentials instead of the one provided as an argument for this case. It appears to have fixed Martin's problem. This patch is needed for NFSv4 mounts and NFSv3 mounts against some non-FreeBSD servers that do not put post operation attributes in the NFSv3 Statfs RPC reply. Tested by: cracauer at cons.org, dim Modified: stable/9/sys/fs/nfs/nfs_commonkrpc.c Directory Properties: stable/9/sys/ (props changed) stable/9/sys/amd64/include/xen/ (props changed) stable/9/sys/boot/ (props changed) stable/9/sys/boot/i386/efi/ (props changed) stable/9/sys/boot/ia64/efi/ (props changed) stable/9/sys/boot/ia64/ski/ (props changed) stable/9/sys/boot/powerpc/boot1.chrp/ (props changed) stable/9/sys/boot/powerpc/ofw/ (props changed) stable/9/sys/cddl/contrib/opensolaris/ (props changed) stable/9/sys/conf/ (props changed) stable/9/sys/contrib/dev/acpica/ (props changed) stable/9/sys/contrib/octeon-sdk/ (props changed) stable/9/sys/contrib/pf/ (props changed) stable/9/sys/contrib/x86emu/ (props changed) Modified: stable/9/sys/fs/nfs/nfs_commonkrpc.c ============================================================================== --- stable/9/sys/fs/nfs/nfs_commonkrpc.c Fri Feb 3 02:15:59 2012 (r230927) +++ stable/9/sys/fs/nfs/nfs_commonkrpc.c Fri Feb 3 03:05:41 2012 (r230928) @@ -472,7 +472,7 @@ newnfs_request(struct nfsrv_descript *nd { u_int32_t *tl; time_t waituntil; - int i, j, set_uid = 0, set_sigset = 0, timeo; + int i, j, set_sigset = 0, timeo; int trycnt, error = 0, usegssname = 0, secflavour = AUTH_SYS; u_int16_t procnum; u_int trylater_delay = 1; @@ -483,8 +483,8 @@ newnfs_request(struct nfsrv_descript *nd enum clnt_stat stat; struct nfsreq *rep = NULL; char *srv_principal = NULL; - uid_t saved_uid = (uid_t)-1; sigset_t oldset; + struct ucred *authcred; if (xidp != NULL) *xidp = 0; @@ -494,6 +494,14 @@ newnfs_request(struct nfsrv_descript *nd return (ESTALE); } + /* + * Set authcred, which is used to acquire RPC credentials to + * the cred argument, by default. The crhold() should not be + * necessary, but will ensure that some future code change + * doesn't result in the credential being free'd prematurely. + */ + authcred = crhold(cred); + /* For client side interruptible mounts, mask off the signals. */ if (nmp != NULL && td != NULL && NFSHASINT(nmp)) { newnfs_set_sigmask(td, &oldset); @@ -532,13 +540,16 @@ newnfs_request(struct nfsrv_descript *nd /* * If there is a client side host based credential, * use that, otherwise use the system uid, if set. + * The system uid is in the nmp->nm_sockreq.nr_cred + * credentials. */ if (nmp->nm_krbnamelen > 0) { usegssname = 1; } else if (nmp->nm_uid != (uid_t)-1) { - saved_uid = cred->cr_uid; - cred->cr_uid = nmp->nm_uid; - set_uid = 1; + KASSERT(nmp->nm_sockreq.nr_cred != NULL, + ("newnfs_request: NULL nr_cred")); + crfree(authcred); + authcred = crhold(nmp->nm_sockreq.nr_cred); } } else if (nmp->nm_krbnamelen == 0 && nmp->nm_uid != (uid_t)-1 && cred->cr_uid == (uid_t)0) { @@ -547,10 +558,13 @@ newnfs_request(struct nfsrv_descript *nd * the system uid is set and this is root, use the * system uid, since root won't have user * credentials in a credentials cache file. + * The system uid is in the nmp->nm_sockreq.nr_cred + * credentials. */ - saved_uid = cred->cr_uid; - cred->cr_uid = nmp->nm_uid; - set_uid = 1; + KASSERT(nmp->nm_sockreq.nr_cred != NULL, + ("newnfs_request: NULL nr_cred")); + crfree(authcred); + authcred = crhold(nmp->nm_sockreq.nr_cred); } if (NFSHASINTEGRITY(nmp)) secflavour = RPCSEC_GSS_KRB5I; @@ -566,13 +580,13 @@ newnfs_request(struct nfsrv_descript *nd * Use the uid that did the mount when the RPC is doing * NFSv4 system operations, as indicated by the * ND_USEGSSNAME flag, for the AUTH_SYS case. + * The credentials in nm_sockreq.nr_cred were used for the + * mount. */ - saved_uid = cred->cr_uid; - if (nmp->nm_uid != (uid_t)-1) - cred->cr_uid = nmp->nm_uid; - else - cred->cr_uid = 0; - set_uid = 1; + KASSERT(nmp->nm_sockreq.nr_cred != NULL, + ("newnfs_request: NULL nr_cred")); + crfree(authcred); + authcred = crhold(nmp->nm_sockreq.nr_cred); } if (nmp != NULL) { @@ -588,12 +602,11 @@ newnfs_request(struct nfsrv_descript *nd auth = authnone_create(); else if (usegssname) auth = nfs_getauth(nrp, secflavour, nmp->nm_krbname, - srv_principal, NULL, cred); + srv_principal, NULL, authcred); else auth = nfs_getauth(nrp, secflavour, NULL, - srv_principal, NULL, cred); - if (set_uid) - cred->cr_uid = saved_uid; + srv_principal, NULL, authcred); + crfree(authcred); if (auth == NULL) { m_freem(nd->nd_mreq); if (set_sigset)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201202030305.q1335fSN067243>