Re: [RFC PATCH v2 2/5] libfs: Check dentry before locking in simple_offset_empty()

[Chuck Lever <chuck.lever@oracle.com>]

On Fri, Nov 29, 2024 at 04:17:56PM +0800, yangerkun wrote:
> 
> 
> 在 2024/11/27 22:36, Chuck Lever 写道:
> > On Wed, Nov 27, 2024 at 11:09:11AM +0800, yangerkun wrote:
> > > Thank you very much for your efforts on this issue!
> > > 
> > > 在 2024/11/26 23:54, cel@kernel.org 写道:
> > > > From: Chuck Lever <chuck.lever@oracle.com>
> > > > 
> > > > Defensive change: Don't try to lock a dentry unless it is positive.
> > > > Trying to lock a negative entry will generate a refcount underflow.
> > > 
> > > Which member trigger this underflow?
> > 
> > dput() encounters a dentry refcount underflow because a negative
> > dentry's refcount is already zero.
> 
> OK, so you mean dentry->d_lockref here.
> 
> But I cannot see why we can trigger this, if it is, it's actually a bug...
> 
> Can you give a more detail why can trigger this?

I think it is possible for the mtree lookup in simple_offset_empty()
to return a pointer to a negative dentry, perhaps due to a race.
There is nothing explicit that prevents a dentry from going negative
while it is still stored in the mtree, although that condition would
be quite brief. So, best to harden the "is dentry positive" check
there so that it acts like other dentry locking code in fs/libfs.c.

Perhaps labeling this patch as a "clean up" would be more clear. I
will remove the "Fixes:" tag -- it does not fix a bug in the current
code.


-- 
Chuck Lever