Commit bb1591b4 authored by Chris Mason's avatar Chris Mason

Btrfs: check prepare_uptodate_page() error code earlier

prepare_pages() may end up calling prepare_uptodate_page() twice if our
write only spans a single page.  But if the first call returns an error,
our page will be unlocked and its not safe to call it again.

This bug goes all the way back to 2011, and it's not something commonly
hit.

While we're here, add a more explicit check for the page being truncated
away.  The bare lock_page() alone is protected only by good thoughts and
i_mutex, which we're sure to regret eventually.
Reported-by: default avatarDave Jones <dsj@fb.com>
Signed-off-by: default avatarChris Mason <clm@fb.com>
parent 1b9b922a
...@@ -1291,7 +1291,8 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans, ...@@ -1291,7 +1291,8 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
* on error we return an unlocked page and the error value * on error we return an unlocked page and the error value
* on success we return a locked page and 0 * on success we return a locked page and 0
*/ */
static int prepare_uptodate_page(struct page *page, u64 pos, static int prepare_uptodate_page(struct inode *inode,
struct page *page, u64 pos,
bool force_uptodate) bool force_uptodate)
{ {
int ret = 0; int ret = 0;
...@@ -1306,6 +1307,10 @@ static int prepare_uptodate_page(struct page *page, u64 pos, ...@@ -1306,6 +1307,10 @@ static int prepare_uptodate_page(struct page *page, u64 pos,
unlock_page(page); unlock_page(page);
return -EIO; return -EIO;
} }
if (page->mapping != inode->i_mapping) {
unlock_page(page);
return -EAGAIN;
}
} }
return 0; return 0;
} }
...@@ -1324,6 +1329,7 @@ static noinline int prepare_pages(struct inode *inode, struct page **pages, ...@@ -1324,6 +1329,7 @@ static noinline int prepare_pages(struct inode *inode, struct page **pages,
int faili; int faili;
for (i = 0; i < num_pages; i++) { for (i = 0; i < num_pages; i++) {
again:
pages[i] = find_or_create_page(inode->i_mapping, index + i, pages[i] = find_or_create_page(inode->i_mapping, index + i,
mask | __GFP_WRITE); mask | __GFP_WRITE);
if (!pages[i]) { if (!pages[i]) {
...@@ -1333,13 +1339,17 @@ static noinline int prepare_pages(struct inode *inode, struct page **pages, ...@@ -1333,13 +1339,17 @@ static noinline int prepare_pages(struct inode *inode, struct page **pages,
} }
if (i == 0) if (i == 0)
err = prepare_uptodate_page(pages[i], pos, err = prepare_uptodate_page(inode, pages[i], pos,
force_uptodate); force_uptodate);
if (i == num_pages - 1) if (!err && i == num_pages - 1)
err = prepare_uptodate_page(pages[i], err = prepare_uptodate_page(inode, pages[i],
pos + write_bytes, false); pos + write_bytes, false);
if (err) { if (err) {
page_cache_release(pages[i]); page_cache_release(pages[i]);
if (err == -EAGAIN) {
err = 0;
goto again;
}
faili = i - 1; faili = i - 1;
goto fail; goto fail;
} }
......
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