On Wed, 3 Sep 2025, Paul Moore wrote: > On Aug 25, 2025 "=?UTF-8?q?Thi=C3=A9baud=20Weksteen?=" 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 > > Acked-by: Stephen Smalley > > Tested-by: Stephen Smalley > > --- > > 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 > > > > +#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 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 > > #include > > #include > > +#include > > > > #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