linux-kernel - Re: [PATCH] dcache: unlock inode->i_lock before goto restart tag in, d_prune

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:   Wed, 15 Apr 2020 18:46:48 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Zhiqiang Liu <liuzhiqiang26@...wei.com>
Cc:     viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, npiggin@...nel.dk, zyan@...hat.com,
        hartleys@...ionengravers.com, Yanxiaodan <yanxiaodan@...wei.com>,
        Mingfangsen <mingfangsen@...wei.com>,
        "wubo (T)" <wubo40@...wei.com>
Subject: Re: [PATCH] dcache: unlock inode->i_lock before goto restart tag in,
 d_prune_aliases

On Wed, Apr 15, 2020 at 09:20:33PM +0800, Zhiqiang Liu wrote:
> From: Zhiqiang Liu <liuzhiqiang26@...wei.com>
> 
> coccicheck reports:
>   fs/dcache.c:1027:1-10: second lock on line 1027
> 
> In d_prune_aliases, before goto restart we should unlock
> inode->i_lock.
> 
> Fixes: 29355c3904e ("d_prune_alias(): just lock the parent and call __dentry_kill()")
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@...wei.com>
> Signed-off-by: Feilong Lin <linfeilong@...wei.com>
> ---
>  fs/dcache.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index b280e07e162b..1532ebe9d9ca 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1030,6 +1030,7 @@ void d_prune_aliases(struct inode *inode)
>  		if (!dentry->d_lockref.count) {
>  			struct dentry *parent = lock_parent(dentry);
>  			if (likely(!dentry->d_lockref.count)) {
> +				spin_unlock(&inode->i_lock);
>  				__dentry_kill(dentry);
>  				dput(parent);
>  				goto restart;
> -- 

Doesn't __dentry_kill() already do the unlock, via dentry_unlink_inode()?

- Eric