From: Dave Chinner <dchinner@redhat.com>
Date: Mon, 10 Oct 2011 01:08:33 +0000 (+0000)
Subject: repair: handle memory allocation failure from blkmap_grow
X-Git-Tag: v3.1.6~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=75372fed044c75742958af3df651d2b1b1b61eae;p=thirdparty%2Fxfsprogs-dev.git

repair: handle memory allocation failure from blkmap_grow

If blkmap_grow fails to allocate a new chunk of memory, it returns
with a null blkmap. The sole caller of blkmap_grow does not check
for this failure, and so will segfault if this error ever occurs.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>
---

diff --git a/repair/bmap.c b/repair/bmap.c
index 0ff9315b1..e29051569 100644
--- a/repair/bmap.c
+++ b/repair/bmap.c
@@ -207,29 +207,34 @@ blkmap_next_off(
  */
 static blkmap_t *
 blkmap_grow(
-	blkmap_t	**blkmapp)
+	blkmap_t	*blkmap)
 {
 	pthread_key_t	key = dblkmap_key;
-	blkmap_t	*blkmap = *blkmapp;
+	blkmap_t	*new_blkmap;
+	int		new_naexts = blkmap->naexts + 4;
 
 	if (pthread_getspecific(key) != blkmap) {
 		key = ablkmap_key;
 		ASSERT(pthread_getspecific(key) == blkmap);
 	}
 
-	blkmap->naexts += 4;
-	blkmap = realloc(blkmap, BLKMAP_SIZE(blkmap->naexts));
-	if (blkmap == NULL)
+	new_blkmap = realloc(blkmap, BLKMAP_SIZE(new_naexts));
+	if (!new_blkmap) {
 		do_error(_("realloc failed in blkmap_grow\n"));
-	*blkmapp = blkmap;
-	pthread_setspecific(key, blkmap);
-	return blkmap;
+		return NULL;
+	}
+	new_blkmap->naexts = new_naexts;
+	pthread_setspecific(key, new_blkmap);
+	return new_blkmap;
 }
 
 /*
  * Set an extent into a block map.
+ *
+ * If this function fails, it leaves the blkmapp untouched so the caller can
+ * handle the error and free the blkmap appropriately.
  */
-void
+int
 blkmap_set_ext(
 	blkmap_t	**blkmapp,
 	xfs_dfiloff_t	o,
@@ -239,9 +244,14 @@ blkmap_set_ext(
 	blkmap_t	*blkmap = *blkmapp;
 	xfs_extnum_t	i;
 
-	if (blkmap->nexts == blkmap->naexts)
-		blkmap = blkmap_grow(blkmapp);
+	if (blkmap->nexts == blkmap->naexts) {
+		blkmap = blkmap_grow(blkmap);
+		if (!blkmap)
+			return ENOMEM;
+		*blkmapp = blkmap;
+	}
 
+	ASSERT(blkmap->nexts < blkmap->naexts);
 	for (i = 0; i < blkmap->nexts; i++) {
 		if (blkmap->exts[i].startoff > o) {
 			memmove(blkmap->exts + i + 1,
@@ -255,4 +265,5 @@ blkmap_set_ext(
 	blkmap->exts[i].startblock = b;
 	blkmap->exts[i].blockcount = c;
 	blkmap->nexts++;
+	return 0;
 }
diff --git a/repair/bmap.h b/repair/bmap.h
index 58abf95fd..118ae1efc 100644
--- a/repair/bmap.h
+++ b/repair/bmap.h
@@ -43,7 +43,7 @@ typedef	struct blkmap {
 blkmap_t	*blkmap_alloc(xfs_extnum_t nex, int whichfork);
 void		blkmap_free(blkmap_t *blkmap);
 
-void		blkmap_set_ext(blkmap_t **blkmapp, xfs_dfiloff_t o,
+int		blkmap_set_ext(blkmap_t **blkmapp, xfs_dfiloff_t o,
 			       xfs_dfsbno_t b, xfs_dfilblks_t c);
 
 xfs_dfsbno_t	blkmap_get(blkmap_t *blkmap, xfs_dfiloff_t o);
diff --git a/repair/dinode.c b/repair/dinode.c
index 5a7453832..39a0cb1eb 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -730,9 +730,27 @@ _("inode %" PRIu64 " - extent offset too large - start %" PRIu64 ", "
 			goto done;
 		}
 
-		if (blkmapp && *blkmapp)
-			blkmap_set_ext(blkmapp, irec.br_startoff,
+		if (blkmapp && *blkmapp) {
+			error = blkmap_set_ext(blkmapp, irec.br_startoff,
 					irec.br_startblock, irec.br_blockcount);
+			if (error) {
+				/*
+				 * we don't want to clear the inode due to an
+				 * internal bmap tracking error, but if we've
+				 * run out of memory then we simply can't
+				 * validate that the filesystem is consistent.
+				 * Hence just abort at this point with an ENOMEM
+				 * error.
+				 */
+				do_abort(
+_("Fatal error: inode %" PRIu64 " - blkmap_set_ext(): %s\n"
+  "\t%s fork, off - %" PRIu64 ", start - %" PRIu64 ", cnt %" PRIu64 "\n"),
+					ino, strerror(error), forkname,
+					irec.br_startoff, irec.br_startblock,
+					irec.br_blockcount);
+			}
+		}
+
 		/*
 		 * Profiling shows that the following loop takes the
 		 * most time in all of xfs_repair.