public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <aelder@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 5/5] repair: prevent blkmap extent count overflows
Date: Thu, 13 Oct 2011 04:58:00 -0500	[thread overview]
Message-ID: <1318499880.3172.13.camel@doink> (raw)
In-Reply-To: <1318208915-14975-6-git-send-email-david@fromorbit.com>

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>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  parent reply	other threads:[~2011-10-13  9:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-10  1:08 [PATCH 0/5, v3] repair: sector size and blkmap fixes Dave Chinner
2011-10-10  1:08 ` [PATCH 1/5] repair: handle repair of image files on large sector size filesystems Dave Chinner
2011-10-10 14:06   ` Christoph Hellwig
2011-10-13  9:57   ` Alex Elder
2011-10-10  1:08 ` [PATCH 2/5] repair: fix a valgrind reported error on i686 Dave Chinner
2011-10-10 14:07   ` Christoph Hellwig
2011-10-13  9:57   ` Alex Elder
2011-10-10  1:08 ` [PATCH 3/5] repair: handle memory allocation failure from blkmap_grow Dave Chinner
2011-10-10 14:14   ` Christoph Hellwig
2011-10-13  9:57   ` Alex Elder
2011-10-10  1:08 ` [PATCH 4/5] repair: don't cache large blkmap allocations Dave Chinner
2011-10-10 14:15   ` Christoph Hellwig
2011-10-13  9:57   ` Alex Elder
2011-10-10  1:08 ` [PATCH 5/5] repair: prevent blkmap extent count overflows Dave Chinner
2011-10-10 14:17   ` Christoph Hellwig
2011-10-13  9:58   ` Alex Elder [this message]
2011-10-10 14:18 ` [PATCH 0/5, v3] repair: sector size and blkmap fixes Christoph Hellwig
2011-10-10 23:12   ` Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
2011-10-09 23:11 [PATCH 0/5, v2] " Dave Chinner
2011-10-09 23:11 ` [PATCH 5/5] repair: prevent blkmap extent count overflows Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1318499880.3172.13.camel@doink \
    --to=aelder@sgi.com \
    --cc=david@fromorbit.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox