Re: [PATCH -next v5 06/32] ext4: recheck extent status tree before block allocation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Reading uninitialized `map->m_seq` on error paths leads to an infinite retry loop in `ext4_map_blocks()`, causing a kernel hard lockup.
--

--- Patch [6]: [PATCH -next v5 06/32] ext4: recheck extent status tree before block allocation ---
commit 6ae9b515f2027351280fce82734e7e8361a281e7
Author: Zhang Yi <[email protected]>

ext4: recheck extent status tree before block allocation

After acquiring i_data_sem in write mode, recheck that the mapping
found via the extent status tree or disk query has not changed. If
we find a valid extent and the sequence number has changed, retry
the entire lookup to obtain the correct trimmed mapping.

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 9dbece14ae569..548a3968c5a7d 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c

[ ... ]

> @@ -820,6 +821,19 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>  	 * with create == 1 flag.
>  	 */
>  	down_write(&EXT4_I(inode)->i_data_sem);
> +
> +	/*
> +	 * Check the validity of the mapping found via the extent status
> +	 * tree or the disk query. A racing truncate may have changed the
> +	 * extent, since writeback does not hold i_rwsem or the folio locks
> +	 * covering the full extent.
> +	 */
> +	if (map->m_seq != READ_ONCE(EXT4_I(inode)->i_es_seq)) {
> +		up_write(&EXT4_I(inode)->i_data_sem);
> +		map->m_flags = 0;
> +		map->m_len = orig_mlen;
> +		goto create_retry;
> +	}

[Severity: High]
Could this cause an infinite loop and hard lockup on disk error?

If ext4_map_query_blocks() encounters an error like -EIO or -EFSCORRUPTED,
it returns < 0 without setting map->m_seq.

However, if EXT4_GET_BLOCKS_CREATE is set, the function does not return
early on error and falls through to this TOCTOU check.

Since callers like _ext4_get_block() allocate struct ext4_map_blocks
on the stack:

fs/ext4/inode.c:_ext4_get_block() {
        struct ext4_map_blocks map;
        ...
        map.m_lblk = iblock;
        map.m_len = bh->b_size >> inode->i_blkbits;

        ret = ext4_map_blocks(..., &map, flags);
}

Will map->m_seq contain uninitialized stack garbage, causing the sequence
check to mismatch and jump to create_retry, which then encounters the exact
same disk error indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6