Commit 2ec0ae3a authored by Theodore Ts'o's avatar Theodore Ts'o

ext4: Fix race in ext4_inode_info.i_cached_extent

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: default avatar"Theodore Ts'o" <tytso@mit.edu>
Reviewed-by: default avatar"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
parent 2a8964d6
...@@ -1841,11 +1841,13 @@ ext4_ext_put_in_cache(struct inode *inode, ext4_lblk_t block, ...@@ -1841,11 +1841,13 @@ ext4_ext_put_in_cache(struct inode *inode, ext4_lblk_t block,
{ {
struct ext4_ext_cache *cex; struct ext4_ext_cache *cex;
BUG_ON(len == 0); BUG_ON(len == 0);
spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
cex = &EXT4_I(inode)->i_cached_extent; cex = &EXT4_I(inode)->i_cached_extent;
cex->ec_type = type; cex->ec_type = type;
cex->ec_block = block; cex->ec_block = block;
cex->ec_len = len; cex->ec_len = len;
cex->ec_start = start; cex->ec_start = start;
spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
} }
/* /*
...@@ -1902,12 +1904,17 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block, ...@@ -1902,12 +1904,17 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block,
struct ext4_extent *ex) struct ext4_extent *ex)
{ {
struct ext4_ext_cache *cex; 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; cex = &EXT4_I(inode)->i_cached_extent;
/* has cache valid data? */ /* has cache valid data? */
if (cex->ec_type == EXT4_EXT_CACHE_NO) if (cex->ec_type == EXT4_EXT_CACHE_NO)
return EXT4_EXT_CACHE_NO; goto errout;
BUG_ON(cex->ec_type != EXT4_EXT_CACHE_GAP && BUG_ON(cex->ec_type != EXT4_EXT_CACHE_GAP &&
cex->ec_type != EXT4_EXT_CACHE_EXTENT); cex->ec_type != EXT4_EXT_CACHE_EXTENT);
...@@ -1918,11 +1925,11 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block, ...@@ -1918,11 +1925,11 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block,
ext_debug("%u cached by %u:%u:%llu\n", ext_debug("%u cached by %u:%u:%llu\n",
block, block,
cex->ec_block, cex->ec_len, cex->ec_start); cex->ec_block, cex->ec_len, cex->ec_start);
return cex->ec_type; ret = cex->ec_type;
} }
errout:
/* not in cache */ spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
return EXT4_EXT_CACHE_NO; return ret;
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment