mm/damon/sysfs: do not ignore callback's return value in damon_sysfs_damon_call()
Subsystems (2)
PATCH 1/1 mm/damon/sysfs: do not ignore callback's return value in damon_sysfs_damon_call()
The callback return value is ignored in damon_sysfs_damon_call(), which means that it is not possible to detect invalid user input when writing commands such as 'commit' to /sys/kernel/mm/damon/admin/kdamonds/<K>/state. Fix it.
--- mm/damon/sysfs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
mm/damon/sysfs.c +4 -1
@@ -1627,12 +1627,15 @@ static int damon_sysfs_damon_call(int (*fn)(void *data), struct damon_sysfs_kdamond *kdamond) {struct damon_call_control call_control = {}; +int err;if (!kdamond->damon_ctx)return -EINVAL ; call_control.fn = fn; call_control.data = kdamond; -return damon_call (kdamond->damon_ctx, &call_control); + err =damon_call (kdamond->damon_ctx, &call_control); + +return err ? err : call_control.return_code; }struct damon_sysfs_schemes_walk_data {
Signature · 1 line
2.43.0
Quoted 4 lines · On Sat, 20 Sep 2025 19:52:36 +0900 Akinobu Mita <[email protected]> wrote:
The callback return value is ignored in
damon_sysfs_damon_call(), which means that it is not possible to detect invalid user input when writing commands such as 'commit' to /sys/kernel/mm/damon/admin/kdamonds/<K>/state. Fix it.
Thank you for finding this issue, Akinobu! Before the introduction of damon_sysfs_damon_call() and its usages, DAMON sysfs interface was returning an error in case of such command failures. So this is fixing a user-visible unintended behavior change. Hence this may deserve Cc-ing stable@ in my opinion.
Quoted 1 line
So, below may better to be added?
Quoted 1 line
Signed-off-by Akinobu Mita <[email protected]>
Quoted 11 lines
--- mm/damon/sysfs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)mm/damon/sysfs.c +0
-0@@ -1627,12 +1627,15 @@ static int damon_sysfs_damon_call(int (*fn)(void *data), struct damon_sysfs_kdamond *kdamond) {
mm/damon/sysfs.c static int damon_sysfs_damon_call(int (*fn)(void *data), · line 1637
Quoted 10 lines
mm/damon/sysfs.c +3
-1struct damon_call_control call_control = {}; +int err;if (!kdamond->damon_ctx)return -EINVAL ; call_control.fn = fn; call_control.data = kdamond; -return damon_call (kdamond->damon_ctx, &call_control); + err =damon_call (kdamond->damon_ctx, &call_control); +
I think the above new line is not really needed, but it is definitely trivial.
Quoted 6 lines
mm/damon/sysfs.c +1
-0+return err ? err : call_control.return_code; }struct damon_sysfs_schemes_walk_data {Signature · 1 line
2.43.0
Thanks,
SJ
SeongJae Park
maintainer+ Andrew
Quoted 59 lines · On Sat, 20 Sep 2025 05:11:31 -0700 SeongJae Park <[email protected]> wrote:
Quoted 4 lines · On Sat, 20 Sep 2025 19:52:36 +0900 Akinobu Mita <[email protected]> wrote:
The callback return value is ignored in
damon_sysfs_damon_call(), which means that it is not possible to detect invalid user input when writing commands such as 'commit' to /sys/kernel/mm/damon/admin/kdamonds/<K>/state. Fix it.Thank you for finding this issue, Akinobu! Before the introduction of
damon_sysfs_damon_call()and its usages, DAMON sysfs interface was returning an error in case of such command failures. So this is fixing a user-visible unintended behavior change. Hence this may deserve Cc-ing stable@ in my opinion.Quoted 1 line
So, below may better to be added?
Fixes f64539dcdb87 ("mm/damon/sysfs: use damon_call() for update_schemes_stats")Cc <[email protected]> # v6.14.xQuoted 1 line
Signed-off-by Akinobu Mita <[email protected]>Reviewed-by SeongJae Park <[email protected]>Quoted 11 lines
--- mm/damon/sysfs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)mm/damon/sysfs.c +0
-0@@ -1627,12 +1627,15 @@ static int damon_sysfs_damon_call(int (*fn)(void *data), struct damon_sysfs_kdamond *kdamond) {mm/damon/sysfs.c static int damon_sysfs_damon_call(int (*fn)(void *data), · line 1637
Quoted 10 lines
mm/damon/sysfs.c +3
-1struct damon_call_control call_control = {}; +int err;if (!kdamond->damon_ctx)return -EINVAL ; call_control.fn = fn; call_control.data = kdamond; -return damon_call (kdamond->damon_ctx, &call_control); + err =damon_call (kdamond->damon_ctx, &call_control); +I think the above new line is not really needed, but it is definitely trivial.
Quoted 6 lines
mm/damon/sysfs.c +1
-0+return err ? err : call_control.return_code; }struct damon_sysfs_schemes_walk_data {Signature · 1 line
2.43.0Thanks,
SJ