Re: [PATCH] mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets test

[Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>]

在 2026/10/29 10:02, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] mm/damon: memory leak of ctx in damon_sysfs_test_add_targets() on skip
> --
>
> commit 15e9d751202cdf7c593524fe7b4266b6f6dd0dbf
> Author: Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>
>
> mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets test
>
> This commit fixes a struct pid reference leak in
> damon_sysfs_test_add_targets() by calling damon_select_ops() to select
> DAMON_OPS_VADDR on the monitoring context before adding targets, ensuring
> that damon_destroy_ctx() invokes damon_va_cleanup_target() to release the
> pid references. If DAMON_OPS_VADDR is not registered, the test cleans up
> its allocations and skips itself.
>
>> diff --git a/mm/damon/tests/sysfs-kunit.h b/mm/damon/tests/sysfs-kunit.h
>> index 138a4b8d14e73..634387c6178b9 100644
>> --- a/mm/damon/tests/sysfs-kunit.h
>> +++ b/mm/damon/tests/sysfs-kunit.h
>> @@ -79,6 +79,14 @@ static void damon_sysfs_test_add_targets(struct kunit *test)
>>   		kunit_skip(test, "ctx alloc fail");
>>   	}
>>   
>> +	if (damon_select_ops(ctx, DAMON_OPS_VADDR)) {
>> +		kfree(sysfs_targets->targets_arr);
>> +		kfree(sysfs_targets);
>> +		kfree(sysfs_target->regions);
>> +		kfree(sysfs_target);
>> +		kunit_skip(test, "vaddr ops not registered");
>> +	}
> [Severity: Medium]
> Does this error path leak ctx when damon_select_ops() fails (for example,
> when CONFIG_DAMON_VADDR is disabled)?
>
> Earlier in damon_sysfs_test_add_targets(), ctx is allocated via
> damon_new_ctx():
>
> 	ctx = damon_new_ctx();
> 	if (!ctx) {
> 		kfree(sysfs_targets->targets_arr);
> 		kfree(sysfs_targets);
> 		kfree(sysfs_target->regions);
> 		kfree(sysfs_target);
> 		kunit_skip(test, "ctx alloc fail");
> 	}
>
> When damon_select_ops(ctx, DAMON_OPS_VADDR) returns a non-zero error,
> kunit_skip() aborts the test immediately without calling
> damon_destroy_ctx(ctx). Should damon_destroy_ctx(ctx) be called before
> kunit_skip() here as well?

Agree, fixed in v2

thanks

xiaolei

>