From owner-svn-src-all@freebsd.org Thu Oct 29 12:56:03 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 85A16450619; Thu, 29 Oct 2020 12:56:03 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CMQRR33Nfz3Sj7; Thu, 29 Oct 2020 12:56:03 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4D7212D7F5; Thu, 29 Oct 2020 12:56:03 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 09TCu3aG025341; Thu, 29 Oct 2020 12:56:03 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 09TCu2BF025339; Thu, 29 Oct 2020 12:56:02 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <202010291256.09TCu2BF025339@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Thu, 29 Oct 2020 12:56:02 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r367130 - in head/sys: kern sys X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: in head/sys: kern sys X-SVN-Commit-Revision: 367130 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Oct 2020 12:56:03 -0000 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