mm/damon/sysfs: fix param_ctx leak on damon_sysfs_new_test_ctx() failure

When damon_sysfs_new_test_ctx() fails in damon_sysfs_commit_input(), param_ctx is leaked because the early return skips the cleanup at the out label. Destroy param_ctx before returning.

---
 mm/damon/sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index 576d1ddd736b..b573b9d60784 100644
@@ -1524,8 +1524,10 @@ static int damon_sysfs_commit_input(void *data)
 	if (IS_ERR(param_ctx))
 		return PTR_ERR(param_ctx);
 	test_ctx = damon_sysfs_new_test_ctx(kdamond->damon_ctx);
-	if (!test_ctx)
+	if (!test_ctx) {
+		damon_destroy_ctx(param_ctx);
 		return -ENOMEM;
+	}
 	err = damon_commit_ctx(test_ctx, param_ctx);
 	if (err)
 		goto out;

2.34.1

Multiple sysfs command paths dereference contexts_arr[0] without first verifying that nr_contexts >= 1. A user can set nr_contexts to 0 via sysfs while DAMON is running, causing NULL pointer dereferences.

Guard all commands (except OFF) at the entry point of damon_sysfs_handle_cmd().

---
 mm/damon/sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

index b573b9d60784..ddc30586c0e6 100644
@@ -1749,6 +1749,9 @@ static int damon_sysfs_update_schemes_tried_regions(
 static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
 		struct damon_sysfs_kdamond *kdamond)
 {
+	if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
+		return -EINVAL;
+
 	switch (cmd) {
 	case DAMON_SYSFS_CMD_ON:
 		return damon_sysfs_turn_damon_on(kdamond);

2.34.1

Nit. There is no 'nr_contexts' in the code. Let's use kdamond->contexts->nr or contexts->nr instead.

It would be nice to explain how users can reproducer the issue. Could you please add below to this part of the commit message?

'''
The issue can be triggered by privileged users like below.

First, start DAMON and make contexts directory empty (kdamond->contexts->nr == 0).

    # damo start
    # cd /sys/kernel/mm/damon/admin/kdamonds/0
    # echo 0 > contexts/nr_contexts

Then, any of below commands will cause the NULL pointer dereference.

    # echo update_schemes_stats > state
    # echo update_schemes_tried_regions > state
    # echo update_schemes_tried_bytes > state
    # echo update_schemes_effective_quotas > state
    # echo update_tuned_intervals > state
'''

I suggested [1] this patch. But I didn't give you 'Revied-by:' tag. So the above Reviewed-by: is not valid.

And this patch looks good to me, so please take my valid Reviewed-by: tag.

From the next time, please add patch version changelog [2] here.

I added this patch into my tree after resolving the trivial things I mentioned above. I will post it as v3 tomorrow, unless Andrew add this to mm.git after resolving the trivial things as I mentioned.

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

Thanks,
SJ

[...]

damon_sysfs_repeat_call_fn() calls damon_sysfs_upd_tuned_intervals(), damon_sysfs_upd_schemes_stats(), and damon_sysfs_upd_schemes_effective_quotas() without checking contexts->nr. If nr_contexts is set to 0 via sysfs while DAMON is running, these functions dereference contexts_arr[0] and cause a NULL pointer dereference. Add the missing check.

---
 mm/damon/sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

index ddc30586c0e6..6a44a2f3d8fc 100644
@@ -1620,9 +1620,12 @@ static int damon_sysfs_repeat_call_fn(void *data)
 
 	if (!mutex_trylock(&damon_sysfs_lock))
 		return 0;
+	if (sysfs_kdamond->contexts->nr != 1)
+		goto out;
 	damon_sysfs_upd_tuned_intervals(sysfs_kdamond);
 	damon_sysfs_upd_schemes_stats(sysfs_kdamond);
 	damon_sysfs_upd_schemes_effective_quotas(sysfs_kdamond);
+out:
 	mutex_unlock(&damon_sysfs_lock);
 	return 0;
 }

2.34.1

From the next time, please add patch changelog here.

[...]
Sashiko also added comments [1] that are very same as those for the previous version of this patch. I replied [2] to those on the thread. In short, nice findings but orthogonal to this patch, and I will work on those separately.

[1] https://sashiko.dev/#/patchset/[email protected]
[2] https://lore.kernel.org/[email protected]

Thanks,
SJ

[...]

From next time, please add patch changelog here.

[...]
Sashiko is adding comments [1] similar to those for the previous version of this patch. I replied [2] on the thread. In short, it is good finding but orthogonal to this patch, and I will work on it.

[1] https://sashiko.dev/#/patchset/[email protected]
[2] https://lore.kernel.org/[email protected]

Thanks,
SJ