linux-kernel - Re: [PATCH v3] securityfs: fix missing of d_delete() in securityfs

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: <CAKtyLkGK3JH7amgskrjMyUz1KZeVtAO_4bA_8iuBYvykgHRsRQ@mail.gmail.com>
Date: Thu, 8 May 2025 18:55:30 -0700
From: Fan Wu <wufan@...nel.org>
To: alexjlzheng@...il.com
Cc: paul@...l-moore.com, jmorris@...ei.org, viro@...iv.linux.org.uk, 
	serge@...lyn.com, greg@...ah.com, chrisw@...l.org, 
	linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Jinliang Zheng <alexjlzheng@...cent.com>
Subject: Re: [PATCH v3] securityfs: fix missing of d_delete() in securityfs_remove()

On Thu, May 8, 2025 at 7:11 AM <alexjlzheng@...il.com> wrote:
>
> From: Jinliang Zheng <alexjlzheng@...cent.com>
>
> Consider the following execution flow:
>
>   Thread 0: securityfs_create_dir("A")
>   Thread 1: cd /sys/kernel/security/A           <- we hold 'A'
>   Thread 0: securityfs_remove(dentry)           <- 'A' don't go away
>   Thread 0: securityfs_create_dir("A")          <- Failed: File exists!
>
> Although the LSM module will not be dynamically added or deleted after
> the kernel is started, it may dynamically add or delete pseudo files
> for status export or function configuration in userspace according to
> different status, which we are not prohibited from doing so.
>
> In addition, securityfs_recursive_remove() avoids this problem by calling
> __d_drop() directly. As a non-recursive version, it is somewhat strange
> that securityfs_remove() does not clean up the deleted dentry.
>
> Fix this by adding d_delete() in securityfs_remove().
>
> Fixes: b67dbf9d4c198 ("[PATCH] add securityfs for all LSMs to use")
> Signed-off-by: Jinliang Zheng <alexjlzheng@...cent.com>
> ---
> changelog:
> v3: Modify the commit message to avoid readers mistakenly thinking that the LSM is being dynamically loaded
> v2: https://lore.kernel.org/all/20250507111204.2585739-1-alexjlzheng@tencent.com/
> v1: https://lore.kernel.org/all/20250425092548.6828-1-alexjlzheng@tencent.com/
> ---
>  security/inode.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/security/inode.c b/security/inode.c
> index da3ab44c8e57..d99baf26350a 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -306,6 +306,7 @@ void securityfs_remove(struct dentry *dentry)
>                         simple_rmdir(dir, dentry);
>                 else
>                         simple_unlink(dir, dentry);
> +               d_delete(dentry);
>                 dput(dentry);
>         }
>         inode_unlock(dir);
> --
> 2.49.0
>
>

Since this could impact efi_secret_unlink(), I would suggest adding linux-efi.

-Fan