On Mon, Aug 11, 2025 at 11:09 PM Stephen Smalley
[off-list ref] wrote:

On Mon, Aug 11, 2025 at 1:34 AM Thiébaud Weksteen [off-list ref] wrote:

quoted

On Fri, Aug 8, 2025 at 10:41 PM Stephen Smalley
[off-list ref] wrote:

quoted

On Fri, Aug 8, 2025 at 7:57 AM Stephen Smalley
[off-list ref] wrote:

Thanks for the review Stephen.

quoted

quoted

On Thu, Aug 7, 2025 at 3:57 AM Thiébaud Weksteen [off-list ref] wrote:

quoted

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.

To provide backward compatibility, I would anticipate that you will
need to define a new SELinux policy capability and make this change
conditional on it being enabled, see:
https://github.com/SELinuxProject/selinux-kernel/wiki/Getting-Started#adding-a-new-selinux-policy-capability
for instructions and links to examples.

I agree. Thanks, I'll add this to the next patchset.

quoted

quoted

Otherwise, see below.

quoted

[1] https://crbug.com/1305267
[2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/ (local)

Signed-off-by: Thiébaud Weksteen <redacted>
---
 mm/memfd.c                          | 16 ++++++++++++++--
 security/selinux/hooks.c            | 15 +++++++++++----
 security/selinux/include/classmap.h |  2 ++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/mm/memfd.c b/mm/memfd.c
index bbe679895ef6..13bff0e91816 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c

@@ -433,6 +433,9 @@ static struct file *alloc_file(const char *name, unsigned int flags)
 {
        unsigned int *file_seals;
        struct file *file;
+       struct inode *inode;
+       int err = 0;
+       const char *anon_name = "[memfd]";

        if (flags & MFD_HUGETLB) {
                file = hugetlb_file_setup(name, 0, VM_NORESERVE,

@@ -444,12 +447,21 @@ 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,
+                       LSM_ANON_INODE_MEMFD,
+                       &QSTR(anon_name), NULL);

Since the anon_name already indicates that this is a memfd, so can't
you already distinguish these via name-based type_transition rules?
Why do we need the enum argument?

On second thought, I see that you are distinguishing not just the
security context/type but also the security class, but the question
remains: can't you compare the name to make this determination?

I definitely can. My initial thought was that a string comparison
would not be adequate here (performance-wise), but I guess calls to
this method are infrequent enough that a strncmp would work here?

Open to data showing otherwise, but would think that a fixed-size
small string comparison would be in the noise here.

I agree.

quoted

On the option of just relying on name-based type transitions, this has
an impact on the size of the policy: for Android, the transition from
ashmem to memfd means that virtually all domains will rely on such a
file. It means that we would need to define a type transition and a
new type for all our domains. (This is an argument on top of the need
for execute_no_trans, as I described in the commit message).

quoted

quoted

quoted

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8d36d5ebb6e5..49742930e706 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -2367,8 +2367,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
        ad.u.file = bprm->file;

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

Here and below I am a little concerned that we could end up reaching
this code on an inode with an isec->sclass that does not define the
execute_no_trans and entrypoint permissions. We should do something to
make that never happens, or check for it and always deny in that case.

I agree. I can add a condition to make sure only these 2 classes (file
or memfd_file) are used here.

quoted

quoted

quoted

                if (rc)
                        return rc;
        } else {

@@ -2378,8 +2378,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;

@@ -2997,6 +2997,13 @@ static int selinux_inode_init_security_anon(struct inode *inode,

                isec->sclass = context_isec->sclass;
                isec->sid = context_isec->sid;
+       } else if (type == LSM_ANON_INODE_MEMFD) {
+               isec->sclass = SECCLASS_MEMFD_FILE;
+               rc = security_transition_sid(
+                       sid, sid,
+                       isec->sclass, name, &isec->sid);

Again, name-based type_transitions ought to be able to distinguish
memfd based on the name argument IIUC.