[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20250509024112.362527-1-alexjlzheng@tencent.com>
Date: Fri, 9 May 2025 10:41:12 +0800
From: Jinliang Zheng <alexjlzheng@...il.com>
To: paul@...l-moore.com
Cc: alexjlzheng@...il.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,
viro@...iv.linux.org.uk,
wufan@...ux.microsoft.com,
linux-efi@...r.kernel.org
Subject: Re: [PATCH v3] securityfs: fix missing of d_delete() in securityfs_remove()
On Thu, 8 May 2025 18:55:30 -0700, Fan Wu <wufan@...nel.org> wrote:
> 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.
Thank you for your reply. :)
Did you mean cc to linux-efi?
thanks,
Jinliang Zheng.
>
> -Fan
Powered by blists - more mailing lists