linux-kernel - Re: [PATCH v2] 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: <1899865.1726491909@warthog.procyon.org.uk>
Date: Mon, 16 Sep 2024 14:05:09 +0100
From: David Howells <dhowells@...hat.com>
To: libaokun@...weicloud.com
Cc: dhowells@...hat.com, netfs@...ts.linux.dev, jlayton@...nel.org,
    brauner@...nel.org, linux-fsdevel@...r.kernel.org,
    hsiangkao@...ux.alibaba.com, jefflexu@...ux.alibaba.com,
    linux-erofs@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
    yangerkun@...wei.com, houtao1@...wei.com, yukuai3@...wei.com,
    wozizhi@...wei.com, Baokun Li <libaokun1@...wei.com>,
    stable@...nel.org
Subject: Re: [PATCH v2] cachefiles: fix dentry leak in cachefiles_open_file()

libaokun@...weicloud.com wrote:

> From: Baokun Li <libaokun1@...wei.com>
> 
> A dentry leak may be caused when a lookup cookie and a cull are concurrent:
> 
>             P1             |             P2
> -----------------------------------------------------------
> cachefiles_lookup_cookie
>   cachefiles_look_up_object
>     lookup_one_positive_unlocked
>      // get dentry
>                             cachefiles_cull
>                               inode->i_flags |= S_KERNEL_FILE;
>     cachefiles_open_file
>       cachefiles_mark_inode_in_use
>         __cachefiles_mark_inode_in_use
>           can_use = false
>           if (!(inode->i_flags & S_KERNEL_FILE))
>             can_use = true
> 	  return false
>         return false
>         // Returns an error but doesn't put dentry
> 
> After that the following WARNING will be triggered when the backend folder
> is umounted:
> 
> ==================================================================
> BUG: Dentry 000000008ad87947{i=7a,n=Dx_1_1.img}  still in use (1) [unmount of ext4 sda]
> WARNING: CPU: 4 PID: 359261 at fs/dcache.c:1767 umount_check+0x5d/0x70
> CPU: 4 PID: 359261 Comm: umount Not tainted 6.6.0-dirty #25
> RIP: 0010:umount_check+0x5d/0x70
> Call Trace:
>  <TASK>
>  d_walk+0xda/0x2b0
>  do_one_tree+0x20/0x40
>  shrink_dcache_for_umount+0x2c/0x90
>  generic_shutdown_super+0x20/0x160
>  kill_block_super+0x1a/0x40
>  ext4_kill_sb+0x22/0x40
>  deactivate_locked_super+0x35/0x80
>  cleanup_mnt+0x104/0x160
> ==================================================================
> 
> Whether cachefiles_open_file() returns true or false, the reference count
> obtained by lookup_positive_unlocked() in cachefiles_look_up_object()
> should be released.
> 
> Therefore release that reference count in cachefiles_look_up_object() to
> fix the above issue and simplify the code.
> 
> Fixes: 1f08c925e7a3 ("cachefiles: Implement backing file wrangling")
> Cc: stable@...nel.org
> Signed-off-by: Baokun Li <libaokun1@...wei.com>

Acked-by: David Howells <dhowells@...hat.com>