Commit 5c48a7df authored by Zhang Yi's avatar Zhang Yi Committed by Theodore Ts'o
Browse files

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 bdf96838 ("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: bdf96838 ("ext4: fix race between truncate and __ext4_journalled_writepage()")
Signed-off-by: default avatarZhang Yi <yi.zhang@huawei.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20211225090937.712867-1-yi.zhang@huawei.com


Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent 298b5c52

fs/ext4/inode.c

+10 −27
Original line number Diff line number Diff line
@@ -1845,30 +1845,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);
@@ -1878,14 +1864,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
@@ -1906,7 +1884,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;
@@ -1916,6 +1895,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);
@@ -1936,9 +1922,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;

}