xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: [PATCH 5/5] repair: prevent blkmap extent count overflows
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 10 Oct 2011 10:11:50 +1100
In-reply-to: <1318201910-11144-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1318201910-11144-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

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@xxxxxxxxxx>
---
 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 3e53457..a6a041a 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

<Prev in Thread] Current Thread [Next in Thread>