[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEjxPJ6G2iK9Yp8eCwbwHQfF1J3WBEVU42kAMQHNuuC_H5QHNw@mail.gmail.com>
Date: Wed, 27 Aug 2025 09:23:47 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: Thiébaud Weksteen <tweek@...gle.com>
Cc: Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
Hugh Dickins <hughd@...gle.com>, Jeff Vander Stoep <jeffv@...gle.com>, Nick Kralevich <nnk@...gle.com>,
Jeff Xu <jeffxu@...gle.com>, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
On Mon, Aug 25, 2025 at 11:18 PM Thiébaud Weksteen <tweek@...gle.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@...gle.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@...il.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
>
Powered by blists - more mailing lists