Re: [RFC PATCH v3 24/28] mm/damon/sysfs: add filters/<F>/path file

[SeongJae Park <sj@kernel.org>]

On Sat, 16 May 2026 19:29:04 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing string trimming in `path_store()` causes double newlines in `path_show()` output.
> --
> 
> commit 355d50ee619dc1c941ec5aef02eb71147957c977
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/sysfs: add filters/<F>/path file
> 
> Introduce a new DAMON sysfs file for letting users setup the target
> memory cgroup of the belonging memory cgroup attribute monitoring.
> 
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index f1b7d79386866..2df1d670ee77d 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> 
> [ ... ]
> 
> > @@ -862,11 +867,46 @@ static ssize_t allow_store(struct kobject *kobj,
> >  	return count;
> >  }
> >  
> > +static ssize_t path_show(struct kobject *kobj,
> > +		struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct damon_sysfs_filter *filter = container_of(kobj,
> > +			struct damon_sysfs_filter, kobj);
> > +	int len;
> > +
> > +	if (!mutex_trylock(&damon_sysfs_lock))
> > +		return -EBUSY;
> > +	len = sysfs_emit(buf, "%s\n", filter->path ? filter->path : "");
> > +	mutex_unlock(&damon_sysfs_lock);
> > +	return len;
> > +}
> > +
> > +static ssize_t path_store(struct kobject *kobj,
> > +		struct kobj_attribute *attr, const char *buf, size_t count)
> > +{
> > +	struct damon_sysfs_filter *filter = container_of(kobj,
> > +			struct damon_sysfs_filter, kobj);
> > +	char *path = kmalloc_objs(*path, size_add(count, 1));
> > +
> > +	if (!path)
> > +		return -ENOMEM;
> > +	strscpy(path, buf, size_add(count, 1));
> 
> Will this strscpy() result in a double newline in the output of path_show()?
> 
> When users write to the path sysfs file using standard tools like echo, the
> input typically includes a trailing newline character. This copy operation
> will store that newline directly into filter->path.
> 
> When read back, path_show() uses sysfs_emit() with "%s\n", which will append
> a second newline.
> 
> While a subsequent commit ("mm/damon/sysfs: setup damon_filter->memcg_id
> from path") uses sysfs_streq() which safely ignores trailing newlines for
> path matching, could we strip the trailing newline here to avoid the
> formatting anomaly in the sysfs output?

Good catch, but quite trivial.  And other similar arbitrary string parameters
work in the same way.  Let's keep this as is for now.

> 
> > +	if (!mutex_trylock(&damon_sysfs_lock)) {
> > +		kfree(path);
> > +		return -EBUSY;
> > +	}
> > +	kfree(filter->path);
> > +	filter->path = path;
> > +	mutex_unlock(&damon_sysfs_lock);
> > +	return count;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260516183712.81393-1-sj@kernel.org?part=24


Thanks,
SJ