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 0ff9315b1437151095180b2397bd816e608cd2bc..e290515692cbceceb025a98962d9df8815885c44 100644 (file)
--- 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 58abf95fdab794282d226d19c1d37c5a9d186790..118ae1efc2e7c80cea8ba82755f0ed9e420e9964 100644 (file)
--- 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 5a7453832660501837804db29633a169a0329aa4..39a0cb1eb73f983a1168cea0d8bda26bed3ef4ac 100644 (file)
--- 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.