Skip site navigation (1)Skip section navigation (2)
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>