[Jaunty] SRU: Fix possible data corupption problem in ext4

[Andy Whitcroft]

On Fri, Jun 19, 2009 at 06:36:41PM +0200, Stefan Bader wrote:
> SRU Justification:
>
> Impact: As mentioned by Ted Tso in  
> https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/330824/comments/191
> there has been a data corruption issue fixed after 2.6.28.y was stopped.
>
> Fix: A cherry pick from upstream from Ted's repo.
>
> Testcase: unfortunately not clearly stated

ACK

Patch looks fine, and seems very appropriate for SRU.

-apw

>
> -- 
>
> When all other means of communication fail, try words!
>
>

> From: Theodore Ts'o <tytso at mit.edu>
> Date: Fri, 15 May 2009 13:07:28 +0000 (-0400)
> Subject: ext4: Fix race in ext4_inode_info.i_cached_extent
> 
> BugLink: https://bugs.launchpad.net/ubuntu/+bug/389555
> 
> commit 2ec0ae3acec47f628179ee95fe2c4da01b5e9fc4 upstream
> 
> backport at http://git.kernel.org/?p=linux/kernel/git/tytso/ext4.git;a=commitdiff;h=16cb5dd9f53e569130584696909d423b6fe38c1e
> 
> If two CPU's simultaneously call ext4_ext_get_blocks() at the same
> time, there is nothing protecting the i_cached_extent structure from
> being used and updated at the same time.  This could potentially cause
> the wrong location on disk to be read or written to, including
> potentially causing the corruption of the block group descriptors
> and/or inode table.
> 
> This bug has been in the ext4 code since almost the very beginning of
> ext4's development.  Fortunately once the data is stored in the page
> cache cache, ext4_get_blocks() doesn't need to be called, so trying to
> replicate this problem to the point where we could identify its root
> cause was *extremely* difficult.  Many thanks to Kevin Shanahan for
> working over several months to be able to reproduce this easily so we
> could finally nail down the cause of the corruption.
> 
> Signed-off-by: "Theodore Ts'o" <tytso at mit.edu>
> Reviewed-by: "Aneesh Kumar K.V" <aneesh.kumar at linux.vnet.ibm.com>
> (cherry picked from commit 2ec0ae3acec47f628179ee95fe2c4da01b5e9fc4)
> ---
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 1ab62cc..df37b2e 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -1742,11 +1742,13 @@ ext4_ext_put_in_cache(struct inode *inode, ext4_lblk_t block,
>  {
>  	struct ext4_ext_cache *cex;
>  	BUG_ON(len == 0);
> +	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
>  	cex = &EXT4_I(inode)->i_cached_extent;
>  	cex->ec_type = type;
>  	cex->ec_block = block;
>  	cex->ec_len = len;
>  	cex->ec_start = start;
> +	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  }
>  
>  /*
> @@ -1803,12 +1805,17 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block,
>  			struct ext4_extent *ex)
>  {
>  	struct ext4_ext_cache *cex;
> +	int ret = EXT4_EXT_CACHE_NO;
>  
> +	/* 
> +	 * We borrow i_block_reservation_lock to protect i_cached_extent
> +	 */
> +	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
>  	cex = &EXT4_I(inode)->i_cached_extent;
>  
>  	/* has cache valid data? */
>  	if (cex->ec_type == EXT4_EXT_CACHE_NO)
> -		return EXT4_EXT_CACHE_NO;
> +		goto errout;
>  
>  	BUG_ON(cex->ec_type != EXT4_EXT_CACHE_GAP &&
>  			cex->ec_type != EXT4_EXT_CACHE_EXTENT);
> @@ -1819,11 +1826,11 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block,
>  		ext_debug("%u cached by %u:%u:%llu\n",
>  				block,
>  				cex->ec_block, cex->ec_len, cex->ec_start);
> -		return cex->ec_type;
> +		ret = cex->ec_type;
>  	}
> -
> -	/* not in cache */
> -	return EXT4_EXT_CACHE_NO;
> +errout:
> +	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> +	return ret;
>  }
>  
>  /*

> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team