kernel BUG at fs/ext4/inode.c:1914 - page_buffers()

Hello,

I have been experimenting this issue:
https://www.spinics.net/lists/linux-ext4/msg86259.html, on a 5.15 kernel.

This issue caused by 5c48a7df9149 ("ext4: fix an use-after-free issue about data=journal writeback mode") is affecting ext4 users with data=journal on all stable kernels.

Jan proposed a fix here
https://www.spinics.net/lists/linux-ext4/msg87054.html which solves the situation for me.

Now this fix is not upstream because the data journaling support has been rewritten. As suggested by Jan, that would mean that we could either backport the following patches from upstream:

bd159398a2d2 ("jdb2: Don't refuse invalidation of already invalidated buffers") d84c9ebdac1e ("ext4: Mark pages with journalled data dirty")
265e72efa99f ("ext4: Keep pages with journalled data dirty")
5e1bdea6391d ("ext4: Clear dirty bit from pages without data to write") 1f1a55f0bf06 ("ext4: Commit transaction before writing back pages in data=journal mode") e360c6ed7274 ("ext4: Drop special handling of journalled data from ext4_sync_file()") c000dfec7e88 ("ext4: Drop special handling of journalled data from extent shifting operations") 783ae448b7a2 ("ext4: Fix special handling of journalled data from extent zeroing") 56c2a0e3d90d ("ext4: Drop special handling of journalled data from ext4_evict_inode()") 7c375870fdc5 ("ext4: Drop special handling of journalled data from ext4_quota_on()") 951cafa6b80e ("ext4: Simplify handling of journalled data in ext4_bmap()") ab382539adcb ("ext4: Update comment in mpage_prepare_extent_to_map()") d0ab8368c175 ("Revert "ext4: Fix warnings when freezing filesystem with journaled data"") 1077b2d53ef5 ("ext4: fix fsync for non-directories")

Or apply the proposed, attached patch. Do you think that would be an option?

Thanks,

Mathieu

From 17ec3d08a7878625c08ab37c45a8dc3c619db7fb Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@xxxxxxx>
Date: Thu, 12 Jan 2023 14:46:12 +0100
Subject: [PATCH] ext4: Fix crash in __ext4_journalled_writepage()

When __ext4_journalled_writepage() unlocks the page, there's nothing that prevents another process from finding the page and reclaiming buffers from it (because we have cleaned the page dirty bit and buffers needn't have the dirty bit set). When that happens we crash in __ext4_journalled_writepage() when trying to get the page buffers. Fix the problem by redirtying the page before unlocking it (so that reclaim and other users know the page isn't written yet) and by checking the page is still dirty after reacquiring the page lock. This should also make sure the page still has buffers.

---
 fs/ext4/inode.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

index 64a783f22105..b9f1fd05cec6 100644
@@ -138,7 +138,6 @@ static inline int ext4_begin_ordered_truncate(struct inode *inode,
 
 static void ext4_invalidatepage(struct page *page, unsigned int offset,
 				unsigned int length);
-static int __ext4_journalled_writepage(struct page *page, unsigned int len);
 static int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
 				  int pextents);
 

@@ -1858,7 +1857,8 @@ int ext4_da_get_block_prep(struct inode *inode, sector_t iblock,
 	return 0;
 }
 
-static int __ext4_journalled_writepage(struct page *page,
+static int __ext4_journalled_writepage(struct writeback_control *wbc,
+				       struct page *page,
 				       unsigned int len)
 {
 	struct address_space *mapping = page->mapping;

@@ -1869,8 +1869,6 @@ static int __ext4_journalled_writepage(struct page *page,
 	struct buffer_head *inode_bh = NULL;
 	loff_t size;
 
-	ClearPageChecked(page);
-
 	if (inline_data) {
 		BUG_ON(page->index != 0);
 		BUG_ON(len > ext4_get_max_inline_size(inode));

@@ -1884,6 +1882,7 @@ static int __ext4_journalled_writepage(struct page *page,
 	 * out from under us.
 	 */
 	get_page(page);
+	redirty_page_for_writepage(wbc, page);
 	unlock_page(page);
 
 	handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE,

@@ -1897,8 +1896,10 @@ static int __ext4_journalled_writepage(struct page *page,
 
 	lock_page(page);
 	put_page(page);
+	ClearPageChecked(page);
 	size = i_size_read(inode);
-	if (page->mapping != mapping || page_offset(page) > size) {
+	if (page->mapping != mapping || page_offset(page) >= size ||
+	    !clear_page_dirty_for_io(page)) {
 		/* The page got truncated from under us */
 		ext4_journal_stop(handle);
 		ret = 0;

@@ -2055,7 +2056,7 @@ static int ext4_writepage(struct page *page,
 		 * It's mmapped pagecache.  Add buffers and journal it.  There
 		 * doesn't seem much point in redirtying the page here.
 		 */
-		return __ext4_journalled_writepage(page, len);
+		return __ext4_journalled_writepage(wbc, page, len);
 
 	ext4_io_submit_init(&io_submit, wbc);
 	io_submit.io_end = ext4_init_io_end(inode, GFP_NOFS);

Backporting the series would be ideal. Is this only for the 5.15 kernel?

Thanks,
Sasha

Hello,

OK. I spotted it on a 5.15 but as far as I understand, this affects all stables with 5c48a7df9149, i.e all stables. Is that correct Jan?

Mathieu

Hello!

Yes, that is correct. Also I have realized that before patches I've already mentioned are applicable, you will also need to pick up:

9462f770eda8 ("ext4: Update stale comment about write constraints") c8e8e16dbbf0 ("ext4: Use nr_to_write directly in mpage_prepare_extent_to_map()") 3f5d30636d2a ("ext4: Mark page for delayed dirtying only if it is pinned") f1496362e9d7 ("ext4: Don't unlock page in ext4_bio_write_page()") eaf2ca10ca4b ("ext4: Move page unlocking out of mpage_submit_page()") d8be7607de03 ("ext4: Move mpage_page_done() calls after error handling") 3f079114bf52 ("ext4: Convert data=journal writeback to use ext4_writepages()") e6c28a26b799 ("ext4: Fix warnings when freezing filesystem with journaled data")
This commit actually gets reverted in the series of patches I have
already mentioned.

So sadly the backport is even larger than what I originally thought.

Honza

Jan Kara <[email protected]>
SUSE Labs, CR

If we're going to backport all of these patches, I'd really would like to see a full regression test run, using something like:

gce-xfstests ltm -c ext4/all -g auto

before and after applying all of these patches, to make sure there are no regression.

(or you can "kvm-xfstests -c ext4/all -g auto" but be prepared for it to take over 24 hours of run time. With gce-xfstesets we start a dozen VM's in parallel so it finishes in about 2.5 hours. See
https://thunk.org/gce-xfstests for more information.)

If you someone who does the backports can send me a pointer to a git branch, I can run the tests for you, if that would be helpful.

Thanks!!

- Ted