linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Smalley <stephen.smalley.work@gmail.com>
To: "Thiébaud Weksteen" <tweek@google.com>
Cc: Paul Moore <paul@paul-moore.com>,
	James Morris <jmorris@namei.org>,
	 Hugh Dickins <hughd@google.com>,
	Jeff Vander Stoep <jeffv@google.com>,
	Nick Kralevich <nnk@google.com>,  Jeff Xu <jeffxu@google.com>,
	linux-kernel@vger.kernel.org,
	 linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	 linux-mm@kvack.org
Subject: Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
Date: Thu, 28 Aug 2025 09:29:48 -0400	[thread overview]
Message-ID: <CAEjxPJ70O5SY=XYJKrQDLkHOO3spD4VSjYCv0LkhYKCvK=GP7Q@mail.gmail.com> (raw)
In-Reply-To: <CAEjxPJ6G2iK9Yp8eCwbwHQfF1J3WBEVU42kAMQHNuuC_H5QHNw@mail.gmail.com>

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


  reply	other threads:[~2025-08-28 13:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-26  3:18 Thiébaud Weksteen
2025-08-27 13:23 ` Stephen Smalley
2025-08-28 13:29   ` Stephen Smalley [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEjxPJ70O5SY=XYJKrQDLkHOO3spD4VSjYCv0LkhYKCvK=GP7Q@mail.gmail.com' \
    --to=stephen.smalley.work@gmail.com \
    --cc=hughd@google.com \
    --cc=jeffv@google.com \
    --cc=jeffxu@google.com \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=nnk@google.com \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    --cc=tweek@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox