repair: prevent blkmap extent count overflows

Fix a bunch of invalid read/write errors due to excessive blkmap
allocations when inode forks are corrupted. These show up some time
after making a blkmap allocation for 536870913 extents on i686,
which is followed some time later by a crash caused bymemory
corruption.

This blkmap allocation size overflows 32 bits in such a
way that it results in a 32 byte allocation and so access to the
second extent results in access beyond the allocated memory and
corrupts random memory.

==5419== Invalid write of size 4
==5419==    at 0x80507DA: blkmap_set_ext (bmap.c:260)
==5419==    by 0x8055CF4: process_bmbt_reclist_int (dinode.c:712)
==5419==    by 0x8056206: process_bmbt_reclist (dinode.c:813)
==5419==    by 0x80579DA: process_exinode (dinode.c:1324)
==5419==    by 0x8059B77: process_dinode_int (dinode.c:2036)
==5419==    by 0x805ABE6: process_dinode (dinode.c:2823)
==5419==    by 0x8052493: process_inode_chunk.isra.4 (dino_chunks.c:777)
==5419==    by 0x8054012: process_aginodes (dino_chunks.c:1024)
==5419==    by 0xFFF: ???
==5419==  Address 0x944cfb8 is 0 bytes after a block of size 32 alloc'd
==5419==    at 0x48E1102: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5419==    by 0x80501F3: blkmap_alloc (bmap.c:56)
==5419==    by 0x80599F5: process_dinode_int (dinode.c:2027)
==5419==    by 0x805ABE6: process_dinode (dinode.c:2823)
==5419==    by 0x8052493: process_inode_chunk.isra.4 (dino_chunks.c:777)
==5419==    by 0x8054012: process_aginodes (dino_chunks.c:1024)
==5419==    by 0xFFF: ???

Add overflow detection code into the blkmap allocation code to avoid
this problem.

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 5fb27bcac82ce25a09e417a4ca1f1e5fec859757..2f1c307c9ac4795fe4afcc26fbcc715c4aa14e0b 100644 (file)
--- a/repair/bmap.c
+++ b/repair/bmap.c
@@ -47,6 +47,17 @@ blkmap_alloc(
        if (nex < 1)
                nex = 1;
 
+       if (nex > BLKMAP_NEXTS_MAX) {
+#if (BITS_PER_LONG == 32)
+               do_warn(
+       _("Number of extents requested in blkmap_alloc (%d) overflows 32 bits.\n"
+         "If this is not a corruption, then you will need a 64 bit system\n"
+         "to repair this filesystem.\n"),
+                       nex);
+#endif
+               return NULL;
+       }
+
        key = whichfork ? ablkmap_key : dblkmap_key;
        blkmap = pthread_getspecific(key);
        if (!blkmap || blkmap->naexts < nex) {
@@ -236,6 +247,23 @@ blkmap_grow(
                ASSERT(pthread_getspecific(key) == blkmap);
        }
 
+       if (new_naexts > BLKMAP_NEXTS_MAX) {
+#if (BITS_PER_LONG == 32)
+               do_error(
+       _("Number of extents requested in blkmap_grow (%d) overflows 32 bits.\n"
+         "You need a 64 bit system to repair this filesystem.\n"),
+                       new_naexts);
+#endif
+               return NULL;
+       }
+       if (new_naexts <= 0) {
+               do_error(
+       _("Number of extents requested in blkmap_grow (%d) overflowed the\n"
+         "maximum number of supported extents (%d).\n"),
+                       new_naexts, BLKMAP_NEXTS_MAX);
+               return NULL;
+       }
+
        new_blkmap = realloc(blkmap, BLKMAP_SIZE(new_naexts));
        if (!new_blkmap) {
                do_error(_("realloc failed in blkmap_grow\n"));

diff --git a/repair/bmap.h b/repair/bmap.h
index 118ae1efc2e7c80cea8ba82755f0ed9e420e9964..19720b1dad514f32127a082d23da7a1e1ded0122 100644 (file)
--- a/repair/bmap.h
+++ b/repair/bmap.h
@@ -40,6 +40,19 @@ typedef      struct blkmap {
 #define        BLKMAP_SIZE(n)  \
        (offsetof(blkmap_t, exts) + (sizeof(bmap_ext_t) * (n)))
 
+/*
+ * For 32 bit platforms, we are limited to extent arrays of 2^31 bytes, which
+ * limits the number of extents in an inode we can check. If we don't limit the
+ * valid range, we can overflow the BLKMAP_SIZE() calculation and allocate less
+ * memory than we think we needed, and hence walk off the end of the array and
+ * corrupt memory.
+ */
+#if BITS_PER_LONG == 32
+#define BLKMAP_NEXTS_MAX       ((INT_MAX / sizeof(bmap_ext_t)) - 1)
+#else
+#define BLKMAP_NEXTS_MAX       INT_MAX
+#endif
+
 blkmap_t       *blkmap_alloc(xfs_extnum_t nex, int whichfork);
 void           blkmap_free(blkmap_t *blkmap);