blk-throttle: fix divide-by-zero on legacy iops limit of 0
Subsystems (3)
PATCH v2 1/1 blk-throttle: fix divide-by-zero on legacy iops limit of 0
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.
---
Changes in v2:
- Use a "void *field" local for the config write so the assignment reads *(u64 *)field / *(unsigned int *)field instead of the (type *)((void *)tg + of_cft(of)->private) casts.
- Use min(v, (u64)UINT_MAX) instead of min_t(u64, v, UINT_MAX).
--- block/blk-throttle.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
block/blk-throttle.c +10 -3
@@ -883,7 +883,12 @@ static unsigned long tg_dispatch_iops_time(struct throtl_grp *tg, struct bio *biu32 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);
@@ -1383,10 +1388,12 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of, tg =blkg_to_tg (ctx.blkg);tg_update_carryover (tg); +void *field = (void *)tg +of_cft (of)->private; +if (is_u64) - *(u64 *)((void *)tg +of_cft (of)->private) = v; + *(u64 *)field = v;else - *(unsigned int *)((void *)tg +of_cft (of)->private) = v; + *(unsigned int *)field =min (v, (u64 )UINT_MAX );tg_conf_updated (tg, false); ret =0 ;
Signature · 1 line
2.43.0