From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bob Peterson <rpeterso@redhat.com>
Date: Tue, 18 Jul 2017 14:53:22 -0400 (EDT)
Subject: [Cluster-devel] [PATCH 2/4] GFS2: Set gl_object in inode
	lookup	only after block type check
In-Reply-To: <20170718182305.27798-3-rpeterso@redhat.com>
References: <20170718182305.27798-1-rpeterso@redhat.com>
	<20170718182305.27798-3-rpeterso@redhat.com>
Message-ID: <1233464494.31977225.1500404002612.JavaMail.zimbra@redhat.com>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

The second half of this patch isn't quite right. I'll rework it
and send a replacement.

Bob Peterson

----- Original Message -----
| Before this patch, the inode glock's gl_object was set after a
| reference was acquired, but before the block type was verified.
| In cases where the block was unlinked, then freed and reused on
| another node, a residule delete callback (delete_work) would try
| to look up the inode, eventually failing the block check, but
| only after it overwrites gl_object with a pointer to the wrong
| inode. This patch moves the assignment of gl_object after the
| block check so it won't be improperly overwritten.
| 
| Likewise, at the end of the function, gfs2_inode_lookup was
| clearing gl_object, even in cases where it wasn't set, such as
| when the block type check fails. The patch only clears it if
| actually set it.
| 
| Signed-off-by: Bob Peterson <rpeterso@redhat.com>
| ---
|  fs/gfs2/inode.c | 7 ++++---
|  1 file changed, 4 insertions(+), 3 deletions(-)
| 
| diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
| index 608e4bf60754..69f66e83920a 100644
| --- a/fs/gfs2/inode.c
| +++ b/fs/gfs2/inode.c
| @@ -145,7 +145,6 @@ struct inode *gfs2_inode_lookup(struct super_block *sb,
| unsigned int type,
|  		if (unlikely(error))
|  			goto fail;
|  		flush_delayed_work(&ip->i_gl->gl_work);
| -		glock_set_object(ip->i_gl, ip);
|  
|  		error = gfs2_glock_get(sdp, no_addr, &gfs2_iopen_glops, CREATE, &io_gl);
|  		if (unlikely(error))
| @@ -170,6 +169,7 @@ struct inode *gfs2_inode_lookup(struct super_block *sb,
| unsigned int type,
|  			}
|  		}
|  
| +		glock_set_object(ip->i_gl, ip);
|  		set_bit(GIF_INVALID, &ip->i_flags);
|  		error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT,
|  		&ip->i_iopen_gh);
|  		if (unlikely(error))
| @@ -207,9 +207,10 @@ struct inode *gfs2_inode_lookup(struct super_block *sb,
| unsigned int type,
|  fail_put:
|  	if (io_gl)
|  		gfs2_glock_put(io_gl);
| -	if (gfs2_holder_initialized(&i_gh))
| +	if (gfs2_holder_initialized(&i_gh)) {
| +		glock_clear_object(ip->i_gl, ip);
|  		gfs2_glock_dq_uninit(&i_gh);
| -	glock_clear_object(ip->i_gl, ip);
| +	}
|  fail:
|  	iget_failed(inode);
|  	return ERR_PTR(error);
| --
| 2.13.3
| 
|