Re: [PATCH] dm cache: fix divide error in too_many_discard_blocks()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] dm cache: 32-bit truncation and divide error in
  cache_create()
- [Low] dm cache: line-wrapped Fixes: trailer in commit message
--

commit 1daf5e2259576307a337e1dd87d8c7878f108ae2
Author: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>

> dm cache: fix divide error in too_many_discard_blocks()
> 
> too_many_discard_blocks() uses sector_div(), whose divisor is a u32.
> discard_block_size is a 64-bit sector_t that calculate_discard_block_size()
> keeps doubling, so once it reaches a multiple of 2^32 the divisor is
> truncated to 0 and the division traps with a divide error. Smaller values
> above 2^32 are also silently truncated, giving a wrong quotient.
> 
> This needs an origin of more than 2^45 sectors, which is unrealistic (this
> is a thing still in 2026!!) for a real device but reachable with a fuzzer.
> 
> Use div64_u64 so the full 64-bit divisor is used.
> 
> Reported-by: syzbot+3e16980543e238b95bd1@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3e16980543e238b95bd1
> Fixes: 08b184514f65d ("dm cache: revert "prevent corruption caused by
>                                 discard_block_size > cache_block_size"")
> Cc: stable@vger.kernel.org
> Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>

[Severity: Low]
The Fixes: trailer is wrapped across two lines with a 13-character SHA,
which should be on a single unwrapped line with a 12-character SHA:

Fixes: 08b184514f65 ("dm cache: revert "prevent corruption caused by discard_block_size > cache_block_size"")

> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 1a5072425c4a1..2d40d6047ef11 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -2383,9 +2383,7 @@ static int create_cache_policy(struct cache *cache, struct cache_args *ca,
>  static bool too_many_discard_blocks(sector_t discard_block_size,
>  				    sector_t origin_size)
>  {
> -	(void) sector_div(origin_size, discard_block_size);
> -
> -	return origin_size > MAX_DISCARD_BLOCKS;
> +	return div64_u64(origin_size, discard_block_size) > MAX_DISCARD_BLOCKS;
>  }
>  
>  static sector_t calculate_discard_block_size(sector_t cache_block_size,

[Severity: Medium]
Does allowing calculate_discard_block_size() to return values >= 2^32 just
move the 32-bit truncation and divide-by-zero into cache_create()?

struct cache declares discard_block_size as uint32_t, and cache_create()
immediately passes it to dm_sector_div_up(), which uses sector_div():

drivers/md/dm-cache-target.c:cache_create() {
	...
	cache->discard_block_size =
		calculate_discard_block_size(cache->sectors_per_block,
					     cache->origin_sectors);
	cache->discard_nr_blocks = to_dblock(dm_sector_div_up(cache->origin_sectors,
							      cache->discard_block_size));
	...
}

When origin_sectors > 2^45 and calculate_discard_block_size() returns a
multiple of 2^32, assigning it to cache->discard_block_size truncates it to
0 and dm_sector_div_up() traps with a divide error.  For non-power-of-two
cache->sectors_per_block where the truncated lower 32 bits are non-zero,
cache->discard_block_size is no longer a multiple of
cache->sectors_per_block, so oblock_to_dblock() can produce dblock indices
>= cache->discard_nr_blocks and hit the BUG_ON() in set_discard() or access
cache->discard_bitset out of bounds.  The other uses of
cache->discard_block_size in oblock_to_dblock(),
calc_discard_block_range(), set_discard_range(), and set_discard_limits()
also use 32-bit division helpers or shifts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106180809.1419-1-drifabdelmalekmohamedsaid@gmail.com?part=1