Re: [PATCH RFC 2/4] landlock: implement memfd detection

["Mickaël Salaün" <mic@digikod.net>]

On Tue, Jul 22, 2025 at 03:56:38PM -0600, Abhinav Saxena wrote:
> Fan Wu <wufan@kernel.org> writes:
> 
> > On Sat, Jul 19, 2025 at 4:13 AM Abhinav Saxena <xandfury@gmail.com> wrote:
> >>
> >> Add is_memfd_file() function to reliably detect memfd files by checking
> >> for “memfd:” prefix in dentry names on shmem-backed files. This
> >> distinguishes true memfd files from regular shmem files.
> >>
> >> Move domain_is_scoped() to domain.c for reuse across subsystems.
> >> Add comprehensive kunit tests for memfd detection edge cases.
> >>
> >> Signed-off-by: Abhinav Saxena <xandfury@gmail.com>
> >> —
> >>  security/landlock/domain.c |  67 +++++++++++++++
> >>  security/landlock/domain.h |   4 +
> >>  security/landlock/fs.c     | 210 +++++++++++++++++++++++++++++++++++++++++++++
> >>  security/landlock/task.c   |  67 —————
> >>  4 files changed, 281 insertions(+), 67 deletions(-)
> >
> > …
> >
> >>
> >> +/**
> >> + * is_memfd_file - Check if file was created via memfd_create()
> >> + * @file: File to check
> >> + *
> >> + * Returns true if @file was created via memfd_create(), false otherwise.
> >> + *
> >> + * memfd files are shmem-backed files with “memfd:” prefix in their dentry name.
> >> + * This is the definitive way to distinguish memfd files from regular shmem
> >> + * files.
> >> + */
> >> +static bool is_memfd_file(struct file *file)
> >> +{
> >> +       const struct dentry *dentry;
> >> +       const unsigned char *name;
> >> +       size_t name_len;
> >> +
> >> +       /* Fast path: basic validation */
> >> +       if (unlikely(!file))
> >> +               return false;
> >> +
> >> +       /* Must be shmem-backed first - this is the cheapest definitive check */
> >> +       if (!shmem_file(file))
> >> +               return false;

In which case a memfd would not be detected by this function?
shmem_file_operations or hugetlbfs_file_operations are always set for
memfd right?

We could export memfd_get_seals() to stay consistent over time.

> >> +
> >> +#ifdef CONFIG_MEMFD_CREATE
> >> +
> >> +       /* Validate dentry and get name info */
> >> +       dentry = file->f_path.dentry;
> >> +       if (unlikely(!dentry))
> >> +               return false;
> >> +
> >> +       name_len = dentry->d_name.len;
> >> +       name = dentry->d_name.name;
> >> +
> >> +       /* memfd files always have “memfd:” prefix (6 characters) */
> >> +       if (name_len < 6 || unlikely(!name))
> >> +               return false;
> >> +
> >> +       /* Check for exact “memfd:” prefix */
> >> +       return memcmp(name, “memfd:”, 6) == 0;
> >> +#else
> >> +       return false;
> >> +#endif
> >
> > I was trying to do something similar early this year but didn’t hear
> > feedback from the linux-mm folks.
> > <https://lore.kernel.org/linux-security-module/20250129203932.22165-1-wufan@kernel.org/>
> >
> > I have considered this approach but didn’t use it. My concern is,
> > potentially a malicious user can create a file in a shmem fs, e.g.
> > tmpfs , with the “memfd:” prefix, which can be used to bypass security
> > policy.

In the case of Landlock that should not be a security issue but a
compatibility issue, which is not better.

> > (Resending this message due to a misconfiguration with my email
> > client. Apologies for any inconvenience.)
> >
> > -Fan
> 
> Hi Fan,
> 
> Thanks for your comments.
> 
> I agree that an LSM hook into memfd_create() would be a much better
> solution. In the absence of such a function, do you think adding a
> `d_unlinked(dentry)` check could serve as an additional verification?
> 
> I say things since I *think* that legitimate memfd files are always
> unlinked while spoofed tmpfs files remain linked. I could be wrong
> though.

We should just use the same checks used by the memfd-related
syscalls/operations to detect such file.

> 
> In any case, we can test this approach using kprobes to validate
> the behavior.
> 
> -Abhinav