Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 3 Dec 1999 23:26:29 +0000 (GMT)
From:      iedowse@maths.tcd.ie
To:        FreeBSD-gnats-submit@freebsd.org
Subject:   kern/15249: nfs_serv.c can vput() junk + more
Message-ID:  <199912032326.aa35944@bell.maths.tcd.ie>

next in thread | raw e-mail | index | archive | help

>Number:         15249
>Category:       kern
>Synopsis:       nfs_serv.c can vput() junk + more
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Dec  3 15:30:01 PST 1999
>Closed-Date:
>Last-Modified:
>Originator:     Ian Dowse
>Release:        FreeBSD 3.3-STABLE i386
>Organization:
		School of Mathematics
		Trinity College Dublin
>Environment:
	All versions of FreeBSD

>Description:

	There are a number of places in nfs_serv.c where certain badly-formed
	NFS requests can result in vput() being called on an uninitialised
	vnode pointer. In nfsrv_getattr(), nfsrv_lookup(), nfsrv_read(),
	and nfsrv_writegather(), the exit code performs a vput(vp) if vp
	is non-NULL. However since vp is not initialised before the macro
	nfsm_srvmtofh() is invoked, it is possible for this exit code to
	get called with a junk vp if nfsm_srvmtofh() does a 'goto nfsmout'.

	Another problem that affects even more functions is that on certain
	(mainly kerboros related) errors nfsrv_fhtovp() does not leave NULL
	in its *vpp argument.

	Finally there is a problem in nqnfsrv_getlease() where it is
	possible to get it to vput(NULL). 
	
>How-To-Repeat:

	To repeat the first problem, send a truncated request to an NFS
	server where the request ends in the middle of the filehandle.

	I haven't cwtried to trigger the others, but it should be fairly
	easy.

>Fix:
	Apply the following patches in src/sys/nfs
	

--- nfs_serv.c.orig	Fri Dec  3 22:03:37 1999
+++ nfs_serv.c	Fri Dec  3 22:22:35 1999
@@ -249,7 +249,7 @@
 	register struct nfs_fattr *fp;
 	struct vattr va;
 	register struct vattr *vap = &va;
-	struct vnode *vp;
+	struct vnode *vp = NULL;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
 	register u_int32_t *tl;
@@ -453,7 +453,7 @@
 	struct ucred *cred = &nfsd->nd_cr;
 	register struct nfs_fattr *fp;
 	struct nameidata nd, ind, *ndp = &nd;
-	struct vnode *vp, *dirp;
+	struct vnode *vp, *dirp = NULL;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
 	register caddr_t cp;
@@ -775,7 +775,7 @@
 	char *cp2;
 	struct mbuf *mb, *mb2, *mreq;
 	struct mbuf *m2;
-	struct vnode *vp;
+	struct vnode *vp = NULL;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
 	struct uio io, *uiop = &io;
@@ -1168,7 +1168,7 @@
 	int ioflags, aftat_ret = 1, s, adjust, v3, zeroing;
 	char *cp2;
 	struct mbuf *mb, *mb2, *mreq, *mrep, *md;
-	struct vnode *vp;
+	struct vnode *vp = NULL;
 	struct uio io, *uiop = &io;
 	u_quad_t frev, cur_usec;
 
--- nfs_subs.c.orig	Fri Dec  3 22:25:21 1999
+++ nfs_subs.c	Fri Dec  3 22:26:57 1999
@@ -1943,6 +1943,7 @@
 	register int i;
 	struct ucred *credanon;
 	int error, exflags;
+	struct vnode *vp;
 #ifdef MNT_EXNORESPORT		/* XXX needs mountd and /etc/exports help yet */
 	struct sockaddr_int *saddr;
 #endif
@@ -1961,7 +1962,7 @@
 	error = VFS_CHECKEXP(mp, nam, &exflags, &credanon);
 	if (error)
 		return (error); 
-	error = VFS_FHTOVP(mp, &fhp->fh_fid, vpp);
+	error = VFS_FHTOVP(mp, &fhp->fh_fid, &vp);
 	if (error)
 		return (error);
 #ifdef MNT_EXNORESPORT
@@ -1969,7 +1970,7 @@
 		saddr = (struct sockaddr_in *)nam;
 		if (saddr->sin_family == AF_INET &&
 		    ntohs(saddr->sin_port) >= IPPORT_RESERVED) {
-			vput(*vpp);
+			vput(vp);
 			return (NFSERR_AUTHERR | AUTH_TOOWEAK);
 		}
 	}
@@ -1979,11 +1980,11 @@
 	 */
 	if (exflags & MNT_EXKERB) {
 		if (!kerbflag) {
-			vput(*vpp);
+			vput(vp);
 			return (NFSERR_AUTHERR | AUTH_TOOWEAK);
 		}
 	} else if (kerbflag) {
-		vput(*vpp);
+		vput(vp);
 		return (NFSERR_AUTHERR | AUTH_TOOWEAK);
 	} else if (cred->cr_uid == 0 || (exflags & MNT_EXPORTANON)) {
 		cred->cr_uid = credanon->cr_uid;
@@ -1996,10 +1997,11 @@
 	else
 		*rdonlyp = 0;
 
-	nfsrv_object_create(*vpp);
+	nfsrv_object_create(vp);
 
 	if (!lockflag)
-		VOP_UNLOCK(*vpp, 0, p);
+		VOP_UNLOCK(vp, 0, p);
+	*vpp = vp;
 	return (0);
 }
 
--- nfs_nqlease.c.orig	Fri Dec  3 22:57:42 1999
+++ nfs_nqlease.c	Fri Dec  3 23:03:56 1999
@@ -769,8 +769,10 @@
 	nfsd->nd_duration = fxdr_unsigned(int, *tl);
 	error = nfsrv_fhtovp(fhp, 1, &vp, cred, slp, nam, &rdonly,
 		(nfsd->nd_flag & ND_KERBAUTH), TRUE);
-	if (error)
+	if (error) {
 		nfsm_reply(0);
+		goto nfsmout;
+	}
 	if (rdonly && flags == ND_WRITE) {
 		error = EROFS;
 		vput(vp);

>Release-Note:
>Audit-Trail:
>Unformatted:


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199912032326.aa35944>