linux-kernel - Re: [PATCH] cachefiles: fix dentry leak in cachefiles_open

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]

Message-ID: <5b7455f8-4637-4ec0-a016-233827131fb2@huaweicloud.com>
Date: Tue, 27 Aug 2024 11:47:43 +0800
From: Baokun Li <libaokun@...weicloud.com>
To: Markus Elfring <Markus.Elfring@....de>
Cc: netfs@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
 linux-erofs@...ts.ozlabs.org, David Howells <dhowells@...hat.com>,
 Jeff Layton <jlayton@...nel.org>, stable@...nel.org,
 LKML <linux-kernel@...r.kernel.org>, Christian Brauner <brauner@...nel.org>,
 Gao Xiang <hsiangkao@...ux.alibaba.com>, Hou Tao <houtao1@...wei.com>,
 Jingbo Xu <jefflexu@...ux.alibaba.com>, Yang Erkun <yangerkun@...wei.com>,
 Yu Kuai <yukuai3@...wei.com>, Zizhi Wo <wozizhi@...wei.com>,
 Baokun Li <libaokun1@...wei.com>
Subject: Re: [PATCH] cachefiles: fix dentry leak in cachefiles_open_file()

On 2024/8/26 21:55, Markus Elfring wrote:
> …
>> Add the missing dput() to cachefiles_open_file() for a quick fix.
> I suggest to use a goto chain accordingly.
>
>
> …

Hi Markus,


Thanks for the suggestion, but I think the current solution is simple
enough that we don't need to add a label to it.

Actually, at first I was going to release the reference count of the
dentry uniformly in cachefiles_look_up_object() and delete all dput()
in cachefiles_open_file(), but this may conflict when backporting
the code to stable. So just keep it simple to facilitate backporting
to stable.

Thanks,
Baokun
>> +++ b/fs/cachefiles/namei.c
>> @@ -554,6 +554,7 @@ static bool cachefiles_open_file(struct cachefiles_object *object,
>>   	if (!cachefiles_mark_inode_in_use(object, d_inode(dentry))) {
>>   		pr_notice("cachefiles: Inode already in use: %pd (B=%lx)\n",
>>   			  dentry, d_inode(dentry)->i_ino);
>> +		dput(dentry);
>>   		return false;
> Please replace two statements by the statement “goto put_dentry;”.
>
>
> …
>> error:
>> 	cachefiles_do_unmark_inode_in_use(object, d_inode(dentry));
> +put_dentry:
>> 	dput(dentry);
>> 	return false;
>> }
> Regards,
> Markus

-- 
With Best Regards,
Baokun Li