[PATCH] ocfs2: fix kernel BUG in ocfs2_page_mkwrite() when write_end fails

[PATCH] ocfs2: fix kernel BUG in ocfs2_page_mkwrite() when write_end fails

[Zhiyuan Zhang]

From: Zhiyuan Zhang <zhiyuanzhang912@gmail.com>

syzbot reported a kernel BUG in the ocfs2 write fault path:

  (syz.0.17,5578,0):ocfs2_write_end_nolock:1945 ERROR: status = -30
  kernel BUG at fs/ocfs2/mmap.c:106!
  Call Trace:
   do_page_mkwrite+0x147/0x310 mm/memory.c:3879
   do_shared_fault mm/memory.c:6145 [inline]
   handle_mm_fault+0x1a53/0x3750 mm/memory.c:6943

__ocfs2_page_mkwrite() calls ocfs2_write_begin_nolock() and then
ocfs2_write_end_nolock(), and uses BUG_ON() to assert that the latter
returns the full length.  But ocfs2_write_end_nolock() can fail: since
commit 7f27ec978b0e ("ocfs2: call ocfs2_journal_access_di() before
ocfs2_journal_dirty() in ocfs2_write_end_nolock()") it gets journal
access to the inode buffer again, because the handle may have been
restarted since ocfs2_write_begin_nolock().  That fails with -EROFS or
-EIO once the journal has been aborted or jbd2 has seen write errors
on the device.  The syzbot reproducer gets there by shrinking the
backing loop device to zero with LOOP_SET_STATUS while writing to a
shared file mapping.

Handle the failure instead of crashing.  ocfs2_write_end_nolock()
leaves the mmap folio locked, because ocfs2_unlock_folios() skips it
when w_target_locked is set so that it can be returned with
VM_FAULT_LOCKED, and do_page_mkwrite() does not unlock it either when
->page_mkwrite() returns an error.  So unlock it and fail the fault;
the faulting task gets SIGBUS, like for the other errors in this
function.

For an mmap write ocfs2_write_end_nolock() returns either len or a
negative errno: inline data has already been converted to extents by
ocfs2_write_begin_nolock(), and copied is equal to len.

The BUG matters for local mounts, where the filesystem is expected to
go read-only and keep running after a journal abort.  On clustered
mounts ocfs2_abort() panics the node anyway as soon as it notices the
aborted journal.

Fixes: 7f27ec978b0e ("ocfs2: call ocfs2_journal_access_di() before ocfs2_journal_dirty() in ocfs2_write_end_nolock()")
Reported-by: syzbot+42984127d45ad96219de@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=42984127d45ad96219de
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5-2
Signed-off-by: Zhiyuan Zhang <zhiyuanzhang912@gmail.com>
---

Notes:
    Testing: syzbot C reproducer in QEMU (x86_64) on a defconfig-based
    kernel with KASAN, lockdep, DEBUG_VM and hung task detection enabled
    (not syzbot's own config).
    
     - Unpatched: BUG in 3 of 3 runs, within 22 iterations.
     - Patched: 437 iterations over 3 runs hit the failing write_end 21
       times (11 x -EROFS, 10 x -EIO), all ending in SIGBUS, with no BUG,
       KASAN, lockdep, DEBUG_VM or hung task report.
     - Negative control: without the folio_unlock(), inode eviction hangs
       on the locked folio in truncate_inode_pages_range() in 3 of 3 runs
       that hit the error.
    
    Not tested on a clustered mount.
    
    AI assistants were used to analyse the report, to write the patch
    and the changelog, and to run the tests above.
    
    ocfs2_dio_wr_get_block() has the same BUG_ON(ret != len).  It is left
    alone here because the pending "migrates ocfs2 DIO from buffer_head to
    iomap" series removes that function.

 fs/ocfs2/mmap.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/mmap.c b/fs/ocfs2/mmap.c
index 6c570157caf1..1c03f45e8965 100644
--- a/fs/ocfs2/mmap.c
+++ b/fs/ocfs2/mmap.c
@@ -103,7 +103,17 @@ static vm_fault_t __ocfs2_page_mkwrite(struct file *file,
 		goto out;
 	}
 	err = ocfs2_write_end_nolock(mapping, pos, len, len, fsdata);
-	BUG_ON(err != len);
+	if (err != len) {
+		/*
+		 * ocfs2_write_end_nolock() keeps the mmap folio locked so
+		 * that it can be returned to the VM with VM_FAULT_LOCKED.
+		 * On failure nobody else will unlock it, so do it here.
+		 */
+		folio_unlock(locked_folio);
+		mlog_errno(err);
+		ret = vmf_error(err);
+		goto out;
+	}
 	ret = VM_FAULT_LOCKED;
 out:
 	return ret;

base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] ocfs2: fix kernel BUG in ocfs2_page_mkwrite() when write_end fails

[Heming Zhao]

On 11/11/26 10:34, Zhiyuan Zhang wrote:
> From: Zhiyuan Zhang <zhiyuanzhang912@gmail.com>
> 
> syzbot reported a kernel BUG in the ocfs2 write fault path:
> 
>    (syz.0.17,5578,0):ocfs2_write_end_nolock:1945 ERROR: status = -30
>    kernel BUG at fs/ocfs2/mmap.c:106!
>    Call Trace:
>     do_page_mkwrite+0x147/0x310 mm/memory.c:3879
>     do_shared_fault mm/memory.c:6145 [inline]
>     handle_mm_fault+0x1a53/0x3750 mm/memory.c:6943
> 
> __ocfs2_page_mkwrite() calls ocfs2_write_begin_nolock() and then
> ocfs2_write_end_nolock(), and uses BUG_ON() to assert that the latter
> returns the full length.  But ocfs2_write_end_nolock() can fail: since
> commit 7f27ec978b0e ("ocfs2: call ocfs2_journal_access_di() before
> ocfs2_journal_dirty() in ocfs2_write_end_nolock()") it gets journal
> access to the inode buffer again, because the handle may have been
> restarted since ocfs2_write_begin_nolock().  That fails with -EROFS or
> -EIO once the journal has been aborted or jbd2 has seen write errors
> on the device.  The syzbot reproducer gets there by shrinking the
> backing loop device to zero with LOOP_SET_STATUS while writing to a
> shared file mapping.
> 
> Handle the failure instead of crashing.  ocfs2_write_end_nolock()
> leaves the mmap folio locked, because ocfs2_unlock_folios() skips it
> when w_target_locked is set so that it can be returned with
> VM_FAULT_LOCKED, and do_page_mkwrite() does not unlock it either when
> ->page_mkwrite() returns an error.  So unlock it and fail the fault;
> the faulting task gets SIGBUS, like for the other errors in this
> function.
> 
> For an mmap write ocfs2_write_end_nolock() returns either len or a
> negative errno: inline data has already been converted to extents by
> ocfs2_write_begin_nolock(), and copied is equal to len.
> 
> The BUG matters for local mounts, where the filesystem is expected to
> go read-only and keep running after a journal abort.  On clustered
> mounts ocfs2_abort() panics the node anyway as soon as it notices the
> aborted journal.
> 
> Fixes: 7f27ec978b0e ("ocfs2: call ocfs2_journal_access_di() before ocfs2_journal_dirty() in ocfs2_write_end_nolock()")
> Reported-by: syzbot+42984127d45ad96219de@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=42984127d45ad96219de
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-fable-5-2
> Signed-off-by: Zhiyuan Zhang <zhiyuanzhang912@gmail.com>

looks good to me.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
> 
> Notes:
>      Testing: syzbot C reproducer in QEMU (x86_64) on a defconfig-based
>      kernel with KASAN, lockdep, DEBUG_VM and hung task detection enabled
>      (not syzbot's own config).
>      
>       - Unpatched: BUG in 3 of 3 runs, within 22 iterations.
>       - Patched: 437 iterations over 3 runs hit the failing write_end 21
>         times (11 x -EROFS, 10 x -EIO), all ending in SIGBUS, with no BUG,
>         KASAN, lockdep, DEBUG_VM or hung task report.
>       - Negative control: without the folio_unlock(), inode eviction hangs
>         on the locked folio in truncate_inode_pages_range() in 3 of 3 runs
>         that hit the error.
>      
>      Not tested on a clustered mount.
>      
>      AI assistants were used to analyse the report, to write the patch
>      and the changelog, and to run the tests above.
>      
>      ocfs2_dio_wr_get_block() has the same BUG_ON(ret != len).  It is left
>      alone here because the pending "migrates ocfs2 DIO from buffer_head to
>      iomap" series removes that function.
> 
>   fs/ocfs2/mmap.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/mmap.c b/fs/ocfs2/mmap.c
> index 6c570157caf1..1c03f45e8965 100644
> --- a/fs/ocfs2/mmap.c
> +++ b/fs/ocfs2/mmap.c
> @@ -103,7 +103,17 @@ static vm_fault_t __ocfs2_page_mkwrite(struct file *file,
>   		goto out;
>   	}
>   	err = ocfs2_write_end_nolock(mapping, pos, len, len, fsdata);
> -	BUG_ON(err != len);
> +	if (err != len) {
> +		/*
> +		 * ocfs2_write_end_nolock() keeps the mmap folio locked so
> +		 * that it can be returned to the VM with VM_FAULT_LOCKED.
> +		 * On failure nobody else will unlock it, so do it here.
> +		 */
> +		folio_unlock(locked_folio);
> +		mlog_errno(err);
> +		ret = vmf_error(err);
> +		goto out;
> +	}
>   	ret = VM_FAULT_LOCKED;
>   out:
>   	return ret;
> 
> base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620