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

[Mateusz Guzik]

Author: mjg
Date: Thu Oct 29 12:56:02 2020
New Revision: 367130
URL: https://svnweb.freebsd.org/changeset/base/367130

Log:
  vfs: add NDREINIT to facilitate repeated namei calls
  
  struct nameidata mixes caller arguments, internal state and output, which
  can be quite error prone.
  
  Recent addition of valdiating ni_resflags uncovered a caller which could
  repeatedly call namei, effectively operating on partially populated state.
  
  Add bare minimium validation this does not happen. The real fix would
  decouple aforementioned state.
  
  Reported by:	pho
  Tested by:	pho (different variant)

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

Modified: head/sys/kern/vfs_lookup.c
==============================================================================
--- head/sys/kern/vfs_lookup.c	Thu Oct 29 11:19:47 2020	(r367129)
+++ head/sys/kern/vfs_lookup.c	Thu Oct 29 12:56:02 2020	(r367130)
@@ -502,6 +502,11 @@ namei(struct nameidata *ndp)
 	cnp = &ndp->ni_cnd;
 	td = cnp->cn_thread;
 #ifdef INVARIANTS
+	KASSERT((ndp->ni_debugflags & NAMEI_DBG_CALLED) == 0,
+	    ("%s: repeated call to namei without NDREINIT", __func__));
+	KASSERT(ndp->ni_debugflags == NAMEI_DBG_INITED,
+	    ("%s: bad debugflags %d", __func__, ndp->ni_debugflags));
+	ndp->ni_debugflags |= NAMEI_DBG_CALLED;
 	/*
 	 * For NDVALIDATE.
 	 *

Modified: head/sys/kern/vfs_vnops.c
==============================================================================
--- head/sys/kern/vfs_vnops.c	Thu Oct 29 11:19:47 2020	(r367129)
+++ head/sys/kern/vfs_vnops.c	Thu Oct 29 12:56:02 2020	(r367130)
@@ -259,6 +259,7 @@ restart:
 				if ((error = vn_start_write(NULL, &mp,
 				    V_XSLEEP | PCATCH)) != 0)
 					return (error);
+				NDREINIT(ndp);
 				goto restart;
 			}
 			if ((vn_open_flags & VN_OPEN_NAMECACHE) != 0)

Modified: head/sys/sys/namei.h
==============================================================================
--- head/sys/sys/namei.h	Thu Oct 29 11:19:47 2020	(r367129)
+++ head/sys/sys/namei.h	Thu Oct 29 12:56:02 2020	(r367130)
@@ -95,11 +95,15 @@ struct nameidata {
 	 */
 	u_int	ni_resflags;
 	/*
+	 * Debug for validating API use by the callers.
+	 */
+	u_short	ni_debugflags;
+	/*
 	 * Shared between namei and lookup/commit routines.
 	 */
+	u_short	ni_loopcnt;		/* count of symlinks encountered */
 	size_t	ni_pathlen;		/* remaining chars in path */
 	char	*ni_next;		/* next location in pathname */
-	u_int	ni_loopcnt;		/* count of symlinks encountered */
 	/*
 	 * Lookup parameters: this structure describes the subset of
 	 * information from the nameidata structure that is passed
@@ -122,7 +126,15 @@ int	cache_fplookup(struct nameidata *ndp, enum cache_f
  *
  * If modifying the list make sure to check whether NDVALIDATE needs updating.
  */
+
 /*
+ * Debug.
+ */
+#define	NAMEI_DBG_INITED	0x0001
+#define	NAMEI_DBG_CALLED	0x0002
+#define	NAMEI_DBG_HADSTARTDIR	0x0004
+
+/*
  * namei operational modifier flags, stored in ni_cnd.flags
  */
 #define	NC_NOMAKEENTRY	0x0001	/* name must not be added to cache */
@@ -215,8 +227,18 @@ int	cache_fplookup(struct nameidata *ndp, enum cache_f
  */
 #ifdef INVARIANTS
 #define NDINIT_PREFILL(arg)	memset(arg, 0xff, sizeof(*arg))
+#define NDINIT_DBG(arg)		{ (arg)->ni_debugflags = NAMEI_DBG_INITED; }
+#define NDREINIT_DBG(arg)	{						\
+	if (((arg)->ni_debugflags & NAMEI_DBG_INITED) == 0)			\
+		panic("namei data not inited");					\
+	if (((arg)->ni_debugflags & NAMEI_DBG_HADSTARTDIR) != 0)		\
+		panic("NDREINIT on namei data with NAMEI_DBG_HADSTARTDIR");	\
+	(arg)->ni_debugflags = NAMEI_DBG_INITED;				\
+}
 #else
 #define NDINIT_PREFILL(arg)	do { } while (0)
+#define NDINIT_DBG(arg)		do { } while (0)
+#define NDREINIT_DBG(arg)	do { } while (0)
 #endif
 
 #define NDINIT_ALL(ndp, op, flags, segflg, namep, dirfd, startdir, rightsp, td)	\
@@ -225,6 +247,7 @@ do {										\
 	cap_rights_t *_rightsp = (rightsp);					\
 	MPASS(_rightsp != NULL);						\
 	NDINIT_PREFILL(_ndp);							\
+	NDINIT_DBG(_ndp);							\
 	_ndp->ni_cnd.cn_nameiop = op;						\
 	_ndp->ni_cnd.cn_flags = flags;						\
 	_ndp->ni_segflg = segflg;						\
@@ -235,6 +258,13 @@ do {										\
 	filecaps_init(&_ndp->ni_filecaps);					\
 	_ndp->ni_cnd.cn_thread = td;						\
 	_ndp->ni_rightsneeded = _rightsp;					\
+} while (0)
+
+#define NDREINIT(ndp)	do {							\
+	struct nameidata *_ndp = (ndp);						\
+	NDREINIT_DBG(_ndp);							\
+	_ndp->ni_resflags = 0;							\
+	_ndp->ni_startdir = NULL;						\
 } while (0)
 
 #define NDF_NO_DVP_RELE		0x00000001