Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Feb 02, 2024 at 04:23:13AM +0000, Al Viro wrote:

>         if (gfs2_glock_is_locked_by_me(ip->i_gl) == NULL) {
>                 int noblock = may_not_block ? GL_NOBLOCK : 0;
>                 error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
>                                            LM_FLAG_ANY | noblock, &i_gh);
>                 if (error)
>                         return error;
>         }
> 
> See the problem?  In RCU mode we carefully fetch ->i_gl and verify
> it's not NULL.  Then we proceed to dereference it again.
> 
> IOW, these ip->i_gl below ought to be replaced with gl, or you are
> risking a compiler fetching the sucker again and getting NULL this
> time around.

Wait, what's going on with this part?
        if (gfs2_holder_initialized(&i_gh))
		gfs2_glock_dq_uninit(&i_gh);

First of all, gfs2_glock_dq_uninit() can bloody well sleep in
gfs2_glock_dq().  What's more, it drops a reference to ->gh_gl,
i.e. our gl, in gfs2_holder_uninit().  Where is the matching
increment of gl's refcount?