svn commit: r364542 - in head/sys: kern sys

[Mateusz Guzik]

Author: mjg
Date: Sun Aug 23 21:06:41 2020
New Revision: 364542
URL: https://svnweb.freebsd.org/changeset/base/364542

Log:
  vfs: validate ndp state after the lookup
  
  The intent is to remove known-to-be-nops NDFREE calls after many lookups.

Modified:
  head/sys/kern/vfs_lookup.c
  head/sys/sys/namei.h

Modified: head/sys/kern/vfs_lookup.c
==============================================================================
--- head/sys/kern/vfs_lookup.c	Sun Aug 23 21:05:54 2020	(r364541)
+++ head/sys/kern/vfs_lookup.c	Sun Aug 23 21:06:41 2020	(r364542)
@@ -482,6 +482,15 @@ namei(struct nameidata *ndp)
 
 	cnp = &ndp->ni_cnd;
 	td = cnp->cn_thread;
+#ifdef INVARIANTS
+	/*
+	 * For NDVALIDATE.
+	 *
+	 * While NDINIT may seem like a more natural place to do it, there are
+	 * callers which directly modify flags past invoking init.
+	 */
+	cnp->cn_origflags = cnp->cn_flags;
+#endif
 	ndp->ni_cnd.cn_cred = ndp->ni_cnd.cn_thread->td_ucred;
 	KASSERT(cnp->cn_cred && td->td_proc, ("namei: bad cred/proc"));
 	KASSERT((cnp->cn_flags & NAMEI_INTERNAL_FLAGS) == 0,
@@ -542,6 +551,8 @@ namei(struct nameidata *ndp)
 		__assert_unreachable();
 		break;
 	case CACHE_FPL_STATUS_HANDLED:
+		if (error == 0)
+			NDVALIDATE(ndp);
 		return (error);
 	case CACHE_FPL_STATUS_PARTIAL:
 		TAILQ_INIT(&ndp->ni_cap_tracker);
@@ -584,6 +595,8 @@ namei(struct nameidata *ndp)
 			SDT_PROBE3(vfs, namei, lookup, return, error,
 			    (error == 0 ? ndp->ni_vp : NULL), false);
 			pwd_drop(pwd);
+			if (error == 0)
+				NDVALIDATE(ndp);
 			return (error);
 		}
 		if (ndp->ni_loopcnt++ >= MAXSYMLINKS) {
@@ -1432,6 +1445,68 @@ void
 		ndp->ni_startdir = NULL;
 	}
 }
+
+#ifdef INVARIANTS
+/*
+ * Validate the final state of ndp after the lookup.
+ *
+ * Historically filesystems were allowed to modify cn_flags. Most notably they
+ * can add SAVENAME to the request, resulting in HASBUF and pushing subsequent
+ * clean up to the consumer. In practice this seems to only concern != LOOKUP
+ * operations.
+ *
+ * As a step towards stricter API contract this routine validates the state to
+ * clean up. Note validation is a work in progress with the intent of becoming
+ * stricter over time.
+ */
+#define NDMODIFYINGFLAGS (LOCKLEAF | LOCKPARENT | WANTPARENT | SAVENAME | SAVESTART | HASBUF)
+void
+NDVALIDATE(struct nameidata *ndp)
+{
+	struct componentname *cnp;
+	u_int64_t used, orig;
+
+	cnp = &ndp->ni_cnd;
+	orig = cnp->cn_origflags;
+	used = cnp->cn_flags;
+	switch (cnp->cn_nameiop) {
+	case LOOKUP:
+		/*
+		 * For plain lookup we require strict conformance -- nothing
+		 * to clean up if it was not requested by the caller.
+		 */
+		orig &= NDMODIFYINGFLAGS;
+		used &= NDMODIFYINGFLAGS;
+		if ((orig & (SAVENAME | SAVESTART)) != 0)
+			orig |= HASBUF;
+		if (orig != used) {
+			goto out_mismatch;
+		}
+		break;
+	case CREATE:
+	case DELETE:
+	case RENAME:
+		/*
+		 * Some filesystems set SAVENAME to provoke HASBUF, accomodate
+		 * for it until it gets fixed.
+		 */
+		orig &= NDMODIFYINGFLAGS;
+		orig |= (SAVENAME | HASBUF);
+		used &= NDMODIFYINGFLAGS;
+		used |= (SAVENAME | HASBUF);
+		if (orig != used) {
+			goto out_mismatch;
+		}
+		break;
+	}
+	return;
+out_mismatch:
+	panic("%s: mismatched flags for op %d: added %" PRIx64 ", "
+	    "removed %" PRIx64" (%" PRIx64" != %" PRIx64"; stored %" PRIx64" != %" PRIx64")",
+	    __func__, cnp->cn_nameiop, used & ~orig, orig &~ used,
+	    orig, used, cnp->cn_origflags, cnp->cn_flags);
+}
+#endif
 
 /*
  * Determine if there is a suitable alternate filename under the specified

Modified: head/sys/sys/namei.h
==============================================================================
--- head/sys/sys/namei.h	Sun Aug 23 21:05:54 2020	(r364541)
+++ head/sys/sys/namei.h	Sun Aug 23 21:06:41 2020	(r364542)
@@ -46,6 +46,7 @@ struct componentname {
 	/*
 	 * Arguments to lookup.
 	 */
+	u_int64_t cn_origflags;	/* flags to namei */
 	u_int64_t cn_flags;	/* flags to namei */
 	struct	thread *cn_thread;/* thread requesting lookup */
 	struct	ucred *cn_cred;	/* credentials */
@@ -250,6 +251,12 @@ void NDFREE(struct nameidata *, const u_int);
 	else								\
 		NDFREE(_ndp, flags);					\
 } while (0)
+
+#ifdef INVARIANTS
+void NDVALIDATE(struct nameidata *);
+#else
+#define NDVALIDATE(ndp)	do { } while (0)
+#endif
 
 int	namei(struct nameidata *ndp);
 int	lookup(struct nameidata *ndp);