[PATCH 5/5] repair: prevent blkmap extent count overflows

Dave Chinner
2011-10-10T01:08:45Z

From: Dave Chinner <dchinner@redhat.com>

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>
---
repair/bmap.c | 28 ++++++++++++++++++++++++++++
repair/bmap.h | 13 +++++++++++++
2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/repair/bmap.c b/repair/bmap.c
index 5fb27bc..2f1c307 100644
--- 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 118ae1e..19720b1 100644
--- 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);

--
1.7.5.4