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

Alex Elder
2011-10-13T09:58:05Z

On Mon, 2011-10-10 at 12:08 +1100, Dave Chinner wrote:
> 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>

I still prefer checking for overflow *before* adding
but it's just not that important.

This looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>