From 641697c62bf0c78cb75ca9e3d36083afc88419b9 Mon Sep 17 00:00:00 2001
From: iedowse <iedowse@FreeBSD.org>
Date: Sat, 12 Jan 2002 03:57:25 +0000
Subject: Fix a few NFSv2 issues that slipped in during the big cleanup. The
 semantics of the nfsm_reply() macro were changed so that the caller has to
 explicitly handle the V2 error case, whereas before, nfsm_reply() did a `goto
 nfsmout' then. A few server ops (setattr, readlink, create, mkdir) weren't
 updated to match, so errors in the V2 case could cause protocol hangs and
 leaked mbufs.

Correct some comments that describe the old nfsm_reply behaviour.

[older, harmless nit] Remove the unnecessary `nfsmreply0' label in
nfsrv_create(), since for its users, the main `ereply' label does
the same thing.
---
 sys/nfsserver/nfs_serv.c | 61 ++++++++++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 33 deletions(-)

(limited to 'sys/nfsserver')

diff --git a/sys/nfsserver/nfs_serv.c b/sys/nfsserver/nfs_serv.c
index 770bc09..46c00d5 100644
--- a/sys/nfsserver/nfs_serv.c
+++ b/sys/nfsserver/nfs_serv.c
@@ -55,8 +55,9 @@ __FBSDID("$FreeBSD$");
  *	returning an error from the server function implies a fatal error
  *	such as a badly constructed rpc request that should be dropped without
  *	a reply.
- *	For Version 3, nfsm_reply() does not return for the error case, since
- *	most version 3 rpcs return more than the status for error cases.
+ *	For nfsm_reply(), the case where error == EBADRPC is treated
+ *	specially; after constructing a reply, it does an immediate
+ *	`goto nfsmout' to avoid getting any V3 post-op status appended.
  *
  * Other notes:
  *	Warning: always pay careful attention to resource cleanup on return
@@ -416,13 +417,12 @@ out:
 	nfsm_reply(NFSX_WCCORFATTR(v3));
 	if (v3) {
 		nfsm_srvwcc_data(preat_ret, &preat, postat_ret, vap);
-		error = 0;
-		goto nfsmout;
-	} else {
-		/* v2 non-error case (see nfsm_reply). */
+	} else if (!error) {
+		/* v2 non-error case. */
 		fp = nfsm_build(struct nfs_fattr *, NFSX_V2FATTR);
 		nfsm_srvfillattr(vap, fp);
 	}
+	error = 0;
 	/* fall through */
 
 nfsmout:
@@ -703,12 +703,11 @@ out:
 	vput(vp);
 	vp = NULL;
 	nfsm_reply(NFSX_POSTOPATTR(v3) + NFSX_UNSIGNED);
-	if (v3) {
+	if (v3)
 		nfsm_srvpostop_attr(getret, &attr);
-		if (error) {
-			error = 0;
-			goto nfsmout;
-		}
+	if (error) {
+		error = 0;
+		goto nfsmout;
 	}
 	if (uiop->uio_resid > 0) {
 		len -= uiop->uio_resid;
@@ -1175,11 +1174,12 @@ ereply:
 			nfsver = boottime;
 		*tl++ = txdr_unsigned(nfsver.tv_sec);
 		*tl = txdr_unsigned(nfsver.tv_usec);
-	} else {
-		/* v2, non-error case (see nfsm_reply). */
+	} else if (!error) {
+		/* v2 non-error case. */
 		fp = nfsm_build(struct nfs_fattr *, NFSX_V2FATTR);
 		nfsm_srvfillattr(vap, fp);
 	}
+	error = 0;
 nfsmout:
 	if (vp)
 		vput(vp);
@@ -1730,12 +1730,12 @@ nfsrv_create(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
 						td);
 				}
 			}
-		} else if (
-			vap->va_type == VCHR ||
-			vap->va_type == VBLK ||
-			vap->va_type == VFIFO
-		) {
+		} else if (vap->va_type == VCHR || vap->va_type == VBLK ||
+		    vap->va_type == VFIFO) {
 			/*
+			 * NFSv2-specific code for creating device nodes
+			 * and fifos.
+			 *
 			 * Handle SysV FIFO node special cases.  All other
 			 * devices require super user to access.
 			 */
@@ -1743,13 +1743,13 @@ nfsrv_create(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
 				vap->va_type = VFIFO;
                         if (vap->va_type != VFIFO &&
                             (error = suser_xxx(cred, 0, 0))) {
-				goto nfsmreply0;
+				goto ereply;
                         }
 			vap->va_rdev = rdev;
 			error = VOP_MKNOD(nd.ni_dvp, &nd.ni_vp, &nd.ni_cnd, vap);
 			if (error) {
 				NDFREE(&nd, NDF_ONLY_PNBUF);
-				goto nfsmreply0;
+				goto ereply;
 			}
 			vput(nd.ni_vp);
 			nd.ni_vp = NULL;
@@ -1773,15 +1773,13 @@ nfsrv_create(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
 
 			error = lookup(&nd);
 			nd.ni_dvp = NULL;
+			if (error)
+				goto ereply;
 
-			if (error != 0) {
-				nfsm_reply(0);
-				/* fall through on certain errors */
-			}
 			nfsrv_object_create(nd.ni_vp);
 			if (nd.ni_cnd.cn_flags & ISSYMLINK) {
 				error = EINVAL;
-				goto nfsmreply0;
+				goto ereply;
 			}
 		} else {
 			error = ENXIO;
@@ -1823,17 +1821,14 @@ ereply:
 			nfsm_srvpostop_attr(0, vap);
 		}
 		nfsm_srvwcc_data(dirfor_ret, &dirfor, diraft_ret, &diraft);
-		error = 0;
-	} else {
-		/* v2 non-error case (see nfsm_reply). */
+	} else if (!error) {
+		/* v2 non-error case. */
 		nfsm_srvfhtom(fhp, v3);
 		fp = nfsm_build(struct nfs_fattr *, NFSX_V2FATTR);
 		nfsm_srvfillattr(vap, fp);
 	}
-	goto nfsmout;
+	error = 0;
 
-nfsmreply0:
-	nfsm_reply(0);
 nfsmout:
 	if (nd.ni_startdir) {
 		vrele(nd.ni_startdir);
@@ -2784,8 +2779,8 @@ out:
 			nfsm_srvpostop_attr(0, vap);
 		}
 		nfsm_srvwcc_data(dirfor_ret, &dirfor, diraft_ret, &diraft);
-	} else {
-		/* v2, non-error case (see nfsm_reply) */
+	} else if (!error) {
+		/* v2 non-error case. */
 		nfsm_srvfhtom(fhp, v3);
 		fp = nfsm_build(struct nfs_fattr *, NFSX_V2FATTR);
 		nfsm_srvfillattr(vap, fp);
-- 
cgit v1.1