linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Bill O'Donnell" <billodo@redhat.com>
To: linux-xfs@vger.kernel.org
Cc: darrick.wong@oracle.com
Subject: [PATCH] xfs: correct null checks and error processing in xfs_initialize_perag
Date: Sat, 28 Jan 2017 13:19:57 -0600	[thread overview]
Message-ID: <20170128191957.13851-1-billodo@redhat.com> (raw)
In-Reply-To: <20170120142642.21698-1-colin.king@canonical.com>

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.

Fixes CoverityScan CID#1397628 ("Dereference after null check")

Reported-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_mount.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 9b9540d..b074d2a 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -190,6 +190,7 @@ xfs_initialize_perag(
 	xfs_agnumber_t	first_initialised = 0;
 	xfs_perag_t	*pag;
 	int		error = -ENOMEM;
+	bool		first_init_done = false;
 
 	/*
 	 * Walk the current per-ag tree so we don't try to initialise AGs
@@ -202,22 +203,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_free_pag;
 
 		spin_lock(&mp->m_perag_lock);
 		if (radix_tree_insert(&mp->m_perag_tree, index, pag)) {
@@ -225,10 +224,15 @@ 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();
+		/* a pag is fully initialized */
+		if (!first_init_done) {
+			first_initialised = index;
+			first_init_done = true;
+		}
 	}
 
 	index = xfs_set_inode_alloc(mp, agcount);
@@ -239,13 +243,24 @@ 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--) {
-		pag = radix_tree_delete(&mp->m_perag_tree, index);
-		xfs_buf_hash_destroy(pag);
-		kmem_free(pag);
+out_unwind_new_pags:
+	/* unwind any newly initialized pags */
+	if (first_init_done) {
+		index--;
+		do {
+			pag = radix_tree_delete(&mp->m_perag_tree, index);
+			if (pag) {
+				xfs_buf_hash_destroy(pag);
+				kmem_free(pag);
+			}
+			if (!index)
+				break;
+			index--;
+		} while (index >= first_initialised);
 	}
 	return error;
 }
-- 
2.9.3


  parent reply	other threads:[~2017-01-28 19:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-20 14:26 [PATCH] xfs: do not call xfs_buf_hash_destroy on a NULL pag Colin King
2017-01-20 16:34 ` Darrick J. Wong
2017-01-20 19:26 ` Eric Sandeen
2017-01-20 20:47   ` Darrick J. Wong
2017-01-20 23:04     ` Colin Ian King
2017-01-24 15:04       ` Bill O'Donnell
2017-01-24 18:34         ` Darrick J. Wong
2017-01-24 21:08 ` [PATCH v2] " Bill O'Donnell
2017-01-24 21:21   ` Darrick J. Wong
2017-01-24 21:28     ` Bill O'Donnell
2017-01-25 19:04 ` [PATCH v3] " Bill O'Donnell
2017-01-26 17:58   ` Eric Sandeen
2017-01-26 18:55     ` Bill O'Donnell
2017-01-26 20:27       ` Eric Sandeen
2017-01-28 19:19 ` Bill O'Donnell [this message]
2017-02-03 22:57   ` [PATCH] xfs: correct null checks and error processing in xfs_initialize_perag Eric Sandeen
2017-02-06 17:08   ` [PATCH v2] " Bill O'Donnell
2017-02-06 19:25     ` Darrick J. Wong
2017-02-07 16:54   ` [PATCH v3] " Bill O'Donnell
2017-02-07 20:49     ` Eric Sandeen
2017-02-07 20:59     ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170128191957.13851-1-billodo@redhat.com \
    --to=billodo@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).