Close a race in NFS lookup processing that could result in stale name cache

entries on one client when a directory was renamed on another client.  The
root cause for the stale entry being trusted is that each per-vnode nfsnode
structure has a single 'n_ctime' timestamp used to validate positive name
cache entries.  However, if there are multiple entries for a single vnode,
they all share a single timestamp.  To fix this, extend the name cache
to allow filesystems to optionally store a timestamp value in each name
cache entry.  The NFS clients now fetch the timestamp associated with
each name cache entry and use that to validate cache hits instead of the
timestamps previously stored in the nfsnode.  Another part of the fix is
that the NFS clients now use timestamps from the post-op attributes of
RPCs when adding name cache entries rather than pulling the timestamps out
of the file's attribute cache.  The latter is subject to races with other
lookups updating the attribute cache concurrently.  Some more details:
- Add a variant of nfsm_postop_attr() to the old NFS client that can return
  a vattr structure with a copy of the post-op attributes.
- Handle lookups of "." as a special case in the NFS clients since the name
  cache does not store name cache entries for ".", so we cannot get a
  useful timestamp.  It didn't really make much sense to recheck the
  attributes on the the directory to validate the namecache hit for "."
  anyway.
- ABI compat shims for the name cache routines are present in this commit
  so that it is safe to MFC.

MFC after:	2 weeks

1 files changed, 58 insertions, 8 deletions

diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c
index e4de804..aa269de 100644
--- a/sys/kern/vfs_cache.c
+++ b/sys/kern/vfs_cache.c
@@ -97,6 +97,8 @@ struct	namecache {
 	TAILQ_ENTRY(namecache) nc_dst;	/* destination vnode list */
 	struct	vnode *nc_dvp;		/* vnode of parent of name */
 	struct	vnode *nc_vp;		/* vnode the name refers to */
+	struct	timespec nc_time;	/* timespec provided by fs */
+	int	nc_ticks;		/* ticks value when entry was added */
 	u_char	nc_flag;		/* flag bits */
 	u_char	nc_nlen;		/* length of name */
 	char	nc_name[0];		/* segment name + nul */
@@ -394,10 +396,12 @@ cache_zap(ncp)
  */
 
 int
-cache_lookup(dvp, vpp, cnp)
+cache_lookup_times(dvp, vpp, cnp, tsp, ticksp)
 	struct vnode *dvp;
 	struct vnode **vpp;
 	struct componentname *cnp;
+	struct timespec *tsp;
+	int *ticksp;
 {
 	struct namecache *ncp;
 	uint32_t hash;
@@ -422,6 +426,10 @@ retry_wlocked:
 			dothits++;
 			SDT_PROBE(vfs, namecache, lookup, hit, dvp, ".",
 			    *vpp, 0, 0);
+			if (tsp != NULL)
+				timespecclear(tsp);
+			if (ticksp != NULL)
+				*ticksp = ticks;
 			goto success;
 		}
 		if (cnp->cn_namelen == 2 && cnp->cn_nameptr[1] == '.') {
@@ -440,19 +448,22 @@ retry_wlocked:
 				CACHE_WUNLOCK();
 				return (0);
 			}
-			if (dvp->v_cache_dd->nc_flag & NCF_ISDOTDOT)
-				*vpp = dvp->v_cache_dd->nc_vp;
+			ncp = dvp->v_cache_dd;
+			if (ncp->nc_flag & NCF_ISDOTDOT)
+				*vpp = ncp->nc_vp;
 			else
-				*vpp = dvp->v_cache_dd->nc_dvp;
+				*vpp = ncp->nc_dvp;
 			/* Return failure if negative entry was found. */
-			if (*vpp == NULL) {
-				ncp = dvp->v_cache_dd;
+			if (*vpp == NULL)
 				goto negative_success;
-			}
 			CTR3(KTR_VFS, "cache_lookup(%p, %s) found %p via ..",
 			    dvp, cnp->cn_nameptr, *vpp);
 			SDT_PROBE(vfs, namecache, lookup, hit, dvp, "..",
 			    *vpp, 0, 0);
+			if (tsp != NULL)
+				*tsp = ncp->nc_time;
+			if (ticksp != NULL)
+				*ticksp = ncp->nc_ticks;
 			goto success;
 		}
 	}
@@ -499,6 +510,10 @@ retry_wlocked:
 		    dvp, cnp->cn_nameptr, *vpp, ncp);
 		SDT_PROBE(vfs, namecache, lookup, hit, dvp, ncp->nc_name,
 		    *vpp, 0, 0);
+		if (tsp != NULL)
+			*tsp = ncp->nc_time;
+		if (ticksp != NULL)
+			*ticksp = ncp->nc_ticks;
 		goto success;
 	}
 
@@ -530,6 +545,10 @@ negative_success:
 		cnp->cn_flags |= ISWHITEOUT;
 	SDT_PROBE(vfs, namecache, lookup, hit_negative, dvp, ncp->nc_name,
 	    0, 0, 0);
+	if (tsp != NULL)
+		*tsp = ncp->nc_time;
+	if (ticksp != NULL)
+		*ticksp = ncp->nc_ticks;
 	CACHE_WUNLOCK();
 	return (ENOENT);
 
@@ -616,10 +635,11 @@ unlock:
  * Add an entry to the cache.
  */
 void
-cache_enter(dvp, vp, cnp)
+cache_enter_time(dvp, vp, cnp, tsp)
 	struct vnode *dvp;
 	struct vnode *vp;
 	struct componentname *cnp;
+	struct timespec *tsp;
 {
 	struct namecache *ncp, *n2;
 	struct nchashhead *ncpp;
@@ -692,6 +712,11 @@ cache_enter(dvp, vp, cnp)
 	ncp->nc_vp = vp;
 	ncp->nc_dvp = dvp;
 	ncp->nc_flag = flag;
+	if (tsp != NULL)
+		ncp->nc_time = *tsp;
+	else
+		timespecclear(&ncp->nc_time);
+	ncp->nc_ticks = ticks;
 	len = ncp->nc_nlen = cnp->cn_namelen;
 	hash = fnv_32_buf(cnp->cn_nameptr, len, FNV1_32_INIT);
 	strlcpy(ncp->nc_name, cnp->cn_nameptr, len + 1);
@@ -708,6 +733,8 @@ cache_enter(dvp, vp, cnp)
 		if (n2->nc_dvp == dvp &&
 		    n2->nc_nlen == cnp->cn_namelen &&
 		    !bcmp(n2->nc_name, cnp->cn_nameptr, n2->nc_nlen)) {
+			n2->nc_time = ncp->nc_time;
+			n2->nc_ticks = ncp->nc_ticks;
 			CACHE_WUNLOCK();
 			cache_free(ncp);
 			return;
@@ -1280,6 +1307,29 @@ vn_commname(struct vnode *vp, char *buf, u_int buflen)
 	return (0);
 }
 
+/* ABI compat shims for old kernel modules. */
+#undef cache_enter
+#undef cache_lookup
+
+void	cache_enter(struct vnode *dvp, struct vnode *vp,
+	    struct componentname *cnp);
+int	cache_lookup(struct vnode *dvp, struct vnode **vpp,
+	    struct componentname *cnp);
+
+void
+cache_enter(struct vnode *dvp, struct vnode *vp, struct componentname *cnp)
+{
+
+	cache_enter_time(dvp, vp, cnp, NULL);
+}
+
+int
+cache_lookup(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp)
+{
+
+	return (cache_lookup_times(dvp, vpp, cnp, NULL, NULL));
+}
+
 /*
  * This function updates path string to vnode's full global path
  * and checks the size of the new path string against the pathlen argument.