Skip to contentSkip to conversation

mm/damon/sysfs-schemes: put stats for scheme_add_dirs() internal error

RFC PATCH In review SeongJae Park 2 participants 3 messages Jun 17

Subsystems (3)
lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

RFC PATCH 1/1 mm/damon/sysfs-schemes: put stats for scheme_add_dirs() internal error

damon_sysfs_scheme_add_dirs() setup the tried_regions directory after the stats directory setup is completed. When the tried_regions directory setup is failed, the setup function ensures the reference for the tried regions directory is released. Hence the error path should put references on setup succeeded directory objects, starting from the stats directory. However, the error path is putting the tried_regions directory instead of the stats directory.

As a direct result, the stats directory object is leaked. Worse yet, if the tried_regions directory setup failed from the initial allocation, the scheme->tried_regions field remains uninitialized. The following kobject_put(&scheme->tried_regions->kobj) call in the error path will dereference the uninitialized memory. The setup failures should not be common. But once it happens, the consequence is quite bad.

Fix this issue by correctly putting the stats directory instead of the tried_regions directory.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/[email protected]

Fixes 5181b75f438d ("mm/damon/sysfs-schemes: implement schemes/tried_regions directory")
Cc <[email protected]> # 6.2.x
---
 mm/damon/sysfs-schemes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
mm/damon/sysfs-schemes.c +4 -4
index 329cfd0bbe9f3..0668a0e539713 100644
@@ -2516,12 +2516,12 @@ static int damon_sysfs_scheme_add_dirs(struct damon_sysfs_scheme *scheme)
 		goto put_filters_watermarks_quotas_access_pattern_out;
 	err = damon_sysfs_scheme_set_tried_regions(scheme);
 	if (err)
-		goto put_tried_regions_out;
+		goto put_stats_out;
 	return 0;
 
-put_tried_regions_out:
-	kobject_put(&scheme->tried_regions->kobj);
-	scheme->tried_regions = NULL;
+put_stats_out:
+	kobject_put(&scheme->stats->kobj);
+	scheme->stats = NULL;
 put_filters_watermarks_quotas_access_pattern_out:
 	kobject_put(&scheme->ops_filters->kobj);
 	scheme->ops_filters = NULL;

base-commit: 2d57ad161a03e7a68b2c036bc4895ef56b4c76a6

Signature · 1 line
2.47.3
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized Pointer Dereference in damon_sysfs_access_pattern_add_dirs() error path. --

commit 3b785df7194b7541f02f158bcc66d968319b86ca
Author: SeongJae Park <[email protected]>
Subject: mm/damon/sysfs-schemes: put stats for scheme_add_dirs() internal error

This commit fixes a bug in damon_sysfs_scheme_add_dirs() where the error path incorrectly puts the uninitialized tried_regions directory instead of the stats directory. This prevents a leak of the stats object and a potential uninitialized pointer dereference.

Quoted 8 lines
mm/damon/sysfs-schemes.c +0 -0
index 329cfd0bbe9f3..0668a0e539713 100644
@@ -2516,12 +2516,12 @@ static int damon_sysfs_scheme_add_dirs(struct damon_sysfs_scheme *scheme)
 		goto put_filters_watermarks_quotas_access_pattern_out;
 	err = damon_sysfs_scheme_set_tried_regions(scheme);
 	if (err)

mm/damon/sysfs-schemes.c static int damon_sysfs_scheme_add_dirs(struct damon_sysfs_scheme *scheme) · line 2524

Quoted 10 lines
mm/damon/sysfs-schemes.c +4 -4
-		goto put_tried_regions_out;
+		goto put_stats_out;
 	return 0;
 
-put_tried_regions_out:
-	kobject_put(&scheme->tried_regions->kobj);
-	scheme->tried_regions = NULL;
+put_stats_out:
+	kobject_put(&scheme->stats->kobj);
+	scheme->stats = NULL;

[Severity: High]
This is a pre-existing issue, but does the identical bug exist in damon_sysfs_access_pattern_add_dirs() within the same file?

Looking at damon_sysfs_access_pattern_alloc(), the access_pattern struct is allocated using kmalloc_obj without zero-initialization, causing its pointer fields like sz, nr_accesses, and age to hold garbage values.

If a directory setup fails in damon_sysfs_access_pattern_add_dirs() due to a memory allocation failure, the error path attempts to clean up the failed object rather than the previously successful allocations:

mm/damon/sysfs-schemes.c:damon_sysfs_access_pattern_add_dirs() {
    ...
put_sz_out:
    kobject_put(&access_pattern->sz->kobj);
    access_pattern->sz = NULL;
    ...
}

Could this cause a kernel panic by dereferencing an uninitialized garbage pointer if the initial allocation fails, exactly like the issue fixed here for scheme->tried_regions?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#2 lore raw
Quoted 56 lines · On Wed, 17 Jun 2026 05:48:32 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized Pointer Dereference in damon_sysfs_access_pattern_add_dirs() error path. --

commit 3b785df7194b7541f02f158bcc66d968319b86ca
Author: SeongJae Park <[email protected]>
Subject: mm/damon/sysfs-schemes: put stats for scheme_add_dirs() internal error

This commit fixes a bug in damon_sysfs_scheme_add_dirs() where the error path incorrectly puts the uninitialized tried_regions directory instead of the stats directory. This prevents a leak of the stats object and a potential uninitialized pointer dereference.

Quoted 8 lines
mm/damon/sysfs-schemes.c +0 -0
index 329cfd0bbe9f3..0668a0e539713 100644
@@ -2516,12 +2516,12 @@ static int damon_sysfs_scheme_add_dirs(struct damon_sysfs_scheme *scheme)
 		goto put_filters_watermarks_quotas_access_pattern_out;
 	err = damon_sysfs_scheme_set_tried_regions(scheme);
 	if (err)

mm/damon/sysfs-schemes.c static int damon_sysfs_scheme_add_dirs(struct damon_sysfs_scheme *scheme) · line 2524

Quoted 10 lines
mm/damon/sysfs-schemes.c +4 -4
-		goto put_tried_regions_out;
+		goto put_stats_out;
 	return 0;
 
-put_tried_regions_out:
-	kobject_put(&scheme->tried_regions->kobj);
-	scheme->tried_regions = NULL;
+put_stats_out:
+	kobject_put(&scheme->stats->kobj);
+	scheme->stats = NULL;

[Severity: High]
This is a pre-existing issue, but does the identical bug exist in damon_sysfs_access_pattern_add_dirs() within the same file?

Looking at damon_sysfs_access_pattern_alloc(), the access_pattern struct is allocated using kmalloc_obj without zero-initialization, causing its pointer fields like sz, nr_accesses, and age to hold garbage values.

If a directory setup fails in damon_sysfs_access_pattern_add_dirs() due to a memory allocation failure, the error path attempts to clean up the failed object rather than the previously successful allocations:

mm/damon/sysfs-schemes.c:damon_sysfs_access_pattern_add_dirs() {
    ...
put_sz_out:
    kobject_put(&access_pattern->sz->kobj);
    access_pattern->sz = NULL;
    ...
}
Quoted 3 lines

Could this cause a kernel panic by dereferencing an uninitialized garbage pointer if the initial allocation fails, exactly like the issue fixed here for scheme->tried_regions?

Nice finding, I will make a fix of this and post as a series together with the scheme dir fix.

Thanks,
SJ

[...]