From 9ffa18884cceb2e5731e422140fad06292de0577 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Mon, 23 Jan 2023 18:58:27 +0100 Subject: [PATCH] gfs2: gl_object races fix Function glock_clear_object() checks if the specified glock is still pointing at the right object and clears the gl_object pointer. To handle the case of incompletely constructed inodes, glock_clear_object() also allows gl_object to be NULL. However, in the teardown case, when iget_failed() is called and the inode is removed from the inode hash, by the time we get to the glock_clear_object() calls in gfs2_put_super() and its helpers, we don't have exclusion against concurrent gfs2_inode_lookup() and gfs2_create_inode() calls, and the inode and iopen glocks may already be pointing at another inode, so the checks in glock_clear_object() are incorrect. To better handle this case, always completely disassociate an inode from its glocks before tearing it down. In addition, get rid of a duplicate glock_clear_object() call in gfs2_evict_inode(). That way, glock_clear_object() will only ever be called when the glock points at the current inode, and the NULL check in glock_clear_object() can be removed. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 4 ++-- fs/gfs2/inode.c | 8 ++++++++ fs/gfs2/super.c | 4 +--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 524f3c9..2868e97 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -883,6 +883,7 @@ void glock_set_object(struct gfs2_glock *gl, void *object) /** * glock_clear_object - clear the gl_object field of a glock * @gl: the glock + * @object: object the glock currently points at */ void glock_clear_object(struct gfs2_glock *gl, void *object) { @@ -892,8 +893,7 @@ void glock_clear_object(struct gfs2_glock *gl, void *object) prev_object = gl->gl_object; gl->gl_object = NULL; spin_unlock(&gl->gl_lockref.lock); - if (gfs2_assert_warn(gl->gl_name.ln_sbd, - prev_object == object || prev_object == NULL)) { + if (gfs2_assert_warn(gl->gl_name.ln_sbd, prev_object == object)) { pr_warn("glock=%u/%llx\n", gl->gl_name.ln_type, (unsigned long long)gl->gl_name.ln_number); diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 614db30..c76fdb8 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -225,6 +225,10 @@ fail: gfs2_glock_dq_uninit(&ip->i_iopen_gh); if (gfs2_holder_initialized(&i_gh)) gfs2_glock_dq_uninit(&i_gh); + if (ip->i_gl) { + gfs2_glock_put(ip->i_gl); + ip->i_gl = NULL; + } iget_failed(inode); return ERR_PTR(error); } @@ -816,6 +820,10 @@ fail_gunlock3: fail_gunlock2: gfs2_glock_put(io_gl); fail_free_inode: + if (ip->i_gl) { + gfs2_glock_put(ip->i_gl); + ip->i_gl = NULL; + } gfs2_rs_deltree(&ip->i_res); gfs2_qa_put(ip); fail_free_acls: diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index 999cc14..de99505 100644 --- a/fs/gfs2/super.c +++ b/fs/gfs2/super.c @@ -1401,10 +1401,8 @@ static void gfs2_evict_inode(struct inode *inode) if (gfs2_rs_active(&ip->i_res)) gfs2_rs_deltree(&ip->i_res); - if (gfs2_holder_initialized(&gh)) { - glock_clear_object(ip->i_gl, ip); + if (gfs2_holder_initialized(&gh)) gfs2_glock_dq_uninit(&gh); - } if (ret && ret != GLR_TRYFAILED && ret != -EROFS) fs_warn(sdp, "gfs2_evict_inode: %d\n", ret); out: -- 2.7.4