Re: [PATCH] nfsd: make use of warning provided by refcount_t

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2024-10-29 at 07:47 +1100, NeilBrown wrote:
> On Tue, 29 Oct 2024, Jeff Layton wrote:
> > On Mon, 2024-10-28 at 09:04 +1100, NeilBrown wrote:
> > > refcount_t, by design, checks for unwanted situations and provides
> > > warnings.  It is rarely useful to have explicit warnings with refcount
> > > usage.
> > > 
> > > In this case we have an explicit warning if a refcount_t reaches zero
> > > when decremented.  Simply using refcount_dec() will provide a similar
> > > warning and also mark the refcount_t as saturated to avoid any possible
> > > use-after-free.
> > > 
> > > This patch drops the warning and uses refcount_dec() instead of
> > > refcount_dec_and_test().
> > > 
> > > Signed-off-by: NeilBrown <neilb@xxxxxxx>
> > > ---
> > >  fs/nfsd/filecache.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> > > index 1408166222c5..c16671135d17 100644
> > > --- a/fs/nfsd/filecache.c
> > > +++ b/fs/nfsd/filecache.c
> > > @@ -1050,7 +1050,7 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
> > >  		 * the last one however, since we should hold another.
> > >  		 */
> > >  		if (nfsd_file_lru_remove(nf))
> > > -			WARN_ON_ONCE(refcount_dec_and_test(&nf->nf_ref));
> > > +			refcount_dec(&nf->nf_ref);
> > 
> > The existing code threw a warning when the counter reached 0. Your
> > change will make the potential warning fire later, after we try to put
> > the last reference and the counter goes to -1. That's probably fine as
> > it should happen later in this function either way.
> 
> The code in refcount_dec() contains:
> 
> 	if (unlikely(old <= 1))
> 		refcount_warn_saturate(r, REFCOUNT_DEC_LEAK);
> 
> 

I stand corrected! I thought it only fired once it went below 0, but I
guess in this case, it does it on a 0->1 transition since this is a
refcount_dec() (sans "test").


> > 
> > >  		goto wait_for_construction;
> > >  	}
> > >  
> > > 
> > > base-commit: 7fa861d5df402b2327f45e0240c1b842f71fec11
> > 
> > Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > 
> 

-- 
Jeff Layton <jlayton@xxxxxxxxxx>





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux