Indeed! Fixed in r367144, thanks. On 10/29/20, Ravi Pokala <rpok...@freebsd.org> wrote: > Hi Mateusz, > > You define NAMEI_DBG_HADSTARTDIR, you check for it being set, but it doesn't > look like you actually set it anywhere...? > > Thanks, > > Ravi (rpokala@) > > -----Original Message----- > From: <owner-src-committ...@freebsd.org> on behalf of Mateusz Guzik > <m...@freebsd.org> > Date: 2020-10-29, Thursday at 05:56 > To: <src-committ...@freebsd.org>, <svn-src-all@freebsd.org>, > <svn-src-h...@freebsd.org> > Subject: svn commit: r367130 - in head/sys: kern sys > > 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 > > >
-- Mateusz Guzik <mjguzik gmail.com> _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
