Commit 0e1edbd9 authored by Nathan Scott's avatar Nathan Scott
Browse files

[XFS] Fix xfs_free_extent related NULL pointer dereference. 



We recently fixed an out-of-space deadlock in XFS, and part of that fix
involved the addition of the XFS_ALLOC_FLAG_FREEING flag to some of the
space allocator calls to indicate they're freeing space, not allocating
it. There was a missed xfs_alloc_fix_freelist condition test that did not
correctly test "flags". The same test would also test an uninitialised
structure field (args->userdata) and depending on its value either would
or would not return early with a critical buffer pointer set to NULL.

This fixes that up, adds asserts to several places to catch future botches
of this nature, and skips sections of xfs_alloc_fix_freelist that are
irrelevent for the space-freeing case.

SGI-PV: 955303
SGI-Modid: xfs-linux-melb:xfs-kern:26743a

Signed-off-by: default avatarNathan Scott <nathans@sgi.com>
parent 9f737633

fs/xfs/xfs_alloc.c

+54 −49
Original line number Diff line number Diff line
@@ -1835,21 +1835,27 @@ xfs_alloc_fix_freelist( 
				&agbp)))

			return error;

		if (!pag->pagf_init) {

			ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);

			ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));

			args->agbp = NULL;

			return 0;

		}

	} else

		agbp = NULL;



	/* If this is a metadata preferred pag and we are user data

	/*

	 * If this is a metadata preferred pag and we are user data

	 * then try somewhere else if we are not being asked to

	 * try harder at this point

	 */

	if (pag->pagf_metadata && args->userdata && flags) {

	if (pag->pagf_metadata && args->userdata &&

	    (flags & XFS_ALLOC_FLAG_TRYLOCK)) {

		ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));

		args->agbp = NULL;

		return 0;

	}



	if (!(flags & XFS_ALLOC_FLAG_FREEING)) {

		need = XFS_MIN_FREELIST_PAG(pag, mp);

		delta = need > pag->pagf_flcount ? need - pag->pagf_flcount : 0;

		/*
@@ -1859,16 +1865,17 @@ xfs_alloc_fix_freelist( 
		longest = (pag->pagf_longest > delta) ?

			(pag->pagf_longest - delta) :

			(pag->pagf_flcount > 0 || pag->pagf_longest > 0);

	if (args->minlen + args->alignment + args->minalignslop - 1 > longest ||

	    (!(flags & XFS_ALLOC_FLAG_FREEING) &&

	     (int)(pag->pagf_freeblks + pag->pagf_flcount -

		   need - args->total) <

	     (int)args->minleft)) {

		if ((args->minlen + args->alignment + args->minalignslop - 1) >

				longest ||

		    ((int)(pag->pagf_freeblks + pag->pagf_flcount -

			   need - args->total) < (int)args->minleft)) {

			if (agbp)

				xfs_trans_brelse(tp, agbp);

			args->agbp = NULL;

			return 0;

		}

	}



	/*

	 * Get the a.g. freespace buffer.

	 * Can fail if we're not blocking on locks, and it's held.
@@ -1878,6 +1885,8 @@ xfs_alloc_fix_freelist( 
				&agbp)))

			return error;

		if (agbp == NULL) {

			ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);

			ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));

			args->agbp = NULL;

			return 0;

		}
@@ -1887,23 +1896,25 @@ xfs_alloc_fix_freelist( 
	 */

	agf = XFS_BUF_TO_AGF(agbp);

	need = XFS_MIN_FREELIST(agf, mp);

	delta = need > be32_to_cpu(agf->agf_flcount) ?

		(need - be32_to_cpu(agf->agf_flcount)) : 0;

	/*

	 * If there isn't enough total or single-extent, reject it.

	 */

	if (!(flags & XFS_ALLOC_FLAG_FREEING)) {

		delta = need > be32_to_cpu(agf->agf_flcount) ?

			(need - be32_to_cpu(agf->agf_flcount)) : 0;

		longest = be32_to_cpu(agf->agf_longest);

		longest = (longest > delta) ? (longest - delta) :

			(be32_to_cpu(agf->agf_flcount) > 0 || longest > 0);

	if (args->minlen + args->alignment + args->minalignslop - 1 > longest ||

	     (!(flags & XFS_ALLOC_FLAG_FREEING) &&

		(int)(be32_to_cpu(agf->agf_freeblks) +

		if ((args->minlen + args->alignment + args->minalignslop - 1) >

				longest ||

		    ((int)(be32_to_cpu(agf->agf_freeblks) +

		     be32_to_cpu(agf->agf_flcount) - need - args->total) <

				(int)args->minleft)) {

			xfs_trans_brelse(tp, agbp);

			args->agbp = NULL;

			return 0;

		}

	}

	/*

	 * Make the freelist shorter if it's too long.

	 */
@@ -1950,13 +1961,12 @@ xfs_alloc_fix_freelist( 
		 * on a completely full ag.

		 */

		if (targs.agbno == NULLAGBLOCK) {

			if (!(flags & XFS_ALLOC_FLAG_FREEING)) {

			if (flags & XFS_ALLOC_FLAG_FREEING)

				break;

			xfs_trans_brelse(tp, agflbp);

			args->agbp = NULL;

			return 0;

		}

			break;

		}

		/*

		 * Put each allocated block on the list.

		 */
@@ -2442,31 +2452,26 @@ xfs_free_extent( 
	xfs_fsblock_t	bno,	/* starting block number of extent */

	xfs_extlen_t	len)	/* length of extent */

{

#ifdef DEBUG

	xfs_agf_t	*agf;	/* a.g. freespace header */

#endif

	xfs_alloc_arg_t	args;	/* allocation argument structure */

	xfs_alloc_arg_t	args;

	int		error;



	ASSERT(len != 0);

	memset(&args, 0, sizeof(xfs_alloc_arg_t));

	args.tp = tp;

	args.mp = tp->t_mountp;

	args.agno = XFS_FSB_TO_AGNO(args.mp, bno);

	ASSERT(args.agno < args.mp->m_sb.sb_agcount);

	args.agbno = XFS_FSB_TO_AGBNO(args.mp, bno);

	args.alignment = 1;

	args.minlen = args.minleft = args.minalignslop = 0;

	down_read(&args.mp->m_peraglock);

	args.pag = &args.mp->m_perag[args.agno];

	if ((error = xfs_alloc_fix_freelist(&args, XFS_ALLOC_FLAG_FREEING)))

		goto error0;

#ifdef DEBUG

	ASSERT(args.agbp != NULL);

	agf = XFS_BUF_TO_AGF(args.agbp);

	ASSERT(args.agbno + len <= be32_to_cpu(agf->agf_length));

	ASSERT((args.agbno + len) <=

		be32_to_cpu(XFS_BUF_TO_AGF(args.agbp)->agf_length));

#endif

	error = xfs_free_ag_extent(tp, args.agbp, args.agno, args.agbno,

		len, 0);

	error = xfs_free_ag_extent(tp, args.agbp, args.agno, args.agbno, len, 0);

error0:

	up_read(&args.mp->m_peraglock);

	return error;

CRA Git | Maintained and supported by SUSTech CRA and CCSE