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: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Bill O'Donnell <billodo@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 9b9540db17a6132472dc35c9ec2edaa5ace0b30d..1f1e4ae441504bd60af93244eebd485a433330ce 100644 (file)
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -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);
        }