On Fri, 19 Aug 2022 14:08:09 -0700 Andrew Morton akpm@linux-foundation.org wrote:
On Fri, 19 Aug 2022 17:19:30 +0000 SeongJae Park sj@kernel.org wrote:
From: Badari Pulavarty badari.pulavarty@intel.com
When user tries to create a DAMON context via the DAMON debugfs interface with a name of an already existing context, the context directory creation silently fails but the context is added in the internal data structure. As a result, memory could leak and DAMON cannot be turned on. An example test case is as below:
# cd /sys/kernel/debug/damon/ # echo "off" > monitor_on # echo paddr > target_ids # echo "abc" > mk_context # echo "abc" > mk_context # echo $$ > abc/target_ids # echo "on" > monitor_on <<< failsThis commit fixes the issue by checking if the name already exist and immediately returning '-EEXIST' in the case.
...
--- a/mm/damon/dbgfs.c +++ b/mm/damon/dbgfs.c @@ -795,7 +795,7 @@ static void dbgfs_destroy_ctx(struct damon_ctx *ctx) */ static int dbgfs_mk_context(char *name) {
- struct dentry *root, **new_dirs, *new_dir;
- struct dentry *root, **new_dirs, *new_dir, *dir; struct damon_ctx **new_ctxs, *new_ctx;
if (damon_nr_running_ctxs()) @@ -817,6 +817,12 @@ static int dbgfs_mk_context(char *name) if (!root) return -ENOENT;
- dir = debugfs_lookup(name, root);
- if (dir) {
dput(dir);return -EEXIST;- }
- new_dir = debugfs_create_dir(name, root); dbgfs_dirs[dbgfs_nr_ctxs] = new_dir;
It would be simpler (and less racy) to check the debugfs_create_dir() return value for IS_ERR()?
I was merely following Greg's previous advice for ignoring the return value[1] of the function, but I might misunderstanding his intention, so CC-ing Greg. Greg, may I ask your opinion?
[1] https://lore.kernel.org/linux-mm/YB1kZaD%2F7omxXztF@kroah.com/
Thanks, SJ