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

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

[Shivank Garg]

Extend anon_inode_make_secure_inode() to take superblock parameter and
make it available via fs.h. This allows other subsystems to create
anonymous inodes with proper security context.

Use this function in secretmem to fix a security regression, where
S_PRIVATE flag wasn't cleared after alloc_anon_inode(), causing
LSM/SELinux checks to be skipped.

Using anon_inode_make_secure_inode() ensures proper security context
initialization through security_inode_init_security_anon().

Fixes: 2bfe15c52612 ("mm: create security context for memfd_secret inodes")
Suggested-by: David Hildenbrand <david@redhat.com>
Suggested-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Mike Rapoport (Microsoft) <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

V3:
- Drop EXPORT to be added later in separate patch for KVM guest_memfd and
  keep this patch focused on fix.

V2: https://lore.kernel.org/all/20250620070328.803704-3-shivankg@amd.com
- Use EXPORT_SYMBOL_GPL_FOR_MODULES() since KVM is the only user.

V1: https://lore.kernel.org/all/20250619073136.506022-2-shivankg@amd.com

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

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index e51e7d88980a..c530405edd15 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 new file (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;
@@ -132,7 +143,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 V3] fs: generalize anon_inode_make_secure_inode() and fix secretmem LSM bypass

[Gupta, Pankaj]

> Extend anon_inode_make_secure_inode() to take superblock parameter and
> make it available via fs.h. This allows other subsystems to create
> anonymous inodes with proper security context.
> 
> Use this function in secretmem to fix a security regression, where
> S_PRIVATE flag wasn't cleared after alloc_anon_inode(), causing
> LSM/SELinux checks to be skipped.
> 
> Using anon_inode_make_secure_inode() ensures proper security context
> initialization through security_inode_init_security_anon().
> 
> Fixes: 2bfe15c52612 ("mm: create security context for memfd_secret inodes")
> Suggested-by: David Hildenbrand <david@redhat.com>
> Suggested-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Signed-off-by: Shivank Garg <shivankg@amd.com>

Relying on 'anon_inode_make_secure_inode' for anon inodes LSM/SELinux
checks seems okay to me.

Acked-by: Pankaj Gupta <pankaj.gupta@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
> 
> V3:
> - Drop EXPORT to be added later in separate patch for KVM guest_memfd and
>    keep this patch focused on fix.
> 
> V2: https://lore.kernel.org/all/20250620070328.803704-3-shivankg@amd.com
> - Use EXPORT_SYMBOL_GPL_FOR_MODULES() since KVM is the only user.
> 
> V1: https://lore.kernel.org/all/20250619073136.506022-2-shivankg@amd.com
> 
>   fs/anon_inodes.c   | 22 +++++++++++++++++-----
>   include/linux/fs.h |  2 ++
>   mm/secretmem.c     |  9 +--------
>   3 files changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index e51e7d88980a..c530405edd15 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 new file (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;
> @@ -132,7 +143,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))

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

[Ira Weiny]

Shivank Garg wrote:
> Extend anon_inode_make_secure_inode() to take superblock parameter and
> make it available via fs.h. This allows other subsystems to create
> anonymous inodes with proper security context.
> 
> Use this function in secretmem to fix a security regression, where
> S_PRIVATE flag wasn't cleared after alloc_anon_inode(), causing
> LSM/SELinux checks to be skipped.
> 
> Using anon_inode_make_secure_inode() ensures proper security context
> initialization through security_inode_init_security_anon().
> 
> Fixes: 2bfe15c52612 ("mm: create security context for memfd_secret inodes")
> Suggested-by: David Hildenbrand <david@redhat.com>
> Suggested-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Reviewed-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

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

[Christian Brauner]

On Thu, 26 Jun 2025 19:14:29 +0000, Shivank Garg wrote:
> Extend anon_inode_make_secure_inode() to take superblock parameter and
> make it available via fs.h. This allows other subsystems to create
> anonymous inodes with proper security context.
> 
> Use this function in secretmem to fix a security regression, where
> S_PRIVATE flag wasn't cleared after alloc_anon_inode(), causing
> LSM/SELinux checks to be skipped.
> 
> [...]

Applied to the vfs-6.17.misc branch of the vfs/vfs.git tree.
Patches in the vfs-6.17.misc 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-6.17.misc

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

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

[Paul Moore]

On Jun 26, 2025 Shivank Garg <shivankg@amd.com> wrote:
> 
> Extend anon_inode_make_secure_inode() to take superblock parameter and
> make it available via fs.h. This allows other subsystems to create
> anonymous inodes with proper security context.
> 
> Use this function in secretmem to fix a security regression, where
> S_PRIVATE flag wasn't cleared after alloc_anon_inode(), causing
> LSM/SELinux checks to be skipped.
> 
> Using anon_inode_make_secure_inode() ensures proper security context
> initialization through security_inode_init_security_anon().
> 
> Fixes: 2bfe15c52612 ("mm: create security context for memfd_secret inodes")
> Suggested-by: David Hildenbrand <david@redhat.com>
> Suggested-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> Acked-by: Pankaj Gupta <pankaj.gupta@amd.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.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
> 
> V3:
> - Drop EXPORT to be added later in separate patch for KVM guest_memfd and
>   keep this patch focused on fix.
> 
> V2: https://lore.kernel.org/all/20250620070328.803704-3-shivankg@amd.com
> - Use EXPORT_SYMBOL_GPL_FOR_MODULES() since KVM is the only user.
> 
> V1: https://lore.kernel.org/all/20250619073136.506022-2-shivankg@amd.com
> 
>  fs/anon_inodes.c   | 22 +++++++++++++++++-----
>  include/linux/fs.h |  2 ++
>  mm/secretmem.c     |  9 +--------
>  3 files changed, 20 insertions(+), 13 deletions(-)

Thanks again for your continued work on this!  I think the patch looks
pretty reasonable, but it would be good to hear a bit about how you've
tested this before ACK'ing the patch.  For example, have you tested this
against any of the LSMs which provide anonymous inode support?

At the very least, the selinux-testsuite has a basic secretmem test, it
would be good to know if the test passes with this patch or if any
additional work is needed to ensure compatibility.

https://github.com/SELinuxProject/selinux-testsuite

--
paul-moore.com

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

[Shivank Garg]

On 7/3/2025 7:43 AM, Paul Moore wrote:
> On Jun 26, 2025 Shivank Garg <shivankg@amd.com> wrote:
>
...
> Thanks again for your continued work on this!  I think the patch looks
> pretty reasonable, but it would be good to hear a bit about how you've
> tested this before ACK'ing the patch.  For example, have you tested this
> against any of the LSMs which provide anonymous inode support?
> 
> At the very least, the selinux-testsuite has a basic secretmem test, it
> would be good to know if the test passes with this patch or if any
> additional work is needed to ensure compatibility.
> 
> https://github.com/SELinuxProject/selinux-testsuite

Hi Paul,

Thank you for pointing me to the selinux-testsuite. I wasn't sure how to properly
test this patch, so your guidance was very helpful.

With the current test policy (test_secretmem.te), I initially encountered the following failures:

~/selinux-testsuite/tests/secretmem# ./test
memfd_secret() failed:  Permission denied
1..6
memfd_secret() failed:  Permission denied
ok 1
ftruncate failed:  Permission denied
unable to mmap secret memory:  Permission denied
not ok 2
#   Failed test at ./test line 23.
ftruncate failed:  Permission denied
unable to mmap secret memory:  Permission denied
ok 3
ftruncate failed:  Permission denied
unable to mmap secret memory:  Permission denied
ok 4
memfd_secret() failed:  Permission denied
ok 5
ftruncate failed:  Permission denied
unable to mmap secret memory:  Permission denied
not ok 6
#   Failed test at ./test line 37.
# Looks like you failed 2 tests of 6.

Using ausearch -m avc, I found denials for create, write, map. For instance:
 avc:  denied  { create } for  pid=11956 comm="secretmem" anonclass=[secretmem] 
...

To resolve this, I updated test_secretmem.te to add additional required
permissions {create, read, write, map}
With this change, all tests now pass successfully:

diff --git a/policy/test_secretmem.te b/policy/test_secretmem.te
index 357f41d..4cce076 100644
--- a/policy/test_secretmem.te
+++ b/policy/test_secretmem.te
@@ -13,12 +13,12 @@ testsuite_domain_type_minimal(test_nocreate_secretmem_t)
 # Domain allowed to create secret memory with the own domain type
 type test_create_secretmem_t;
 testsuite_domain_type_minimal(test_create_secretmem_t)
-allow test_create_secretmem_t self:anon_inode create;
+allow test_create_secretmem_t self:anon_inode { create read write map };

 # Domain allowed to create secret memory with the own domain type and allowed to map WX
 type test_create_wx_secretmem_t;
 testsuite_domain_type_minimal(test_create_wx_secretmem_t)
-allow test_create_wx_secretmem_t self:anon_inode create;
+allow test_create_wx_secretmem_t self:anon_inode { create read write map };
 allow test_create_wx_secretmem_t self:process execmem;

 # Domain not allowed to create secret memory via a type transition to a private type
@@ -30,4 +30,4 @@ type_transition test_nocreate_transition_secretmem_t test_nocreate_transition_se
 type test_create_transition_secretmem_t;
 testsuite_domain_type_minimal(test_create_transition_secretmem_t)
 type_transition test_create_transition_secretmem_t test_create_transition_secretmem_t:anon_inode test_secretmem_inode_t "[secretmem]";
-allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode create;
+allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode { create read write map };

Does this approach look correct to you? Please let me know if my understanding
makes sense and what should be my next step for patch.

Thanks,
Shivank

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

[Shivank Garg]

On 7/1/2025 2:03 PM, Christian Brauner wrote:
> On Thu, 26 Jun 2025 19:14:29 +0000, Shivank Garg wrote:
>> Extend anon_inode_make_secure_inode() to take superblock parameter and
>> make it available via fs.h. This allows other subsystems to create
>> anonymous inodes with proper security context.
>>
>> Use this function in secretmem to fix a security regression, where
>> S_PRIVATE flag wasn't cleared after alloc_anon_inode(), causing
>> LSM/SELinux checks to be skipped.
>>
>> [...]
> 
> Applied to the vfs-6.17.misc branch of the vfs/vfs.git tree.
> Patches in the vfs-6.17.misc 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-6.17.misc
> 
> [1/1] fs: generalize anon_inode_make_secure_inode() and fix secretmem LSM bypass
>       https://git.kernel.org/vfs/vfs/c/4dc65f072c2b


Hi Christian,

I think there may have been a mix-up with the patch versions that got merged.

We had agreed to use V3 of the patch (without EXPORT), which appears to be 
correctly merged in the vfs tree:
https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.all&id=4dc65f072c2b30ae3653b76208a926f767c402a0

However, it looks like V2 (with EXPORT_SYMBOL_GPL_FOR_MODULES) was merged into 
Linus's tree instead:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cbe4134ea4bc493239786220bd69cb8a13493190

Thanks,
Shivank

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

[Paul Moore]

On Fri, Jul 4, 2025 at 6:41 AM Shivank Garg <shivankg@amd.com> wrote:
> On 7/3/2025 7:43 AM, Paul Moore wrote:
> > On Jun 26, 2025 Shivank Garg <shivankg@amd.com> wrote:
>
> ...
>
> > Thanks again for your continued work on this!  I think the patch looks
> > pretty reasonable, but it would be good to hear a bit about how you've
> > tested this before ACK'ing the patch.  For example, have you tested this
> > against any of the LSMs which provide anonymous inode support?
> >
> > At the very least, the selinux-testsuite has a basic secretmem test, it
> > would be good to know if the test passes with this patch or if any
> > additional work is needed to ensure compatibility.
> >
> > https://github.com/SELinuxProject/selinux-testsuite
>
> Hi Paul,
>
> Thank you for pointing me to the selinux-testsuite. I wasn't sure how to properly
> test this patch, so your guidance was very helpful.
>
> With the current test policy (test_secretmem.te), I initially encountered the following failures:
>
> ~/selinux-testsuite/tests/secretmem# ./test
> memfd_secret() failed:  Permission denied
> 1..6
> memfd_secret() failed:  Permission denied
> ok 1
> ftruncate failed:  Permission denied
> unable to mmap secret memory:  Permission denied
> not ok 2

...

> To resolve this, I updated test_secretmem.te to add additional required
> permissions {create, read, write, map}
> With this change, all tests now pass successfully:
>
> diff --git a/policy/test_secretmem.te b/policy/test_secretmem.te
> index 357f41d..4cce076 100644
> --- a/policy/test_secretmem.te
> +++ b/policy/test_secretmem.te
> @@ -13,12 +13,12 @@ testsuite_domain_type_minimal(test_nocreate_secretmem_t)
>  # Domain allowed to create secret memory with the own domain type
>  type test_create_secretmem_t;
>  testsuite_domain_type_minimal(test_create_secretmem_t)
> -allow test_create_secretmem_t self:anon_inode create;
> +allow test_create_secretmem_t self:anon_inode { create read write map };
>
>  # Domain allowed to create secret memory with the own domain type and allowed to map WX
>  type test_create_wx_secretmem_t;
>  testsuite_domain_type_minimal(test_create_wx_secretmem_t)
> -allow test_create_wx_secretmem_t self:anon_inode create;
> +allow test_create_wx_secretmem_t self:anon_inode { create read write map };

I believe this domain also needs the anon_inode/execute permission.

>  allow test_create_wx_secretmem_t self:process execmem;
>
>  # Domain not allowed to create secret memory via a type transition to a private type
> @@ -30,4 +30,4 @@ type_transition test_nocreate_transition_secretmem_t test_nocreate_transition_se
>  type test_create_transition_secretmem_t;
>  testsuite_domain_type_minimal(test_create_transition_secretmem_t)
>  type_transition test_create_transition_secretmem_t test_create_transition_secretmem_t:anon_inode test_secretmem_inode_t "[secretmem]";
> -allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode create;
> +allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode { create read write map };
>
> Does this approach look correct to you? Please let me know if my understanding
> makes sense and what should be my next step for patch.

[NOTE: added selinux@vger and selinux-refpolicy@vger to the To/CC line]

Hi Shivank,

My apologies for not responding earlier, Friday was a holiday and I
was away over the weekend.  Getting back at it this morning I ran into
the same failures as you described, and had to make similar changes to
the selinux-testsuite policy (see the anon_inode/execute comment
above, I also added the capability/ipc_lock permission as needed).

Strictly speaking this is a regression in the kernel, even if the new
behavior is correct.  I'm CC'ing the SELinux and Reference Policy
lists so that the policy devs can take a look and see what impacts
there might be to the various public SELinux policies.  If this looks
like it may be a significant issue, we'll need to work around this
with a SELinux "policy capability" or some other compatibility
solution.

-- 
paul-moore.com

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

[Chris PeBenito]

On 7/7/2025 4:01 PM, Paul Moore wrote:
> On Fri, Jul 4, 2025 at 6:41 AM Shivank Garg <shivankg@amd.com> wrote:
>> On 7/3/2025 7:43 AM, Paul Moore wrote:
>>> On Jun 26, 2025 Shivank Garg <shivankg@amd.com> wrote:
>>
>> ...
>>
>>> Thanks again for your continued work on this!  I think the patch looks
>>> pretty reasonable, but it would be good to hear a bit about how you've
>>> tested this before ACK'ing the patch.  For example, have you tested this
>>> against any of the LSMs which provide anonymous inode support?
>>>
>>> At the very least, the selinux-testsuite has a basic secretmem test, it
>>> would be good to know if the test passes with this patch or if any
>>> additional work is needed to ensure compatibility.
>>>
>>> https://github.com/SELinuxProject/selinux-testsuite
>>
>> Hi Paul,
>>
>> Thank you for pointing me to the selinux-testsuite. I wasn't sure how to properly
>> test this patch, so your guidance was very helpful.
>>
>> With the current test policy (test_secretmem.te), I initially encountered the following failures:
>>
>> ~/selinux-testsuite/tests/secretmem# ./test
>> memfd_secret() failed:  Permission denied
>> 1..6
>> memfd_secret() failed:  Permission denied
>> ok 1
>> ftruncate failed:  Permission denied
>> unable to mmap secret memory:  Permission denied
>> not ok 2
> 
> ...
> 
>> To resolve this, I updated test_secretmem.te to add additional required
>> permissions {create, read, write, map}
>> With this change, all tests now pass successfully:
>>
>> diff --git a/policy/test_secretmem.te b/policy/test_secretmem.te
>> index 357f41d..4cce076 100644
>> --- a/policy/test_secretmem.te
>> +++ b/policy/test_secretmem.te
>> @@ -13,12 +13,12 @@ testsuite_domain_type_minimal(test_nocreate_secretmem_t)
>>   # Domain allowed to create secret memory with the own domain type
>>   type test_create_secretmem_t;
>>   testsuite_domain_type_minimal(test_create_secretmem_t)
>> -allow test_create_secretmem_t self:anon_inode create;
>> +allow test_create_secretmem_t self:anon_inode { create read write map };
>>
>>   # Domain allowed to create secret memory with the own domain type and allowed to map WX
>>   type test_create_wx_secretmem_t;
>>   testsuite_domain_type_minimal(test_create_wx_secretmem_t)
>> -allow test_create_wx_secretmem_t self:anon_inode create;
>> +allow test_create_wx_secretmem_t self:anon_inode { create read write map };
> 
> I believe this domain also needs the anon_inode/execute permission.
> 
>>   allow test_create_wx_secretmem_t self:process execmem;
>>
>>   # Domain not allowed to create secret memory via a type transition to a private type
>> @@ -30,4 +30,4 @@ type_transition test_nocreate_transition_secretmem_t test_nocreate_transition_se
>>   type test_create_transition_secretmem_t;
>>   testsuite_domain_type_minimal(test_create_transition_secretmem_t)
>>   type_transition test_create_transition_secretmem_t test_create_transition_secretmem_t:anon_inode test_secretmem_inode_t "[secretmem]";
>> -allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode create;
>> +allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode { create read write map };
>>
>> Does this approach look correct to you? Please let me know if my understanding
>> makes sense and what should be my next step for patch.
> 
> [NOTE: added selinux@vger and selinux-refpolicy@vger to the To/CC line]
> 
> Hi Shivank,
> 
> My apologies for not responding earlier, Friday was a holiday and I
> was away over the weekend.  Getting back at it this morning I ran into
> the same failures as you described, and had to make similar changes to
> the selinux-testsuite policy (see the anon_inode/execute comment
> above, I also added the capability/ipc_lock permission as needed).
> 
> Strictly speaking this is a regression in the kernel, even if the new
> behavior is correct.  I'm CC'ing the SELinux and Reference Policy
> lists so that the policy devs can take a look and see what impacts
> there might be to the various public SELinux policies.  If this looks
> like it may be a significant issue, we'll need to work around this
> with a SELinux "policy capability" or some other compatibility
> solution.

In refpolicy, there are 34 rules for anon_inode and they all have { 
create read write map } -- none of them have the execute permission.  Of 
these, only 4 are explict and could potentially be broken.  The 
remaining get it due to being unconfined, thus can be immediately fixed, 
since it's unconfined.

IMO, this is very low impact.

-- 
Chris PeBenito

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

[Paul Moore]

On Mon, Jul 7, 2025 at 4:38 PM Chris PeBenito <pebenito@ieee.org> wrote:
> On 7/7/2025 4:01 PM, Paul Moore wrote:
> >
> > Strictly speaking this is a regression in the kernel, even if the new
> > behavior is correct.  I'm CC'ing the SELinux and Reference Policy
> > lists so that the policy devs can take a look and see what impacts
> > there might be to the various public SELinux policies.  If this looks
> > like it may be a significant issue, we'll need to work around this
> > with a SELinux "policy capability" or some other compatibility
> > solution.
>
> In refpolicy, there are 34 rules for anon_inode and they all have {
> create read write map } -- none of them have the execute permission.  Of
> these, only 4 are explict and could potentially be broken.  The
> remaining get it due to being unconfined, thus can be immediately fixed,
> since it's unconfined.
>
> IMO, this is very low impact.

Thanks Chris, I think it's worth leaving the kernel code as-is and
just patching the selinux-testsuite.  I'll send out a patch for that
tomorrow.

-- 
paul-moore.com