Re: [RFC PATCH 3/3] nfsd: vet the opened dentry after setting a delegation

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2022-07-14 at 17:16 +0000, Chuck Lever III wrote:
> 
> > On Jul 14, 2022, at 1:11 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Thu, 2022-07-14 at 16:53 +0000, Chuck Lever III wrote:
> > > 
> > > > On Jul 14, 2022, at 11:28 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > > > 
> > > > Between opening a file and setting a delegation on it, someone could
> > > > rename or unlink the dentry. If this happens, we do not want to grant a
> > > > delegation on the open.
> > > > 
> > > > Take the i_rwsem before setting the delegation. If we're granted a
> > > > lease, redo the lookup of the file being opened and validate that the
> > > > resulting dentry matches the one in the open file description. We only
> > > > need to do this for the CLAIM_NULL open case however.
> > > > 
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > > fs/nfsd/nfs4state.c | 55 ++++++++++++++++++++++++++++++++++++++++-----
> > > > 1 file changed, 50 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > > index 347794028c98..e5c7f6690d2d 100644
> > > > --- a/fs/nfsd/nfs4state.c
> > > > +++ b/fs/nfsd/nfs4state.c
> > > > @@ -1172,6 +1172,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_file *fp,
> > > > 
> > > > void
> > > > nfs4_put_stid(struct nfs4_stid *s)
> > > > +	__releases(&clp->cl_lock)
> > > > {
> > > > 	struct nfs4_file *fp = s->sc_file;
> > > > 	struct nfs4_client *clp = s->sc_client;
> > > 
> > > This hunk causes a bunch of new sparse warnings:
> > > 
> > > /home/cel/src/linux/klimt/include/linux/list.h:137:19: warning: context imbalance in 'put_clnt_odstate' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:1174:1: warning: context imbalance in 'nfs4_put_stid' - different lock contexts for basic block
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:1230:13: warning: context imbalance in 'destroy_unhashed_deleg' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:1528:17: warning: context imbalance in 'release_lock_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:1624:17: warning: context imbalance in 'release_last_closed_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:2212:22: warning: context imbalance in '__destroy_client' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:4481:17: warning: context imbalance in 'nfsd4_find_and_lock_existing_open' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:4557:25: warning: context imbalance in 'init_open_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:4606:17: warning: context imbalance in 'move_to_close_lru' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:4752:13: warning: context imbalance in 'nfsd4_cb_recall_release' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:5003:17: warning: context imbalance in 'nfs4_check_deleg' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:5392:9: warning: context imbalance in 'nfs4_set_delegation' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:5467:9: warning: context imbalance in 'nfs4_open_delegation' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:5619:17: warning: context imbalance in 'nfsd4_process_open2' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:5638:17: warning: context imbalance in 'nfsd4_cleanup_open_state' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:5934:27: warning: context imbalance in 'nfs4_laundromat' - different lock contexts for basic block
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6160:17: warning: context imbalance in 'nfsd4_lookup_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6374:25: warning: context imbalance in 'nfs4_preprocess_stateid_op' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6422:9: warning: context imbalance in 'nfsd4_free_lock_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6459:28: warning: context imbalance in 'nfsd4_free_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6528:17: warning: context imbalance in 'nfs4_preprocess_seqid_op' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6545:17: warning: context imbalance in 'nfs4_preprocess_confirmed_seqid_op' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6588:9: warning: context imbalance in 'nfsd4_open_confirm' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6657:9: warning: context imbalance in 'nfsd4_open_downgrade' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6730:9: warning: context imbalance in 'nfsd4_close' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:6762:9: warning: context imbalance in 'nfsd4_delegreturn' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:7034:17: warning: context imbalance in 'init_lock_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:7063:17: warning: context imbalance in 'find_or_create_lock_stateid' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:7362:17: warning: context imbalance in 'nfsd4_lock' - unexpected unlock
> > > /home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:7535:9: warning: context imbalance in 'nfsd4_locku' - unexpected unlock
> > > 
> > > Let's repair the "/home/cel/src/linux/klimt/fs/nfsd/nfs4state.c:1185:9:
> > > warning: context imbalance in 'nfs4_put_stid' - unexpected unlock" message
> > > in a separate patch.
> > > 
> > 
> > 
> > Yeah, I saw that too after I sent this. That hunk doesn't belong in
> > here. I'll drop it from my local copy.
> 
> Well, I'm interested in trying to get rid of the existing sparse
> warnings too, since those tend to block our ability to see new
> warnings that arise.
> 
> If you have suggestions or proposals, please send them!
> 

We should definitely annotate these functions that have unbalanced
locking like this one. That's the usual way to silence these sorts of
warnings. I'm hoping Neil's patches will make it easier to address.

-- 
Jeff Layton <jlayton@kernel.org>