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

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

On Fri, Jan 19, 2024 at 10:20:47PM +0100, Andreas Gruenbacher wrote:
> Hello,
> 
> Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
> GL_NOBLOCK flag for non-blocking lookups"):
> 
> * First, parent can now be NULL and dereferencing it in
>   gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
>   work;
> 
> * Second, gfs2_dir_check() can still sleep, which breaks LOOKUP_RCU
>   mode.

Looks sane, but there's another piece of fun in gfs_permission():
        gl = rcu_dereference_check(ip->i_gl, !may_not_block);
        if (unlikely(!gl)) {
                /* inode is getting torn down, must be RCU mode */
                WARN_ON_ONCE(!may_not_block);
                return -ECHILD;
        }
        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.