linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] memfd,selinux: call security_inode_init_security_anon
@ 2025-08-26  3:18 Thiébaud Weksteen
  2025-08-27 13:23 ` Stephen Smalley
  2025-09-03 21:26 ` Paul Moore
  0 siblings, 2 replies; 11+ messages in thread
From: Thiébaud Weksteen @ 2025-08-26  3:18 UTC (permalink / raw)
  To: Paul Moore, James Morris, Stephen Smalley, Hugh Dickins,
	Jeff Vander Stoep, Nick Kralevich, Jeff Xu
  Cc: Thiébaud Weksteen, linux-kernel, linux-security-module,
	selinux, linux-mm

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>
---
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) {
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;
+
 	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(
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 5665aa5e7853..3ec85142771f 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -179,6 +179,8 @@ const struct security_class_mapping secclass_map[] = {
 	{ "anon_inode", { COMMON_FILE_PERMS, NULL } },
 	{ "io_uring", { "override_creds", "sqpoll", "cmd", "allowed", NULL } },
 	{ "user_namespace", { "create", NULL } },
+	{ "memfd_file",
+	  { COMMON_FILE_PERMS, "execute_no_trans", "entrypoint", NULL } },
 	/* last one */ { NULL, {} }
 };
 
diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h
index 7405154e6c42..dabcc9f14dde 100644
--- a/security/selinux/include/policycap.h
+++ b/security/selinux/include/policycap.h
@@ -17,6 +17,7 @@ enum {
 	POLICYDB_CAP_NETLINK_XPERM,
 	POLICYDB_CAP_NETIF_WILDCARD,
 	POLICYDB_CAP_GENFS_SECLABEL_WILDCARD,
+	POLICYDB_CAP_MEMFD_CLASS,
 	__POLICYDB_CAP_MAX
 };
 #define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1)
diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h
index d8962fcf2ff9..8e96f2a816b6 100644
--- a/security/selinux/include/policycap_names.h
+++ b/security/selinux/include/policycap_names.h
@@ -20,6 +20,7 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = {
 	"netlink_xperm",
 	"netif_wildcard",
 	"genfs_seclabel_wildcard",
+	"memfd_class",
 };
 /* clang-format on */
 
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 8201e6a3ac0f..72c963f54148 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -209,6 +209,11 @@ static inline bool selinux_policycap_netif_wildcard(void)
 		selinux_state.policycap[POLICYDB_CAP_NETIF_WILDCARD]);
 }
 
+static inline bool selinux_policycap_memfd_class(void)
+{
+	return READ_ONCE(selinux_state.policycap[POLICYDB_CAP_MEMFD_CLASS]);
+}
+
 struct selinux_policy_convert_data;
 
 struct selinux_load_state {
-- 
2.51.0.261.g7ce5a0a67e-goog



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-08-26  3:18 [PATCH] memfd,selinux: call security_inode_init_security_anon Thiébaud Weksteen
@ 2025-08-27 13:23 ` Stephen Smalley
  2025-08-28 13:29   ` Stephen Smalley
  2025-09-03 16:56   ` Stephen Smalley
  2025-09-03 21:26 ` Paul Moore
  1 sibling, 2 replies; 11+ messages in thread
From: Stephen Smalley @ 2025-08-27 13:23 UTC (permalink / raw)
  To: Thiébaud Weksteen
  Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
	Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
	selinux, linux-mm

On Mon, Aug 25, 2025 at 11:18 PM Thiébaud Weksteen <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>

This looks good to me, but do you have a test for it, preferably via
patch for the selinux-testsuite?
See https://github.com/SELinuxProject/selinux-testsuite/commit/023b79b8319e5fe222fb5af892c579593e1cbc50
for an example.

Otherwise, you can add my:
Acked-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) {
> 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;
> +
>         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(
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 5665aa5e7853..3ec85142771f 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -179,6 +179,8 @@ const struct security_class_mapping secclass_map[] = {
>         { "anon_inode", { COMMON_FILE_PERMS, NULL } },
>         { "io_uring", { "override_creds", "sqpoll", "cmd", "allowed", NULL } },
>         { "user_namespace", { "create", NULL } },
> +       { "memfd_file",
> +         { COMMON_FILE_PERMS, "execute_no_trans", "entrypoint", NULL } },
>         /* last one */ { NULL, {} }
>  };
>
> diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h
> index 7405154e6c42..dabcc9f14dde 100644
> --- a/security/selinux/include/policycap.h
> +++ b/security/selinux/include/policycap.h
> @@ -17,6 +17,7 @@ enum {
>         POLICYDB_CAP_NETLINK_XPERM,
>         POLICYDB_CAP_NETIF_WILDCARD,
>         POLICYDB_CAP_GENFS_SECLABEL_WILDCARD,
> +       POLICYDB_CAP_MEMFD_CLASS,
>         __POLICYDB_CAP_MAX
>  };
>  #define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1)
> diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h
> index d8962fcf2ff9..8e96f2a816b6 100644
> --- a/security/selinux/include/policycap_names.h
> +++ b/security/selinux/include/policycap_names.h
> @@ -20,6 +20,7 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = {
>         "netlink_xperm",
>         "netif_wildcard",
>         "genfs_seclabel_wildcard",
> +       "memfd_class",
>  };
>  /* clang-format on */
>
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 8201e6a3ac0f..72c963f54148 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -209,6 +209,11 @@ static inline bool selinux_policycap_netif_wildcard(void)
>                 selinux_state.policycap[POLICYDB_CAP_NETIF_WILDCARD]);
>  }
>
> +static inline bool selinux_policycap_memfd_class(void)
> +{
> +       return READ_ONCE(selinux_state.policycap[POLICYDB_CAP_MEMFD_CLASS]);
> +}
> +
>  struct selinux_policy_convert_data;
>
>  struct selinux_load_state {
> --
> 2.51.0.261.g7ce5a0a67e-goog
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-08-27 13:23 ` Stephen Smalley
@ 2025-08-28 13:29   ` Stephen Smalley
  2025-08-29  3:17     ` Thiébaud Weksteen
  2025-08-29 10:56     ` Paul Moore
  2025-09-03 16:56   ` Stephen Smalley
  1 sibling, 2 replies; 11+ messages in thread
From: Stephen Smalley @ 2025-08-28 13:29 UTC (permalink / raw)
  To: Thiébaud Weksteen
  Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
	Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
	selinux, linux-mm

On Wed, Aug 27, 2025 at 9:23 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Aug 25, 2025 at 11:18 PM Thiébaud Weksteen <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>
>
> This looks good to me, but do you have a test for it, preferably via
> patch for the selinux-testsuite?
> See https://github.com/SELinuxProject/selinux-testsuite/commit/023b79b8319e5fe222fb5af892c579593e1cbc50
> for an example.
>
> Otherwise, you can add my:
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Also, we'll need a corresponding patch to define the new policy
capability in libsepol, and will need to de-conflict with the other
pending patches that are also trying to claim the next available
policy capability bit (so you may end up with a different one
upstream).

>
> > ---
> > 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) {
> > 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;
> > +
> >         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(
> > diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> > index 5665aa5e7853..3ec85142771f 100644
> > --- a/security/selinux/include/classmap.h
> > +++ b/security/selinux/include/classmap.h
> > @@ -179,6 +179,8 @@ const struct security_class_mapping secclass_map[] = {
> >         { "anon_inode", { COMMON_FILE_PERMS, NULL } },
> >         { "io_uring", { "override_creds", "sqpoll", "cmd", "allowed", NULL } },
> >         { "user_namespace", { "create", NULL } },
> > +       { "memfd_file",
> > +         { COMMON_FILE_PERMS, "execute_no_trans", "entrypoint", NULL } },
> >         /* last one */ { NULL, {} }
> >  };
> >
> > diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h
> > index 7405154e6c42..dabcc9f14dde 100644
> > --- a/security/selinux/include/policycap.h
> > +++ b/security/selinux/include/policycap.h
> > @@ -17,6 +17,7 @@ enum {
> >         POLICYDB_CAP_NETLINK_XPERM,
> >         POLICYDB_CAP_NETIF_WILDCARD,
> >         POLICYDB_CAP_GENFS_SECLABEL_WILDCARD,
> > +       POLICYDB_CAP_MEMFD_CLASS,
> >         __POLICYDB_CAP_MAX
> >  };
> >  #define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1)
> > diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h
> > index d8962fcf2ff9..8e96f2a816b6 100644
> > --- a/security/selinux/include/policycap_names.h
> > +++ b/security/selinux/include/policycap_names.h
> > @@ -20,6 +20,7 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = {
> >         "netlink_xperm",
> >         "netif_wildcard",
> >         "genfs_seclabel_wildcard",
> > +       "memfd_class",
> >  };
> >  /* clang-format on */
> >
> > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> > index 8201e6a3ac0f..72c963f54148 100644
> > --- a/security/selinux/include/security.h
> > +++ b/security/selinux/include/security.h
> > @@ -209,6 +209,11 @@ static inline bool selinux_policycap_netif_wildcard(void)
> >                 selinux_state.policycap[POLICYDB_CAP_NETIF_WILDCARD]);
> >  }
> >
> > +static inline bool selinux_policycap_memfd_class(void)
> > +{
> > +       return READ_ONCE(selinux_state.policycap[POLICYDB_CAP_MEMFD_CLASS]);
> > +}
> > +
> >  struct selinux_policy_convert_data;
> >
> >  struct selinux_load_state {
> > --
> > 2.51.0.261.g7ce5a0a67e-goog
> >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-08-28 13:29   ` Stephen Smalley
@ 2025-08-29  3:17     ` Thiébaud Weksteen
  2025-08-29 10:56     ` Paul Moore
  1 sibling, 0 replies; 11+ messages in thread
From: Thiébaud Weksteen @ 2025-08-29  3:17 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
	Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
	selinux, linux-mm

On Thu, Aug 28, 2025 at 11:30 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Aug 27, 2025 at 9:23 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Mon, Aug 25, 2025 at 11:18 PM Thiébaud Weksteen <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>
> >
> > This looks good to me, but do you have a test for it, preferably via
> > patch for the selinux-testsuite?
> > See https://github.com/SELinuxProject/selinux-testsuite/commit/023b79b8319e5fe222fb5af892c579593e1cbc50
> > for an example.

Not yet, I only tested internally on Android. Let me get a change
ready for selinux-testsuite.

> >
> > Otherwise, you can add my:
> > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Thanks for the review!

>
> Also, we'll need a corresponding patch to define the new policy
> capability in libsepol, and will need to de-conflict with the other
> pending patches that are also trying to claim the next available
> policy capability bit (so you may end up with a different one
> upstream).

Ack. Thanks for the heads-up. Happy to rebase the commit if that helps.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-08-28 13:29   ` Stephen Smalley
  2025-08-29  3:17     ` Thiébaud Weksteen
@ 2025-08-29 10:56     ` Paul Moore
  1 sibling, 0 replies; 11+ messages in thread
From: Paul Moore @ 2025-08-29 10:56 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Thiébaud Weksteen, James Morris, Hugh Dickins,
	Jeff Vander Stoep, Nick Kralevich, Jeff Xu, linux-kernel,
	linux-security-module, selinux, linux-mm

On Thu, Aug 28, 2025 at 9:30 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Wed, Aug 27, 2025 at 9:23 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > On Mon, Aug 25, 2025 at 11:18 PM Thiébaud Weksteen <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 ...
>
> Also, we'll need a corresponding patch to define the new policy
> capability in libsepol, and will need to de-conflict with the other
> pending patches that are also trying to claim the next available
> policy capability bit (so you may end up with a different one
> upstream).

My apologies for the late reply, I have limited network access this
week and haven't yet been able to give this a proper review, but I
expect things to get back to normal next week.  That said, Stephen's
comments about a test suite addition are important, and I would like
to see a test addition before merging this code both to ensure this
works on a wider range of SELinux based systems beyond Android (you
should also test this on something other than Android, e.g. a modern
Fedora system) and to provide a reliable test that we can use to test
for regressions in the future.

As far as the policy capability bit offset is concerned, don't worry
too much about that right now.  Allocated magic numbers like the
policy capability bits are never really fixed until they land in an
upstream tree (technically not until they land in a proper tagged
release from Linus); if/when a patch is merged that requires a new
capability bit I simply assign it the next unused offset at the time
the patch is merged.  Other approaches either end up potentially
creating holes in the capability bitmap (yuck) or creating merge
dependencies between otherwise independent pact{sets} (extra double
yuck).

-- 
paul-moore.com


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-08-27 13:23 ` Stephen Smalley
  2025-08-28 13:29   ` Stephen Smalley
@ 2025-09-03 16:56   ` Stephen Smalley
  1 sibling, 0 replies; 11+ messages in thread
From: Stephen Smalley @ 2025-09-03 16:56 UTC (permalink / raw)
  To: Thiébaud Weksteen
  Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
	Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
	selinux, linux-mm

On Wed, Aug 27, 2025 at 9:23 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Aug 25, 2025 at 11:18 PM Thiébaud Weksteen <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>
>
> This looks good to me, but do you have a test for it, preferably via
> patch for the selinux-testsuite?
> See https://github.com/SELinuxProject/selinux-testsuite/commit/023b79b8319e5fe222fb5af892c579593e1cbc50
> for an example.
>
> Otherwise, you can add my:
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

And now having run the tests posted in:
    https://lore.kernel.org/selinux/20250902055401.618729-1-tweek@google.com/
you can also add my:
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-08-26  3:18 [PATCH] memfd,selinux: call security_inode_init_security_anon Thiébaud Weksteen
  2025-08-27 13:23 ` Stephen Smalley
@ 2025-09-03 21:26 ` Paul Moore
  2025-09-16  5:06   ` Hugh Dickins
  1 sibling, 1 reply; 11+ messages in thread
From: Paul Moore @ 2025-09-03 21:26 UTC (permalink / raw)
  To: Thiébaud Weksteen, James Morris, Stephen Smalley,
	Hugh Dickins, Jeff Vander Stoep, Nick Kralevich, Jeff Xu,
	Baolin Wang
  Cc: Thiébaud Weksteen, linux-kernel, linux-security-module,
	selinux, linux-mm

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-09-03 21:26 ` Paul Moore
@ 2025-09-16  5:06   ` Hugh Dickins
  2025-09-16 15:26     ` Paul Moore
  0 siblings, 1 reply; 11+ messages in thread
From: Hugh Dickins @ 2025-09-16  5:06 UTC (permalink / raw)
  To: Paul Moore
  Cc: Thiébaud Weksteen, James Morris, Stephen Smalley,
	Hugh Dickins, Jeff Vander Stoep, Nick Kralevich, Jeff Xu,
	Baolin Wang, linux-kernel, linux-security-module, selinux,
	linux-mm

[-- Attachment #1: Type: text/plain, Size: 8087 bytes --]

On Wed, 3 Sep 2025, Paul Moore wrote:
> 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.

So far as I can tell, seems okay to me:
Acked-by: Hugh Dickins <hughd@google.com>

If I'd responded earlier (sorry), I would have asked for it just to use
&QSTR("[memfd]") directly in the call, rather than indirecting through
unnecessary #define MEMFD_ANON_NAME "[memfd]"; never mind, that's all.

Please do take this, along with the rest, through your security tree:
mm.git contains no conflicting change to mm/memfd.c at present.

Thanks,
Hugh

> 
> > 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-09-16  5:06   ` Hugh Dickins
@ 2025-09-16 15:26     ` Paul Moore
  2025-09-17  0:34       ` Thiébaud Weksteen
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Moore @ 2025-09-16 15:26 UTC (permalink / raw)
  To: Hugh Dickins, Thiébaud Weksteen
  Cc: James Morris, Stephen Smalley, Jeff Vander Stoep, Nick Kralevich,
	Jeff Xu, Baolin Wang, linux-kernel, linux-security-module,
	selinux, linux-mm

On Tue, Sep 16, 2025 at 1:07 AM Hugh Dickins <hughd@google.com> wrote:
> On Wed, 3 Sep 2025, Paul Moore wrote:
> > 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>

...

> > 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.
>
> So far as I can tell, seems okay to me:
> Acked-by: Hugh Dickins <hughd@google.com>
>
> If I'd responded earlier (sorry), I would have asked for it just to use
> &QSTR("[memfd]") directly in the call, rather than indirecting through
> unnecessary #define MEMFD_ANON_NAME "[memfd]"; never mind, that's all.
>
> Please do take this, along with the rest, through your security tree:
> mm.git contains no conflicting change to mm/memfd.c at present.

Thanks Hugh, it turns out we ended up having a discussion on the
SELinux side (proper return values for error conditions) and I'm going
to hold off on this until after the upcoming merge window to give time
for that discussion to run its course.  The good news is that gives
Thiébaud an opportunity to do the qstr fixup you wanted.

Thiébaud, are you okay with making the change Hugh has requested?

-- 
paul-moore.com


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-09-16 15:26     ` Paul Moore
@ 2025-09-17  0:34       ` Thiébaud Weksteen
  2025-09-17  1:08         ` Hugh Dickins
  0 siblings, 1 reply; 11+ messages in thread
From: Thiébaud Weksteen @ 2025-09-17  0:34 UTC (permalink / raw)
  To: Paul Moore, Hugh Dickins
  Cc: James Morris, Stephen Smalley, Jeff Vander Stoep, Nick Kralevich,
	Jeff Xu, Baolin Wang, linux-kernel, linux-security-module,
	selinux, linux-mm

On Wed, Sep 17, 2025 at 1:26 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Sep 16, 2025 at 1:07 AM Hugh Dickins <hughd@google.com> wrote:
> > On Wed, 3 Sep 2025, Paul Moore wrote:
> > > 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>
>
> ...
>
> > > 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.
> >
> > So far as I can tell, seems okay to me:
> > Acked-by: Hugh Dickins <hughd@google.com>
> >
> > If I'd responded earlier (sorry), I would have asked for it just to use
> > &QSTR("[memfd]") directly in the call, rather than indirecting through
> > unnecessary #define MEMFD_ANON_NAME "[memfd]"; never mind, that's all.
> >

Thanks for the review Hugh. In our case, it is necessary to expose
MEMFD_ANON_NAME as there is a string comparison done in
security/selinux/hooks.c (see selinux_inode_init_security_anon
changes).
I would argue it is cleaner to reference the same constant. The
alternative here would be to have 2 copies of it, with the risk of
them being out-of-sync at some point.

> > Please do take this, along with the rest, through your security tree:
> > mm.git contains no conflicting change to mm/memfd.c at present.
>
> Thanks Hugh, it turns out we ended up having a discussion on the
> SELinux side (proper return values for error conditions) and I'm going
> to hold off on this until after the upcoming merge window to give time
> for that discussion to run its course.  The good news is that gives
> Thiébaud an opportunity to do the qstr fixup you wanted.
>
> Thiébaud, are you okay with making the change Hugh has requested?
>
> --
> paul-moore.com


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
  2025-09-17  0:34       ` Thiébaud Weksteen
@ 2025-09-17  1:08         ` Hugh Dickins
  0 siblings, 0 replies; 11+ messages in thread
From: Hugh Dickins @ 2025-09-17  1:08 UTC (permalink / raw)
  To: Thiébaud Weksteen
  Cc: Paul Moore, Hugh Dickins, James Morris, Stephen Smalley,
	Jeff Vander Stoep, Nick Kralevich, Jeff Xu, Baolin Wang,
	linux-kernel, linux-security-module, selinux, linux-mm

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

On Wed, 17 Sep 2025, Thiébaud Weksteen wrote:
> On Wed, Sep 17, 2025 at 1:26 AM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > So far as I can tell, seems okay to me:
> > > Acked-by: Hugh Dickins <hughd@google.com>
> > >
> > > If I'd responded earlier (sorry), I would have asked for it just to use
> > > &QSTR("[memfd]") directly in the call, rather than indirecting through
> > > unnecessary #define MEMFD_ANON_NAME "[memfd]"; never mind, that's all.
> > >
> 
> Thanks for the review Hugh. In our case, it is necessary to expose
> MEMFD_ANON_NAME as there is a string comparison done in
> security/selinux/hooks.c (see selinux_inode_init_security_anon
> changes).
> I would argue it is cleaner to reference the same constant. The
> alternative here would be to have 2 copies of it, with the risk of
> them being out-of-sync at some point.

Oh, I'm sorry, I simply misread your patch, and thought that the
#define MEMFD_ANON_NAME "[memfd]" came at the start of mm/memfd.c,
whereas it's in include/linux/memfd.h just before mm/memfd.c.

Yes, you're perfectly correct to do it that way then, ignore me.
(I do have to reflect on why three instances of MEMFD_ANON_NAME
are safer than two instances of "[memfd]", but you are correct.)

That does lead me to look quickly at the security/selinux/hooks.c
end of the patch: I don't particularly love what I see there, but
that's none of my business, you and Paul have constraints to meet
there which I'm entirely unfamiliar with.

Hugh

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-09-17  1:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26  3:18 [PATCH] memfd,selinux: call security_inode_init_security_anon Thiébaud Weksteen
2025-08-27 13:23 ` Stephen Smalley
2025-08-28 13:29   ` Stephen Smalley
2025-08-29  3:17     ` Thiébaud Weksteen
2025-08-29 10:56     ` Paul Moore
2025-09-03 16:56   ` Stephen Smalley
2025-09-03 21:26 ` Paul Moore
2025-09-16  5:06   ` Hugh Dickins
2025-09-16 15:26     ` Paul Moore
2025-09-17  0:34       ` Thiébaud Weksteen
2025-09-17  1:08         ` Hugh Dickins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).