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

[SJ Park <sj@kernel.org>]

On Mon, 06 Jul 2026 15:19:38 +0000 sashiko-bot@kernel.org wrote:

> 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.

A minor issue for unlikely user setup.  But I agree this is better to be fixed,
and the core layer parameters validation makes this easy to fix.  I will work
on fixing this, in the next version of this series, or separately.

> 
> 
> [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?

Yes, but only for one aggregation interval.  I don't think this is an issue.

> 
> > +
> > +	return err ? err : count;
> > +}
> > +
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706141912.88445-1-sj@kernel.org?part=13


Thanks,
SJ

[...]