Re: [RFC PATCH v1.1 13/16] mm/damon/sysfs: implement probe/weight file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unbounded `weight` from sysfs causes an integer overflow in `damon_probe_hits_wsum()`, which corrupts the core region scoring algorithm.
- [High] Setting a large `weight` via sysfs trivially bypasses the `max_nr_regions` limit, leading to unbounded kernel memory usage and CPU starvation.
--

commit 13d5322850c993787f439a6daf8065f8717e7f80
Author: SJ Park <sj@kernel.org>

mm/damon/sysfs: implement probe/weight file

This commit implements a new sysfs file, weight, under the probe directory.
Users can set the probe weight and enable monitoring using it.

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index b65651498e0d1..a8ef7903bf94f 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c

[ ... ]

> @@ -1100,12 +1101,35 @@ static void damon_sysfs_probe_rm_dirs(struct damon_sysfs_probe *probe)

[ ... ]

> +static ssize_t weight_store(struct kobject *kobj,
> +		struct kobj_attribute *attr, const char *buf, size_t count)
> +{
> +	struct damon_sysfs_probe *probe = container_of(kobj,
> +			struct damon_sysfs_probe, kobj);
> +	int err = kstrtouint(buf, 0, &probe->weight);

[Severity: High]
Does this allow an unbounded unsigned int weight to be set, potentially
causing an integer overflow during calculation?

In damon_probe_hits_wsum(), the score is calculated by multiplying the
hits and the weight:

mm/damon/core.c:damon_probe_hits_wsum() {
    ...
    sum += r->last_probe_hits[i++] * probe->weight;
    ...
}

Because weight is allowed to be up to UINT_MAX, and probe_hits can be up to
255, could this calculation wrap around? If it wraps around, regions with a
higher number of probe hits might erroneously compute a lower weighted sum
than colder regions, which seems like it would break the ability to
accurately track memory.


[Severity: High]
Could a large weight bypass the max_nr_regions limit and lead to unbounded
region growth?

In kdamond_merge_regions(), the merge loop condition checks against max_thres:

mm/damon/core.c:kdamond_merge_regions() {
    ...
    } while (nr_regions > c->attrs.max_nr_regions &&
             threshold / 2 < max_thres);
    ...
}

The initial threshold scales with the unbounded weight. If a user configures a
large weight, does threshold / 2 immediately exceed max_thres on the first
loop iteration?

This would cause the loop to terminate prematurely, failing to iteratively
increase the threshold to merge regions. Could this allow the number of
regions to grow without bound and lead to severe CPU starvation?

> +
> +	return err ? err : count;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=13