Commit 7dfee17b authored by Dave Chinner's avatar Dave Chinner Committed by Dave Chinner
Browse files

xfs: validate block number being freed before adding to xefi 



Bad things happen in defered extent freeing operations if it is
passed a bad block number in the xefi. This can come from a bogus
agno/agbno pair from deferred agfl freeing, or just a bad fsbno
being passed to __xfs_free_extent_later(). Either way, it's very
difficult to diagnose where a null perag oops in EFI creation
is coming from when the operation that queued the xefi has already
been completed and there's no longer any trace of it around....

Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDarrick J. Wong <djwong@kernel.org>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent 3148ebf2

fs/xfs/libxfs/xfs_ag.c

+4 −1
Original line number Diff line number Diff line
@@ -984,7 +984,10 @@ xfs_ag_shrink_space( 
		if (err2 != -ENOSPC)

			goto resv_err;



		__xfs_free_extent_later(*tpp, args.fsbno, delta, NULL, true);

		err2 = __xfs_free_extent_later(*tpp, args.fsbno, delta, NULL,

				true);

		if (err2)

			goto resv_err;



		/*

		 * Roll the transaction before trying to re-init the per-ag

fs/xfs/libxfs/xfs_alloc.c

+13 −3
Original line number Diff line number Diff line
@@ -2431,7 +2431,7 @@ xfs_agfl_reset( 
 * the real allocation can proceed. Deferring the free disconnects freeing up

 * the AGFL slot from freeing the block.

 */

STATIC void

static int

xfs_defer_agfl_block(

	struct xfs_trans		*tp,

	xfs_agnumber_t			agno,
@@ -2450,17 +2450,21 @@ xfs_defer_agfl_block( 
	xefi->xefi_blockcount = 1;

	xefi->xefi_owner = oinfo->oi_owner;



	if (XFS_IS_CORRUPT(mp, !xfs_verify_fsbno(mp, xefi->xefi_startblock)))

		return -EFSCORRUPTED;



	trace_xfs_agfl_free_defer(mp, agno, 0, agbno, 1);



	xfs_extent_free_get_group(mp, xefi);

	xfs_defer_add(tp, XFS_DEFER_OPS_TYPE_AGFL_FREE, &xefi->xefi_list);

	return 0;

}



/*

 * Add the extent to the list of extents to be free at transaction end.

 * The list is maintained sorted (by block number).

 */

void

int

__xfs_free_extent_later(

	struct xfs_trans		*tp,

	xfs_fsblock_t			bno,
@@ -2487,6 +2491,9 @@ __xfs_free_extent_later( 
#endif

	ASSERT(xfs_extfree_item_cache != NULL);



	if (XFS_IS_CORRUPT(mp, !xfs_verify_fsbext(mp, bno, len)))

		return -EFSCORRUPTED;



	xefi = kmem_cache_zalloc(xfs_extfree_item_cache,

			       GFP_KERNEL | __GFP_NOFAIL);

	xefi->xefi_startblock = bno;
@@ -2510,6 +2517,7 @@ __xfs_free_extent_later( 


	xfs_extent_free_get_group(mp, xefi);

	xfs_defer_add(tp, XFS_DEFER_OPS_TYPE_FREE, &xefi->xefi_list);

	return 0;

}



#ifdef DEBUG
@@ -2670,7 +2678,9 @@ xfs_alloc_fix_freelist( 
			goto out_agbp_relse;



		/* defer agfl frees */

		xfs_defer_agfl_block(tp, args->agno, bno, &targs.oinfo);

		error = xfs_defer_agfl_block(tp, args->agno, bno, &targs.oinfo);

		if (error)

			goto out_agbp_relse;

	}



	targs.tp = tp;

fs/xfs/libxfs/xfs_alloc.h

+3 −3
Original line number Diff line number Diff line
@@ -230,7 +230,7 @@ xfs_buf_to_agfl_bno( 
	return bp->b_addr;

}



void __xfs_free_extent_later(struct xfs_trans *tp, xfs_fsblock_t bno,

int __xfs_free_extent_later(struct xfs_trans *tp, xfs_fsblock_t bno,

		xfs_filblks_t len, const struct xfs_owner_info *oinfo,

		bool skip_discard);
@@ -254,14 +254,14 @@ void xfs_extent_free_get_group(struct xfs_mount *mp, 
#define XFS_EFI_ATTR_FORK	(1U << 1) /* freeing attr fork block */

#define XFS_EFI_BMBT_BLOCK	(1U << 2) /* freeing bmap btree block */



static inline void

static inline int

xfs_free_extent_later(

	struct xfs_trans		*tp,

	xfs_fsblock_t			bno,

	xfs_filblks_t			len,

	const struct xfs_owner_info	*oinfo)

{

	__xfs_free_extent_later(tp, bno, len, oinfo, false);

	return __xfs_free_extent_later(tp, bno, len, oinfo, false);

}

fs/xfs/libxfs/xfs_bmap.c

+8 −2
Original line number Diff line number Diff line
@@ -572,8 +572,12 @@ xfs_bmap_btree_to_extents( 
	cblock = XFS_BUF_TO_BLOCK(cbp);

	if ((error = xfs_btree_check_block(cur, cblock, 0, cbp)))

		return error;



	xfs_rmap_ino_bmbt_owner(&oinfo, ip->i_ino, whichfork);

	xfs_free_extent_later(cur->bc_tp, cbno, 1, &oinfo);

	error = xfs_free_extent_later(cur->bc_tp, cbno, 1, &oinfo);

	if (error)

		return error;



	ip->i_nblocks--;

	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, -1L);

	xfs_trans_binval(tp, cbp);
@@ -5230,10 +5234,12 @@ xfs_bmap_del_extent_real( 
		if (xfs_is_reflink_inode(ip) && whichfork == XFS_DATA_FORK) {

			xfs_refcount_decrease_extent(tp, del);

		} else {

			__xfs_free_extent_later(tp, del->br_startblock,

			error = __xfs_free_extent_later(tp, del->br_startblock,

					del->br_blockcount, NULL,

					(bflags & XFS_BMAPI_NODISCARD) ||

					del->br_state == XFS_EXT_UNWRITTEN);

			if (error)

				goto done;

		}

	}

fs/xfs/libxfs/xfs_bmap_btree.c

+5 −2
Original line number Diff line number Diff line
@@ -268,11 +268,14 @@ xfs_bmbt_free_block( 
	struct xfs_trans	*tp = cur->bc_tp;

	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));

	struct xfs_owner_info	oinfo;

	int			error;



	xfs_rmap_ino_bmbt_owner(&oinfo, ip->i_ino, cur->bc_ino.whichfork);

	xfs_free_extent_later(cur->bc_tp, fsbno, 1, &oinfo);

	ip->i_nblocks--;

	error = xfs_free_extent_later(cur->bc_tp, fsbno, 1, &oinfo);

	if (error)

		return error;



	ip->i_nblocks--;

	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);

	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, -1L);

	return 0;