Re: [PATCH] selftests/damon: Add missing NULL checks after malloc()

Re: [PATCH] selftests/damon: Add missing NULL checks after malloc()

[SJ Park]

Cc-ing DAMON mailing list.  Please Cc DAMON mailing list for DAMON patches.

Hello longlong,

On Tue, 21 Jul 2026 10:58:02 +0800 longlong yan <yanlonglong@kylinos.cn> wrote:

> Add NULL checks after each malloc() call, printing an error message
> to stderr and returning -1 on failure.

Thank you for this patch!

Could you please elaborate why this change is needed, though?  What is the user
impacting issue that this patch is fixing?  Is this fixing a false positive or
negative test results?  Or, just making the allocation failure easier to find?

> 
> Fixes: b5906f5f7359 ("selftests/damon: add a test for update_schemes_tried_regions sysfs command")
> Fixes: c94df805c774 ("selftests/damon: implement a program for even-numbered memory regions access")

If this patch is only making the allocation failure easier to detect, I think
it is an improvement rather than a fix.  If that's the case, I think we don't
need above Fixes: tags.

If this is a real bug fix, maybe it deserves stable@ porting.  Please separate
this into two patches for each bug and Cc stable@ for easy stable@ porting.


Thanks,
SJ

[...]

[PATCH v2] selftests/damon: Add missing NULL checks after malloc()

[longlong yan]

In low-memory scenarios, malloc() can fail. Without checking,
the test will dereference NULL and crash, causing the test harness
to report a failure that is unrelated to DAMON functionality.
This is a false negative: the test should skip or report a resource error,
not crash and mask the actual test result.

Add NULL checks after each malloc() call, printing an error message
to stderr and returning -1 on failure.

Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
---
 tools/testing/selftests/damon/access_memory.c      | 11 ++++++++++-
 tools/testing/selftests/damon/access_memory_even.c | 11 ++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/damon/access_memory.c b/tools/testing/selftests/damon/access_memory.c
index 567793b11107..e1a8e050dcd4 100644
--- a/tools/testing/selftests/damon/access_memory.c
+++ b/tools/testing/selftests/damon/access_memory.c
@@ -38,8 +38,17 @@ int main(int argc, char *argv[])
 		mode = ACCESS_MODE_REPEAT;
 
 	regions = malloc(sizeof(*regions) * nr_regions);
-	for (i = 0; i < nr_regions; i++)
+	if (!regions) {
+		fprintf(stderr, "Failed to allocate regions array\n");
+		return -1;
+	}
+	for (i = 0; i < nr_regions; i++) {
 		regions[i] = malloc(sz_region);
+		if (!regions[i]) {
+			fprintf(stderr, "Failed to allocate region %d\n", i);
+			return -1;
+		}
+	}
 
 	do {
 		for (i = 0; i < nr_regions; i++) {
diff --git a/tools/testing/selftests/damon/access_memory_even.c b/tools/testing/selftests/damon/access_memory_even.c
index 93f3a71bcfd4..a443835c119c 100644
--- a/tools/testing/selftests/damon/access_memory_even.c
+++ b/tools/testing/selftests/damon/access_memory_even.c
@@ -26,8 +26,17 @@ int main(int argc, char *argv[])
 	sz_region = atoi(argv[2]);
 
 	regions = malloc(sizeof(*regions) * nr_regions);
-	for (i = 0; i < nr_regions; i++)
+	if (!regions) {
+		fprintf(stderr, "Failed to allocate regions array\n");
+		return -1;
+	}
+	for (i = 0; i < nr_regions; i++) {
 		regions[i] = malloc(sz_region);
+		if (!regions[i]) {
+			fprintf(stderr, "Failed to allocate region %d\n", i);
+			return -1;
+		}
+	}
 
 	while (1) {
 		for (i = 0; i < nr_regions; i++) {
-- 
2.43.0

Re: [PATCH v2] selftests/damon: Add missing NULL checks after malloc()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced error path returns -1 to prevent an OOM crash, but this does not coordinate with the Python test harnesses to actually skip the test or report a resource error. Instead, it causes tests to either silently pass (false positive) or crash with Python exceptions.
--

--- Patch [1]: [PATCH v2] selftests/damon: Add missing NULL checks after malloc() ---
commit c48bfd12db1afa969a2ab38d496264e74f1b6b1e
Author: longlong yan <yanlonglong@kylinos.cn>

selftests/damon: Add missing NULL checks after malloc()

This commit adds NULL checks after malloc() calls in access_memory.c and 
access_memory_even.c to prevent crashes during low-memory conditions.

> diff --git a/tools/testing/selftests/damon/access_memory.c b/tools/testing/selftests/damon/access_memory.c
> --- a/tools/testing/selftests/damon/access_memory.c
> +++ b/tools/testing/selftests/damon/access_memory.c
> @@ -38,8 +38,17 @@ int main(int argc, char *argv[])
>  		mode = ACCESS_MODE_REPEAT;
>  
>  	regions = malloc(sizeof(*regions) * nr_regions);
> -	for (i = 0; i < nr_regions; i++)
> +	if (!regions) {
> +		fprintf(stderr, "Failed to allocate regions array\n");
> +		return -1;

[Severity: Medium]
Will returning -1 here coordinate correctly with the Python test harnesses
like damos_quota.py and sysfs_update_schemes_tried_regions_wss_estimation.py?

If the Python scripts do not check the subprocess return code, an early exit
might leave data arrays empty. Depending on the script, this can either lead
to a Python IndexError (e.g., when accessing wss_collected in
pass_wss_estimation) or a silent test pass that masks the out of memory
condition entirely.

Should this return KSFT_SKIP (4) instead to properly notify the test harness,
or do the Python scripts need updating to verify the return code?

> +	}
> +	for (i = 0; i < nr_regions; i++) {
>  		regions[i] = malloc(sz_region);
> +		if (!regions[i]) {
> +			fprintf(stderr, "Failed to allocate region %d\n", i);
> +			return -1;
> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721060542.326-1-yanlonglong@kylinos.cn?part=1

Re: [PATCH v2] selftests/damon: Add missing NULL checks after malloc()

[SJ Park]

Hello longlong,


From the next time, when someone asked questions to your patch, please answer
the questions and keep the discussion until the next action item becomes clear.
Please don't post a new version without making sure such discussion is done.

Also, don't post a new version of a patch as a reply to the previous version.
Post as a new thread from the next time.

On Tue, 21 Jul 2026 14:05:42 +0800 longlong yan <yanlonglong@kylinos.cn> wrote:

> In low-memory scenarios, malloc() can fail. Without checking,
> the test will dereference NULL and crash, causing the test harness
> to report a failure that is unrelated to DAMON functionality.

'test harness' means the DAMON selftests that internally runs the access_memory
and access_memory_even programs, suc as damos_apply_interval.py, right?  Could
you share more details about how the test harness report a failure in the
situation?

> This is a false negative:

Shouldn't it be called false positive, as this is a context of tests?

> the test should skip or report a resource error,
> not crash and mask the actual test result.
> 
> Add NULL checks after each malloc() call, printing an error message
> to stderr and returning -1 on failure.

The above description reads like this patch makes the DAMON selftets skip or
report a resource error.  But I don't show how that works.  Could you please
elaborate?

> 
> Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
> ---

Please add changelog on the commentary area [1], with links to the previous
revisions.

[1] https://docs.kernel.org/process/submitting-patches.html#commentary


Thanks,
SJ

[...]