Re: [PATCH] memfd,selinux: call security_inode_init_security_anon

["Thiébaud Weksteen" <tweek@google.com>]

On Wed, Sep 17, 2025 at 1:26 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Sep 16, 2025 at 1:07 AM Hugh Dickins <hughd@google.com> wrote:
> > On Wed, 3 Sep 2025, Paul Moore wrote:
> > > On Aug 25, 2025 "=?UTF-8?q?Thi=C3=A9baud=20Weksteen?=" <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>
> > > > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> > > > Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>
> ...
>
> > > 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 <hughd@google.com>
> >
> > 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.
> >

Thanks for the review Hugh. In our case, it is necessary to expose
MEMFD_ANON_NAME as there is a string comparison done in
security/selinux/hooks.c (see selinux_inode_init_security_anon
changes).
I would argue it is cleaner to reference the same constant. The
alternative here would be to have 2 copies of it, with the risk of
them being out-of-sync at some point.

> > 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, it turns out we ended up having a discussion on the
> SELinux side (proper return values for error conditions) and I'm going
> to hold off on this until after the upcoming merge window to give time
> for that discussion to run its course.  The good news is that gives
> Thiébaud an opportunity to do the qstr fixup you wanted.
>
> Thiébaud, are you okay with making the change Hugh has requested?
>
> --
> paul-moore.com