[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEjxPJ5q0eriGjo1tdfN+pzBBN5OeyfMaYp_sNQcOg-rDaXVCA@mail.gmail.com>
Date: Mon, 8 Sep 2025 12:32:28 -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>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
Isaac Manjarres <isaacmanjarres@...gle.com>, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [PATCH v2] memfd,selinux: call security_inode_init_security_anon
On Sun, Sep 7, 2025 at 9:34 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>
> Acked-by: Stephen Smalley <stephen.smalley.work@...il.com>
> Tested-by: Stephen Smalley <stephen.smalley.work@...il.com>
> ---
> Changes since v1:
> - Move test of class earlier in selinux_bprm_creds_for_exec
> - Remove duplicate call to security_transition_sid
>
> 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 | 26 +++++++++++++++++-----
> 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, 44 insertions(+), 7 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874bf7d..6adf2f393ed9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2315,6 +2316,9 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> new_tsec = selinux_cred(bprm->cred);
> isec = inode_security(inode);
>
> + if (isec->sclass != SECCLASS_FILE && isec->sclass != SECCLASS_MEMFD_FILE)
> + return -EPERM;
> +
Sorry, I should have mentioned this earlier, but usually we try to
avoid triggering silent denials from SELinux since it provides no hint
to the user as to what went wrong or how to resolve.
Arguably reaching this code would be suggestive of a kernel bug but I
know that BUG_ON() is frowned upon these days.
Maybe we should WARN_ON_ONCE() here or similar? We also rarely return
-EPERM from SELinux outside of capability checks since usually EPERM
means a failed capability check
(vs -EACCES). Defer to Paul on how/if he wants to handle this and
whether it requires re-spinning this patch or just a follow-on one.
> /* Default to the current task SID. */
> new_tsec->sid = old_tsec->sid;
> new_tsec->osid = old_tsec->sid;
Powered by blists - more mailing lists