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

[Paul Moore <paul@paul-moore.com>]

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