Re: [PATCH] blk-throttle: fix divide-by-zero on legacy iops limit of 0

Haris Iqbal <[email protected]>
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

On 7/14/26 12:35, Tao Cui wrote:
> From: Tao Cui <[email protected]>
> 
> Writing a multiple of 2^32 (e.g. 4294967296) to a legacy cgroup v1
> throttle iops file (blkio.throttle.{read,write}_iops_device) silently
> truncates to 0: tg_set_conf() stores the sscanf-parsed u64 value into
> an unsigned int field with no clamping. The cgroup v2 path,
> tg_set_limit(), already clamps the same kind of value with
> min_t(u64, val, UINT_MAX), but the legacy path never did. Note that
> the "!v -> U64_MAX" mapping only catches an explicit zero and does not
> catch a value that truncates to zero.
> 
> With iops stored as 0, tg_update_has_rules() sets has_rules_iops[] and
> the next IO reaches tg_within_iops_limit(), which computes
> 
>      jiffy_wait = max(jiffy_wait, HZ / iops_limit + 1);
> 
> triggering a divide-by-zero oops.
> 
> Fix it in two places:
> 
>    * tg_set_conf(): clamp the value to UINT_MAX, consistent with
>      tg_set_limit(). This closes the truncation root cause (and the
>      general silent truncation for any value above UINT_MAX).
> 
>    * tg_dispatch_iops_time(): treat iops_limit == 0 as unlimited so the
>      divide in tg_within_iops_limit() is never reached, defending
>      against any future path that could produce a zero limit.
> 

Does this need a "Fixes:" tag.

> Signed-off-by: Tao Cui <[email protected]>
> ---
>   block/blk-throttle.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index ffc3b70065d4..3f3c1374f4b2 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -883,7 +883,12 @@ static unsigned long tg_dispatch_iops_time(struct throtl_grp *tg, struct bio *bi
>   	u32 iops_limit = tg_iops_limit(tg, rw);
>   	unsigned long iops_wait;
>   
> -	if (iops_limit == UINT_MAX || tg->flags & THROTL_TG_CANCELING)
> +	/*
> +	 * iops_limit == 0 is not a valid limit. Treat it as unlimited so we
> +	 * never reach the HZ / iops_limit divide in tg_within_iops_limit().
> +	 */
> +	if (iops_limit == UINT_MAX || iops_limit == 0 ||
> +	    tg->flags & THROTL_TG_CANCELING)
>   		return 0;
>   
>   	tg_update_slice(tg, rw);
> @@ -1386,7 +1391,8 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of,
>   	if (is_u64)
>   		*(u64 *)((void *)tg + of_cft(of)->private) = v;
>   	else
> -		*(unsigned int *)((void *)tg + of_cft(of)->private) = v;
> +		*(unsigned int *)((void *)tg + of_cft(of)->private) =
> +			min_t(u64, v, UINT_MAX);
>   
>   	tg_conf_updated(tg, false);
>   	ret = 0;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.