linux-kernel - Re: [PATCH] hfs: fix a memleak in hfs_find

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 29 Jan 2024 20:54:02 +0800 (GMT+08:00)
From: alexious@....edu.cn
To: "Viacheslav Dubeyko" <slava@...eyko.com>
Cc: "Andrew Morton" <akpm@...ux-foundation.org>,
	"Desmond Cheong Zhi Xi" <desmondcheongzx@...il.com>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hfs: fix a memleak in hfs_find_init

> > On 22 Jan 2024, at 20:27, Zhipeng Lu <alexious@....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.

Thanks,
Zhipeng.

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