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 _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"