Subject: kern/20791: nfsd should reply ENAMETOOLONG correctly
To: None <gnats-bugs@gnats.netbsd.org>
From: None <yamt@mwd.biglobe.ne.jp>
List: netbsd-bugs
Date: 03/19/2003 08:31:13
>Number:         20791
>Category:       kern
>Synopsis:       nfsd should reply ENAMETOOLONG correctly
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 18 15:32:00 PST 2003
>Closed-Date:
>Last-Modified:
>Originator:     YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
>Release:        NetBSD 1.6P
>Organization:

>Environment:
	
	
System: NetBSD kaeru 1.6P NetBSD 1.6P (build.kaeru) #: Wed Mar 19 00:08:18 JST 2003 takashi@kaeru:/usr/home/takashi/work/kernel/build.kaeru i386
Architecture: i386
Machine: i386
>Description:
	our nfsd discard requests with too long names as BADRPC.
	however, for NFSv3, it should reply ENAMETOOLONG because
	there's no name length limit like 1024 in the protocol spec.
>How-To-Repeat:
	use clients like linux, which don't respect PATHCONF.
>Fix:
	apply the following patch. (compile tested only)


Index: nfs_subs.c
===================================================================
RCS file: /cvs/NetBSD/src/sys/nfs/nfs_subs.c,v
retrieving revision 1.109
diff -u -p -r1.109 nfs_subs.c
--- nfs_subs.c	2003/02/26 06:31:19	1.109
+++ nfs_subs.c	2003/03/18 23:21:48
@@ -1880,7 +1880,7 @@ int
 nfs_namei(ndp, fhp, len, slp, nam, mdp, dposp, retdirp, p, kerbflag, pubflag)
 	struct nameidata *ndp;
 	fhandle_t *fhp;
-	int len;
+	uint32_t len;
 	struct nfssvc_sock *slp;
 	struct mbuf *nam;
 	struct mbuf **mdp;
Index: nfs_serv.c
===================================================================
RCS file: /cvs/NetBSD/src/sys/nfs/nfs_serv.c,v
retrieving revision 1.67
diff -u -p -r1.67 nfs_serv.c
--- nfs_serv.c	2003/02/26 06:31:18	1.67
+++ nfs_serv.c	2003/03/18 23:21:48
@@ -364,7 +364,8 @@ nfsrv_lookup(nfsd, slp, procp, mrq)
 	u_int32_t *tl;
 	int32_t t1;
 	caddr_t bpos;
-	int error = 0, cache, len, dirattr_ret = 1;
+	int error = 0, cache, dirattr_ret = 1;
+	uint32_t len;
 	int v3 = (nfsd->nd_flag & ND_NFSV3), pubflag;
 	char *cp2;
 	struct mbuf *mb, *mreq;
@@ -587,7 +588,8 @@ nfsrv_read(nfsd, slp, procp, mrq)
 	int i;
 	caddr_t bpos;
 	int error = 0, rdonly, cache, cnt, len, left, siz, tlen, getret;
-	int v3 = (nfsd->nd_flag & ND_NFSV3), reqlen;
+	int v3 = (nfsd->nd_flag & ND_NFSV3);
+	uint32_t reqlen;
 	char *cp2;
 	struct mbuf *mb, *mreq;
 	struct mbuf *m2;
@@ -1732,7 +1734,8 @@ nfsrv_rename(nfsd, slp, procp, mrq)
 	u_int32_t *tl;
 	int32_t t1;
 	caddr_t bpos;
-	int error = 0, cache, len, len2, fdirfor_ret = 1, fdiraft_ret = 1;
+	int error = 0, cache, fdirfor_ret = 1, fdiraft_ret = 1;
+	uint32_t len, len2;
 	int tdirfor_ret = 1, tdiraft_ret = 1;
 	int v3 = (nfsd->nd_flag & ND_NFSV3);
 	char *cp2;
@@ -1784,7 +1787,15 @@ nfsrv_rename(nfsd, slp, procp, mrq)
 	}
 	fvp = fromnd.ni_vp;
 	nfsm_srvmtofh(tfhp);
-	nfsm_strsiz(len2, NFS_MAXNAMLEN);
+	if (v3) {
+		nfsm_dissect(tl, uint32_t *, NFSX_UNSIGNED);
+		len2 = fxdr_unsigned(uint32_t, *tl);
+		/* len2 will be checked by nfs_namei */
+	}
+	else {
+		/* NFSv2 */
+		nfsm_strsiz(len2, NFS_MAXNAMLEN);
+	}
 	cred->cr_uid = saved_uid;
 	tond.ni_cnd.cn_cred = cred;
 	tond.ni_cnd.cn_nameiop = RENAME;
@@ -2042,7 +2053,8 @@ nfsrv_symlink(nfsd, slp, procp, mrq)
 	char *bpos, *pathcp = NULL, *cp2;
 	struct uio io;
 	struct iovec iv;
-	int error = 0, cache, len, len2, dirfor_ret = 1, diraft_ret = 1;
+	int error = 0, cache, dirfor_ret = 1, diraft_ret = 1;
+	uint32_t len, len2;
 	int v3 = (nfsd->nd_flag & ND_NFSV3);
 	struct mbuf *mb, *mreq;
 	struct vnode *dirp = (struct vnode *)0;
@@ -2071,9 +2083,20 @@ nfsrv_symlink(nfsd, slp, procp, mrq)
 	if (error)
 		goto out;
 	VATTR_NULL(&va);
-	if (v3)
+	if (v3) {
 		nfsm_srvsattr(&va);
-	nfsm_strsiz(len2, NFS_MAXPATHLEN);
+		nfsm_dissect(tl, uint32_t *, NFSX_UNSIGNED);
+		len2 = fxdr_unsigned(uint32_t, *tl);
+		if (len2 > PATH_MAX) {
+			/* XXX should check _PC_NO_TRUNC */
+			error = ENAMETOOLONG;
+			goto abortop;
+		}
+	}
+	else {
+		/* NFSv2 */
+		nfsm_strsiz(len2, NFS_MAXPATHLEN);
+	}
 	pathcp = malloc(len2 + 1, M_TEMP, M_WAITOK);
 	iv.iov_base = pathcp;
 	iv.iov_len = len2;
@@ -2091,13 +2114,15 @@ nfsrv_symlink(nfsd, slp, procp, mrq)
 	}
 	*(pathcp + len2) = '\0';
 	if (nd.ni_vp) {
+		error = EEXIST;
+abortop:
 		VOP_ABORTOP(nd.ni_dvp, &nd.ni_cnd);
 		if (nd.ni_dvp == nd.ni_vp)
 			vrele(nd.ni_dvp);
 		else
 			vput(nd.ni_dvp);
-		vrele(nd.ni_vp);
-		error = EEXIST;
+		if (nd.ni_vp)
+			vrele(nd.ni_vp);
 		goto out;
 	}
 	nqsrv_getl(nd.ni_dvp, ND_WRITE);
Index: nfsm_subs.h
===================================================================
RCS file: /cvs/NetBSD/src/sys/nfs/nfsm_subs.h,v
retrieving revision 1.24
diff -u -p -r1.24 nfsm_subs.h
--- nfsm_subs.h	2003/02/26 07:33:57	1.24
+++ nfsm_subs.h	2003/03/18 23:21:48
@@ -296,26 +296,24 @@
 				
 
 #define	nfsm_strsiz(s,m) \
-		{ nfsm_dissect(tl,u_int32_t *,NFSX_UNSIGNED); \
-		if (((s) = fxdr_unsigned(int32_t,*tl)) > (m)) { \
+		{ nfsm_dissect(tl,uint32_t *,NFSX_UNSIGNED); \
+		if (((s) = fxdr_unsigned(uint32_t,*tl)) > (m)) { \
 			m_freem(mrep); \
 			error = EBADRPC; \
 			goto nfsmout; \
 		} }
 
 #define	nfsm_srvstrsiz(s,m) \
-		{ nfsm_dissect(tl,u_int32_t *,NFSX_UNSIGNED); \
-		if (((s) = fxdr_unsigned(int32_t,*tl)) > (m) || (s) <= 0) { \
+		{ nfsm_dissect(tl,uint32_t *,NFSX_UNSIGNED); \
+		if (((s) = fxdr_unsigned(uint32_t,*tl)) > (m) || (s) <= 0) { \
 			error = EBADRPC; \
 			nfsm_reply(0); \
 		} }
 
 #define	nfsm_srvnamesiz(s) \
-		{ nfsm_dissect(tl,u_int32_t *,NFSX_UNSIGNED); \
-		if (((s) = fxdr_unsigned(int32_t,*tl)) > NFS_MAXNAMLEN) \
+		{ nfsm_dissect(tl,uint32_t *,NFSX_UNSIGNED); \
+		if (((s) = fxdr_unsigned(uint32_t,*tl)) > NFS_MAXNAMLEN) \
 			error = NFSERR_NAMETOL; \
-		if ((s) <= 0) \
-			error = EBADRPC; \
 		if (error) \
 			nfsm_reply(0); \
 		}
>Release-Note:
>Audit-Trail:
>Unformatted: