DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup
@ 2026-07-08  8:01 Guangshuo Li
  2026-07-08  8:20 ` sashiko-bot
  2026-07-08 14:17 ` SJ Park
  0 siblings, 2 replies; 4+ messages in thread
From: Guangshuo Li @ 2026-07-08  8:01 UTC (permalink / raw)
  To: SeongJae Park, Andrew Morton, damon, linux-mm, linux-kernel; +Cc: Guangshuo Li

damon_sysfs_scheme_add_dirs() sets up the tried_regions directory after
the stats directory has already been created.

If damon_sysfs_scheme_set_tried_regions() fails, scheme->tried_regions
has not been assigned. The error path nevertheless jumps to
put_tried_regions_out and dereferences scheme->tried_regions while
trying to put its kobject.

This can dereference uninitialized memory because struct
damon_sysfs_scheme is allocated with kmalloc-style allocation and
scheme->tried_regions is only assigned after tried_regions setup
succeeds.

On tried_regions setup failure, start cleanup from the last successfully
created directory, stats, instead of trying to put tried_regions.

Fixes: 5181b75f438d ("mm/damon/sysfs-schemes: implement schemes/tried_regions directory")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 mm/damon/sysfs-schemes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 329cfd0bbe9f..0668a0e53971 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -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;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup
  2026-07-08  8:01 [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup Guangshuo Li
@ 2026-07-08  8:20 ` sashiko-bot
  2026-07-08 14:22   ` SJ Park
  2026-07-08 14:17 ` SJ Park
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-07-08  8:20 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: damon

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

Pre-existing issues:
- [High] Uninitialized pointer dereference in error path of damon_sysfs_access_pattern_add_dirs()
--

--- Patch [1]: [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup ---
commit b97caf8d3b939c01dca3a53c5340d70ede6714d2
Author: Guangshuo Li <lgs201920130244@gmail.com>

mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup

This commit fixes an uninitialized pointer dereference in the error cleanup
path of damon_sysfs_scheme_add_dirs(). It changes the failure jump target 
for damon_sysfs_scheme_set_tried_regions() to avoid calling kobject_put()
on the uninitialized tried_regions pointer.

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 329cfd0bbe9f3..0668a0e539713 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -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;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this cleanup,
I noticed the exact same uninitialized pointer dereference exists earlier in
the same file.

If damon_sysfs_access_pattern_add_range_dir() fails during the setup of "sz",
does the error path dereference an uninitialized pointer?

mm/damon/sysfs-schemes.c:damon_sysfs_access_pattern_add_dirs() {
    ...
	err = damon_sysfs_access_pattern_add_range_dir(access_pattern,
			&access_pattern->sz, "sz");
	if (err)
		goto put_sz_out;
    ...
put_sz_out:
	kobject_put(&access_pattern->sz->kobj);
	access_pattern->sz = NULL;
	return err;
}

Since damon_sysfs_access_pattern is allocated with kmalloc_obj(), its fields
are uninitialized. If the directory creation fails, the function returns an
error without writing to the pointer. The error path then jumps to put_sz_out
and calls kobject_put() on the garbage pointer access_pattern->sz, identical
to the bug fixed by this patch.

Could this be addressed with a similar fix?

>  put_filters_watermarks_quotas_access_pattern_out:
>  	kobject_put(&scheme->ops_filters->kobj);
>  	scheme->ops_filters = NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708080107.727043-1-lgs201920130244@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup
  2026-07-08  8:01 [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup Guangshuo Li
  2026-07-08  8:20 ` sashiko-bot
@ 2026-07-08 14:17 ` SJ Park
  1 sibling, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-07-08 14:17 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: SJ Park, Andrew Morton, damon, linux-mm, linux-kernel

Hello Guangshuo,

On Wed,  8 Jul 2026 16:01:07 +0800 Guangshuo Li <lgs201920130244@gmail.com> wrote:

> damon_sysfs_scheme_add_dirs() sets up the tried_regions directory after
> the stats directory has already been created.
> 
> If damon_sysfs_scheme_set_tried_regions() fails, scheme->tried_regions
> has not been assigned. The error path nevertheless jumps to
> put_tried_regions_out and dereferences scheme->tried_regions while
> trying to put its kobject.
> 
> This can dereference uninitialized memory because struct
> damon_sysfs_scheme is allocated with kmalloc-style allocation and
> scheme->tried_regions is only assigned after tried_regions setup
> succeeds.
> 
> On tried_regions setup failure, start cleanup from the last successfully
> created directory, stats, instead of trying to put tried_regions.

Thank you for this patch!  However, it seems a same fix is already merged into
the mainline as commit 05ea83ee88ca ("mm/damon/sysfs-schemes: put stats for
scheme_add_dirs() internal error").

So, unfortunately we will not be able to merge this patch.


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup
  2026-07-08  8:20 ` sashiko-bot
@ 2026-07-08 14:22   ` SJ Park
  0 siblings, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-07-08 14:22 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, Guangshuo Li, damon

On Wed, 08 Jul 2026 08:20:09 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Uninitialized pointer dereference in error path of damon_sysfs_access_pattern_add_dirs()
> --
> 
> --- Patch [1]: [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup ---
> commit b97caf8d3b939c01dca3a53c5340d70ede6714d2
> Author: Guangshuo Li <lgs201920130244@gmail.com>
> 
> mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup
> 
> This commit fixes an uninitialized pointer dereference in the error cleanup
> path of damon_sysfs_scheme_add_dirs(). It changes the failure jump target 
> for damon_sysfs_scheme_set_tried_regions() to avoid calling kobject_put()
> on the uninitialized tried_regions pointer.
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index 329cfd0bbe9f3..0668a0e539713 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> > @@ -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;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but while reviewing this cleanup,
> I noticed the exact same uninitialized pointer dereference exists earlier in
> the same file.
> 
> If damon_sysfs_access_pattern_add_range_dir() fails during the setup of "sz",
> does the error path dereference an uninitialized pointer?
> 
> mm/damon/sysfs-schemes.c:damon_sysfs_access_pattern_add_dirs() {
>     ...
> 	err = damon_sysfs_access_pattern_add_range_dir(access_pattern,
> 			&access_pattern->sz, "sz");
> 	if (err)
> 		goto put_sz_out;
>     ...
> put_sz_out:
> 	kobject_put(&access_pattern->sz->kobj);
> 	access_pattern->sz = NULL;
> 	return err;
> }
> 
> Since damon_sysfs_access_pattern is allocated with kmalloc_obj(), its fields
> are uninitialized. If the directory creation fails, the function returns an
> error without writing to the pointer. The error path then jumps to put_sz_out
> and calls kobject_put() on the garbage pointer access_pattern->sz, identical
> to the bug fixed by this patch.
> 
> Could this be addressed with a similar fix?

This is already fixed in the mainline by commit d58fdbe37a82
("mm/damon/sysfs-schemes: fix dir put orders in access_pattern_add_dirs()") to
my understanding.   Let me know if I'm missing something.

> 
> >  put_filters_watermarks_quotas_access_pattern_out:
> >  	kobject_put(&scheme->ops_filters->kobj);
> >  	scheme->ops_filters = NULL;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708080107.727043-1-lgs201920130244@gmail.com?part=1


Thanks,
SJ

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-08 14:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08  8:01 [PATCH] mm/damon/sysfs-schemes: fix tried_regions setup failure cleanup Guangshuo Li
2026-07-08  8:20 ` sashiko-bot
2026-07-08 14:22   ` SJ Park
2026-07-08 14:17 ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox