Commit b20fe473 authored by Bill O'Donnell's avatar Bill O'Donnell Committed by Darrick J. Wong
Browse files

xfs: correct null checks and error processing in xfs_initialize_perag 



If pag cannot be allocated, the current error exit path will trip
a null pointer deference error when calling xfs_buf_hash_destroy
with a null pag.  Fix this by adding a new error exit labels and
jumping to those accordingly, avoiding the hash destroy and
unnecessary kmem_free on pag.

Up to three things need to be properly unwound:

1) pag memory allocation
2) xfs_buf_hash_init
3) radix_tree_insert

For any given iteration through the loop, any of the above which
succeed must be unwound for /this/ pag, and then all prior
initialized pags must be unwound.

Addresses-Coverity-Id: 1397628 ("Dereference after null check")

Reported-by: default avatarColin Ian King <colin.king@canonical.com>
Signed-off-by: default avatarBill O'Donnell <billodo@redhat.com>
Reviewed-by: default avatarEric Sandeen <sandeen@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent c5ecb423

fs/xfs/xfs_mount.c

+15 −9
Original line number Diff line number Diff line
@@ -187,7 +187,7 @@ xfs_initialize_perag( 
	xfs_agnumber_t	*maxagi)

{

	xfs_agnumber_t	index;

	xfs_agnumber_t	first_initialised = 0;

	xfs_agnumber_t	first_initialised = NULLAGNUMBER;

	xfs_perag_t	*pag;

	int		error = -ENOMEM;
@@ -202,22 +202,20 @@ xfs_initialize_perag( 
			xfs_perag_put(pag);

			continue;

		}

		if (!first_initialised)

			first_initialised = index;



		pag = kmem_zalloc(sizeof(*pag), KM_MAYFAIL);

		if (!pag)

			goto out_unwind;

			goto out_unwind_new_pags;

		pag->pag_agno = index;

		pag->pag_mount = mp;

		spin_lock_init(&pag->pag_ici_lock);

		mutex_init(&pag->pag_ici_reclaim_lock);

		INIT_RADIX_TREE(&pag->pag_ici_root, GFP_ATOMIC);

		if (xfs_buf_hash_init(pag))

			goto out_unwind;

			goto out_free_pag;



		if (radix_tree_preload(GFP_NOFS))

			goto out_unwind;

			goto out_hash_destroy;



		spin_lock(&mp->m_perag_lock);

		if (radix_tree_insert(&mp->m_perag_tree, index, pag)) {
@@ -225,10 +223,13 @@ xfs_initialize_perag( 
			spin_unlock(&mp->m_perag_lock);

			radix_tree_preload_end();

			error = -EEXIST;

			goto out_unwind;

			goto out_hash_destroy;

		}

		spin_unlock(&mp->m_perag_lock);

		radix_tree_preload_end();

		/* first new pag is fully initialized */

		if (first_initialised == NULLAGNUMBER)

			first_initialised = index;

	}



	index = xfs_set_inode_alloc(mp, agcount);
@@ -239,11 +240,16 @@ xfs_initialize_perag( 
	mp->m_ag_prealloc_blocks = xfs_prealloc_blocks(mp);

	return 0;



out_unwind:

out_hash_destroy:

	xfs_buf_hash_destroy(pag);

out_free_pag:

	kmem_free(pag);

	for (; index > first_initialised; index--) {

out_unwind_new_pags:

	/* unwind any prior newly initialized pags */

	for (index = first_initialised; index < agcount; index++) {

		pag = radix_tree_delete(&mp->m_perag_tree, index);

		if (!pag)

			break;

		xfs_buf_hash_destroy(pag);

		kmem_free(pag);

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE