[Top] [All Lists]

Re: Possible memory corruption in xfs_buf_iomove()

To: Dale Stephenson <dalestephenson@xxxxxxx>
Subject: Re: Possible memory corruption in xfs_buf_iomove()
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 1 Feb 2011 09:51:31 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <86FBE6A1-CDCD-4FA3-B389-8D8C75A67CAB@xxxxxxx>
References: <86FBE6A1-CDCD-4FA3-B389-8D8C75A67CAB@xxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Mon, Jan 31, 2011 at 04:29:16PM -0500, Dale Stephenson wrote:
> At Snap Appliance, we were running into appeared to be random oops
> when setting/reading large attributes.  Trevor Heathorn was able to
> trace the problem down to the function xfs_buf_iomove() in
> linux-2.6-xfs/xfs_buf.c.  The size of the copy within the loop is
> calculated as a minimum of the page size and bp->b_count_desired,
> minus the applicable offsets.  This can result in a copy size
> greater than the remaining size of the data length, which in the
> case of a read caused whatever else is in the buffer to be written
> past the end of the data area we had allocated.  He found that this
> fix prevented the oops:

Good catch! Do you have a test case that reproduced the problem? If
so, can you post that, too, so we can push it into xfstests as a
regression test?

[ As an aside, if you've got other regression tests that you'd like
to have run regularly during mainline developement and can be
integrated into xfstests, we can help make that happen. ]

> --- xfs_buf.c 17 Sep 2009 00:19:49 -0000      1.4
> +++ xfs_buf.c 31 Jan 2011 20:22:59 -0000      1.5
> @@ -1313,6 +1313,7 @@
>                             PAGE_CACHE_SIZE-cpoff, bp->b_count_desired-boff);
>               ASSERT(((csize + cpoff) <= PAGE_CACHE_SIZE));
> +             csize = min(csize, (bend - boff));
>               switch (mode) {
>               case XBRW_ZERO:

Yup, definitely needed. Can you add a Signed-off-by tag to this with
an appropriate commit message (pretty much what you've already
included with this report), we can get this fix into mainline


Dave Chinner

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