From owner-svn-src-all@freebsd.org Thu Oct 29 18:44:19 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 B4EED45A9BB; Thu, 29 Oct 2020 18:44:19 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm1-x344.google.com (mail-wm1-x344.google.com [IPv6:2a00:1450:4864:20::344]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CMZ9H2n42z49tc; Thu, 29 Oct 2020 18:44:19 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm1-x344.google.com with SMTP id e2so813700wme.1; Thu, 29 Oct 2020 11:44:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=tBJcQl8tRLBurowHbTB6wpmNCLPzogv7WnoGzueIZp4=; b=A6XEUveCTtcDIaJMwxZmbk/q0UXE8kL77J15r2q5ut5nGp+eXJCuhueXDwFHxC0nVm ZezVHNcJTMDiecDAzI+XuXS/GNwreGuXBxFbt62UhnIG8flRY2q7BBtEyIaTlzThC7/R Qabarj+YjX9izlqXgBwvHRtgQDpk/jnSeTD3/dGT79MLNrb3HGUgEOF2/OdSMyEESAQM too/+WkS5lTjEAUGsH5nhTaaJ6vHNCza6c3KOjzokTxgtQiOvRYUHuOeh+7Id2TiLvd+ R2TTW8KtMXM7hC9l940i0JUGgfX0u6wEED3I75JwlsWoKkMeKBmrLmF4U2J2gFgyfkk5 WCwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=tBJcQl8tRLBurowHbTB6wpmNCLPzogv7WnoGzueIZp4=; b=ABCj2isWm7AGbmwIXOlrRiaHgWG0qEgHzTFmVnbELS55g6UabQjyLTYj6YCFCclu/+ kuOXQKxGyefEgxSqVdvOZ63DHcSt5E/hNFa3lNd3rGX6xAvxWETrlHgcEzLYrvXqQfnr i6mpbPOyJ8tIKSEsU7DYDwILAVeX+9bNwxkGMYHleSfoAxe/a98r1B9eY6GaRN/YgXxC sVx5xAbmRo0USTIklDEDIn+X8Hjx4gWkNLzfGOJ8pNJjucjGQO6U77UcqPjq8yWew6NI YVleSJO8wOjkr61Ida5edH5CayM7/Zjt0GyaKWYaiRZlJO0gd0H2iR/AeQpewNa6fnbA S/Zg== X-Gm-Message-State: AOAM531eY8/NBjeLx7nEkFeKtKZdCObo6RN2jsWEgTVn/m7XpoM6/f5D cm4cq4xzOa0EImWH7daty3cipwg65JgE+MMwyYmveHOEoaE= X-Google-Smtp-Source: ABdhPJzVr4Lmg3aWGvC0qVVv8RkhwvevuGirufiHxE0HyH4pgN1TyKC9BXTRiYSo9CDvaEZzNNSzoS5hEXxTVq6yrwc= X-Received: by 2002:a1c:2441:: with SMTP id k62mr615952wmk.10.1603997057189; Thu, 29 Oct 2020 11:44:17 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a5d:4c4f:0:0:0:0:0 with HTTP; Thu, 29 Oct 2020 11:44:16 -0700 (PDT) In-Reply-To: References: <202010291256.09TCu2BF025339@repo.freebsd.org> From: Mateusz Guzik Date: Thu, 29 Oct 2020 19:44:16 +0100 Message-ID: Subject: Re: svn commit: r367130 - in head/sys: kern sys To: Ravi Pokala Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4CMZ9H2n42z49tc X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] 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:44:19 -0000 Indeed! Fixed in r367144, thanks. On 10/29/20, Ravi Pokala wrote: > Hi Mateusz, > > You define NAMEI_DBG_HADSTARTDIR, you check for it being set, but it does= n'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, > 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 wou= ld > 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 > 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 =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, > rightsp, 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 > > > --=20 Mateusz Guzik