Re: [PATCH] memfd,selinux: call security_inode_init_security_anon

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

On Aug 25, 2025 "=?UTF-8?q?Thi=C3=A9baud=20Weksteen?=" <tweek@google.com> wrote:
> 
> Prior to this change, no security hooks were called at the creation of a
> memfd file. It means that, for SELinux as an example, it will receive
> the default type of the filesystem that backs the in-memory inode. In
> most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
> be hugetlbfs. Both can be considered implementation details of memfd.
> 
> It also means that it is not possible to differentiate between a file
> coming from memfd_create and a file coming from a standard tmpfs mount
> point.
> 
> Additionally, no permission is validated at creation, which differs from
> the similar memfd_secret syscall.
> 
> Call security_inode_init_security_anon during creation. This ensures
> that the file is setup similarly to other anonymous inodes. On SELinux,
> it means that the file will receive the security context of its task.
> 
> The ability to limit fexecve on memfd has been of interest to avoid
> potential pitfalls where /proc/self/exe or similar would be executed
> [1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
> similarly to the file class. These access vectors may not make sense for
> the existing "anon_inode" class. Therefore, define and assign a new
> class "memfd_file" to support such access vectors.
> 
> Guard these changes behind a new policy capability named "memfd_class".
> 
> [1] https://crbug.com/1305267
> [2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
> 
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> Changes since RFC:
> - Remove enum argument, simply compare the anon inode name
> - Introduce a policy capability for compatility
> - Add validation of class in selinux_bprm_creds_for_exec
> 
>  include/linux/memfd.h                      |  2 ++
>  mm/memfd.c                                 | 14 +++++++++--
>  security/selinux/hooks.c                   | 27 ++++++++++++++++++----
>  security/selinux/include/classmap.h        |  2 ++
>  security/selinux/include/policycap.h       |  1 +
>  security/selinux/include/policycap_names.h |  1 +
>  security/selinux/include/security.h        |  5 ++++
>  7 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/memfd.h b/include/linux/memfd.h
> index 6f606d9573c3..cc74de3dbcfe 100644
> --- a/include/linux/memfd.h
> +++ b/include/linux/memfd.h
> @@ -4,6 +4,8 @@
>  
>  #include <linux/file.h>
>  
> +#define MEMFD_ANON_NAME "[memfd]"
> +
>  #ifdef CONFIG_MEMFD_CREATE
>  extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg);
>  struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx);
> diff --git a/mm/memfd.c b/mm/memfd.c
> index bbe679895ef6..63b439eb402a 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -433,6 +433,8 @@ static struct file *alloc_file(const char *name, unsigned int flags)
>  {
>  	unsigned int *file_seals;
>  	struct file *file;
> +	struct inode *inode;
> +	int err = 0;
>  
>  	if (flags & MFD_HUGETLB) {
>  		file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> @@ -444,12 +446,20 @@ static struct file *alloc_file(const char *name, unsigned int flags)
>  	}
>  	if (IS_ERR(file))
>  		return file;
> +
> +	inode = file_inode(file);
> +	err = security_inode_init_security_anon(inode,
> +			&QSTR(MEMFD_ANON_NAME), NULL);
> +	if (err) {
> +		fput(file);
> +		file = ERR_PTR(err);
> +		return file;
> +	}
> +
>  	file->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
>  	file->f_flags |= O_LARGEFILE;
>  
>  	if (flags & MFD_NOEXEC_SEAL) {
> -		struct inode *inode = file_inode(file);
> -
>  		inode->i_mode &= ~0111;
>  		file_seals = memfd_file_seals_ptr(file);
>  		if (file_seals) {

Hugh, Baolin, and shmem/mm folks, are you okay with the changes above? If
so it would be nice to get an ACK from one of you.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874bf7d..429b2269b35a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -93,6 +93,7 @@
>  #include <linux/fanotify.h>
>  #include <linux/io_uring/cmd.h>
>  #include <uapi/linux/lsm.h>
> +#include <linux/memfd.h>
>  
>  #include "avc.h"
>  #include "objsec.h"
> @@ -2366,9 +2367,12 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
>  	ad.type = LSM_AUDIT_DATA_FILE;
>  	ad.u.file = bprm->file;
>  
> +	if (isec->sclass != SECCLASS_FILE && isec->sclass != SECCLASS_MEMFD_FILE)
> +		return -EPERM;

In the interest of failing fast, this should probably be moved up in the
function to just after where @isec is set.  There are also a number of
checks that happen prior to this placement, but after the isec assignment.
While I don't think any of those checks should be an issue, I'd rather
not to have to worry about those and just fail the non-FILE/MEMFD_FILE
case as soon as we can in selinux_bprm_creds_for_exec().

>  	if (new_tsec->sid == old_tsec->sid) {
> -		rc = avc_has_perm(old_tsec->sid, isec->sid,
> -				  SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
> +		rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
> +				  FILE__EXECUTE_NO_TRANS, &ad);
>  		if (rc)
>  			return rc;
>  	} else {
> @@ -2378,8 +2382,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
>  		if (rc)
>  			return rc;
>  
> -		rc = avc_has_perm(new_tsec->sid, isec->sid,
> -				  SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
> +		rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
> +				  FILE__ENTRYPOINT, &ad);
>  		if (rc)
>  			return rc;
>  
> @@ -2974,10 +2978,18 @@ static int selinux_inode_init_security_anon(struct inode *inode,
>  	struct common_audit_data ad;
>  	struct inode_security_struct *isec;
>  	int rc;
> +	bool is_memfd = false;
>  
>  	if (unlikely(!selinux_initialized()))
>  		return 0;
>  
> +	if (name != NULL && name->name != NULL &&
> +	    !strcmp(name->name, MEMFD_ANON_NAME)) {
> +		if (!selinux_policycap_memfd_class())
> +			return 0;
> +		is_memfd = true;
> +	}
> +
>  	isec = selinux_inode(inode);
>  
>  	/*
> @@ -2996,6 +3008,13 @@ static int selinux_inode_init_security_anon(struct inode *inode,
>  
>  		isec->sclass = context_isec->sclass;
>  		isec->sid = context_isec->sid;
> +	} else if (is_memfd) {
> +		isec->sclass = SECCLASS_MEMFD_FILE;
> +		rc = security_transition_sid(
> +			sid, sid,
> +			isec->sclass, name, &isec->sid);
> +		if (rc)
> +			return rc;
>  	} else {
>  		isec->sclass = SECCLASS_ANON_INODE;
>  		rc = security_transition_sid(

We're duplicating the security_transition_sid() call which seems less
than ideal, how about something like this?

  if (context_inode) {
    /* ... existing stuff ... */
  } else {
    if (is_memfd)
      isec->sclass = SECCLASS_MEMFD_FILE;
    else
      isec->sclass = SECCLASS_ANON_INODE;
    rc = security_transition_sid(...);
    if (rc)
      return rc;
  }

--
paul-moore.com