[PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Shivank Garg]

Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
anonymous inodes with proper security context. This replaces the current
pattern of calling alloc_anon_inode() followed by
inode_init_security_anon() for creating security context manually.

This change also fixes a security regression in secretmem where the
S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
LSM/SELinux checks to be bypassed for secretmem file descriptors.

As guest_memfd currently resides in the KVM module, we need to export this
symbol for use outside the core kernel. In the future, guest_memfd might be
moved to core-mm, at which point the symbols no longer would have to be
exported. When/if that happens is still unclear.

Fixes: 2bfe15c52612 ("mm: create security context for memfd_secret inodes")
Suggested-by: David Hildenbrand <david@redhat.com>
Suggested-by: Mike Rapoport <rppt@kernel.org>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
The handling of the S_PRIVATE flag for these inodes was discussed
extensively ([1], [2], [3]).

As per discussion [3] with Mike and Paul, KVM guest_memfd and secretmem
results in user-visible file descriptors, so they should be subject to
LSM/SELinux security policies rather than bypassing them with S_PRIVATE.

[1] https://lore.kernel.org/all/b9e5fa41-62fd-4b3d-bb2d-24ae9d3c33da@redhat.com
[2] https://lore.kernel.org/all/cover.1748890962.git.ackerleytng@google.com
[3] https://lore.kernel.org/all/aFOh8N_rRdSi_Fbc@kernel.org

 fs/anon_inodes.c   | 23 ++++++++++++++++++-----
 include/linux/fs.h |  2 ++
 mm/secretmem.c     |  9 +--------
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index e51e7d88980a..ca6cfb1cd496 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -98,14 +98,25 @@ static struct file_system_type anon_inode_fs_type = {
 	.kill_sb	= kill_anon_super,
 };
 
-static struct inode *anon_inode_make_secure_inode(
-	const char *name,
-	const struct inode *context_inode)
+/**
+ * anon_inode_make_secure_inode - allocate an anonymous inode with security context
+ * @sb:		[in]	Superblock to allocate from
+ * @name:	[in]	Name of the class of the newfile (e.g., "secretmem")
+ * @context_inode:
+ *		[in]	Optional parent inode for security inheritance
+ *
+ * The function ensures proper security initialization through the LSM hook
+ * security_inode_init_security_anon().
+ *
+ * Return:	Pointer to new inode on success, ERR_PTR on failure.
+ */
+struct inode *anon_inode_make_secure_inode(struct super_block *sb, const char *name,
+					   const struct inode *context_inode)
 {
 	struct inode *inode;
 	int error;
 
-	inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+	inode = alloc_anon_inode(sb);
 	if (IS_ERR(inode))
 		return inode;
 	inode->i_flags &= ~S_PRIVATE;
@@ -118,6 +129,7 @@ static struct inode *anon_inode_make_secure_inode(
 	}
 	return inode;
 }
+EXPORT_SYMBOL_GPL(anon_inode_make_secure_inode);
 
 static struct file *__anon_inode_getfile(const char *name,
 					 const struct file_operations *fops,
@@ -132,7 +144,8 @@ static struct file *__anon_inode_getfile(const char *name,
 		return ERR_PTR(-ENOENT);
 
 	if (make_inode) {
-		inode =	anon_inode_make_secure_inode(name, context_inode);
+		inode =	anon_inode_make_secure_inode(anon_inode_mnt->mnt_sb,
+						     name, context_inode);
 		if (IS_ERR(inode)) {
 			file = ERR_CAST(inode);
 			goto err;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b085f161ed22..040c0036320f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3608,6 +3608,8 @@ extern int simple_write_begin(struct file *file, struct address_space *mapping,
 extern const struct address_space_operations ram_aops;
 extern int always_delete_dentry(const struct dentry *);
 extern struct inode *alloc_anon_inode(struct super_block *);
+struct inode *anon_inode_make_secure_inode(struct super_block *sb, const char *name,
+					   const struct inode *context_inode);
 extern int simple_nosetlease(struct file *, int, struct file_lease **, void **);
 extern const struct dentry_operations simple_dentry_operations;
 
diff --git a/mm/secretmem.c b/mm/secretmem.c
index 589b26c2d553..9a11a38a6770 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -195,18 +195,11 @@ static struct file *secretmem_file_create(unsigned long flags)
 	struct file *file;
 	struct inode *inode;
 	const char *anon_name = "[secretmem]";
-	int err;
 
-	inode = alloc_anon_inode(secretmem_mnt->mnt_sb);
+	inode = anon_inode_make_secure_inode(secretmem_mnt->mnt_sb, anon_name, NULL);
 	if (IS_ERR(inode))
 		return ERR_CAST(inode);
 
-	err = security_inode_init_security_anon(inode, &QSTR(anon_name), NULL);
-	if (err) {
-		file = ERR_PTR(err);
-		goto err_free_inode;
-	}
-
 	file = alloc_file_pseudo(inode, secretmem_mnt, "secretmem",
 				 O_RDWR, &secretmem_fops);
 	if (IS_ERR(file))
-- 
2.43.0

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christian Brauner]

On Thu, 19 Jun 2025 07:31:37 +0000, Shivank Garg wrote:
> Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> anonymous inodes with proper security context. This replaces the current
> pattern of calling alloc_anon_inode() followed by
> inode_init_security_anon() for creating security context manually.
> 
> This change also fixes a security regression in secretmem where the
> S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> LSM/SELinux checks to be bypassed for secretmem file descriptors.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass
      https://git.kernel.org/vfs/vfs/c/c696307648ea

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Vlastimil Babka]

On 6/19/25 09:31, Shivank Garg wrote:
> Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> anonymous inodes with proper security context. This replaces the current
> pattern of calling alloc_anon_inode() followed by
> inode_init_security_anon() for creating security context manually.
> 
> This change also fixes a security regression in secretmem where the
> S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> LSM/SELinux checks to be bypassed for secretmem file descriptors.
> 
> As guest_memfd currently resides in the KVM module, we need to export this

Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
explicit for KVM?

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Shivank Garg]

On 6/19/2025 2:43 PM, Vlastimil Babka wrote:
> On 6/19/25 09:31, Shivank Garg wrote:
>> Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
>> anonymous inodes with proper security context. This replaces the current
>> pattern of calling alloc_anon_inode() followed by
>> inode_init_security_anon() for creating security context manually.
>>
>> This change also fixes a security regression in secretmem where the
>> S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
>> LSM/SELinux checks to be bypassed for secretmem file descriptors.
>>
>> As guest_memfd currently resides in the KVM module, we need to export this
> 
> Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
> explicit for KVM?
> 

Thanks for the suggestion.
I wasn't aware of this earlier. I think it makes sense to use it now.

So, the code would look like this:

+EXPORT_SYMBOL_GPL_FOR_MODULES(anon_inode_make_secure_inode, "kvm");

which builds fine for me. I hope this is the correct usage.

Thanks,
Shivank

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christian Brauner]

On Thu, Jun 19, 2025 at 11:13:49AM +0200, Vlastimil Babka wrote:
> On 6/19/25 09:31, Shivank Garg wrote:
> > Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> > anonymous inodes with proper security context. This replaces the current
> > pattern of calling alloc_anon_inode() followed by
> > inode_init_security_anon() for creating security context manually.
> > 
> > This change also fixes a security regression in secretmem where the
> > S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> > LSM/SELinux checks to be bypassed for secretmem file descriptors.
> > 
> > As guest_memfd currently resides in the KVM module, we need to export this
> 
> Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
> explicit for KVM?

Oh? Enlighten me about that, if you have a second, please. 

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Mike Rapoport]

On Thu, Jun 19, 2025 at 12:38:25PM +0200, Christian Brauner wrote:
> On Thu, Jun 19, 2025 at 11:13:49AM +0200, Vlastimil Babka wrote:
> > On 6/19/25 09:31, Shivank Garg wrote:
> > > Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> > > anonymous inodes with proper security context. This replaces the current
> > > pattern of calling alloc_anon_inode() followed by
> > > inode_init_security_anon() for creating security context manually.
> > > 
> > > This change also fixes a security regression in secretmem where the
> > > S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> > > LSM/SELinux checks to be bypassed for secretmem file descriptors.
> > > 
> > > As guest_memfd currently resides in the KVM module, we need to export this
> > 
> > Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
> > explicit for KVM?
> 
> Oh? Enlighten me about that, if you have a second, please. 

From Documentation/core-api/symbol-namespaces.rst:

The macro takes a comma separated list of module names, allowing only those
modules to access this symbol. Simple tail-globs are supported.

For example::

  EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")

will limit usage of this symbol to modules whoes name matches the given
patterns.


-- 
Sincerely yours,
Mike.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christian Brauner]

On Thu, Jun 19, 2025 at 02:01:22PM +0300, Mike Rapoport wrote:
> On Thu, Jun 19, 2025 at 12:38:25PM +0200, Christian Brauner wrote:
> > On Thu, Jun 19, 2025 at 11:13:49AM +0200, Vlastimil Babka wrote:
> > > On 6/19/25 09:31, Shivank Garg wrote:
> > > > Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> > > > anonymous inodes with proper security context. This replaces the current
> > > > pattern of calling alloc_anon_inode() followed by
> > > > inode_init_security_anon() for creating security context manually.
> > > > 
> > > > This change also fixes a security regression in secretmem where the
> > > > S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> > > > LSM/SELinux checks to be bypassed for secretmem file descriptors.
> > > > 
> > > > As guest_memfd currently resides in the KVM module, we need to export this
> > > 
> > > Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
> > > explicit for KVM?
> > 
> > Oh? Enlighten me about that, if you have a second, please. 
> 
> From Documentation/core-api/symbol-namespaces.rst:
> 
> The macro takes a comma separated list of module names, allowing only those
> modules to access this symbol. Simple tail-globs are supported.
> 
> For example::
> 
>   EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
> 
> will limit usage of this symbol to modules whoes name matches the given
> patterns.

Is that still mostly advisory and can still be easily circumenvented?

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Mike Rapoport]

On Thu, Jun 19, 2025 at 02:06:17PM +0200, Christian Brauner wrote:
> On Thu, Jun 19, 2025 at 02:01:22PM +0300, Mike Rapoport wrote:
> > On Thu, Jun 19, 2025 at 12:38:25PM +0200, Christian Brauner wrote:
> > > On Thu, Jun 19, 2025 at 11:13:49AM +0200, Vlastimil Babka wrote:
> > > > On 6/19/25 09:31, Shivank Garg wrote:
> > > > > Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> > > > > anonymous inodes with proper security context. This replaces the current
> > > > > pattern of calling alloc_anon_inode() followed by
> > > > > inode_init_security_anon() for creating security context manually.
> > > > > 
> > > > > This change also fixes a security regression in secretmem where the
> > > > > S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> > > > > LSM/SELinux checks to be bypassed for secretmem file descriptors.
> > > > > 
> > > > > As guest_memfd currently resides in the KVM module, we need to export this
> > > > 
> > > > Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
> > > > explicit for KVM?
> > > 
> > > Oh? Enlighten me about that, if you have a second, please. 
> > 
> > From Documentation/core-api/symbol-namespaces.rst:
> > 
> > The macro takes a comma separated list of module names, allowing only those
> > modules to access this symbol. Simple tail-globs are supported.
> > 
> > For example::
> > 
> >   EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
> > 
> > will limit usage of this symbol to modules whoes name matches the given
> > patterns.
> 
> Is that still mostly advisory and can still be easily circumenvented?

The commit message says

   will limit the use of said function to kvm.ko, any other module trying
   to use this symbol will refure to load (and get modpost build
   failures).
 
-- 
Sincerely yours,
Mike.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Sean Christopherson]

On Thu, Jun 19, 2025, Mike Rapoport wrote:
> On Thu, Jun 19, 2025 at 02:06:17PM +0200, Christian Brauner wrote:
> > On Thu, Jun 19, 2025 at 02:01:22PM +0300, Mike Rapoport wrote:
> > > On Thu, Jun 19, 2025 at 12:38:25PM +0200, Christian Brauner wrote:
> > > > On Thu, Jun 19, 2025 at 11:13:49AM +0200, Vlastimil Babka wrote:
> > > > > On 6/19/25 09:31, Shivank Garg wrote:
> > > > > > Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> > > > > > anonymous inodes with proper security context. This replaces the current
> > > > > > pattern of calling alloc_anon_inode() followed by
> > > > > > inode_init_security_anon() for creating security context manually.
> > > > > > 
> > > > > > This change also fixes a security regression in secretmem where the
> > > > > > S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> > > > > > LSM/SELinux checks to be bypassed for secretmem file descriptors.
> > > > > > 
> > > > > > As guest_memfd currently resides in the KVM module, we need to export this
> > > > > 
> > > > > Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
> > > > > explicit for KVM?
> > > > 
> > > > Oh? Enlighten me about that, if you have a second, please. 
> > > 
> > > From Documentation/core-api/symbol-namespaces.rst:
> > > 
> > > The macro takes a comma separated list of module names, allowing only those
> > > modules to access this symbol. Simple tail-globs are supported.
> > > 
> > > For example::
> > > 
> > >   EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
> > > 
> > > will limit usage of this symbol to modules whoes name matches the given
> > > patterns.
> > 
> > Is that still mostly advisory and can still be easily circumenvented?

Yes and no.  For out-of-tree modules, it's mostly advisory.  Though I can imagine
if someone tries to report a bug because their module is masquerading as e.g. kvm,
then they will be told to go away (in far less polite words :-D).

For in-tree modules, the restriction is much more enforceable.  Renaming a module
to circumvent a restricted export will raise major red flags, and getting "proper"
access to a symbol would require an ack from the relevant maintainers.  E.g. for
many KVM-induced exports, it's not that other module writers are trying to misbehave,
there simply aren't any guardrails to deter them from using a "dangerous" export.
 
The other big benefit I see is documentation, e.g. both for readers/developers to
understand the intent, and for auditing purposes (I would be shocked if there
aren't exports that were KVM-induced, but that are no longer necessary).

And we can utilize the framework to do additional hardening.  E.g. for exports
that exist solely for KVM, I plan on adding wrappers so that the symbols are
exproted if and only if KVM is enabled in the kernel .config[*].  Again, that's
far from perfect, e.g. AFAIK every distro enables KVM, but it should help keep
everyone honest.

[*] https://lore.kernel.org/all/ZzJOoFFPjrzYzKir@google.com 

> The commit message says
> 
>    will limit the use of said function to kvm.ko, any other module trying
>    to use this symbol will refure to load (and get modpost build
>    failures).

To Christian's point, the restrictions are trivial to circumvent by out-of-tree
modules.  E.g. to get access to the above, simply name your module kvm-lol.ko or
whatever.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Shivank Garg]

On 6/20/2025 8:32 PM, Sean Christopherson wrote:
> On Thu, Jun 19, 2025, Mike Rapoport wrote:
>> On Thu, Jun 19, 2025 at 02:06:17PM +0200, Christian Brauner wrote:
>>> On Thu, Jun 19, 2025 at 02:01:22PM +0300, Mike Rapoport wrote:
>>>> On Thu, Jun 19, 2025 at 12:38:25PM +0200, Christian Brauner wrote:
>>>>> On Thu, Jun 19, 2025 at 11:13:49AM +0200, Vlastimil Babka wrote:
>>>>>> On 6/19/25 09:31, Shivank Garg wrote:
>>>>>>> Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
>>>>>>> anonymous inodes with proper security context. This replaces the current
>>>>>>> pattern of calling alloc_anon_inode() followed by
>>>>>>> inode_init_security_anon() for creating security context manually.
>>>>>>>
>>>>>>> This change also fixes a security regression in secretmem where the
>>>>>>> S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
>>>>>>> LSM/SELinux checks to be bypassed for secretmem file descriptors.
>>>>>>>
>>>>>>> As guest_memfd currently resides in the KVM module, we need to export this
>>>>>>
>>>>>> Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
>>>>>> explicit for KVM?
>>>>>
>>>>> Oh? Enlighten me about that, if you have a second, please. 
>>>>
>>>> From Documentation/core-api/symbol-namespaces.rst:
>>>>
>>>> The macro takes a comma separated list of module names, allowing only those
>>>> modules to access this symbol. Simple tail-globs are supported.
>>>>
>>>> For example::
>>>>
>>>>   EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
>>>>
>>>> will limit usage of this symbol to modules whoes name matches the given
>>>> patterns.
>>>
>>> Is that still mostly advisory and can still be easily circumenvented?
> 
> Yes and no.  For out-of-tree modules, it's mostly advisory.  Though I can imagine
> if someone tries to report a bug because their module is masquerading as e.g. kvm,
> then they will be told to go away (in far less polite words :-D).
> 
> For in-tree modules, the restriction is much more enforceable.  Renaming a module
> to circumvent a restricted export will raise major red flags, and getting "proper"
> access to a symbol would require an ack from the relevant maintainers.  E.g. for
> many KVM-induced exports, it's not that other module writers are trying to misbehave,
> there simply aren't any guardrails to deter them from using a "dangerous" export.
>  
> The other big benefit I see is documentation, e.g. both for readers/developers to
> understand the intent, and for auditing purposes (I would be shocked if there
> aren't exports that were KVM-induced, but that are no longer necessary).
> 
> And we can utilize the framework to do additional hardening.  E.g. for exports
> that exist solely for KVM, I plan on adding wrappers so that the symbols are
> exproted if and only if KVM is enabled in the kernel .config[*].  Again, that's
> far from perfect, e.g. AFAIK every distro enables KVM, but it should help keep
> everyone honest.
> 
> [*] https://lore.kernel.org/all/ZzJOoFFPjrzYzKir@google.com 
> 
>> The commit message says
>>
>>    will limit the use of said function to kvm.ko, any other module trying
>>    to use this symbol will refure to load (and get modpost build
>>    failures).
> 
> To Christian's point, the restrictions are trivial to circumvent by out-of-tree
> modules.  E.g. to get access to the above, simply name your module kvm-lol.ko or
> whatever.

Thanks for the info.

I have posted the revised patch with EXPORT_SYMBOL_GPL_FOR_MODULES:
https://lore.kernel.org/linux-mm/20250620070328.803704-3-shivankg@amd.com

Please review when you have a chance.

Thanks,
Shivank

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christian Brauner]

On Fri, Jun 20, 2025 at 08:02:18AM -0700, Sean Christopherson wrote:
> On Thu, Jun 19, 2025, Mike Rapoport wrote:
> > On Thu, Jun 19, 2025 at 02:06:17PM +0200, Christian Brauner wrote:
> > > On Thu, Jun 19, 2025 at 02:01:22PM +0300, Mike Rapoport wrote:
> > > > On Thu, Jun 19, 2025 at 12:38:25PM +0200, Christian Brauner wrote:
> > > > > On Thu, Jun 19, 2025 at 11:13:49AM +0200, Vlastimil Babka wrote:
> > > > > > On 6/19/25 09:31, Shivank Garg wrote:
> > > > > > > Export anon_inode_make_secure_inode() to allow KVM guest_memfd to create
> > > > > > > anonymous inodes with proper security context. This replaces the current
> > > > > > > pattern of calling alloc_anon_inode() followed by
> > > > > > > inode_init_security_anon() for creating security context manually.
> > > > > > > 
> > > > > > > This change also fixes a security regression in secretmem where the
> > > > > > > S_PRIVATE flag was not cleared after alloc_anon_inode(), causing
> > > > > > > LSM/SELinux checks to be bypassed for secretmem file descriptors.
> > > > > > > 
> > > > > > > As guest_memfd currently resides in the KVM module, we need to export this
> > > > > > 
> > > > > > Could we use the new EXPORT_SYMBOL_GPL_FOR_MODULES() thingy to make this
> > > > > > explicit for KVM?
> > > > > 
> > > > > Oh? Enlighten me about that, if you have a second, please. 
> > > > 
> > > > From Documentation/core-api/symbol-namespaces.rst:
> > > > 
> > > > The macro takes a comma separated list of module names, allowing only those
> > > > modules to access this symbol. Simple tail-globs are supported.
> > > > 
> > > > For example::
> > > > 
> > > >   EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
> > > > 
> > > > will limit usage of this symbol to modules whoes name matches the given
> > > > patterns.
> > > 
> > > Is that still mostly advisory and can still be easily circumenvented?
> 
> Yes and no.  For out-of-tree modules, it's mostly advisory.  Though I can imagine
> if someone tries to report a bug because their module is masquerading as e.g. kvm,
> then they will be told to go away (in far less polite words :-D).
> 
> For in-tree modules, the restriction is much more enforceable.  Renaming a module
> to circumvent a restricted export will raise major red flags, and getting "proper"
> access to a symbol would require an ack from the relevant maintainers.  E.g. for
> many KVM-induced exports, it's not that other module writers are trying to misbehave,
> there simply aren't any guardrails to deter them from using a "dangerous" export.
>  
> The other big benefit I see is documentation, e.g. both for readers/developers to
> understand the intent, and for auditing purposes (I would be shocked if there
> aren't exports that were KVM-induced, but that are no longer necessary).
> 
> And we can utilize the framework to do additional hardening.  E.g. for exports
> that exist solely for KVM, I plan on adding wrappers so that the symbols are
> exproted if and only if KVM is enabled in the kernel .config[*].  Again, that's
> far from perfect, e.g. AFAIK every distro enables KVM, but it should help keep
> everyone honest.
> 
> [*] https://lore.kernel.org/all/ZzJOoFFPjrzYzKir@google.com 
> 
> > The commit message says
> > 
> >    will limit the use of said function to kvm.ko, any other module trying
> >    to use this symbol will refure to load (and get modpost build
> >    failures).
> 
> To Christian's point, the restrictions are trivial to circumvent by out-of-tree
> modules.  E.g. to get access to the above, simply name your module kvm-lol.ko or
> whatever.

Thanks for all the details!
I'm more than happy to switch a bunch of our exports so that we only
allow them for specific modules. But for that we also need
EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christoph Hellwig]

On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
> I'm more than happy to switch a bunch of our exports so that we only
> allow them for specific modules. But for that we also need
> EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.

Huh?  Any export for a specific in-tree module (or set thereof) is
by definition internals and an _GPL export if perfectly fine and
expected.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christoph Hellwig]

On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
> > I'm more than happy to switch a bunch of our exports so that we only
> > allow them for specific modules. But for that we also need
> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
> 
> Huh?  Any export for a specific in-tree module (or set thereof) is
> by definition internals and an _GPL export if perfectly fine and
> expected.

.. the only thing we should do is to drop the pointless _GPL in the
name entirely.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Vlastimil Babka]

On 6/23/25 16:01, Christoph Hellwig wrote:
> On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
>> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
>> > I'm more than happy to switch a bunch of our exports so that we only
>> > allow them for specific modules. But for that we also need
>> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
>> 
>> Huh?  Any export for a specific in-tree module (or set thereof) is
>> by definition internals and an _GPL export if perfectly fine and
>> expected.

Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
modules, so external module with GPL and matching name can import.

But if we're targetting in-tree stuff like kvm, we don't need to provide a
non-GPL variant I think?

> .. the only thing we should do is to drop the pointless _GPL in the
> name entirely.

I'd agree if it was indeed limited to in-tree modules.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christoph Hellwig]

On Mon, Jun 23, 2025 at 04:21:15PM +0200, Vlastimil Babka wrote:
> On 6/23/25 16:01, Christoph Hellwig wrote:
> > On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
> >> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
> >> > I'm more than happy to switch a bunch of our exports so that we only
> >> > allow them for specific modules. But for that we also need
> >> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
> >> 
> >> Huh?  Any export for a specific in-tree module (or set thereof) is
> >> by definition internals and an _GPL export if perfectly fine and
> >> expected.
> 
> Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
> modules, so external module with GPL and matching name can import.

Sure, technically they can.  But that's not the intent of the export,
but rather abusing it.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Peter Zijlstra]

On Mon, Jun 23, 2025 at 04:21:15PM +0200, Vlastimil Babka wrote:
> On 6/23/25 16:01, Christoph Hellwig wrote:
> > On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
> >> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
> >> > I'm more than happy to switch a bunch of our exports so that we only
> >> > allow them for specific modules. But for that we also need
> >> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
> >> 
> >> Huh?  Any export for a specific in-tree module (or set thereof) is
> >> by definition internals and an _GPL export if perfectly fine and
> >> expected.
> 
> Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
> modules, so external module with GPL and matching name can import.
> 
> But if we're targetting in-tree stuff like kvm, we don't need to provide a
> non-GPL variant I think?

So the purpose was to limit specific symbols to known in-tree module
users (hence GPL only).

Eg. KVM; x86 exports a fair amount of low level stuff just because KVM.
Nobody else should be touching those symbols.

If you have a pile of symbols for !GPL / out-of-tree consumers, it
doesn't really make sense to limit the export to a named set of modules,
does it?

So yes, nothing limits things to in-tree modules per-se. The
infrastructure only really cares about module names (and implicitly
trusts the OS to not overwrite existing kernel modules etc.). So you
could add an out-of-tree module name to the list (or have an out-of-free
module have a name that matches a glob; "kvm-vmware" would match "kvm-*"
for example).

But that is very much beyond the intention of things.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christian Brauner]

On Mon, Jun 23, 2025 at 04:28:36PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 23, 2025 at 04:21:15PM +0200, Vlastimil Babka wrote:
> > On 6/23/25 16:01, Christoph Hellwig wrote:
> > > On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
> > >> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
> > >> > I'm more than happy to switch a bunch of our exports so that we only
> > >> > allow them for specific modules. But for that we also need
> > >> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
> > >> 
> > >> Huh?  Any export for a specific in-tree module (or set thereof) is
> > >> by definition internals and an _GPL export if perfectly fine and
> > >> expected.
> > 
> > Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
> > modules, so external module with GPL and matching name can import.
> > 
> > But if we're targetting in-tree stuff like kvm, we don't need to provide a
> > non-GPL variant I think?
> 
> So the purpose was to limit specific symbols to known in-tree module
> users (hence GPL only).
> 
> Eg. KVM; x86 exports a fair amount of low level stuff just because KVM.
> Nobody else should be touching those symbols.
> 
> If you have a pile of symbols for !GPL / out-of-tree consumers, it
> doesn't really make sense to limit the export to a named set of modules,
> does it?
> 
> So yes, nothing limits things to in-tree modules per-se. The
> infrastructure only really cares about module names (and implicitly
> trusts the OS to not overwrite existing kernel modules etc.). So you
> could add an out-of-tree module name to the list (or have an out-of-free
> module have a name that matches a glob; "kvm-vmware" would match "kvm-*"
> for example).
> 
> But that is very much beyond the intention of things.

So I'm not well-versed in all the GPL vs non-GPL exports. I'm thinking
of cases like EXPORT_SYMBOL(fget_task_next); That's exposed to gfs2 (and
bpf but that's built-in). I see no reason to risk spreading the usage of
that special-thing to anywhere else. So I would use
EXPORT_*_FOR_MODULES(gfs2) for this and we'd notice if anything else is
trying to use that thing.

Another excellent candidate is:

  /*
   * synchronous analog of fput(); for kernel threads that might be needed
   * in some umount() (and thus can't use flush_delayed_fput() without
   * risking deadlocks), need to wait for completion of __fput() and know
   * for this specific struct file it won't involve anything that would
   * need them.  Use only if you really need it - at the very least,
   * don't blindly convert fput() by kernel thread to that.
   */
  void __fput_sync(struct file *file)
  {
          if (file_ref_put(&file->f_ref))
                  __fput(file);
  }
  EXPORT_SYMBOL(__fput_sync);

That thing worries me to no end because that can be used to wreak all
kinds of havoc and I want that thing tied down so no one can even look
at it without getting a compile time or runtime error that we can
immediately notice. So for that as well I want to allow-list modules
that we have explictly acknowledged to use it.

But iiuc I can't just switch that non-GPL exported symbol to a GPL
exported symbol. And I don't want to be involved in some kind of
ideological warfare around that stuff.

I care about not growing more users of __fput_sync(). So any advice is
appreciated.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Christian Brauner]

On Tue, Jun 24, 2025 at 11:02:16AM +0200, Christian Brauner wrote:
> On Mon, Jun 23, 2025 at 04:28:36PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 23, 2025 at 04:21:15PM +0200, Vlastimil Babka wrote:
> > > On 6/23/25 16:01, Christoph Hellwig wrote:
> > > > On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
> > > >> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
> > > >> > I'm more than happy to switch a bunch of our exports so that we only
> > > >> > allow them for specific modules. But for that we also need
> > > >> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
> > > >> 
> > > >> Huh?  Any export for a specific in-tree module (or set thereof) is
> > > >> by definition internals and an _GPL export if perfectly fine and
> > > >> expected.
> > > 
> > > Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
> > > modules, so external module with GPL and matching name can import.
> > > 
> > > But if we're targetting in-tree stuff like kvm, we don't need to provide a
> > > non-GPL variant I think?
> > 
> > So the purpose was to limit specific symbols to known in-tree module
> > users (hence GPL only).
> > 
> > Eg. KVM; x86 exports a fair amount of low level stuff just because KVM.
> > Nobody else should be touching those symbols.
> > 
> > If you have a pile of symbols for !GPL / out-of-tree consumers, it
> > doesn't really make sense to limit the export to a named set of modules,
> > does it?

I don't understand that argument. I don't care if out-of-tree users
abuse some symbol because:

* If they ever show up with a complaint we'll tell them to go away. 
* If they want to be merged upstream, we'll tell them to either change
  the code in question to not rely on the offending symbol or we decide
  that it's ok for them to use it and allow-list them.

I do however very much care about in-tree consumers even for non-GPLd
symbols. I want anyone who tries to use a symbol that we decided
requires substantial arguments to be used to come to us and justify it.
So EXPORT_*_FOR_MODULES() would certainly help with that.

The other things is that using EXPORT_SYMBOL() or even
EXPORT_SYMBOL_GPL() sends the wrong message to other module-like
wanna-be consumers of these functions. I'm specifically thinking about
bpf. They more than once argued that anything exposed to modules can
also be exposed as a bpf kfunc as the stability guarantees are
comparable.

And it is not an insane argument. Being able to use
EXPOR_SYMBOL_FOR_MODULES() would also allow to communicate "Hey, this
very much just exists for the purpose of this one-off consumer that
really can't do without it or without some other ugly hack.".

Because this is where the pain for us is: If you do large-scale
refactorings (/me glares at Al, Christoph, and in the mirror) the worst
case is finding out that some special-purpose helper has grown N new
users with a bunch of them using it completely wrong and now having to
massage core code to not break something that's technically inherently
broken.

Out-of-tree consumers have zero relevance for all of this. For all I
care they don't exist. It's about the maintainers ability to chop off
the Kraken's tentacles.

> > 
> > So yes, nothing limits things to in-tree modules per-se. The
> > infrastructure only really cares about module names (and implicitly
> > trusts the OS to not overwrite existing kernel modules etc.). So you
> > could add an out-of-tree module name to the list (or have an out-of-free
> > module have a name that matches a glob; "kvm-vmware" would match "kvm-*"
> > for example).
> > 
> > But that is very much beyond the intention of things.
> 
> So I'm not well-versed in all the GPL vs non-GPL exports. I'm thinking
> of cases like EXPORT_SYMBOL(fget_task_next); That's exposed to gfs2 (and
> bpf but that's built-in). I see no reason to risk spreading the usage of
> that special-thing to anywhere else. So I would use
> EXPORT_*_FOR_MODULES(gfs2) for this and we'd notice if anything else is
> trying to use that thing.
> 
> Another excellent candidate is:
> 
>   /*
>    * synchronous analog of fput(); for kernel threads that might be needed
>    * in some umount() (and thus can't use flush_delayed_fput() without
>    * risking deadlocks), need to wait for completion of __fput() and know
>    * for this specific struct file it won't involve anything that would
>    * need them.  Use only if you really need it - at the very least,
>    * don't blindly convert fput() by kernel thread to that.
>    */
>   void __fput_sync(struct file *file)
>   {
>           if (file_ref_put(&file->f_ref))
>                   __fput(file);
>   }
>   EXPORT_SYMBOL(__fput_sync);
> 
> That thing worries me to no end because that can be used to wreak all
> kinds of havoc and I want that thing tied down so no one can even look
> at it without getting a compile time or runtime error that we can
> immediately notice. So for that as well I want to allow-list modules
> that we have explictly acknowledged to use it.
> 
> But iiuc I can't just switch that non-GPL exported symbol to a GPL
> exported symbol. And I don't want to be involved in some kind of
> ideological warfare around that stuff.
> 
> I care about not growing more users of __fput_sync(). So any advice is
> appreciated.

s/./?/

That was supposed to be a question mark.

IOW, can I add EXPORT_SYMBOL_FOR_MODULES() as a companion to
EXPORT_SYMBOL_GPL_FOR_MODULES()?

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Vlastimil Babka]

On 6/25/25 11:05, Christian Brauner wrote:
> On Tue, Jun 24, 2025 at 11:02:16AM +0200, Christian Brauner wrote:
> I don't understand that argument. I don't care if out-of-tree users
> abuse some symbol because:
> 
> * If they ever show up with a complaint we'll tell them to go away. 
> * If they want to be merged upstream, we'll tell them to either change
>   the code in question to not rely on the offending symbol or we decide
>   that it's ok for them to use it and allow-list them.
> 
> I do however very much care about in-tree consumers even for non-GPLd
> symbols. I want anyone who tries to use a symbol that we decided
> requires substantial arguments to be used to come to us and justify it.
> So EXPORT_*_FOR_MODULES() would certainly help with that.
> 
> The other things is that using EXPORT_SYMBOL() or even
> EXPORT_SYMBOL_GPL() sends the wrong message to other module-like
> wanna-be consumers of these functions. I'm specifically thinking about
> bpf. They more than once argued that anything exposed to modules can
> also be exposed as a bpf kfunc as the stability guarantees are
> comparable.
> 
> And it is not an insane argument. Being able to use
> EXPOR_SYMBOL_FOR_MODULES() would also allow to communicate "Hey, this
> very much just exists for the purpose of this one-off consumer that
> really can't do without it or without some other ugly hack.".
> 
> Because this is where the pain for us is: If you do large-scale
> refactorings (/me glares at Al, Christoph, and in the mirror) the worst
> case is finding out that some special-purpose helper has grown N new
> users with a bunch of them using it completely wrong and now having to
> massage core code to not break something that's technically inherently
> broken.
> 
> Out-of-tree consumers have zero relevance for all of this. For all I
> care they don't exist. It's about the maintainers ability to chop off
> the Kraken's tentacles.

Then I think you can just use EXPORT_SYMBOL_GPL_FOR_MODULES() as it is
today. It's intended for a particular in-tree module, which is by definition
going to be GPL. I doubt anyone will come complaining that you've cut off
their ability to fake the name of the in-tree module while having non-GPL
license. So I don't really see the danger of causing holy license wars
there. The _FOR_MODULES() part is restricting enough even without _GPL_.

But if we can indeed enforce in-tree-ness and drop _GPL_ from the name, it
would be cleaner IMHO.

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Vlastimil Babka]

On 6/23/25 16:28, Peter Zijlstra wrote:
> On Mon, Jun 23, 2025 at 04:21:15PM +0200, Vlastimil Babka wrote:
>> On 6/23/25 16:01, Christoph Hellwig wrote:
>> > On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
>> >> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
>> >> > I'm more than happy to switch a bunch of our exports so that we only
>> >> > allow them for specific modules. But for that we also need
>> >> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
>> >> 
>> >> Huh?  Any export for a specific in-tree module (or set thereof) is
>> >> by definition internals and an _GPL export if perfectly fine and
>> >> expected.
>> 
>> Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
>> modules, so external module with GPL and matching name can import.
>> 
>> But if we're targetting in-tree stuff like kvm, we don't need to provide a
>> non-GPL variant I think?
> 
> So the purpose was to limit specific symbols to known in-tree module
> users (hence GPL only).
> 
> Eg. KVM; x86 exports a fair amount of low level stuff just because KVM.
> Nobody else should be touching those symbols.
> 
> If you have a pile of symbols for !GPL / out-of-tree consumers, it
> doesn't really make sense to limit the export to a named set of modules,
> does it?
> 
> So yes, nothing limits things to in-tree modules per-se. The
> infrastructure only really cares about module names (and implicitly
> trusts the OS to not overwrite existing kernel modules etc.). So you
> could add an out-of-tree module name to the list (or have an out-of-free
> module have a name that matches a glob; "kvm-vmware" would match "kvm-*"
> for example).
> 
> But that is very much beyond the intention of things.

So AFAIK we have a way to recognize out of tree modules when loading, as
there's a taint just for that. Then the same mechanism could perhaps just
refuse loading them if they use any _FOR_MODULES() export, regardless of
name? Then the _GPL_ part would become implicit and redundant and we could
drop it as Christoph suggested?

Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix secretmem LSM bypass

[David Hildenbrand]

On 25.06.25 10:02, Vlastimil Babka wrote:
> On 6/23/25 16:28, Peter Zijlstra wrote:
>> On Mon, Jun 23, 2025 at 04:21:15PM +0200, Vlastimil Babka wrote:
>>> On 6/23/25 16:01, Christoph Hellwig wrote:
>>>> On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
>>>>> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
>>>>>> I'm more than happy to switch a bunch of our exports so that we only
>>>>>> allow them for specific modules. But for that we also need
>>>>>> EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
>>>>>
>>>>> Huh?  Any export for a specific in-tree module (or set thereof) is
>>>>> by definition internals and an _GPL export if perfectly fine and
>>>>> expected.
>>>
>>> Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
>>> modules, so external module with GPL and matching name can import.
>>>
>>> But if we're targetting in-tree stuff like kvm, we don't need to provide a
>>> non-GPL variant I think?
>>
>> So the purpose was to limit specific symbols to known in-tree module
>> users (hence GPL only).
>>
>> Eg. KVM; x86 exports a fair amount of low level stuff just because KVM.
>> Nobody else should be touching those symbols.
>>
>> If you have a pile of symbols for !GPL / out-of-tree consumers, it
>> doesn't really make sense to limit the export to a named set of modules,
>> does it?
>>
>> So yes, nothing limits things to in-tree modules per-se. The
>> infrastructure only really cares about module names (and implicitly
>> trusts the OS to not overwrite existing kernel modules etc.). So you
>> could add an out-of-tree module name to the list (or have an out-of-free
>> module have a name that matches a glob; "kvm-vmware" would match "kvm-*"
>> for example).
>>
>> But that is very much beyond the intention of things.
> 
> So AFAIK we have a way to recognize out of tree modules when loading, as
> there's a taint just for that. Then the same mechanism could perhaps just
> refuse loading them if they use any _FOR_MODULES() export, regardless of
> name? Then the _GPL_ part would become implicit and redundant and we could
> drop it as Christoph suggested?

If that is possible, that sounds indeed nice.

-- 
Cheers,

David / dhildenb