From owner-svn-src-all@freebsd.org Thu Oct 29 18:35:15 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 A916A45A4ED; Thu, 29 Oct 2020 18:35:15 +0000 (UTC) (envelope-from rpokala@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (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 "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CMYyq3yr2z488G; Thu, 29 Oct 2020 18:35:15 +0000 (UTC) (envelope-from rpokala@freebsd.org) Received: from [192.168.1.10] (c-98-207-126-143.hsd1.ca.comcast.net [98.207.126.143]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: rpokala) by smtp.freebsd.org (Postfix) with ESMTPSA id DD53525F90; Thu, 29 Oct 2020 18:35:14 +0000 (UTC) (envelope-from rpokala@freebsd.org) User-Agent: Microsoft-MacOutlook/16.42.20101102 Date: Thu, 29 Oct 2020 11:35:11 -0700 Subject: Re: svn commit: r367130 - in head/sys: kern sys From: Ravi Pokala To: Mateusz Guzik , , , Message-ID: Thread-Topic: svn commit: r367130 - in head/sys: kern sys References: <202010291256.09TCu2BF025339@repo.freebsd.org> In-Reply-To: <202010291256.09TCu2BF025339@repo.freebsd.org> Mime-version: 1.0 Content-type: text/plain; charset="UTF-8" Content-transfer-encoding: quoted-printable 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 18:35:15 -0000 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@) =EF=BB=BF-----Original Message----- From: on behalf of Mateusz Guzik Date: 2020-10-29, Thursday at 05:56 To: , , 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, w= hich can be quite error prone. Recent addition of valdiating ni_resflags uncovered a caller which co= uld repeatedly call namei, effectively operating on partially populated s= tate. 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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D --- 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 =3D &ndp->ni_cnd; td =3D cnp->cn_thread; #ifdef INVARIANTS + KASSERT((ndp->ni_debugflags & NAMEI_DBG_CALLED) =3D=3D 0, + ("%s: repeated call to namei without NDREINIT", __func__)); + KASSERT(ndp->ni_debugflags =3D=3D NAMEI_DBG_INITED, + ("%s: bad debugflags %d", __func__, ndp->ni_debugflags)); + ndp->ni_debugflags |=3D NAMEI_DBG_CALLED; /* * For NDVALIDATE. * Modified: head/sys/kern/vfs_vnops.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D --- 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 =3D vn_start_write(NULL, &mp, V_XSLEEP | PCATCH)) !=3D 0) return (error); + NDREINIT(ndp); goto restart; } if ((vn_open_flags & VN_OPEN_NAMECACHE) !=3D 0) Modified: head/sys/sys/namei.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D --- 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 cac= he_f * * If modifying the list make sure to check whether NDVALIDATE needs u= pdating. */ + /* + * 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 cac= he_f */ #ifdef INVARIANTS #define NDINIT_PREFILL(arg) memset(arg, 0xff, sizeof(*arg)) +#define NDINIT_DBG(arg) { (arg)->ni_debugflags =3D NAMEI_DBG_INITED; } +#define NDREINIT_DBG(arg) { \ + if (((arg)->ni_debugflags & NAMEI_DBG_INITED) =3D=3D 0) \ + panic("namei data not inited"); \ + if (((arg)->ni_debugflags & NAMEI_DBG_HADSTARTDIR) !=3D 0) \ + panic("NDREINIT on namei data with NAMEI_DBG_HADSTARTDIR"); \ + (arg)->ni_debugflags =3D 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, rig= htsp, td) \ @@ -225,6 +247,7 @@ do { \ cap_rights_t *_rightsp =3D (rightsp); \ MPASS(_rightsp !=3D NULL); \ NDINIT_PREFILL(_ndp); \ + NDINIT_DBG(_ndp); \ _ndp->ni_cnd.cn_nameiop =3D op; \ _ndp->ni_cnd.cn_flags =3D flags; \ _ndp->ni_segflg =3D segflg; \ @@ -235,6 +258,13 @@ do { \ filecaps_init(&_ndp->ni_filecaps); \ _ndp->ni_cnd.cn_thread =3D td; \ _ndp->ni_rightsneeded =3D _rightsp; \ +} while (0) + +#define NDREINIT(ndp) do { \ + struct nameidata *_ndp =3D (ndp); \ + NDREINIT_DBG(_ndp); \ + _ndp->ni_resflags =3D 0; \ + _ndp->ni_startdir =3D NULL; \ } while (0) #define NDF_NO_DVP_RELE 0x00000001