Message-ID: <CADhLXY7AVVfxeTtDfEJXsYvk66CV7vsRMVw4P8PEn3rgOuSOLA@mail.gmail.com>
Date: Wed, 3 Dec 2025 07:07:57 +0530
From: Deepanshu Kartikey <kartikey406@...il.com>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzbot+b0a0670332b6b3230a0a@...kaller.appspotmail.com, tytso@....edu, 
	adilger.kernel@...ger.ca, djwong@...nel.org
Subject: Re: [PATCH v2] ext4: check folio uptodate state in ext4_page_mkwrite()

On Tue, Dec 2, 2025 at 5:54 PM Zhang Yi <yi.zhang@...weicloud.com> wrote:
>
> Hi Deepanshu!
>
> On 11/30/2025 10:06 AM, Deepanshu Kartikey wrote:
> > On Sat, Nov 22, 2025 at 7:27 AM Deepanshu Kartikey
> > <kartikey406@...il.com> wrote:
> >>
> >> When delayed block allocation fails due to filesystem corruption,
> >> ext4's writeback error handling invalidates affected folios by calling
> >> mpage_release_unused_pages() with invalidate=true, which explicitly
> >> clears the uptodate flag:
> >>
> >>     static void mpage_release_unused_pages(..., bool invalidate)
> >>     {
> >>         ...
> >>         if (invalidate) {
> >>             block_invalidate_folio(folio, 0, folio_size(folio));
> >>             folio_clear_uptodate(folio);
> >>         }
> >>     }
> >>
> >> If ext4_page_mkwrite() is subsequently called on such a non-uptodate
> >> folio, it can proceed to mark the folio dirty without checking its
> >> state. This triggers a warning in __folio_mark_dirty():
> >>
> >>     WARNING: CPU: 0 PID: 5 at mm/page-writeback.c:2960
> >>     __folio_mark_dirty+0x578/0x880
> >>
> >>     Call Trace:
> >>      fault_dirty_shared_page+0x16e/0x2d0
> >>      do_wp_page+0x38b/0xd20
> >>      handle_pte_fault+0x1da/0x450
> >>      __handle_mm_fault+0x652/0x13b0
> >>      handle_mm_fault+0x22a/0x6f0
> >>      do_user_addr_fault+0x200/0x8a0
> >>      exc_page_fault+0x81/0x1b0
> >>
> >> This scenario occurs when:
> >> 1. A write with delayed allocation marks a folio dirty (uptodate=1)
> >> 2. Writeback attempts block allocation but detects filesystem corruption
> >> 3. Error handling calls mpage_release_unused_pages(invalidate=true),
> >>    which clears the uptodate flag via folio_clear_uptodate()
> >> 4. A subsequent ftruncate() triggers ext4_truncate()
> >> 5. ext4_block_truncate_page() attempts to zero the page tail
> >> 6. This triggers a write fault on the mmap'd page
> >> 7. ext4_page_mkwrite() is called with the non-uptodate folio
> >> 8. Without checking uptodate, it proceeds to mark the folio dirty
> >> 9. __folio_mark_dirty() triggers: WARN_ON_ONCE(!folio_test_uptodate())
>
> Thank you a lot for analyzing this issue and the fix patch. As I was
> going through the process of understanding this issue, I had one
> question. Is the code flow that triggers the warning as follows?
>
> wp_page_shared()
>   do_page_mkwrite()
>     ext4_page_mkwrite()
>       block_page_mkwrite()   //The default delalloc path
>         block_commit_write()
>           mark_buffer_dirty()
>             __folio_mark_dirty(0)  //'warn' is false, doesn't trigger warning
>         folio_mark_dirty()
>           ext4_dirty_folio()
>             block_dirty_folio  //newly_dirty is false, doesn't call __folio_mark_dirty()
>   fault_dirty_shared_page()
>     folio_mark_dirty()  //Trigger warning ?
>
> This folio has been marked as dirty. How was this warning triggered?
> Am I missing something?
>
> Thanks,
> Yi.
>

Hi Yi,

Thank you for your question about the exact flow that triggers the warning.

You're correct that the code paths within ext4_page_mkwrite() and
block_page_mkwrite() call __folio_mark_dirty() with warn=0, so no
warning occurs there. The warning actually triggers later, in
fault_dirty_shared_page() after page_mkwrite returns.

Here's the complete flow:

  wp_page_shared()
    ↓
    do_page_mkwrite()
      ↓
      ext4_page_mkwrite()
        ↓
        block_page_mkwrite()
          ↓
          mark_buffer_dirty() → __folio_mark_dirty(warn=0)  // No warning
        ↓
        Returns success
    ↓
    fault_dirty_shared_page(vma, folio)  ← Warning triggers here
      ↓
      folio_mark_dirty(folio)
        ↓
        ext4_dirty_folio()
          ↓
          block_dirty_folio()
            ↓
            if (!folio_test_set_dirty(folio))  // Folio not already dirty
              __folio_mark_dirty(folio, mapping, 1)  ← warn=1, triggers WARNING

The key is that the folio can become non-uptodate between when it's
initially read and when wp_page_shared() is called. This happens when:

1. Delayed block allocation fails due to filesystem corruption
2. Error handling in mpage_release_unused_pages() explicitly clears uptodate:

     if (invalidate) {
         block_invalidate_folio(folio, 0, folio_size(folio));
         folio_clear_uptodate(folio);
     }

3. A subsequent operation (like ftruncate) triggers ext4_block_truncate_page()
4. This causes a write fault on the mmap'd page
5. wp_page_shared() is called with the now-non-uptodate folio

>From my debug logs with a test kernel:

  [22.387777] EXT4-fs error: lblock 0 mapped to illegal pblock 0
  [22.389798] EXT4-fs: Delayed block allocation failed... error 117
  [22.390401] EXT4-fs: This should not happen!! Data will be lost

  [22.399463] EXT4-fs error: Corrupt filesystem

  [22.400513] WP_PAGE_SHARED: ENTER folio=... uptodate=0 dirty=0
  [22.401953] WP_PAGE_SHARED: page_mkwrite failed, returning 2

With my fix, ext4_page_mkwrite() detects the non-uptodate state and
returns VM_FAULT_SIGBUS before block_page_mkwrite() is called,
preventing wp_page_shared() from reaching fault_dirty_shared_page().

Without the fix, the sequence would be:
- ext4_page_mkwrite() succeeds (doesn't check uptodate)
- block_page_mkwrite() marks buffers dirty (warn=0, no warning)
- Returns to wp_page_shared()
- fault_dirty_shared_page() calls folio_mark_dirty()
- block_dirty_folio() finds folio not dirty (uptodate=0, dirty=0)
- Calls __folio_mark_dirty() with warn=1
- WARNING triggers: WARN_ON_ONCE(warn && !folio_test_uptodate(folio)
&& !folio_test_dirty(folio))

The syzbot call trace confirms this:

  Call Trace:
   fault_dirty_shared_page+0x16e/0x2d0
   do_wp_page+0x38b/0xd20
   handle_pte_fault+0x1da/0x450

Does this clarify the flow?

Best regards,
Deepanshu

Powered by blists - more mailing lists