2021-12-25 08:58:15

by Zhang Yi

Subject: [PATCH] ext4: fix an use-after-free issue about data=journal writeback mode

Our syzkaller report an use-after-free issue that accessing the freed
buffer_head on the writeback page in __ext4_journalled_writepage(). The
problem is that if there was a truncate racing with the data=journalled
writeback procedure, the writeback length could become zero and
bget_one() refuse to get buffer_head's refcount, then the truncate
procedure release buffer once we drop page lock, finally, the last
ext4_walk_page_buffers() trigger the use-after-free problem.

sync truncate
ext4_sync_file()
file_write_and_wait_range()
ext4_setattr(0)
inode->i_size = 0
ext4_writepage()
len = 0
__ext4_journalled_writepage()
page_bufs = page_buffers(page)
ext4_walk_page_buffers(bget_one) <- does not get refcount
do_invalidatepage()
free_buffer_head()
ext4_walk_page_buffers(page_bufs) <- trigger use-after-free

After commit bdf96838aea6 ("ext4: fix race between truncate and
__ext4_journalled_writepage()"), we have already handled the racing
case, so the bget_one() and bput_one() are not needed. So this patch
simply remove these hunk, and recheck the i_size to make it safe.

Fixes: bdf96838aea6 ("ext4: fix race between truncate and __ext4_journalled_writepage()")
Signed-off-by: Zhang Yi <[email protected]>
Cc: [email protected]
---
fs/ext4/inode.c | 37 ++++++++++---------------------------
1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bfd3545f1e5d..cd94a70dd3d5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1844,30 +1844,16 @@ int ext4_da_get_block_prep(struct inode *inode, sector_t iblock,
return 0;
}

-static int bget_one(handle_t *handle, struct inode *inode,
- struct buffer_head *bh)
-{
- get_bh(bh);
- return 0;
-}
-
-static int bput_one(handle_t *handle, struct inode *inode,
- struct buffer_head *bh)
-{
- put_bh(bh);
- return 0;
-}
-
static int __ext4_journalled_writepage(struct page *page,
unsigned int len)
{
struct address_space *mapping = page->mapping;
struct inode *inode = mapping->host;
- struct buffer_head *page_bufs = NULL;
handle_t *handle = NULL;
int ret = 0, err = 0;
int inline_data = ext4_has_inline_data(inode);
struct buffer_head *inode_bh = NULL;
+ loff_t size;

ClearPageChecked(page);

@@ -1877,14 +1863,6 @@ static int __ext4_journalled_writepage(struct page *page,
inode_bh = ext4_journalled_write_inline_data(inode, len, page);
if (inode_bh == NULL)
goto out;
- } else {
- page_bufs = page_buffers(page);
- if (!page_bufs) {
- BUG();
- goto out;
- }
- ext4_walk_page_buffers(handle, inode, page_bufs, 0, len,
- NULL, bget_one);
}
/*
* We need to release the page lock before we start the
@@ -1905,7 +1883,8 @@ static int __ext4_journalled_writepage(struct page *page,

lock_page(page);
put_page(page);
- if (page->mapping != mapping) {
+ size = i_size_read(inode);
+ if (page->mapping != mapping || page_offset(page) > size) {
/* The page got truncated from under us */
ext4_journal_stop(handle);
ret = 0;
@@ -1915,6 +1894,13 @@ static int __ext4_journalled_writepage(struct page *page,
if (inline_data) {
ret = ext4_mark_inode_dirty(handle, inode);
} else {
+ struct buffer_head *page_bufs = page_buffers(page);
+
+ if (page->index == size >> PAGE_SHIFT)
+ len = size & ~PAGE_MASK;
+ else
+ len = PAGE_SIZE;
+
ret = ext4_walk_page_buffers(handle, inode, page_bufs, 0, len,
NULL, do_journal_get_write_access);

@@ -1935,9 +1921,6 @@ static int __ext4_journalled_writepage(struct page *page,
out:
unlock_page(page);
out_no_pagelock:
- if (!inline_data && page_bufs)
- ext4_walk_page_buffers(NULL, inode, page_bufs, 0, len,
- NULL, bput_one);
brelse(inode_bh);
return ret;
}
--
2.31.1

2021-12-27 15:16:43

by Theodore Ts'o

Subject: Re: [PATCH] ext4: fix an use-after-free issue about data=journal writeback mode

On Sat, 25 Dec 2021 17:09:37 +0800, Zhang Yi wrote:
> Our syzkaller report an use-after-free issue that accessing the freed
> buffer_head on the writeback page in __ext4_journalled_writepage(). The
> problem is that if there was a truncate racing with the data=journalled
> writeback procedure, the writeback length could become zero and
> bget_one() refuse to get buffer_head's refcount, then the truncate
> procedure release buffer once we drop page lock, finally, the last
> ext4_walk_page_buffers() trigger the use-after-free problem.
>
> [...]

Nice catch. Applied, thanks!

[1/1] ext4: fix an use-after-free issue about data=journal writeback mode
commit: 856dd2096e2a01f6eb2c9d60f6e0cd587aa273a8

Best regards,
--
Theodore Ts'o <[email protected]>