avoid divide-by-zero due to max_nr_accesses overflow
Subsystems (3)
SeongJae Park
maintainerv2 cover avoid divide-by-zero due to max_nr_accesses overflow
Changes from v1
(https://lore.kernel.org/damon/[email protected]/)
- Add Reported-by: Jakub Acs <[email protected]>
The maximum nr_accesses of given DAMON context can be calculated by dividing the aggregation interval by the sampling interval. Some logics in DAMON uses the maximum nr_accesses as a divisor. Hence, the value shouldn't be zero. Such case is avoided since DAMON avoids setting the agregation interval as samller than the sampling interval. However, since nr_accesses is unsigned int while the intervals are unsigned long, the maximum nr_accesses could be zero while casting.
Avoid the divide-by-zero by implementing a function that handles the corner case (first patch), and replaces the vulnerable direct max nr_accesses calculations (remaining patches).
Note that the patches for the replacements are divided for broken commits, to make backporting on required tres easier. Especially, the last patch is for a patch that not yet merged into the mainline but in mm tree.
SeongJae Park (5):
mm/damon: implement a function for max nr_accesses safe calculation mm/damon/core: avoid divide-by-zero during monitoring results update mm/damon/ops-common: avoid divide-by-zero during region hotness
calculation
mm/damon/lru_sort: avoid divide-by-zero in hot threshold calculation mm/damon/core: avoid divide-by-zero from pseudo-moving window length
calculation
include/linux/damon.h | 7 +++++++ mm/damon/core.c | 12 +++--------- mm/damon/lru_sort.c | 4 +--- mm/damon/ops-common.c | 5 ++--- 4 files changed, 13 insertions(+), 15 deletions(-)
base-commit: e845524c56a529768a8793e96304db09134eafdf
Signature · 1 line
2.34.1
SeongJae Park
maintainerPATCH v2 1/5 mm/damon: implement a function for max nr_accesses safe calculation
The maximum nr_accesses of given DAMON context can be calculated by dividing the aggregation interval by the sampling interval. Some logics in DAMON uses the maximum nr_accesses as a divisor. Hence, the value shouldn't be zero. Such case is avoided since DAMON avoids setting the agregation interval as samller than the sampling interval. However, since nr_accesses is unsigned int while the intervals are unsigned long, the maximum nr_accesses could be zero while casting. Implement a function that handles the corner case.
Note that this commit is not fixing the real issue since this is only introducing the safe function that will replaces the problematic divisions. The replacements will be made by followup commits, to make backporting on stable series easier.
--- include/linux/damon.h | 7 +++++++ 1 file changed, 7 insertions(+)
include/linux/damon.h +7 -0
@@ -681,6 +681,13 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)return ctx->ops.id ==DAMON_OPS_VADDR || ctx->ops.id ==DAMON_OPS_FVADDR ; } +static inline unsigned int damon_max_nr_accesses (const struct damon_attrs *attrs) +{ +/* {aggr,sample}_interval are unsigned long, hence could overflow */ +return min (attrs->aggr_interval / attrs->sample_interval, + (unsigned long )UINT_MAX ); +} +int damon_start (struct damon_ctx **ctxs,int nr_ctxs,bool exclusive);int damon_stop (struct damon_ctx **ctxs,int nr_ctxs);
Signature · 1 line
2.34.1
SeongJae Park
maintainerPATCH v2 2/5 mm/damon/core: avoid divide-by-zero during monitoring results update
In reply to SeongJae ParkWhen monitoring attributes are changed, DAMON updates access rate of the monitoring results accordingly. For that, it divides some values by the maximum nr_accesses. However, due to the type of the related variables, simple division-based calculation of the divisor can return zero. As a result, divide-by-zero is possible. Fix it by using damon_max_nr_accesses(), which handles the case.
--- mm/damon/core.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
mm/damon/core.c +2 -8
@@ -500,20 +500,14 @@ static unsigned int damon_age_for_new_attrs(unsigned int age,static unsigned int damon_accesses_bp_to_nr_accesses (unsigned int accesses_bp,struct damon_attrs *attrs) { -unsigned int max_nr_accesses = - attrs->aggr_interval / attrs->sample_interval; - -return accesses_bp * max_nr_accesses /10000 ; +return accesses_bp *damon_max_nr_accesses (attrs) /10000 ; }/* convert nr_accesses to access ratio in bp (per 10,000) */ static unsigned int damon_nr_accesses_to_accesses_bp (unsigned int nr_accesses,struct damon_attrs *attrs) { -unsigned int max_nr_accesses = - attrs->aggr_interval / attrs->sample_interval; - -return nr_accesses *10000 / max_nr_accesses; +return nr_accesses *10000 /damon_max_nr_accesses (attrs); } static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
Signature · 1 line
2.34.1
SeongJae Park
maintainerPATCH v2 3/5 mm/damon/ops-common: avoid divide-by-zero during region hotness calculation
In reply to SeongJae ParkWhen calculating the hotness of each region for the under-quota regions prioritization, DAMON divides some values by the maximum nr_accesses. However, due to the type of the related variables, simple division-based calculation of the divisor can return zero. As a result, divide-by-zero is possible. Fix it by using damon_max_nr_accesses(), which handles the case.
--- mm/damon/ops-common.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
mm/damon/ops-common.c +2 -3
@@ -73,7 +73,6 @@ void damon_pmdp_mkold(pmd_t *pmd, struct vm_area_struct *vma, unsigned long addrint damon_hot_score (struct damon_ctx *c,struct damon_region *r,struct damos *s) { -unsigned int max_nr_accesses;int freq_subscore;unsigned int age_in_sec;int age_in_log, age_subscore;
@@ -81,8 +80,8 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,unsigned int age_weight = s->quota.weight_age;int hotness; - max_nr_accesses = c->attrs.aggr_interval / c->attrs.sample_interval; - freq_subscore = r->nr_accesses *DAMON_MAX_SUBSCORE / max_nr_accesses; + freq_subscore = r->nr_accesses *DAMON_MAX_SUBSCORE / +damon_max_nr_accesses (&c->attrs); age_in_sec = (unsigned long )r->age * c->attrs.aggr_interval /1000000 ; for (age_in_log = 0; age_in_log < DAMON_MAX_AGE_IN_LOG && age_in_sec;
Signature · 1 line
2.34.1
SeongJae Park
maintainerPATCH v2 4/5 mm/damon/lru_sort: avoid divide-by-zero in hot threshold calculation
In reply to SeongJae ParkWhen calculating the hotness threshold for lru_prio scheme of DAMON_LRU_SORT, the module divides some values by the maximum nr_accesses. However, due to the type of the related variables, simple division-based calculation of the divisor can return zero. As a result, divide-by-zero is possible. Fix it by using damon_max_nr_accesses(), which handles the case.
--- mm/damon/lru_sort.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
mm/damon/lru_sort.c +1 -3
@@ -195,9 +195,7 @@ static int damon_lru_sort_apply_parameters(void)if (err)return err; -/* aggr_interval / sample_interval is the maximum nr_accesses */ - hot_thres = damon_lru_sort_mon_attrs.aggr_interval / - damon_lru_sort_mon_attrs.sample_interval * + hot_thres =damon_max_nr_accesses (&damon_lru_sort_mon_attrs) * hot_thres_access_freq /1000 ; scheme =damon_lru_sort_new_hot_scheme (hot_thres);if (!scheme)
Signature · 1 line
2.34.1
SeongJae Park
maintainerPATCH v2 5/5 mm/damon/core: avoid divide-by-zero from pseudo-moving window length calculation
In reply to SeongJae ParkWhen calculating the pseudo-moving access rate, DAMON divides some values by the maximum nr_accesses. However, due to the type of the related variables, simple division-based calculation of the divisor can return zero. As a result, divide-by-zero is possible. Fix it by using damon_max_nr_accesses(), which handles the case.
Note that this is a fix for a commit that not in the mainline but mm tree.
---
Note that this is for a patch in mm-stable that not yet merged into the mainline.
mm/damon/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/damon/core.c +1 -1
@@ -1665,7 +1665,7 @@ void damon_update_region_access_rate(struct damon_region *r, bool accessed,* aggr_interval, owing to validation of damon_set_attrs(). */ if (attrs->sample_interval) - len_window = attrs->aggr_interval / attrs->sample_interval; + len_window =damon_max_nr_accesses (attrs); r->nr_accesses_bp =damon_moving_sum (r->nr_accesses_bp, r->last_nr_accesses *10000 , len_window, accessed ?10000 :0 );
Signature · 1 line
2.34.1