public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, linux-xfs@vger.kernel.org,
	Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>,
	Christoph Hellwig <hch@lst.de>,
	"Darrick J. Wong" <djwong@kernel.org>,
	Chandan Babu R <chandanbabu@kernel.org>,
	Catherine Hoang <catherine.hoang@oracle.com>
Subject: [PATCH 6.6 273/396] xfs: ensure logflagsp is initialized in xfs_bmap_del_extent_real
Date: Mon,  1 Apr 2024 17:45:22 +0200	[thread overview]
Message-ID: <20240401152556.051182296@linuxfoundation.org> (raw)
In-Reply-To: <20240401152547.867452742@linuxfoundation.org>

6.6-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>

commit e6af9c98cbf0164a619d95572136bfb54d482dd6 upstream.

In the case of returning -ENOSPC, ensure logflagsp is initialized by 0.
Otherwise the caller __xfs_bunmapi will set uninitialized illegal
tmp_logflags value into xfs log, which might cause unpredictable error
in the log recovery procedure.

Also, remove the flags variable and set the *logflagsp directly, so that
the code should be more robust in the long run.

Fixes: 1b24b633aafe ("xfs: move some more code into xfs_bmap_del_extent_real")
Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/xfs/libxfs/xfs_bmap.c |   73 +++++++++++++++++++----------------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5014,7 +5014,6 @@ xfs_bmap_del_extent_real(
 	xfs_fileoff_t		del_endoff;	/* first offset past del */
 	int			do_fx;	/* free extent at end of routine */
 	int			error;	/* error return value */
-	int			flags = 0;/* inode logging flags */
 	struct xfs_bmbt_irec	got;	/* current extent entry */
 	xfs_fileoff_t		got_endoff;	/* first offset past got */
 	int			i;	/* temp state */
@@ -5027,6 +5026,8 @@ xfs_bmap_del_extent_real(
 	uint32_t		state = xfs_bmap_fork_to_state(whichfork);
 	struct xfs_bmbt_irec	old;
 
+	*logflagsp = 0;
+
 	mp = ip->i_mount;
 	XFS_STATS_INC(mp, xs_del_exlist);
 
@@ -5039,7 +5040,6 @@ xfs_bmap_del_extent_real(
 	ASSERT(got_endoff >= del_endoff);
 	ASSERT(!isnullstartblock(got.br_startblock));
 	qfield = 0;
-	error = 0;
 
 	/*
 	 * If it's the case where the directory code is running with no block
@@ -5055,13 +5055,13 @@ xfs_bmap_del_extent_real(
 	    del->br_startoff > got.br_startoff && del_endoff < got_endoff)
 		return -ENOSPC;
 
-	flags = XFS_ILOG_CORE;
+	*logflagsp = XFS_ILOG_CORE;
 	if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
 		if (!(bflags & XFS_BMAPI_REMAP)) {
 			error = xfs_rtfree_blocks(tp, del->br_startblock,
 					del->br_blockcount);
 			if (error)
-				goto done;
+				return error;
 		}
 
 		do_fx = 0;
@@ -5076,11 +5076,9 @@ xfs_bmap_del_extent_real(
 	if (cur) {
 		error = xfs_bmbt_lookup_eq(cur, &got, &i);
 		if (error)
-			goto done;
-		if (XFS_IS_CORRUPT(mp, i != 1)) {
-			error = -EFSCORRUPTED;
-			goto done;
-		}
+			return error;
+		if (XFS_IS_CORRUPT(mp, i != 1))
+			return -EFSCORRUPTED;
 	}
 
 	if (got.br_startoff == del->br_startoff)
@@ -5097,17 +5095,15 @@ xfs_bmap_del_extent_real(
 		xfs_iext_prev(ifp, icur);
 		ifp->if_nextents--;
 
-		flags |= XFS_ILOG_CORE;
+		*logflagsp |= XFS_ILOG_CORE;
 		if (!cur) {
-			flags |= xfs_ilog_fext(whichfork);
+			*logflagsp |= xfs_ilog_fext(whichfork);
 			break;
 		}
 		if ((error = xfs_btree_delete(cur, &i)))
-			goto done;
-		if (XFS_IS_CORRUPT(mp, i != 1)) {
-			error = -EFSCORRUPTED;
-			goto done;
-		}
+			return error;
+		if (XFS_IS_CORRUPT(mp, i != 1))
+			return -EFSCORRUPTED;
 		break;
 	case BMAP_LEFT_FILLING:
 		/*
@@ -5118,12 +5114,12 @@ xfs_bmap_del_extent_real(
 		got.br_blockcount -= del->br_blockcount;
 		xfs_iext_update_extent(ip, state, icur, &got);
 		if (!cur) {
-			flags |= xfs_ilog_fext(whichfork);
+			*logflagsp |= xfs_ilog_fext(whichfork);
 			break;
 		}
 		error = xfs_bmbt_update(cur, &got);
 		if (error)
-			goto done;
+			return error;
 		break;
 	case BMAP_RIGHT_FILLING:
 		/*
@@ -5132,12 +5128,12 @@ xfs_bmap_del_extent_real(
 		got.br_blockcount -= del->br_blockcount;
 		xfs_iext_update_extent(ip, state, icur, &got);
 		if (!cur) {
-			flags |= xfs_ilog_fext(whichfork);
+			*logflagsp |= xfs_ilog_fext(whichfork);
 			break;
 		}
 		error = xfs_bmbt_update(cur, &got);
 		if (error)
-			goto done;
+			return error;
 		break;
 	case 0:
 		/*
@@ -5154,18 +5150,18 @@ xfs_bmap_del_extent_real(
 		new.br_state = got.br_state;
 		new.br_startblock = del_endblock;
 
-		flags |= XFS_ILOG_CORE;
+		*logflagsp |= XFS_ILOG_CORE;
 		if (cur) {
 			error = xfs_bmbt_update(cur, &got);
 			if (error)
-				goto done;
+				return error;
 			error = xfs_btree_increment(cur, 0, &i);
 			if (error)
-				goto done;
+				return error;
 			cur->bc_rec.b = new;
 			error = xfs_btree_insert(cur, &i);
 			if (error && error != -ENOSPC)
-				goto done;
+				return error;
 			/*
 			 * If get no-space back from btree insert, it tried a
 			 * split, and we have a zero block reservation.  Fix up
@@ -5178,33 +5174,28 @@ xfs_bmap_del_extent_real(
 				 */
 				error = xfs_bmbt_lookup_eq(cur, &got, &i);
 				if (error)
-					goto done;
-				if (XFS_IS_CORRUPT(mp, i != 1)) {
-					error = -EFSCORRUPTED;
-					goto done;
-				}
+					return error;
+				if (XFS_IS_CORRUPT(mp, i != 1))
+					return -EFSCORRUPTED;
 				/*
 				 * Update the btree record back
 				 * to the original value.
 				 */
 				error = xfs_bmbt_update(cur, &old);
 				if (error)
-					goto done;
+					return error;
 				/*
 				 * Reset the extent record back
 				 * to the original value.
 				 */
 				xfs_iext_update_extent(ip, state, icur, &old);
-				flags = 0;
-				error = -ENOSPC;
-				goto done;
-			}
-			if (XFS_IS_CORRUPT(mp, i != 1)) {
-				error = -EFSCORRUPTED;
-				goto done;
+				*logflagsp = 0;
+				return -ENOSPC;
 			}
+			if (XFS_IS_CORRUPT(mp, i != 1))
+				return -EFSCORRUPTED;
 		} else
-			flags |= xfs_ilog_fext(whichfork);
+			*logflagsp |= xfs_ilog_fext(whichfork);
 
 		ifp->if_nextents++;
 		xfs_iext_next(ifp, icur);
@@ -5228,7 +5219,7 @@ xfs_bmap_del_extent_real(
 					((bflags & XFS_BMAPI_NODISCARD) ||
 					del->br_state == XFS_EXT_UNWRITTEN));
 			if (error)
-				goto done;
+				return error;
 		}
 	}
 
@@ -5243,9 +5234,7 @@ xfs_bmap_del_extent_real(
 	if (qfield && !(bflags & XFS_BMAPI_REMAP))
 		xfs_trans_mod_dquot_byino(tp, ip, qfield, (long)-nblks);
 
-done:
-	*logflagsp = flags;
-	return error;
+	return 0;
 }
 
 /*



  parent reply	other threads:[~2024-04-01 16:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240401152547.867452742@linuxfoundation.org>
2024-04-01 15:45 ` [PATCH 6.6 253/396] xfs: move the xfs_rtbitmap.c declarations to xfs_rtbitmap.h Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 254/396] xfs: convert rt bitmap extent lengths to xfs_rtbxlen_t Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 255/396] xfs: consider minlen sized extents in xfs_rtallocate_extent_block Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 256/396] xfs: dont leak recovered attri intent items Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 257/396] xfs: use xfs_defer_pending objects to recover " Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 258/396] xfs: pass the xfs_defer_pending object to iop_recover Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 259/396] xfs: transfer recovered intent item ownership in ->iop_recover Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 260/396] xfs: make rextslog computation consistent with mkfs Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 261/396] xfs: fix 32-bit truncation in xfs_compute_rextslog Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 262/396] xfs: dont allow overly small or large realtime volumes Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 263/396] xfs: make xchk_iget safer in the presence of corrupt inode btrees Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 264/396] xfs: remove unused fields from struct xbtree_ifakeroot Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 265/396] xfs: recompute growfsrtfree transaction reservation while growing rt volume Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 266/396] xfs: fix an off-by-one error in xreap_agextent_binval Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 267/396] xfs: force all buffers to be written during btree bulk load Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 268/396] xfs: add missing nrext64 inode flag check to scrub Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 269/396] xfs: initialise di_crc in xfs_log_dinode Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 270/396] xfs: short circuit xfs_growfs_data_private() if delta is zero Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 271/396] xfs: add lock protection when remove perag from radix tree Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 272/396] xfs: fix perag leak when growfs fails Greg Kroah-Hartman
2024-04-01 15:45 ` Greg Kroah-Hartman [this message]
2024-04-01 15:45 ` [PATCH 6.6 274/396] xfs: update dir3 leaf block metadata after swap Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 275/396] xfs: reset XFS_ATTR_INCOMPLETE filter on node removal Greg Kroah-Hartman
2024-04-01 15:45 ` [PATCH 6.6 276/396] xfs: remove conditional building of rt geometry validator functions Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240401152556.051182296@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=catherine.hoang@oracle.com \
    --cc=chandanbabu@kernel.org \
    --cc=djwong@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-xfs@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=zhangjiachen.jaycee@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox