From: Dave Chinner <dchinner@xxxxxxxxxx>
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@xxxxxxxxxx>
---
repair/bmap.c | 33 ++++++++++++++++++++++-----------
repair/bmap.h | 2 +-
repair/dinode.c | 22 ++++++++++++++++++++--
3 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/repair/bmap.c b/repair/bmap.c
index 0ff9315..e290515 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 58abf95..118ae1e 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 5a74538..39a0cb1 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.
--
1.7.5.4
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
