| Message ID | 20240201130027.3058006-1-alexious@zju.edu.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] hfs: fix a memleak in hfs_find_init | expand |
> On 1 Feb 2024, at 16:00, Zhipeng Lu <alexious@zju.edu.cn> wrote: > > In every caller of hfs_find_init, `ptr` won't be freed when hfs_find_init > fails, but will be freed when somewhere after hfs_find_init fails. Current statement sounds confusing for my taste. I don’t follow it. What do you mean here? > This suggests that hfs_find_init should proberly free `ptr` in its own > error-handling to prevent `ptr` from leaking. I assume you meant properly here? > > In particular, When the switch statment goes to default and return an error, > `ptr` should be freed. I assume you meant statement here? Also, you mentioning `ptr` but we freed fd->search_key here. It sounds confusing too. > > Fixes: b3b2177a2d79 ("hfs: add lock nesting notation to hfs_find_init") > Signed-off-by: Zhipeng Lu <alexious@zju.edu.cn> > --- > Changelog: > > v2: Improve commit message to be more clear. Currently, it sounds slightly more confusing. :) Thanks, Slava. > --- > fs/hfs/bfind.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/hfs/bfind.c b/fs/hfs/bfind.c > index ef9498a6e88a..7aa3b9aba4d1 100644 > --- a/fs/hfs/bfind.c > +++ b/fs/hfs/bfind.c > @@ -36,6 +36,7 @@ int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd) > mutex_lock_nested(&tree->tree_lock, ATTR_BTREE_MUTEX); > break; > default: > + kfree(fd->search_key); > return -EINVAL; > } > return 0; > -- > 2.34.1 >
diff --git a/fs/hfs/bfind.c b/fs/hfs/bfind.c index ef9498a6e88a..7aa3b9aba4d1 100644 --- a/fs/hfs/bfind.c +++ b/fs/hfs/bfind.c @@ -36,6 +36,7 @@ int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd) mutex_lock_nested(&tree->tree_lock, ATTR_BTREE_MUTEX); break; default: + kfree(fd->search_key); return -EINVAL; } return 0;
In every caller of hfs_find_init, `ptr` won't be freed when hfs_find_init fails, but will be freed when somewhere after hfs_find_init fails. This suggests that hfs_find_init should proberly free `ptr` in its own error-handling to prevent `ptr` from leaking. In particular, When the switch statment goes to default and return an error, `ptr` should be freed. Fixes: b3b2177a2d79 ("hfs: add lock nesting notation to hfs_find_init") Signed-off-by: Zhipeng Lu <alexious@zju.edu.cn> --- Changelog: v2: Improve commit message to be more clear. --- fs/hfs/bfind.c | 1 + 1 file changed, 1 insertion(+)