Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 16 Jul 2020 22:18:40 -0400
From:   "J. Bruce Fields" <bfields@fieldses.org>
To:     NeilBrown <neilb@suse.de>
Cc:     Jeff Layton <jlayton@kernel.org>, linux-nfs@vger.kernel.org
Subject: Re: nfs4_show_superblock considered harmful :-)
Message-ID: <20200717021840.GD18568@fieldses.org>
References: <871rn38suc.fsf@notabene.neil.brown.name>
 <20200529220608.GA22758@fieldses.org>
 <87a71n7dek.fsf@notabene.neil.brown.name>
 <20200715185456.GE15543@fieldses.org>
 <20200716171958.GB18568@fieldses.org>
 <87tuy7vxeb.fsf@notabene.neil.brown.name>
 <20200717010301.GC18568@fieldses.org>
 <87r1tbvsey.fsf@notabene.neil.brown.name>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87r1tbvsey.fsf@notabene.neil.brown.name>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Fri, Jul 17, 2020 at 11:31:17AM +1000, NeilBrown wrote:
> On Thu, Jul 16 2020, J. Bruce Fields wrote:
> 
> > On Fri, Jul 17, 2020 at 09:43:40AM +1000, NeilBrown wrote:
> >> On Thu, Jul 16 2020, J. Bruce Fields wrote:
> >> > --- a/fs/nfsd/nfs4state.c
> >> > +++ b/fs/nfsd/nfs4state.c
> >> > @@ -507,6 +507,16 @@ find_any_file(struct nfs4_file *f)
> >> >  	return ret;
> >> >  }
> >> >  
> >> > +static struct nfsd_file *find_deleg_file(struct nfs4_file *f)
> >> > +{
> >> > +	struct nfsd_file *ret;
> >> > +
> >> > +	spin_lock(&f->fi_lock);
> >> > +	ret = nfsd_file_get(f->fi_deleg_file);
> >> 
> >> A test on f->fi_deleg_file being non-NULL would make this look safer.
> >> It would  also make the subsequent test on the return value appear sane.
> >
> > Yes, thanks!-b.
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index c2a2e56c896d..6e8811e7c134 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -509,10 +509,11 @@ find_any_file(struct nfs4_file *f)
> >  
> >  static struct nfsd_file *find_deleg_file(struct nfs4_file *f)
> >  {
> > -	struct nfsd_file *ret;
> > +	struct nfsd_file *ret = NULL;
> >  
> >  	spin_lock(&f->fi_lock);
> > -	ret = nfsd_file_get(f->fi_deleg_file);
> > +	if (f->fi_deleg_file)
> > +		ret = nfsd_file_get(f->fi_deleg_file);
> >  	spin_unlock(&f->fi_lock);
> >  	return ret;
> >  }
> 
> Reviewed-by: NeilBrown <neilb@suse.de>
> 
> for the whole patch.

Thanks!--b.