[PATCH v4] ext4: fix kernel BUG in ext4_write_inline_data_end

[PATCH v4] ext4: fix kernel BUG in ext4_write_inline_data_end

[Aditya Prakash Srivastava]

When the data=journal mount option is used, the ext4_journalled_write_end()
function incorrectly calls ext4_write_inline_data_end() without checking
if the EXT4_STATE_MAY_INLINE_DATA flag is still set on the inode.

If a previous attempt to convert the inline data to an extent failed (e.g.
due to ENOSPC), the EXT4_STATE_MAY_INLINE_DATA flag is cleared, but
the EXT4_INODE_INLINE_DATA flag remains set. In this scenario, the next
call to ext4_write_begin() will not prepare the inline data xattr for
writing, but ext4_journalled_write_end() will incorrectly attempt to write
to it, triggering a BUG_ON(pos + len > EXT4_I(inode)->i_inline_size) in
ext4_write_inline_data() since i_inline_size was not expanded.

Additionally, two separate TOCTOU race conditions exist due to concurrent
ext4_page_mkwrite() execution:
1) A concurrent ext4_page_mkwrite() can execute ext4_convert_inline_data()
between write_begin and write_end, clearing the inline flags. Since block
buffers were not allocated in write_begin, this results in a NULL pointer
dereference in the write_end fallback paths because folio_buffers(folio) is
NULL.
2) If ext4_convert_inline_data() clears the flags exactly after the inline
flags checks pass in write_end, but before ext4_write_inline_data_end()
acquires the xattr semaphore, the subsequent check will hit a panic via
BUG_ON(!ext4_has_inline_data(inode)).

Fix these issues completely by:
1) Having write_end functions (ext4_write_end(),
ext4_journalled_write_end(), and ext4_da_do_write_end()) return 0
(VFS retry) if they fall through to the block fallback path and detect
that folio_buffers(folio) is NULL, after safely stopping any active
journal handle (protecting against a NULL handle panic in
ext4_put_nojournal()).
2) Replacing BUG_ON(!ext4_has_inline_data(inode)) inside
ext4_write_inline_data_end() with a graceful error path. If the inline flag
is cleared after locking the xattr, we unlock the xattr, release the iloc,
unlock/put the folio, stop the journal, and return 0 to trigger a retry.

Reported-by: syzbot+0c89d865531d053abb2d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0c89d865531d053abb2d
Fixes: 3fdcfb668fd7 ("ext4: add journalled write support for inline data")
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
v4:
  - Address critical TOCTOU race condition (reported by Sashiko AI review):
    * Scenario: A buffered write holds the folio lock and evaluates the inline
      flags checks in write_end to true. Before it enters or locks the xattr_sem
      in ext4_write_inline_data_end(), a concurrent memory-mapped page fault
      (ext4_page_mkwrite()) converts the inline data to an extent. This page fault
      bypasses the folio lock (since ext4_convert_inline_data() runs lockless),
      acquires the xattr_sem, and clears the inline flags. When the buffered write
      resumes and enters ext4_write_inline_data_end(), it acquires the xattr_sem
      and immediately triggers BUG_ON(!ext4_has_inline_data(inode)) causing a
      kernel panic.
    * Fix: Replace the BUG_ON() with a graceful error-handling retry path that
      releases all resources (locks/buffers/folios/journals) and returns 0.
v3:
  - Fix journal handle leak and NULL handle crash (reported by Sashiko AI review):
    * Scenario 1 (leak): During a delayed allocation write (ext4_da_write_begin),
      inline data was prepared and a transaction handle started. If a concurrent
      page fault converts the inline data before write_end, ext4_da_write_end()
      falls through to ext4_da_do_write_end(). If the fallback check for
      !folio_buffers(folio) returns 0 to retry without calling ext4_journal_stop(),
      the transaction handle is leaked open-ended, eventually hanging the filesystem.
    * Scenario 2 (crash): If we blindly call ext4_journal_stop() on a NULL handle
      (e.g., when no transaction was started because we never took the inline path),
      __ext4_journal_stop() delegates to ext4_put_nojournal(NULL) which triggers
      BUG_ON(ref_cnt == 0), panicking the kernel.
    * Fix: Retrieve the active handle in ext4_da_do_write_end() and stop it
      if non-NULL. Also explicitly check "if (handle)" before calling
      ext4_journal_stop() in ext4_write_end() and ext4_journalled_write_end().
v2:
  - Address TOCTOU race condition (reported by Sashiko AI review):
    * Scenario: A concurrent ext4_page_mkwrite() converts inline data to extents
      and clears the flags between ext4_write_begin() and write_end(). The
      write_end function falls through to the block fallback path. Since block
      buffers were not allocated in write_begin (because it took the inline path),
      folio_buffers(folio) is NULL, causing a NULL pointer dereference in
      ext4_journalled_zero_new_buffers() or ext4_walk_page_buffers(), or silent
      data loss in the standard write path.
    * Fix: Have the write_end functions return 0 if folio_buffers(folio) is NULL,
      triggering a safe VFS-level retry. On the next write attempt, the inline
      flags will be detected as cleared, and blocks/buffers will be properly allocated.
 fs/ext4/inline.c |  9 ++++++++-
 fs/ext4/inode.c  | 24 ++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 8045e4ff270c..161136e84661 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -812,7 +812,14 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
 			goto out;
 		}
 		ext4_write_lock_xattr(inode, &no_expand);
-		BUG_ON(!ext4_has_inline_data(inode));
+		if (unlikely(!ext4_has_inline_data(inode))) {
+			ext4_write_unlock_xattr(inode, &no_expand);
+			brelse(iloc.bh);
+			folio_unlock(folio);
+			folio_put(folio);
+			ext4_journal_stop(handle);
+			return 0;
+		}
 
 		/*
 		 * ei->i_inline_off may have changed since
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c2c2d6ac7f3d..bc2688e03c19 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1455,6 +1455,14 @@ static int ext4_write_end(const struct kiocb *iocb,
 		return ext4_write_inline_data_end(inode, pos, len, copied,
 						  folio);
 
+	if (unlikely(!folio_buffers(folio))) {
+		folio_unlock(folio);
+		folio_put(folio);
+		if (handle)
+			ext4_journal_stop(handle);
+		return 0;
+	}
+
 	copied = block_write_end(pos, len, copied, folio);
 	/*
 	 * it's important to update i_size while still holding folio lock:
@@ -1560,10 +1568,19 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
 
 	BUG_ON(!ext4_handle_valid(handle));
 
-	if (ext4_has_inline_data(inode))
+	if (ext4_has_inline_data(inode) &&
+	    ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
 		return ext4_write_inline_data_end(inode, pos, len, copied,
 						  folio);
 
+	if (unlikely(!folio_buffers(folio))) {
+		folio_unlock(folio);
+		folio_put(folio);
+		if (handle)
+			ext4_journal_stop(handle);
+		return 0;
+	}
+
 	if (unlikely(copied < len) && !folio_test_uptodate(folio)) {
 		copied = 0;
 		ext4_journalled_zero_new_buffers(handle, inode, folio,
@@ -3231,7 +3248,10 @@ static int ext4_da_do_write_end(struct address_space *mapping,
 	if (unlikely(!folio_buffers(folio))) {
 		folio_unlock(folio);
 		folio_put(folio);
-		return -EIO;
+		handle = ext4_journal_current_handle();
+		if (handle)
+			ext4_journal_stop(handle);
+		return 0;
 	}
 	/*
 	 * block_write_end() will mark the inode as dirty with I_DIRTY_PAGES
-- 
2.47.3

Re: [PATCH v4] ext4: fix kernel BUG in ext4_write_inline_data_end

[Jan Kara]

On Tue 09-06-26 06:20:05, Aditya Prakash Srivastava wrote:
> When the data=journal mount option is used, the ext4_journalled_write_end()
> function incorrectly calls ext4_write_inline_data_end() without checking
> if the EXT4_STATE_MAY_INLINE_DATA flag is still set on the inode.
> 
> If a previous attempt to convert the inline data to an extent failed (e.g.
> due to ENOSPC), the EXT4_STATE_MAY_INLINE_DATA flag is cleared, but
> the EXT4_INODE_INLINE_DATA flag remains set. In this scenario, the next
> call to ext4_write_begin() will not prepare the inline data xattr for
> writing, but ext4_journalled_write_end() will incorrectly attempt to write
> to it, triggering a BUG_ON(pos + len > EXT4_I(inode)->i_inline_size) in
> ext4_write_inline_data() since i_inline_size was not expanded.
> 
> Additionally, two separate TOCTOU race conditions exist due to concurrent
> ext4_page_mkwrite() execution:
> 1) A concurrent ext4_page_mkwrite() can execute ext4_convert_inline_data()
> between write_begin and write_end, clearing the inline flags. Since block
> buffers were not allocated in write_begin, this results in a NULL pointer
> dereference in the write_end fallback paths because folio_buffers(folio) is
> NULL.
> 2) If ext4_convert_inline_data() clears the flags exactly after the inline
> flags checks pass in write_end, but before ext4_write_inline_data_end()
> acquires the xattr semaphore, the subsequent check will hit a panic via
> BUG_ON(!ext4_has_inline_data(inode)).

Yes, locking of inline data writes is broken (and difficult to fix). Your
v1 patch was actually simple and obvious improvement of the situation.
These additional fixes belong into separate patches.

> Fix these issues completely by:
> 1) Having write_end functions (ext4_write_end(),
> ext4_journalled_write_end(), and ext4_da_do_write_end()) return 0
> (VFS retry) if they fall through to the block fallback path and detect
> that folio_buffers(folio) is NULL, after safely stopping any active
> journal handle (protecting against a NULL handle panic in
> ext4_put_nojournal()).
> 2) Replacing BUG_ON(!ext4_has_inline_data(inode)) inside
> ext4_write_inline_data_end() with a graceful error path. If the inline flag
> is cleared after locking the xattr, we unlock the xattr, release the iloc,
> unlock/put the folio, stop the journal, and return 0 to trigger a retry.
> 
> Reported-by: syzbot+0c89d865531d053abb2d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0c89d865531d053abb2d
> Fixes: 3fdcfb668fd7 ("ext4: add journalled write support for inline data")
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> ---
> v4:
>   - Address critical TOCTOU race condition (reported by Sashiko AI review):
>     * Scenario: A buffered write holds the folio lock and evaluates the inline
>       flags checks in write_end to true. Before it enters or locks the xattr_sem
>       in ext4_write_inline_data_end(), a concurrent memory-mapped page fault
>       (ext4_page_mkwrite()) converts the inline data to an extent. This page fault
>       bypasses the folio lock (since ext4_convert_inline_data() runs lockless),
>       acquires the xattr_sem, and clears the inline flags. When the buffered write
>       resumes and enters ext4_write_inline_data_end(), it acquires the xattr_sem
>       and immediately triggers BUG_ON(!ext4_has_inline_data(inode)) causing a
>       kernel panic.
>     * Fix: Replace the BUG_ON() with a graceful error-handling retry path that
>       releases all resources (locks/buffers/folios/journals) and returns 0.
> v3:
>   - Fix journal handle leak and NULL handle crash (reported by Sashiko AI review):
>     * Scenario 1 (leak): During a delayed allocation write (ext4_da_write_begin),
>       inline data was prepared and a transaction handle started. If a concurrent
>       page fault converts the inline data before write_end, ext4_da_write_end()
>       falls through to ext4_da_do_write_end(). If the fallback check for
>       !folio_buffers(folio) returns 0 to retry without calling ext4_journal_stop(),
>       the transaction handle is leaked open-ended, eventually hanging the filesystem.
>     * Scenario 2 (crash): If we blindly call ext4_journal_stop() on a NULL handle
>       (e.g., when no transaction was started because we never took the inline path),
>       __ext4_journal_stop() delegates to ext4_put_nojournal(NULL) which triggers
>       BUG_ON(ref_cnt == 0), panicking the kernel.
>     * Fix: Retrieve the active handle in ext4_da_do_write_end() and stop it
>       if non-NULL. Also explicitly check "if (handle)" before calling
>       ext4_journal_stop() in ext4_write_end() and ext4_journalled_write_end().
> v2:
>   - Address TOCTOU race condition (reported by Sashiko AI review):
>     * Scenario: A concurrent ext4_page_mkwrite() converts inline data to extents
>       and clears the flags between ext4_write_begin() and write_end(). The
>       write_end function falls through to the block fallback path. Since block
>       buffers were not allocated in write_begin (because it took the inline path),
>       folio_buffers(folio) is NULL, causing a NULL pointer dereference in
>       ext4_journalled_zero_new_buffers() or ext4_walk_page_buffers(), or silent
>       data loss in the standard write path.
>     * Fix: Have the write_end functions return 0 if folio_buffers(folio) is NULL,
>       triggering a safe VFS-level retry. On the next write attempt, the inline
>       flags will be detected as cleared, and blocks/buffers will be properly allocated.
>  fs/ext4/inline.c |  9 ++++++++-
>  fs/ext4/inode.c  | 24 ++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 8045e4ff270c..161136e84661 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -812,7 +812,14 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
>  			goto out;
>  		}
>  		ext4_write_lock_xattr(inode, &no_expand);
> -		BUG_ON(!ext4_has_inline_data(inode));
> +		if (unlikely(!ext4_has_inline_data(inode))) {
> +			ext4_write_unlock_xattr(inode, &no_expand);
> +			brelse(iloc.bh);
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			ext4_journal_stop(handle);
> +			return 0;
> +		}

This deserves a comment before the 'if' that we could have raced with
ext4_page_mkwrite() converting the inode and so we just retry the whole
write.

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c2c2d6ac7f3d..bc2688e03c19 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1455,6 +1455,14 @@ static int ext4_write_end(const struct kiocb *iocb,
>  		return ext4_write_inline_data_end(inode, pos, len, copied,
>  						  folio);
>  
> +	if (unlikely(!folio_buffers(folio))) {
> +		folio_unlock(folio);
> +		folio_put(folio);
> +		if (handle)
> +			ext4_journal_stop(handle);
> +		return 0;
> +	}
> +

Ouch, this is a crude hack. I think much cleaner solution would be for
ext4_write_begin() to set in fsdata in what state it prepared the inode
(inline, extent based) - we already use that mechanism to communicate some
state for delayed allocations. Then ->write_end handler will use fsdata
(not inode state) to determine what function to call. IMHO the code will be
much more obvious that way.

> @@ -3231,7 +3248,10 @@ static int ext4_da_do_write_end(struct address_space *mapping,
>  	if (unlikely(!folio_buffers(folio))) {
>  		folio_unlock(folio);
>  		folio_put(folio);
> -		return -EIO;
> +		handle = ext4_journal_current_handle();
> +		if (handle)
> +			ext4_journal_stop(handle);
> +		return 0;
>  	}
>  	/*
>  	 * block_write_end() will mark the inode as dirty with I_DIRTY_PAGES

Huh, what is this about? It definitely looks very suspicious...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v4] ext4: fix kernel BUG in ext4_write_inline_data_end

[Aditya Prakash Srivastava]

Hi Jan,

Thank you very much for the incredibly detailed review and the design
insights!

I completely agree with your suggestion. Rushing too many fixes for
complex, concurrent race conditions into a single patch makes the
code harder to review and risks introducing subtle regressions.

Let's go ahead with the simple and straightforward v1 patch (which
has your Reviewed-by) to fix the original syzbot crash for now.

I will take your excellent suggestion to use fsdata for state
communication between write_begin and write_end, and I will work on
formulating a separate, cleaner patch series in the future to
address the remaining concurrent locking races you mentioned.

I will withdraw this v4 thread for now.

Thanks again for your guidance!

Best regards,
Aditya Prakash Srivastava

Re: [PATCH v4] ext4: fix kernel BUG in ext4_write_inline_data_end

[Theodore Ts'o]

On Tue, 09 Jun 2026 06:20:05 +0000, Aditya Prakash Srivastava wrote:
> When the data=journal mount option is used, the ext4_journalled_write_end()
> function incorrectly calls ext4_write_inline_data_end() without checking
> if the EXT4_STATE_MAY_INLINE_DATA flag is still set on the inode.
> 
> If a previous attempt to convert the inline data to an extent failed (e.g.
> due to ENOSPC), the EXT4_STATE_MAY_INLINE_DATA flag is cleared, but
> the EXT4_INODE_INLINE_DATA flag remains set. In this scenario, the next
> call to ext4_write_begin() will not prepare the inline data xattr for
> writing, but ext4_journalled_write_end() will incorrectly attempt to write
> to it, triggering a BUG_ON(pos + len > EXT4_I(inode)->i_inline_size) in
> ext4_write_inline_data() since i_inline_size was not expanded.
> 
> [...]

Applied, thanks!

[1/1] ext4: fix kernel BUG in ext4_write_inline_data_end
      commit: ad09aa45965d3fafaf9963bc78109b73c0f9ac8d

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>