Re: [PATCH] hfs: fix a memleak in hfs_find_init

[alexious]

> > On 22 Jan 2024, at 20:27, Zhipeng Lu <alexious@xxxxxxxxxx> wrote:
> > 
> > When the switch statment goes to default and return an error, ptr should
> > be freed since it is allocated in hfs_find_init.
> > 
> 
> Do you have any memory leaks report? Could you share it in the comments?
> Which use-case reproduces the issue? It will be easier to review the fix
> If you can share the path of reproduction.
> 
> Thanks,
> Slava.

Well, we found this potential memory leak by static analysis.

We found that all of hfs_find_init's callers won't release `ptr` when 
hfs_find_init fails, while they will do release `ptr` when functions 
that after hfs_find_init fails. This tactic observation suggests that
hfs_find_init proberly should release `ptr` when it fails, i.e. in the
default branch of switch in this patch.

Besides, we noticed another implementation of hfs_find_init in 
fs/hfsplus/bfind.c, which is essentially identical to the one in 
this patch (in fs/hfs/bfind.c) but calling `BUG();` in default branch
to trigger an error-handling.

Thanks,
Zhipeng.


> 
> > Fixes: b3b2177a2d79 ("hfs: add lock nesting notation to hfs_find_init")
> > Signed-off-by: Zhipeng Lu <alexious@xxxxxxxxxx>
> > ---
> > 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
> >