Re: [PATCH 5/6] mm/damon/sysfs: support aging_flush

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `aging_flush` field in `struct damon_sysfs_context` is not initialized during allocation, leading to undefined default behavior and a minor information leak.
--

commit d37fc04bb82ae96572d3c46e4e494980a145a1c1
Author: Krishna Iyer <kiyer@crusoe.ai>

mm/damon/sysfs: support aging_flush

This commit exposes the aging_flush monitoring context option via a new DAMON
sysfs context file. It accepts boolean strings to configure the value, which
the commit message states defaults to N.

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index e3858ffab4b22..c8bc9cfbe4cf5 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -1403,6 +1403,7 @@ struct damon_sysfs_context {
>  	struct kobject kobj;
>  	enum damon_ops_id ops_id;
>  	unsigned long addr_unit;
> +	bool aging_flush;

[Severity: Medium]
Since this structure is allocated using kmalloc_obj() in
damon_sysfs_context_alloc(), does this leave the new aging_flush field
uninitialized?

The commit message mentions it "defaults to N", but without an explicit
initialization in damon_sysfs_context_alloc(), it appears it could inherit a
random value from the heap:

mm/damon/sysfs.c:damon_sysfs_context_alloc() {
    ...
	struct damon_sysfs_context *context = kmalloc_obj(*context);
    ...
	context->pause = false;
	return context;
}

>  	struct damon_sysfs_attrs *attrs;
>  	struct damon_sysfs_targets *targets;
>  	struct damon_sysfs_schemes *schemes;

[ ... ]

> @@ -1618,6 +1619,29 @@ static ssize_t pause_store(struct kobject *kobj, struct kobj_attribute *attr,
>  	return count;
>  }
>  
> +static ssize_t aging_flush_show(struct kobject *kobj,
> +		struct kobj_attribute *attr, char *buf)
> +{
> +	struct damon_sysfs_context *context = container_of(kobj,
> +			struct damon_sysfs_context, kobj);
> +
> +	return sysfs_emit(buf, "%c\n", context->aging_flush ? 'Y' : 'N');

[Severity: Medium]
If aging_flush is left uninitialized during allocation, could reading it here
expose uninitialized kernel memory to user-space?

> +}
> +
> +static ssize_t aging_flush_store(struct kobject *kobj,

[ ... ]

> @@ -2102,6 +2130,7 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
>  	if (err)
>  		return err;
>  	ctx->addr_unit = READ_ONCE(sys_ctx->addr_unit);
> +	ctx->aging_flush = READ_ONCE(sys_ctx->aging_flush);

[Severity: Medium]
Could reading an uninitialized sys_ctx->aging_flush here cause unpredictable
behavior by randomly applying the uninitialized value to the core DAMON
context?

>  	/* addr_unit is respected by only DAMON_OPS_PADDR */
>  	if (ops_id == DAMON_OPS_PADDR)
>  		ctx->min_region_sz = max(

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830051407.50008-1-kiyer@crusoe.ai?part=5