linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: Abhinav Saxena <xandfury@gmail.com>, Paul Moore <paul@paul-moore.com>
Cc: "Fan Wu" <wufan@kernel.org>, "Günther Noack" <gnoack@google.com>,
	"James Morris" <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <nick.desaulniers+lkml@gmail.com>,
	"Bill Wendling" <morbo@google.com>,
	"Justin Stitt" <justinstitt@google.com>,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	llvm@lists.linux.dev, "Daniel Verkamp" <dverkamp@chromium.org>,
	"Jeff Xu" <jeffxu@chromium.org>
Subject: Re: [PATCH RFC 2/4] landlock: implement memfd detection
Date: Thu, 18 Sep 2025 16:26:38 +0200	[thread overview]
Message-ID: <20250918.eXaiF5Ejoing@digikod.net> (raw)
In-Reply-To: <20250918.kieghied1ahS@digikod.net>

On Thu, Sep 18, 2025 at 04:07:48PM +0200, Mickaël Salaün wrote:
> 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

Hmm, we cannot use hook_file_alloc_security() to enforce restriction on
memfd creation because the inode mode is set after this call by
memfd.c:alloc_file().  So we should indeed add a new LSM hook.  That
would also improve security checks on file creation for other file
types.

> > `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
> 

  reply	other threads:[~2025-09-18 14:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-19 11:13 [PATCH RFC 0/4] landlock: add LANDLOCK_SCOPE_MEMFD_EXEC execution Abhinav Saxena
2025-07-19 11:13 ` [PATCH RFC 1/4] landlock: add LANDLOCK_SCOPE_MEMFD_EXEC scope Abhinav Saxena
2025-07-19 11:13 ` [PATCH RFC 2/4] landlock: implement memfd detection Abhinav Saxena
2025-07-20  7:32   ` Fan Wu
2025-07-22 21:56     ` Abhinav Saxena
2025-09-18 14:07       ` Mickaël Salaün
2025-09-18 14:26         ` Mickaël Salaün [this message]
2025-07-19 11:13 ` [PATCH RFC 3/4] landlock: add memfd exec LSM hooks and scoping Abhinav Saxena
2025-07-19 11:13 ` [PATCH RFC 4/4] selftests/landlock: add memfd execution tests Abhinav Saxena
2025-09-18 14:07 ` [PATCH RFC 0/4] landlock: add LANDLOCK_SCOPE_MEMFD_EXEC execution Mickaël Salaün
2025-09-28 23:37   ` Abhinav Saxena
2025-11-20 15:42     ` Mickaël Salaün

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250918.eXaiF5Ejoing@digikod.net \
    --to=mic@digikod.net \
    --cc=dverkamp@chromium.org \
    --cc=gnoack@google.com \
    --cc=jeffxu@chromium.org \
    --cc=jmorris@namei.org \
    --cc=justinstitt@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=paul@paul-moore.com \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=wufan@kernel.org \
    --cc=xandfury@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).