fs/anon_inodes.c | 23 ++++++++++++++++++----- include/linux/fs.h | 2 ++ mm/secretmem.c | 9 +-------- 3 files changed, 21 insertions(+), 13 deletions(-)
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
result 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
V1->V2: Use EXPORT_SYMBOL_GPL_FOR_MODULES() since KVM is the only user.
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..1d847a939f29 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_FOR_MODULES(anon_inode_make_secure_inode, "kvm");
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.0On 20.06.25 09:03, 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
> 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>
In general, LGTM, but I think the actual fix should be separated from
exporting it for guest_memfd purposes?
Also makes backporting easier, when EXPORT_SYMBOL_GPL_FOR_MODULES does
not exist yet ...
Leaving deciding about that to fs people.
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
On 6/23/2025 7:21 PM, David Hildenbrand wrote:
> On 20.06.25 09:03, 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
>> 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>
>
>
> In general, LGTM, but I think the actual fix should be separated from exporting it for guest_memfd purposes?
>
> Also makes backporting easier, when EXPORT_SYMBOL_GPL_FOR_MODULES does not exist yet ...
>
I agree. I did not think about backporting conflicts when sending the patch.
Christian, I can send it as 2 separate patches to make it easier?
Thanks,
Shivank
On 6/23/25 16:08, Shivank Garg wrote:
>
>
> On 6/23/2025 7:21 PM, David Hildenbrand wrote:
>> On 20.06.25 09:03, 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
>>> 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>
>>
>>
>> In general, LGTM, but I think the actual fix should be separated from exporting it for guest_memfd purposes?
>>
>> Also makes backporting easier, when EXPORT_SYMBOL_GPL_FOR_MODULES does not exist yet ...
>>
> I agree. I did not think about backporting conflicts when sending the patch.
>
> Christian, I can send it as 2 separate patches to make it easier?
The proper way is to send the fix without the export, and then add the
export only when adding its user.
> Thanks,
> Shivank
On 6/23/25 16:13, Vlastimil Babka wrote: > On 6/23/25 16:08, Shivank Garg wrote: >> >> >>> >>> In general, LGTM, but I think the actual fix should be separated from exporting it for guest_memfd purposes? >>> >>> Also makes backporting easier, when EXPORT_SYMBOL_GPL_FOR_MODULES does not exist yet ... >>> >> I agree. I did not think about backporting conflicts when sending the patch. >> >> Christian, I can send it as 2 separate patches to make it easier? > > The proper way is to send the fix without the export, and then add the > export only when adding its user. Note: AFAIU either way the new user would be depending on a patch in a vfs tree (maybe scheduled for an 6.16 rc and not the next merge window?) if that's an issue for the development. >> Thanks, >> Shivank >
On 6/23/2025 7:58 PM, Vlastimil Babka wrote: > On 6/23/25 16:13, Vlastimil Babka wrote: >> On 6/23/25 16:08, Shivank Garg wrote: >>> >>> >>>> >>>> In general, LGTM, but I think the actual fix should be separated from exporting it for guest_memfd purposes? >>>> >>>> Also makes backporting easier, when EXPORT_SYMBOL_GPL_FOR_MODULES does not exist yet ... >>>> >>> I agree. I did not think about backporting conflicts when sending the patch. >>> >>> Christian, I can send it as 2 separate patches to make it easier? >> >> The proper way is to send the fix without the export, and then add the >> export only when adding its user. > > Note: AFAIU either way the new user would be depending on a patch in a vfs > tree (maybe scheduled for an 6.16 rc and not the next merge window?) if > that's an issue for the development. Thanks Vlastimil. I have sent a revised patch [1] without EXPORT. The EXPORT can be added later through the KVM tree with the guest_memfd changes. Hopefully, anon_inode_make_secure_inode() change will be merged by then. Christian, could you please replace the current patch with V3 [1]? And Would you also be willing to provide your Acked-by when EXPORT_SYMBOL_GPL_FOR_MODULES change addition is submitted later? Thank you for the patience and review :) [1] https://lore.kernel.org/all/20250626191425.9645-5-shivankg@amd.com Best Regards, Shivank
On Fri, 20 Jun 2025 07:03:30 +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/cbe4134ea4bc
On Fri, Jun 20, 2025 at 07:03:30AM +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.
>
> 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>
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> 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
> result 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
>
> V1->V2: Use EXPORT_SYMBOL_GPL_FOR_MODULES() since KVM is the only user.
>
> 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..1d847a939f29 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_FOR_MODULES(anon_inode_make_secure_inode, "kvm");
>
> 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
>
--
Sincerely yours,
Mike.
© 2016 - 2025 Red Hat, Inc.