8270 dnlc_reverse_lookup() is unsafe at any speed

Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Toomas Soome <tsoome@me.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

3 files changed, 14 insertions, 121 deletions

diff --git a/usr/src/uts/common/fs/dnlc.c b/usr/src/uts/common/fs/dnlc.c
index 25112ac4aa..0ec57ff7f7 100644
--- a/usr/src/uts/common/fs/dnlc.c
+++ b/usr/src/uts/common/fs/dnlc.c
@@ -20,6 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, Joyent, Inc.
  * Copyright (c) 2017 by Delphix. All rights reserved.
  */
 
@@ -67,12 +68,12 @@
 /*
  * We want to be able to identify files that are referenced only by the DNLC.
  * When adding a reference from the DNLC, call VN_HOLD_DNLC instead of VN_HOLD,
- * since multiple DNLC references should only be counted once in v_count. This
- * file contains only two(2) calls to VN_HOLD, renamed VN_HOLD_CALLER in the
- * hope that no one will mistakenly add a VN_HOLD to this file. (Unfortunately
- * it is not possible to #undef VN_HOLD and retain VN_HOLD_CALLER. Ideally a
- * Makefile rule would grep uncommented C tokens to check that VN_HOLD is
- * referenced only once in this file, to define VN_HOLD_CALLER.)
+ * since multiple DNLC references should only be counted once in v_count. The
+ * VN_HOLD macro itself is aliased to VN_HOLD_CALLER in this file to help
+ * differentiate the behaviors.  (Unfortunately it is not possible to #undef
+ * VN_HOLD and retain VN_HOLD_CALLER. Ideally a Makefile rule would grep
+ * uncommented C tokens to check that VN_HOLD is referenced only once in this
+ * file, to define VN_HOLD_CALLER.)
  */
 #define	VN_HOLD_CALLER	VN_HOLD
 #define	VN_HOLD_DNLC(vp)	{	\
@@ -634,7 +635,7 @@ dnlc_lookup(vnode_t *dp, const char *name)
 			 * put a hold on it.
 			 */
 			vp = ncp->vp;
-			VN_HOLD_CALLER(vp); /* VN_HOLD 1 of 2 in this file */
+			VN_HOLD_CALLER(vp);
 			mutex_exit(&hp->hash_lock);
 			ncstats.hits++;
 			ncs.ncs_hits.value.ui64++;
@@ -922,50 +923,6 @@ dnlc_fs_purge1(vnodeops_t *vop)
 }
 
 /*
- * Perform a reverse lookup in the DNLC.  This will find the first occurrence of
- * the vnode.  If successful, it will return the vnode of the parent, and the
- * name of the entry in the given buffer.  If it cannot be found, or the buffer
- * is too small, then it will return NULL.  Note that this is a highly
- * inefficient function, since the DNLC is constructed solely for forward
- * lookups.
- */
-vnode_t *
-dnlc_reverse_lookup(vnode_t *vp, char *buf, size_t buflen)
-{
-	nc_hash_t *nch;
-	ncache_t *ncp;
-	vnode_t *pvp;
-
-	if (!doingcache)
-		return (NULL);
-
-	for (nch = nc_hash; nch < &nc_hash[nc_hashsz]; nch++) {
-		mutex_enter(&nch->hash_lock);
-		ncp = nch->hash_next;
-		while (ncp != (ncache_t *)nch) {
-			/*
-			 * We ignore '..' entries since it can create
-			 * confusion and infinite loops.
-			 */
-			if (ncp->vp == vp && !(ncp->namlen == 2 &&
-			    0 == bcmp(ncp->name, "..", 2)) &&
-			    ncp->namlen < buflen) {
-				bcopy(ncp->name, buf, ncp->namlen);
-				buf[ncp->namlen] = '\0';
-				pvp = ncp->dp;
-				/* VN_HOLD 2 of 2 in this file */
-				VN_HOLD_CALLER(pvp);
-				mutex_exit(&nch->hash_lock);
-				return (pvp);
-			}
-			ncp = ncp->hash_next;
-		}
-		mutex_exit(&nch->hash_lock);
-	}
-
-	return (NULL);
-}
-/*
  * Utility routine to search for a cache entry. Return the
  * ncache entry if found, NULL otherwise.
  */
diff --git a/usr/src/uts/common/fs/lookup.c b/usr/src/uts/common/fs/lookup.c
index 55ffb94805..da0eae21e5 100644
--- a/usr/src/uts/common/fs/lookup.c
+++ b/usr/src/uts/common/fs/lookup.c
@@ -22,6 +22,7 @@
 /*
  * Copyright 2015 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 1988, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, Joyent, Inc.
  */
 
 /*	Copyright (c) 1983, 1984, 1985, 1986, 1987, 1988, 1989 AT&T	*/
@@ -1166,38 +1167,6 @@ dirtopath(vnode_t *vrootp, vnode_t *vp, char *buf, size_t buflen, int flags,
 		}
 
 		/*
-		 * Try to obtain the path component from dnlc cache
-		 * before searching through the directory.
-		 */
-		if ((cmpvp = dnlc_reverse_lookup(vp, dbuf, dlen)) != NULL) {
-			/*
-			 * If we got parent vnode as a result,
-			 * then the answered path is correct.
-			 */
-			if (VN_CMP(cmpvp, pvp)) {
-				VN_RELE(cmpvp);
-				complen = strlen(dbuf);
-				bufloc -= complen;
-				if (bufloc <= buf) {
-					err = ENAMETOOLONG;
-					goto out;
-				}
-				bcopy(dbuf, bufloc, complen);
-
-				/* Prepend a slash to the current path */
-				*--bufloc = '/';
-
-				/* And continue with the next component */
-				VN_RELE(vp);
-				vp = pvp;
-				pvp = NULL;
-				continue;
-			} else {
-				VN_RELE(cmpvp);
-			}
-		}
-
-		/*
 		 * Search the parent directory for the entry corresponding to
 		 * this vnode.
 		 */
@@ -1269,10 +1238,9 @@ vnodetopath_common(vnode_t *vrootp, vnode_t *vp, char *buf, size_t buflen,
     cred_t *cr, int flags)
 {
 	pathname_t pn, rpn;
-	int ret, len;
-	vnode_t *compvp, *pvp, *realvp;
+	int ret;
+	vnode_t *compvp, *realvp;
 	proc_t *p = curproc;
-	char path[MAXNAMELEN];
 	int doclose = 0;
 
 	/*
@@ -1409,41 +1377,10 @@ notcached:
 	pn_free(&pn);
 
 	if (vp->v_type != VDIR) {
-		/*
-		 * If we don't have a directory, try to find it in the dnlc via
-		 * reverse lookup.  Once this is found, we can use the regular
-		 * directory search to find the full path.
-		 */
-		if ((pvp = dnlc_reverse_lookup(vp, path, MAXNAMELEN)) != NULL) {
-			/*
-			 * Check if we have read privilege so, that
-			 * we can lookup the path in the directory
-			 */
-			ret = 0;
-			if ((flags & LOOKUP_CHECKREAD)) {
-				ret = VOP_ACCESS(pvp, VREAD, 0, cr, NULL);
-			}
-			if (ret == 0) {
-				ret = dirtopath(vrootp, pvp, buf, buflen,
-				    flags, cr);
-			}
-			if (ret == 0) {
-				len = strlen(buf);
-				if (len + strlen(path) + 1 >= buflen) {
-					ret = ENAMETOOLONG;
-				} else {
-					if (buf[len - 1] != '/')
-						buf[len++] = '/';
-					bcopy(path, buf + len,
-					    strlen(path) + 1);
-				}
-			}
-
-			VN_RELE(pvp);
-		} else
-			ret = ENOENT;
-	} else
+		ret = ENOENT;
+	} else {
 		ret = dirtopath(vrootp, vp, buf, buflen, flags, cr);
+	}
 
 	VN_RELE(vrootp);
 	if (doclose) {
diff --git a/usr/src/uts/common/sys/dnlc.h b/usr/src/uts/common/sys/dnlc.h
index 070506ee31..26f1f1dc0c 100644
--- a/usr/src/uts/common/sys/dnlc.h
+++ b/usr/src/uts/common/sys/dnlc.h
@@ -187,7 +187,6 @@ void	dnlc_purge_vp(vnode_t *);
 int	dnlc_purge_vfsp(vfs_t *, int);
 void	dnlc_remove(vnode_t *, const char *);
 int	dnlc_fs_purge1(struct vnodeops *);
-vnode_t	*dnlc_reverse_lookup(vnode_t *, char *, size_t);
 void	dnlc_reduce_cache(void *);
 
 #endif	/* defined(_KERNEL) */