lkml.org 
[lkml]   [2024]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] hfs: fix a memleak in hfs_find_init
From
Date


> On 29 Jan 2024, at 15:54, alexious@zju.edu.cn wrote:
>
>>> On 22 Jan 2024, at 20:27, Zhipeng Lu <alexious@zju.edu.cn> 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.
>

I see. I believe it makes sense to add all of this explanation
into comment section. Modification looks good. Mostly, hfs_find_exit()
does freeing resources and if hfs_find_init() fails, then hfs_find_exit()
is never called. Maybe, it makes sense to set fd->tree = NULL too but
it is not critical, as far as I can see.

Could you please rework the comment section of the patch?

Thanks,
Slava.


\
 
 \ /
  Last update: 2024-05-27 14:38    [W:0.043 / U:0.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site