Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0bce587f7b012c6ee1cab04923946c64d928e059.camel@kernel.org>
Subject: Re: [PATCH v2 0/2] nfsd: close potential race between open and
 delegation
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Neil Brown <neilb@suse.de>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Date:   Fri, 15 Jul 2022 20:23:57 -0400
In-Reply-To: <6F1AD05A-B3C7-4B2E-84CA-D08BF1093F82@oracle.com>
References: <20220715183257.41129-1-jlayton@kernel.org>
         <6F1AD05A-B3C7-4B2E-84CA-D08BF1093F82@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.3 (3.44.3-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2022-07-15 at 18:42 +0000, Chuck Lever III wrote:
>=20
> > On Jul 15, 2022, at 2:32 PM, Jeff Layton <jlayton@kernel.org> wrote:
> >=20
> > v2:
> > - use nfsd_lookup_dentry instead of lookup_one_len
> >=20
> > Here's a respin of the patches to fix up the potential race between an
> > open and delegation. I took Neil's advice an changed over to use
> > nfsd_lookup_dentry.
> >=20
> > This patchset is based on top of Neil's recent patchset entitled:
> >=20
> >    [PATCH 0/8] NFSD: clean up locking.
>=20
> Thanks to both of you for pursuing this work! I think there are
> some good improvements here.
>=20
> Note that there are some outstanding review comments (aside from
> the disagreement about how to refactor nfsd_create) so I expect
> Neil will be reposting his series. This is just to note that, as
> long as your series is based on his, I will consider your series
> as RFC until his base series is stable and pulled.
>=20
> I'll review again today or over the weekend.
>=20

Thanks. I think I'll be able to adapt this approach on top of whatever
Neil comes up with.

>=20
> > Tested with xfstests and it seemed to behave. I haven't done any testin=
g
> > to ensure that the race is actually fixed, mainly because I don't have =
a
> > way to reliably reproduce it.
>=20
> That's the thing: we don't have many tests that use multiple clients
> targeting the same set of files.
>=20
>=20

Yeah, it's a difficult problem. Testing delegation behavior is
particularly difficult since the client doesn't have a lot of control
over them being granted in the first place.

> > Jeff Layton (2):
> >  nfsd: drop fh argument from alloc_init_deleg
> >  nfsd: vet the opened dentry after setting a delegation
> >=20
> > fs/nfsd/nfs4state.c | 58 ++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 49 insertions(+), 9 deletions(-)
> >=20
> > --=20
> > 2.36.1
> >=20
>=20
> --
> Chuck Lever
>=20
>=20
>=20

--=20
Jeff Layton <jlayton@kernel.org>