Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d4a97a04 authored by Brian Foster's avatar Brian Foster Committed by Dave Chinner
Browse files

xfs: add missing bmap cancel calls in error paths 



If a failure occurs after the bmap free list is populated and before
xfs_bmap_finish() completes successfully (which returns a partial
list on failure), the bmap free list must be cancelled. Otherwise,
the extent items on the list are never freed and a memory leak
occurs.

Several random error paths throughout the code suffer this problem.
Fix these up such that xfs_bmap_cancel() is always called on error.

Signed-off-by: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent 146e54b7

fs/xfs/libxfs/xfs_bmap.c

+1 −0
Original line number Diff line number Diff line
@@ -5945,6 +5945,7 @@ xfs_bmap_split_extent( 
	return xfs_trans_commit(tp);



out:

	xfs_bmap_cancel(&free_list);

	xfs_trans_cancel(tp);

	return error;

}

fs/xfs/xfs_bmap_util.c

+6 −4
Original line number Diff line number Diff line
@@ -1474,7 +1474,7 @@ xfs_shift_file_space( 
				XFS_DIOSTRAT_SPACE_RES(mp, 0), 0,

				XFS_QMOPT_RES_REGBLKS);

		if (error)

			goto out;

			goto out_trans_cancel;



		xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
@@ -1488,18 +1488,20 @@ xfs_shift_file_space( 
				&done, stop_fsb, &first_block, &free_list,

				direction, XFS_BMAP_MAX_SHIFT_EXTENTS);

		if (error)

			goto out;

			goto out_bmap_cancel;



		error = xfs_bmap_finish(&tp, &free_list, &committed);

		if (error)

			goto out;

			goto out_bmap_cancel;



		error = xfs_trans_commit(tp);

	}



	return error;



out:

out_bmap_cancel:

	xfs_bmap_cancel(&free_list);

out_trans_cancel:

	xfs_trans_cancel(tp);

	return error;

}

fs/xfs/xfs_inode.c

+5 −4
Original line number Diff line number Diff line
@@ -1791,14 +1791,15 @@ xfs_inactive_ifree( 
	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_ICOUNT, -1);



	/*

	 * Just ignore errors at this point.  There is nothing we can

	 * do except to try to keep going. Make sure it's not a silent

	 * error.

	 * Just ignore errors at this point.  There is nothing we can do except

	 * to try to keep going. Make sure it's not a silent error.

	 */

	error = xfs_bmap_finish(&tp,  &free_list, &committed);

	if (error)

	if (error) {

		xfs_notice(mp, "%s: xfs_bmap_finish returned error %d",

			__func__, error);

		xfs_bmap_cancel(&free_list);

	}

	error = xfs_trans_commit(tp);

	if (error)

		xfs_notice(mp, "%s: xfs_trans_commit returned error %d",

fs/xfs/xfs_rtalloc.c

+29 −28
Original line number Diff line number Diff line
@@ -757,31 +757,30 @@ xfs_rtallocate_extent_size( 
/*

 * Allocate space to the bitmap or summary file, and zero it, for growfs.

 */

STATIC int				/* error */

STATIC int

xfs_growfs_rt_alloc(

	xfs_mount_t	*mp,		/* file system mount point */

	struct xfs_mount	*mp,		/* file system mount point */

	xfs_extlen_t		oblocks,	/* old count of blocks */

	xfs_extlen_t		nblocks,	/* new count of blocks */

	xfs_inode_t	*ip)		/* inode (bitmap/summary) */

	struct xfs_inode	*ip)		/* inode (bitmap/summary) */

{

	xfs_fileoff_t		bno;		/* block number in file */

	xfs_buf_t	*bp;		/* temporary buffer for zeroing */

	struct xfs_buf		*bp;	/* temporary buffer for zeroing */

	int			committed;	/* transaction committed flag */

	xfs_daddr_t		d;		/* disk block address */

	int			error;		/* error return value */

	xfs_fsblock_t		firstblock;/* first block allocated in xaction */

	xfs_bmap_free_t	flist;		/* list of freed blocks */

	struct xfs_bmap_free	flist;		/* list of freed blocks */

	xfs_fsblock_t		fsbno;		/* filesystem block for bno */

	xfs_bmbt_irec_t	map;		/* block map output */

	struct xfs_bmbt_irec	map;		/* block map output */

	int			nmap;		/* number of block maps */

	int			resblks;	/* space reservation */

	struct xfs_trans	*tp;



	/*

	 * Allocate space to the file, as necessary.

	 */

	while (oblocks < nblocks) {

		xfs_trans_t	*tp;



		tp = xfs_trans_alloc(mp, XFS_TRANS_GROWFSRT_ALLOC);

		resblks = XFS_GROWFSRT_SPACE_RES(mp, nblocks - oblocks);

		/*
@@ -790,7 +789,7 @@ xfs_growfs_rt_alloc( 
		error = xfs_trans_reserve(tp, &M_RES(mp)->tr_growrtalloc,

					  resblks, 0);

		if (error)

			goto error_cancel;

			goto out_trans_cancel;

		/*

		 * Lock the inode.

		 */
@@ -808,16 +807,16 @@ xfs_growfs_rt_alloc( 
		if (!error && nmap < 1)

			error = -ENOSPC;

		if (error)

			goto error_cancel;

			goto out_bmap_cancel;

		/*

		 * Free any blocks freed up in the transaction, then commit.

		 */

		error = xfs_bmap_finish(&tp, &flist, &committed);

		if (error)

			goto error_cancel;

			goto out_bmap_cancel;

		error = xfs_trans_commit(tp);

		if (error)

			goto error;

			return error;

		/*

		 * Now we need to clear the allocated blocks.

		 * Do this one block per transaction, to keep it simple.
@@ -832,7 +831,7 @@ xfs_growfs_rt_alloc( 
			error = xfs_trans_reserve(tp, &M_RES(mp)->tr_growrtzero,

						  0, 0);

			if (error)

				goto error_cancel;

				goto out_trans_cancel;

			/*

			 * Lock the bitmap inode.

			 */
@@ -846,9 +845,7 @@ xfs_growfs_rt_alloc( 
				mp->m_bsize, 0);

			if (bp == NULL) {

				error = -EIO;

error_cancel:

				xfs_trans_cancel(tp);

				goto error;

				goto out_trans_cancel;

			}

			memset(bp->b_addr, 0, mp->m_sb.sb_blocksize);

			xfs_trans_log_buf(tp, bp, 0, mp->m_sb.sb_blocksize - 1);
@@ -857,16 +854,20 @@ xfs_growfs_rt_alloc( 
			 */

			error = xfs_trans_commit(tp);

			if (error)

				goto error;

				return error;

		}

		/*

		 * Go on to the next extent, if any.

		 */

		oblocks = map.br_startoff + map.br_blockcount;

	}



	return 0;



error:

out_bmap_cancel:

	xfs_bmap_cancel(&flist);

out_trans_cancel:

	xfs_trans_cancel(tp);

	return error;

}