Commit e6af9c98 authored by Jiachen Zhang's avatar Jiachen Zhang Committed by Chandan Babu R
Browse files

xfs: ensure logflagsp is initialized in xfs_bmap_del_extent_real 



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: 1b24b633 ("xfs: move some more code into xfs_bmap_del_extent_real")
Signed-off-by: default avatarJiachen Zhang <zhangjiachen.jaycee@bytedance.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatar"Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: default avatarChandan Babu R <chandanbabu@kernel.org>
parent 08e54ca4

fs/xfs/libxfs/xfs_bmap.c

+31 −42
Original line number Diff line number Diff line
@@ -5010,7 +5010,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 */
@@ -5023,6 +5022,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);
@@ -5035,7 +5036,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
@@ -5051,13 +5051,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;
@@ -5072,11 +5072,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)
@@ -5093,17 +5091,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:

		/*
@@ -5114,12 +5110,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:

		/*
@@ -5128,12 +5124,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:

		/*
@@ -5150,18 +5146,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
@@ -5174,33 +5170,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);
@@ -5224,7 +5215,7 @@ xfs_bmap_del_extent_real( 
					((bflags & XFS_BMAPI_NODISCARD) ||

					del->br_state == XFS_EXT_UNWRITTEN));

			if (error)

				goto done;

				return error;

		}

	}
@@ -5239,9 +5230,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;

}



/*