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: <CAKtyLkGyaoHr_xVGrWCTSFkqyf8b+hkOX1A0vyOpZUkTcTGtvQ@mail.gmail.com>
Date: Fri, 25 Apr 2025 22:57:08 -0700
From: Fan Wu <wufan@...nel.org>
To: Jinliang Zheng <alexjlzheng@...il.com>
Cc: paul@...l-moore.com, alexjlzheng@...cent.com, chrisw@...l.org, 
	greg@...ah.com, jmorris@...ei.org, linux-kernel@...r.kernel.org, 
	linux-security-module@...r.kernel.org, serge@...lyn.com, 
	stable@...r.kernel.org
Subject: Re: [PATCH] securityfs: fix missing of d_delete() in securityfs_remove()

On Fri, Apr 25, 2025 at 9:15 PM Jinliang Zheng <alexjlzheng@...il.com> wrote:
>
> On Fri, 25 Apr 2025 18:06:32 -0400, Paul Moore wrote:
> > 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!
>
> Thank you for your reply. :)
>
> >
> > 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
>
> Sorry for that, I will read it. And thank you for your pointing it out.
>
> >
> > 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.
>
> I don't think that not supporting dynamic loading/unloading of LSMs means
> that directories/files under securityfs cannot be dynamically added/deleted.
>
> The example code in the commit message is just to quickly show the problem,
> not the actual usage scenario.
>
> I'm not sure whether existing LSMs have dynamic addition/deletion of files,
> but I don't think we should prohibit these operations.
>
> Moreover, since securityfs provides the securityfs_remove() interface, it
> is necessary to handle the deletion of dentry whenever it is used. What's
> more, we have EXPORT_SYMBOL_GPL(securityfs_remove).
>
> (By the way, the reason why I noticed this problem is because I needed to
> dynamically create/delete configuration directories/files when implementing
> an LSM. Of course, I am not dynamically loading/unloading LSM, but
> dynamically adding/deleting directories/files under securityfs according to
> the status during LSM operation.)
>
> Therefore, I think we need this patch and strongly recommend it. At least,
> it has no harm. Hahahaha
>
> thanks,
> Jinliang Zheng :)
>

We have added securityfs_recursive_remove() for this purpose.

To the best of my knowledge, IPE is the only LSM that will delete
dentry during normal operation.

-Fan