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

[Zhang Yi <yizhang089@gmail.com>]

On 10/9/2026 7:33 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] ext4: NULL pointer dereference in pagecache_write() during dynamic fsverity enablement
> --
> 
> commit 3391a6fc6c975d43c433b32dd18bae4da4784860
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: partially enable iomap for the buffered I/O path of regular files
>      
> Introduce ext4_enable_buffered_iomap() to determine whether a regular file
> inode should use the iomap buffered I/O path. If unsupported features like
> fsverity are enabled at the superblock level, the inode silently falls back
> to the buffer_head path.
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index c3a6f3dce3d86..a07ce368e6e82 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
> [ ... ]
>> @@ -6076,6 +6098,81 @@ static int check_igot_inode(struct inode *inode, ext4_iget_flags flags,
> [ ... ]
>> +void ext4_enable_buffered_iomap(struct inode *inode)
>> +{
>> +	struct super_block *sb = inode->i_sb;
>> +
>> +	if (!S_ISREG(inode->i_mode))
>> +		return;
>> +	if (ext4_test_inode_flag(inode, EXT4_INODE_EA_INODE))
>> +		return;
>> +
>> +	/* Unsupported Features */
>> +	if (ext4_has_feature_inline_data(sb))
>> +		return;
>> +	if (ext4_has_feature_verity(sb))
>> +		return;
> 
> [Severity: Critical]
> Does this logic handle dynamic fsverity enablement correctly?
> 
> If FS_IOC_ENABLE_VERITY is executed on a file where the verity feature was not
> initially set on the superblock, ext4_has_feature_verity(sb) will be false at
> inode initialization. This causes the inode to be assigned to the iomap
> buffered I/O path, setting mapping->a_ops = &ext4_iomap_aops.
> 
> When the user executes the ioctl, ext4_begin_enable_verity() does not check if
> the inode is using the iomap path. It proceeds to write the verity descriptor
> via ext4_write_verity_descriptor(), which calls pagecache_write().
> 
> fs/ext4/verity.c:pagecache_write() {
>      ...
>      res = aops->write_begin(NULL, mapping, pos, n, &folio, &fsdata);
>      ...
> }
> 
> Because pagecache_write() unconditionally dereferences aops->write_begin, and
> ext4_iomap_aops does not implement write_begin (it is NULL), can this lead to
> an immediate NULL pointer dereference?
> 
> Since this can be triggered by unprivileged users with write access via an
> ioctl, could this be exploited as a local Denial of Service?
> 

This is a false positive.  An inode is put on the buffered iomap path
only when the superblock does not have the verity feature, and that
feature is not allowed to be enabled at runtime.  It is a ro_compat
feature set by mkfs/tune2fs with the filesystem unmounted, and there is
no in-kernel setter for it.

More importantly, the only way to reach pagecache_write() from
ext4_begin_enable_verity() is FS_IOC_ENABLE_VERITY, and ext4's ioctl
handler gates it on the same ext4_has_feature_verity() check, so the
ioctl is rejected before ext4_begin_enable_verity() is ever called and
aops->write_begin() is never dereferenced.

Thanks,
Yi.