Until now, smbfs_fullpath() computed the full path starting from the

vnode and following back the chain of n_parent pointers up to the root,
without acquiring the locks of the n_parent vnodes analyzed during the
computation. This is immediately wrong because if the vnode lock is not
held there's no guarantee on the validity of the vnode pointer or the data.
In order to fix, store the whole path in the smbnode structure so that
smbfs_fullpath() can use this information.

Discussed with:		kib
Reported and tested by:		pho
Sponsored by:		iXsystems inc.

5 files changed, 63 insertions, 60 deletions

diff --git a/sys/fs/smbfs/smbfs_io.c b/sys/fs/smbfs/smbfs_io.c
index 686c64d..1018fae 100644
--- a/sys/fs/smbfs/smbfs_io.c
+++ b/sys/fs/smbfs/smbfs_io.c
@@ -97,7 +97,7 @@ smbfs_readvdir(struct vnode *vp, struct uio *uio, struct ucred *cred)
 		bzero((caddr_t)&de, DE_SIZE);
 		de.d_reclen = DE_SIZE;
 		de.d_fileno = (offset == 0) ? np->n_ino :
-		    (np->n_parent ? VTOSMB(np->n_parent)->n_ino : 2);
+		    (np->n_parent ? np->n_parentino : 2);
 		if (de.d_fileno == 0)
 			de.d_fileno = 0x7ffffffd + offset;
 		de.d_namlen = offset + 1;
diff --git a/sys/fs/smbfs/smbfs_node.c b/sys/fs/smbfs/smbfs_node.c
index 66696ca..8fb9c80 100644
--- a/sys/fs/smbfs/smbfs_node.c
+++ b/sys/fs/smbfs/smbfs_node.c
@@ -98,8 +98,9 @@ smbfs_vnode_cmp(struct vnode *vp, void *_sc)
 }
 
 static int
-smbfs_node_alloc(struct mount *mp, struct vnode *dvp,
-	const char *name, int nmlen, struct smbfattr *fap, struct vnode **vpp)
+smbfs_node_alloc(struct mount *mp, struct vnode *dvp, const char *dirnm, 
+	int dirlen, const char *name, int nmlen, char sep, 
+	struct smbfattr *fap, struct vnode **vpp)
 {
 	struct vattr vattr;
 	struct thread *td = curthread;	/* XXX */
@@ -107,7 +108,8 @@ smbfs_node_alloc(struct mount *mp, struct vnode *dvp,
 	struct smbnode *np, *dnp;
 	struct vnode *vp, *vp2;
 	struct smbcmp sc;
-	int error;
+	char *p, *rpath;
+	int error, rplen;
 
 	sc.n_parent = dvp;
 	sc.n_nmlen = nmlen;
@@ -173,18 +175,36 @@ smbfs_node_alloc(struct mount *mp, struct vnode *dvp,
 		return (error);
 	vp = *vpp;
 	np = malloc(sizeof *np, M_SMBNODE, M_WAITOK | M_ZERO);
+	rplen = dirlen;
+	if (sep != '\0')
+		rplen++;
+	rplen += nmlen;
+	rpath = malloc(rplen + 1, M_SMBNODENAME, M_WAITOK);
+	p = rpath;
+	bcopy(dirnm, p, dirlen);
+	p += dirlen;
+	if (sep != '\0')
+		*p++ = sep;
+	if (name != NULL) {
+		bcopy(name, p, nmlen);
+		p += nmlen;
+	}
+	MPASS(p == rpath + rplen);
 	lockmgr(vp->v_vnlock, LK_EXCLUSIVE, NULL);
 	/* Vnode initialization */
 	vp->v_type = fap->fa_attr & SMB_FA_DIR ? VDIR : VREG;
 	vp->v_data = np;
 	np->n_vnode = vp;
 	np->n_mount = VFSTOSMBFS(mp);
+	np->n_rpath = rpath;
+	np->n_rplen = rplen;
 	np->n_nmlen = nmlen;
 	np->n_name = smbfs_name_alloc(name, nmlen);
 	np->n_ino = fap->fa_ino;
 	if (dvp) {
 		ASSERT_VOP_LOCKED(dvp, "smbfs_node_alloc");
 		np->n_parent = dvp;
+		np->n_parentino = VTOSMB(dvp)->n_ino;
 		if (/*vp->v_type == VDIR &&*/ (dvp->v_vflag & VV_ROOT) == 0) {
 			vref(dvp);
 			np->n_flag |= NREFPARENT;
@@ -209,14 +229,23 @@ int
 smbfs_nget(struct mount *mp, struct vnode *dvp, const char *name, int nmlen,
 	struct smbfattr *fap, struct vnode **vpp)
 {
-	struct smbnode *np;
+	struct smbnode *dnp, *np;
 	struct vnode *vp;
-	int error;
+	int error, sep;
 
 	*vpp = NULL;
-	error = smbfs_node_alloc(mp, dvp, name, nmlen, fap, &vp);
+	dnp = (dvp) ? VTOSMB(dvp) : NULL;
+	sep = 0;
+	if (dnp != NULL) {
+		sep = SMBFS_DNP_SEP(dnp); 
+		error = smbfs_node_alloc(mp, dvp, dnp->n_rpath, dnp->n_rplen, 
+		    name, nmlen, sep, fap, &vp); 
+	} else
+		error = smbfs_node_alloc(mp, NULL, "\\", 1, name, nmlen, 
+		    sep, fap, &vp); 
 	if (error)
 		return error;
+	MPASS(vp != NULL);
 	np = VTOSMB(vp);
 	if (fap)
 		smbfs_attr_cacheenter(vp, fap);
@@ -256,6 +285,8 @@ smbfs_reclaim(ap)
 	vfs_hash_remove(vp);
 	if (np->n_name)
 		smbfs_name_free(np->n_name);
+	if (np->n_rpath)
+		free(np->n_rpath, M_SMBNODENAME);
 	free(np, M_SMBNODE);
 	vp->v_data = NULL;
 	if (dvp != NULL) {
diff --git a/sys/fs/smbfs/smbfs_node.h b/sys/fs/smbfs/smbfs_node.h
index 3e703da..9dbc973 100644
--- a/sys/fs/smbfs/smbfs_node.h
+++ b/sys/fs/smbfs/smbfs_node.h
@@ -53,9 +53,12 @@ struct smbnode {
 	struct timespec		n_atime;	/* last access time */
 	u_quad_t		n_size;
 	long			n_ino;
+	long			n_parentino;	/* parent inode number */
 	int			n_dosattr;
 	u_int16_t		n_fid;		/* file handle */
 	int			n_rwstate;	/* granted access mode */
+	int			n_rplen;
+	char *			n_rpath;
 	u_char			n_nmlen;
 	u_char *		n_name;
 	struct smbfs_fctx *	n_dirseq;	/* ff context */
@@ -72,6 +75,8 @@ struct smbcmp {
 #define VTOSMB(vp)	((struct smbnode *)(vp)->v_data)
 #define SMBTOV(np)	((struct vnode *)(np)->n_vnode)
 
+#define	SMBFS_DNP_SEP(dnp)	((dnp->n_rplen > 1) ? '\\' : '\0')
+
 struct vop_getpages_args;
 struct vop_inactive_args;
 struct vop_putpages_args;
diff --git a/sys/fs/smbfs/smbfs_subr.c b/sys/fs/smbfs/smbfs_subr.c
index b95f92b..aa03101 100644
--- a/sys/fs/smbfs/smbfs_subr.c
+++ b/sys/fs/smbfs/smbfs_subr.c
@@ -107,44 +107,6 @@ smb_dos2unixtime(u_int dd, u_int dt, u_int dh, int tzoff,
 	smb_time_server2local(tsp->tv_sec, tzoff, tsp);
 }
 
-static int
-smb_fphelp(struct mbchain *mbp, struct smb_vc *vcp, struct smbnode *np,
-	int caseopt)
-{
-	struct smbmount *smp= np->n_mount;
-	struct smbnode **npp = smp->sm_npstack;
-	int i, error = 0;
-
-/*	simple_lock(&smp->sm_npslock);*/
-	i = 0;
-	while (np->n_parent) {
-		if (i++ == SMBFS_MAXPATHCOMP) {
-/*			simple_unlock(&smp->sm_npslock);*/
-			return ENAMETOOLONG;
-		}
-		*npp++ = np;
-		if ((np->n_flag & NREFPARENT) == 0)
-			break;
-		np = VTOSMB(np->n_parent);
-	}
-/*	if (i == 0)
-		return smb_put_dmem(mbp, vcp, "\\", 2, caseopt);*/
-	while (i--) {
-		np = *--npp;
-		if (SMB_UNICODE_STRINGS(vcp))
-			error = mb_put_uint16le(mbp, '\\');
-		else
-			error = mb_put_uint8(mbp, '\\');
-		if (error)
-			break;
-		error = smb_put_dmem(mbp, vcp, np->n_name, np->n_nmlen, caseopt);
-		if (error)
-			break;
-	}
-/*	simple_unlock(&smp->sm_npslock);*/
-	return error;
-}
-
 int
 smbfs_fullpath(struct mbchain *mbp, struct smb_vc *vcp, struct smbnode *dnp,
 	const char *name, int nmlen)
@@ -160,23 +122,28 @@ smbfs_fullpath(struct mbchain *mbp, struct smb_vc *vcp, struct smbnode *dnp,
 	if (SMB_DIALECT(vcp) < SMB_DIALECT_LANMAN1_0)
 		caseopt |= SMB_CS_UPPER;
 	if (dnp != NULL) {
-		error = smb_fphelp(mbp, vcp, dnp, caseopt);
-		if (error)
-			return error;
-	}
-	if (name) {
-		if (SMB_UNICODE_STRINGS(vcp))
-			error = mb_put_uint16le(mbp, '\\');
-		else
-			error = mb_put_uint8(mbp, '\\');
-		if (error)
-			return error;
-		error = smb_put_dmem(mbp, vcp, name, nmlen, caseopt);
+		error = smb_put_dmem(mbp, vcp, dnp->n_rpath, dnp->n_rplen, 
+		    caseopt);
 		if (error)
 			return error;
+		if (name) {
+			/* Put the separator */
+			if (SMB_UNICODE_STRINGS(vcp))
+				error = mb_put_uint16le(mbp, '\\');
+			else
+				error = mb_put_uint8(mbp, '\\');
+			if (error)
+				return error;
+			/* Put the name */
+			error = smb_put_dmem(mbp, vcp, name, nmlen, caseopt);
+			if (error)
+				return error;
+		}
 	}
-	error = mb_put_uint8(mbp, 0);
-	if (SMB_UNICODE_STRINGS(vcp) && error == 0)
+	/* Put NULL terminator. */
+	if (SMB_UNICODE_STRINGS(vcp))
+		error = mb_put_uint16le(mbp, 0);
+	else
 		error = mb_put_uint8(mbp, 0);
 	return error;
 }
diff --git a/sys/fs/smbfs/smbfs_vfsops.c b/sys/fs/smbfs/smbfs_vfsops.c
index 11dba2a..781857d 100644
--- a/sys/fs/smbfs/smbfs_vfsops.c
+++ b/sys/fs/smbfs/smbfs_vfsops.c
@@ -320,7 +320,7 @@ smbfs_root(struct mount *mp, int flags, struct vnode **vpp)
 	error = smbfs_smb_lookup(NULL, NULL, 0, &fattr, scred);
 	if (error)
 		goto out;
-	error = smbfs_nget(mp, NULL, "TheRooT", 7, &fattr, &vp);
+	error = smbfs_nget(mp, NULL, NULL, 0, &fattr, &vp);
 	if (error)
 		goto out;
 	ASSERT_VOP_LOCKED(vp, "smbfs_root");