Re: [PATCH v6 30/31] ext4: partially enable iomap for the buffered I/O path of regular files

[Zhang Yi <yizhang089@gmail.com>]

On 9/3/2026 10:04 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Concurrent buffered reads during dynamic transition of the journal data flag cause type confusion on folio->private, leading to memory corruption.
> --

This is a false positive. Buffered reads do take invalidate_lock.
filemap_create_folio() and filemap_update_page() acquire
filemap_invalidate_lock_shared() before installing the folio and calling
->read_folio(). Readahead and page faults do the same.

Thanks,
Yi.

> 
> commit a5650f621dac34bfe427e11f706ab1c16d533afc
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: partially enable iomap for the buffered I/O path of regular files
> 
> This commit introduces ext4_enable_buffered_iomap() to determine whether a
> regular file inode should use the iomap buffered I/O path. It also modifies
> ext4_change_inode_journal_flag() to dynamically toggle the buffered I/O
> path and address_space operations on an active inode when the per-inode
> data journal flag is changed.
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
> [ ... ]
>> @@ -7598,9 +7695,10 @@ int ext4_change_inode_journal_flag(struct inode *inode, int val)
>>   	 * the inode's in-core data-journaling state flag now.
>>   	 */
>>   
>> -	if (val)
>> +	if (val) {
>>   		ext4_set_inode_flag(inode, EXT4_INODE_JOURNAL_DATA);
>> -	else {
>> +		ext4_disable_buffered_iomap(inode);
>> +	} else {
>>   		err = jbd2_journal_flush(journal, 0);
>>   		if (err < 0) {
>>   			jbd2_journal_unlock_updates(journal);
>> @@ -7609,6 +7707,7 @@ int ext4_change_inode_journal_flag(struct inode *inode, int val)
>>   			return err;
>>   		}
>>   		ext4_clear_inode_flag(inode, EXT4_INODE_JOURNAL_DATA);
>> +		ext4_enable_buffered_iomap(inode);
>>   	}
>>   	ext4_set_aops(inode);
> 
> [Severity: High]
> Can this lead to memory corruption due to a type confusion on folio->private?
> 
> When toggling the journal data flag on an active file,
> ext4_change_inode_journal_flag() truncates the page cache, but then blocks
> on jbd2_journal_lock_updates(journal).
> 
> During this blocking window, a concurrent thread could perform a buffered
> read on the same file. Because ext4_file_read_iter() calls
> generic_file_read_iter(), which handles buffered reads without acquiring
> the inode lock or invalidate_lock, the concurrent read can instantiate new
> folios into the page cache using the old address space operations (for
> example, attaching a buffer_head structure to folio->private).
> 
> When jbd2_journal_lock_updates() returns, this thread will proceed to
> switch the operations via ext4_set_aops(inode).
> 
> Will subsequent operations on those concurrently populated folios invoke the
> new iomap operations, misinterpreting the buffer_head pointer in
> folio->private as an iomap_folio_state pointer?
>