Re: [PATCH 4/4] iomap: fix out-of-bounds bitmap_set() with zero-length range

[Zhang Yi <yi.zhang@huaweicloud.com>]

On 5/14/2026 11:08 PM, Joanne Koong wrote:
> On Wed, May 13, 2026 at 11:35 PM Zhang Yi <yi.zhang@huaweicloud.com> wrote:
>>
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> ifs_set_range_dirty() and ifs_set_range_uptodate() compute last_blk
>> as (off + len - 1) >> i_blkbits.  When off is 0 and len is 0, the
>> unsigned subtraction underflows to SIZE_MAX, producing a huge
>> last_blk and nr_blks value that causes bitmap_set() to write far
>> beyond the ifs->state allocation.
>>
>> Regarding ifs_set_range_uptodate(), it is temporarily safe because len
>> cannot be passed in as 0. However, for ifs_set_range_dirty() this is
>> reachable from __iomap_write_end(): when copy_folio_from_iter_atomic()
>> returns 0 (e.g. user buffer fault) and the folio is already uptodate,
>> the guard at the top of __iomap_write_end() does not trigger because
>> !folio_test_uptodate() is false, and iomap_set_range_dirty() is called
>> with copied == 0.
> 
> Is the Fixes: 4ce02c679722  ("iomap: Add per-block dirty state
> tracking to improve performance") tag needed for this?
> 
>>
>> Add a !len guard to both functions before the computation, so that a
>> zero-length range is a no-op.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> ---
>>  fs/iomap/buffered-io.c | 23 +++++++++++++++--------
>>  1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
>> index 27ab33edbdee..6fe5f7e998fd 100644
>> --- a/fs/iomap/buffered-io.c
>> +++ b/fs/iomap/buffered-io.c
>> @@ -67,11 +67,14 @@ static bool ifs_set_range_uptodate(struct folio *folio,
>>                 struct iomap_folio_state *ifs, size_t off, size_t len)
>>  {
>>         struct inode *inode = folio->mapping->host;
>> -       unsigned int first_blk = off >> inode->i_blkbits;
>> -       unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
>> -       unsigned int nr_blks = last_blk - first_blk + 1;
>> +       unsigned int first_blk, last_blk;
>>
>> -       bitmap_set(ifs->state, first_blk, nr_blks);
>> +       if (!len)
>> +               return true;
> 
> I think both callers of ifs_set_range_uptodate() use the return value
> to decide whether to mark the folio uptodate or not - does this still
> need to return ifs_is_fully_uptodate(folio, ifs) instead of always
> true?
> 

Yeah, you are right! I missed that, will fix.

Thanks,
Yi.

> Thanks,
> Joanne
> 
>> +
>> +       first_blk = off >> inode->i_blkbits;
>> +       last_blk = (off + len - 1) >> inode->i_blkbits;
>> +       bitmap_set(ifs->state, first_blk, last_blk - first_blk + 1);
>>         return ifs_is_fully_uptodate(folio, ifs);
>>  }