linux-kernel - Re: [PATCH] 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: <CAHC9VhROjCvwaEJ1-Vc9SQU-x3wmZjeFknxkFGJcpPL28fGm1w@mail.gmail.com>
Date: Fri, 25 Apr 2025 18:06:32 -0400
From: Paul Moore <paul@...l-moore.com>
To: alexjlzheng@...il.com
Cc: jmorris@...ei.org, 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>, stable@...r.kernel.org
Subject: Re: [PATCH] securityfs: fix missing of d_delete() in securityfs_remove()

On Fri, Apr 25, 2025 at 5:25 AM <alexjlzheng@...il.com> wrote:
>
> From: Jinliang Zheng <alexjlzheng@...cent.com>
>
> Consider the following module code:
>
>   static struct dentry *dentry;
>
>   static int __init securityfs_test_init(void)
>   {
>           dentry = securityfs_create_dir("standon", NULL);
>           return PTR_ERR(dentry);
>   }
>
>   static void __exit securityfs_test_exit(void)
>   {
>           securityfs_remove(dentry);
>   }
>
>   module_init(securityfs_test_init);
>   module_exit(securityfs_test_exit);
>
> and then:
>
>   insmod /path/to/thismodule
>   cd /sys/kernel/security/standon     <- we hold 'standon'
>   rmmod thismodule                    <- 'standon' don't go away
>   insmod /path/to/thismodule          <- Failed: File exists!

A quick procedural note, and you may have gotten an email about this
from the stable kernel folks already, you generally shouldn't add the
stable alias to your emails directly.  You may want to look at the
kernel docs on the stable kernel if you haven't already:

* https://docs.kernel.org/process/stable-kernel-rules.html

Beyond that, we don't currently support dynamically loading or
unloading LSMs so the immediate response to the reproducer above is
"don't do that, we don't support it" :)  However, if you see a similar
problem with a LSM properly registered with the running kernel please
let us know.

> 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>
> Cc: <stable@...r.kernel.org>
> ---
>  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

-- 
paul-moore.com