[PATCH 1/2] gfs2: release the statfs glock when gfs2_clear_beyond_eof() fails

[PATCH 1/2] gfs2: release the statfs glock when gfs2_clear_beyond_eof() fails

[Joseph Qi]

gfs2_file_buffered_write() acquires the master statfs glock before
writing to the rindex and releases it once the write is done.  The only
error path in between used to be the failure of that acquisition, which
gfs2_glock_nq_init() unwinds by itself, so the cleanup code at the end of
the function never had to deal with a held statfs glock.

Commit 4982e58669b1 ("gfs2: page poisoning fix") added a call to
gfs2_clear_beyond_eof() right after the acquisition and made it jump to
out_unlock on failure.  That path only drops the inode glock and then
frees statfs_gh, so a failed write to the rindex leaves the master
statfs glock held exclusively and frees a holder that is still linked
into the glock's holder list.  The GL_NOCACHE demote is requested by
__gfs2_glock_dq(), which is never reached, so the glock stays locked for
the rest of the mount, and the glock and pid references taken by
__gfs2_holder_init() leak as well.  Walking the stale list entry later
on, for instance from do_promote() or from a demote callback, is a
use-after-free.

gfs2_clear_beyond_eof() zeroes the space beyond eof, which for the
rindex ends up in adjust_fs_space(), so the call cannot be moved out
from under the statfs glock.  Release the holder in the error path
instead, guarded by gfs2_holder_initialized() so that the paths which
never acquired it and the one which already released it are unaffected.

Fixes: 4982e58669b1 ("gfs2: page poisoning fix")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/gfs2/file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 1efd0679badd9..2452d3ecf09c7 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1087,6 +1087,8 @@ static ssize_t gfs2_file_buffered_write(struct kiocb *iocb,
 		goto retry;
 	}
 out_unlock:
+	if (inode == sdp->sd_rindex && gfs2_holder_initialized(statfs_gh))
+		gfs2_glock_dq_uninit(statfs_gh);
 	if (gfs2_holder_queued(gh))
 		gfs2_glock_dq(gh);
 out_uninit:
-- 
2.39.3

[PATCH 2/2] gfs2: hold the master statfs glock while operating on the rindex

[Joseph Qi]

Writes to the rindex reach adjust_fs_space() through gfs2_iomap_end(),
which reads the master statfs block, merges the local statfs deltas into
it and dirties it again through update_statfs().
gfs2_file_buffered_write() therefore takes that glock exclusively before
writing to the rindex.

gfs2_fallocate() and gfs2_setattr() do not, so fallocate(2) and
truncate(2) on the rindex reach adjust_fs_space() with nothing held on
sd_statfs_inode.  gfs2_fallocate() admits the rindex on purpose, since
gfs2_grow reserves rindex space with it, and the rindex size is only
page aligned when the resource group count is a multiple of 64, so
gfs2_clear_beyond_eof() usually zeroes the tail of the last page there
and gfs2_iomap_end() sees IOMAP_ZERO on the rindex.  Punching a hole in
the rindex goes the same way through __gfs2_punch_hole(), and growing or
truncating it goes through gfs2_setattr_size(), which reaches
gfs2_clear_beyond_eof() in do_grow() and gfs2_block_zero_range() in
trunc_start().

Another node holding the master statfs glock can update the block while
this one works from the copy it read, and that update is lost.

Take the glock in gfs2_fallocate() and gfs2_setattr() for the rindex,
mirroring gfs2_file_buffered_write().  Both already hold the inode glock
at that point, so the acquisition order matches.

Fixes: 2257e468a63b ("gfs2: implement gfs2_block_zero_range using iomap_zero_range")
Fixes: 4982e58669b1 ("gfs2: page poisoning fix")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/gfs2/file.c  | 20 +++++++++++++++++++-
 fs/gfs2/inode.c | 24 +++++++++++++++++++++---
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 2452d3ecf09c7..e37b9cd78d972 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1391,9 +1391,11 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	struct gfs2_glock *gl = gfs2_inode_glock(inode);
 	struct gfs2_inode *ip = GFS2_I(inode);
-	struct gfs2_holder gh;
+	struct gfs2_holder gh, statfs_gh;
 	int ret;
 
+	gfs2_holder_mark_uninitialized(&statfs_gh);
+
 	if (mode & ~(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE))
 		return -EOPNOTSUPP;
 	/* fallocate is needed by gfs2_grow to reserve space in the rindex */
@@ -1407,6 +1409,20 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
 	if (ret)
 		goto out_uninit;
 
+	/*
+	 * Growing the rindex or punching a hole in it ends up in
+	 * adjust_fs_space(), which updates the master statfs block, so hold
+	 * that glock like gfs2_file_buffered_write() does.
+	 */
+	if (inode == sdp->sd_rindex) {
+		struct gfs2_glock *m_gl = gfs2_inode_glock(sdp->sd_statfs_inode);
+
+		ret = gfs2_glock_nq_init(m_gl, LM_ST_EXCLUSIVE, GL_NOCACHE,
+					 &statfs_gh);
+		if (ret)
+			goto out_unlock;
+	}
+
 	if (!(mode & FALLOC_FL_KEEP_SIZE) &&
 	    (offset + len) > inode->i_size) {
 		ret = inode_newsize_ok(inode, offset + len);
@@ -1428,6 +1444,8 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
 
 	put_write_access(inode);
 out_unlock:
+	if (gfs2_holder_initialized(&statfs_gh))
+		gfs2_glock_dq_uninit(&statfs_gh);
 	gfs2_glock_dq(&gh);
 out_uninit:
 	gfs2_holder_uninit(&gh);
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 2c2dc459c0375..51d406d913793 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2112,9 +2112,11 @@ static int gfs2_setattr(const struct mnt_idmap *idmap,
 	struct inode *inode = d_inode(dentry);
 	struct gfs2_glock *gl = gfs2_inode_glock(inode);
 	struct gfs2_inode *ip = GFS2_I(inode);
-	struct gfs2_holder i_gh;
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	struct gfs2_holder i_gh, statfs_gh;
 	int error;
 
+	gfs2_holder_mark_uninitialized(&statfs_gh);
 	error = gfs2_qa_get(ip);
 	if (error)
 		return error;
@@ -2131,9 +2133,25 @@ static int gfs2_setattr(const struct mnt_idmap *idmap,
 	if (error)
 		goto error;
 
-	if (attr->ia_valid & ATTR_SIZE)
+	if (attr->ia_valid & ATTR_SIZE) {
+		/*
+		 * Growing or truncating the rindex ends up in
+		 * adjust_fs_space(), which updates the master statfs block, so
+		 * hold that glock like gfs2_file_buffered_write() does.
+		 */
+		if (inode == sdp->sd_rindex) {
+			struct gfs2_glock *m_gl;
+
+			m_gl = gfs2_inode_glock(sdp->sd_statfs_inode);
+			error = gfs2_glock_nq_init(m_gl, LM_ST_EXCLUSIVE,
+						   GL_NOCACHE, &statfs_gh);
+			if (error)
+				goto error;
+		}
 		error = gfs2_setattr_size(inode, attr->ia_size);
-	else if (attr->ia_valid & (ATTR_UID | ATTR_GID))
+		if (gfs2_holder_initialized(&statfs_gh))
+			gfs2_glock_dq_uninit(&statfs_gh);
+	} else if (attr->ia_valid & (ATTR_UID | ATTR_GID))
 		error = setattr_chown(inode, attr);
 	else {
 		error = gfs2_setattr_simple(inode, attr);
-- 
2.39.3