Re: [PATCH v3 6/9] ext4: improve EXT4_GET_BLOCKS_CACHED_NOWAIT handling in ext4_map_blocks

[Baokun Li <libaokun@linux.alibaba.com>]

On 2026/6/26 16:50, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Encountering an invalid block during EXT4_GET_BLOCKS_CACHED_NOWAIT causes ext4_error_inode() to block while in RCU-walk mode.
>
> Pre-existing issues:
> - [High] check_block_validity() is bypassed for unwritten extents because EXT4_MAP_MAPPED is not set, leaving the filesystem vulnerable to arbitrary block reads.
> --
>
> commit 16a7856d68391995dd0a737adac6e20b3e6ddb76
> Author: Baokun Li <libaokun@linux.alibaba.com>
>
> ext4: improve EXT4_GET_BLOCKS_CACHED_NOWAIT handling in ext4_map_blocks
>
> When EXT4_GET_BLOCKS_CACHED_NOWAIT is set and the extent status cache hits,
> ext4_map_blocks() returns immediately without running check_block_validity().
> This allows malicious extents from crafted filesystem images to bypass
> validation if they have been cached by a previous blocking read.
>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index a60ecf0bebd04..2479f8d93ec8b 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -759,8 +759,9 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>>  			BUG();
>>  		}
>>  
>> +		/* Skip blocking operations and jump to extent validation. */
>>  		if (flags & EXT4_GET_BLOCKS_CACHED_NOWAIT)
>> -			return retval;
>> +			goto found;
> [Severity: High]
> Can jumping to the extent validation cause a scheduling while atomic BUG?
>
> If a crafted filesystem provides a malicious extent that is successfully cached,
> a subsequent path walk in RCU mode will call ext4_getblk() with
> EXT4_GET_BLOCKS_CACHED_NOWAIT.
>
> Jumping to found executes check_block_validity(). If the extent is malicious,
> this calls ext4_error_inode(), which can block:
>
> check_block_validity()
>   __check_block_validity()
>     ext4_error_inode()
>       ext4_handle_error()
>         ext4_commit_super()
>           wait_on_buffer()
>
> Since RCU-walk mode holds rcu_read_lock(), will blocking in wait_on_buffer()
> cause a crash?

Ah~ not doing check_block_validity() now will directly use potentially
abnormal extents, but doing check_block_validity() might sleep in atomic
context. It seems that validation before inserting into the es tree is the
thorough solution.

> [Severity: High]
> This isn't a bug introduced by this patch, but does the extent validation
> actually check unwritten extents?
>
> When looking up an unwritten extent in the ES cache, the flags are set to
> EXT4_MAP_UNWRITTEN but omit EXT4_MAP_MAPPED:
>
> ext4_map_blocks() {
>     ...
>     map->m_flags |= ext4_es_is_written(&es) ?
>                     EXT4_MAP_MAPPED : EXT4_MAP_UNWRITTEN;
>     ...
> }
>
> At the found label, check_block_validity() is only called if EXT4_MAP_MAPPED
> is set:
>
> found:
>     if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
>         ret = check_block_validity(inode, map);
>         ...
>
> Since unwritten extents skip check_block_validity(), could an attacker craft
> a filesystem with an unwritten extent in a directory pointing to critical
> metadata, forcing ext4_getblk() to read those blocks and leak their contents?
>
This is a false positive:

1. Symlinks cannot have unwritten extents. ext4 only creates
   unwritten extents via fallocate for regular files; symlink
   data is always fully written at creation time.

2. Reading unwritten extents does not leak stale data.

3. Writing to unwritten extents triggers re-validation. The
   unwritten-to-written conversion re-maps through the full
   ext4_map_blocks() path (without CACHED_NOWAIT), which runs
   check_block_validity() before any user data reaches the
   extent.