* Re: [PATCH v1] security/safesetid: fix comment and error handling
From: Micah Morton @ 2026-03-18 20:49 UTC (permalink / raw)
To: yanwei.gao; +Cc: paul, linux-security-module
In-Reply-To: <20260303024025.37916-1-gaoyanwei.tx@gmail.com>
On Mon, Mar 2, 2026 at 6:40 PM yanwei.gao <gaoyanwei.tx@gmail.com> wrote:
>
> - Fix comment in lsm.c: use CAP_SETGID instead of CAP_SETUID in the
> GID capability check comment to match the actual logic.
> - In handle_policy_update(), set err = -EINVAL and goto out_free_buf
> when policy type is neither UID nor GID, so the error is returned
> to the caller instead of only logging.
> - In safesetid_init_securityfs(), return ret directly when
> policy_dir creation fails instead of goto error (no cleanup needed
> at that point).
>
> Signed-off-by: yanwei.gao <gaoyanwei.tx@gmail.com>
> ---
> security/safesetid/lsm.c | 2 +-
> security/safesetid/securityfs.c | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index d5fb949050dd..a7b68e65996c 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -128,7 +128,7 @@ static int safesetid_security_capable(const struct cred *cred,
> if (setid_policy_lookup((kid_t){.gid = cred->gid}, INVALID_ID, GID) == SIDPOL_DEFAULT)
> return 0;
> /*
> - * Reject use of CAP_SETUID for functionality other than calling
> + * Reject use of CAP_SETGID for functionality other than calling
Looks good.
> * set*gid() (e.g. setting up userns gid mappings).
> */
> pr_warn("Operation requires CAP_SETGID, which is not available to GID %u for operations besides approved set*gid transitions\n",
> diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
> index a71e548065a9..50682abd342b 100644
> --- a/security/safesetid/securityfs.c
> +++ b/security/safesetid/securityfs.c
> @@ -224,6 +224,8 @@ static ssize_t handle_policy_update(struct file *file,
> } else {
> /* Error, policy type is neither UID or GID */
> pr_warn("error: bad policy type");
> + err = -EINVAL;
> + goto out_free_buf;
This makes sense to me and matches how things are done in the
functions above in verify_ruleset() and parse_policy_line().
Is this code actually reachable? I guess if verify_ruleset returns
-EINVAL due to a bad policy type value the code still hits this spot?
> }
> err = len;
>
> @@ -321,7 +323,7 @@ int __init safesetid_init_securityfs(void)
> policy_dir = securityfs_create_dir("safesetid", NULL);
> if (IS_ERR(policy_dir)) {
> ret = PTR_ERR(policy_dir);
> - goto error;
> + return ret;
We still need the `securityfs_remove(policy_dir)` call to clean up the
dir created with `policy_dir = securityfs_create_dir("safesetid",
NULL)` don't we?
> }
>
> uid_policy_file = securityfs_create_file("uid_allowlist_policy", 0600,
> --
> 2.43.5
>
^ permalink raw reply
* Re: [PATCH RFC] security: add LSM blob and hooks for namespaces
From: danieldurning.work @ 2026-03-18 20:17 UTC (permalink / raw)
To: brauner; +Cc: mic, jmorris, linux-kernel, linux-security-module, paul
In-Reply-To: <20260312.eNg0oog8eeHi@digikod.net>
On Thu, Mar 12, 2026, at 11:10:32AM +0100, Mickaël Salaün <mic@digikod.net> wrote:
> On Tue, Feb 17, 2026 at 12:33:28PM +0100, Paul Moore wrote:
> > On February 17, 2026 9:54:42 AM Christian Brauner <brauner@kernel.org> wrote:
> > > On Mon, Feb 16, 2026 at 07:53:11PM +0100, Paul Moore wrote:
> > > > On February 16, 2026 2:52:34 PM Christian Brauner <brauner@kernel.org> wrote:
> > > > > All namespace types now share the same ns_common infrastructure. Extend
> > > > > this to include a security blob so LSMs can start managing namespaces
> > > > > uniformly without having to add one-off hooks or security fields to
> > > > > every individual namespace type.
> > > > >
> > > > > Add a ns_security pointer to ns_common and the corresponding lbs_ns
> > > > > blob size to lsm_blob_sizes. Allocation and freeing hooks are called
> > > > > from the common __ns_common_init() and __ns_common_free() paths so
> > > > > every namespace type gets covered in one go. All information about the
> > > > > namespace type and the appropriate casting helpers to get at the
> > > > > containing namespace are available via ns_common making it
> > > > > straightforward for LSMs to differentiate when they need to.
> > > > >
> > > > > A namespace_install hook is called from validate_ns() during setns(2)
> > > > > giving LSMs a chance to enforce policy on namespace transitions.
> > > > >
> > > > > Individual namespace types can still have their own specialized security
> > > > > hooks when needed. This is just the common baseline that makes it easy
> > > > > to track and manage namespaces from the security side without requiring
> > > > > every namespace type to reinvent the wheel.
> > > > >
> > > > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > > > ---
> > > > > include/linux/lsm_hook_defs.h | 3 ++
> > > > > include/linux/lsm_hooks.h | 1 +
> > > > > include/linux/ns/ns_common_types.h | 3 ++
> > > > > include/linux/security.h | 20 ++++++++++
> > > > > kernel/nscommon.c | 12 ++++++
> > > > > kernel/nsproxy.c | 8 +++-
> > > > > security/lsm_init.c | 2 +
> > > > > security/security.c | 76 ++++++++++++++++++++++++++++++++++++++
> > > > > 8 files changed, 124 insertions(+), 1 deletion(-)
> > > >
> > > > I still have limited network access for a few more days, but a couple of
> > > > quick comments in no particular order ...
> > > >
> > > > Generally speaking we don't add things to the LSM interface without a user,
> > > > and I can't think of a good reason why we would want to do things
> > > > differently here. This means that when you propose something like this you
> > > > should also propose an addition to one of the in-tree LSMs to make use of
> > > > it. While the guidance doc linked below (also linked in the LSM MAINTAINERS
> > > > entry) doesn't have any guidance for the LSM blobs as they are generally a
> > > > byproduct of the hooks, if you are looking for some general info I think the
> > > > bits on adding a new LSM hook would be very close to what we would expect
> > > > for blob additions.
> > > >
> > > > https://github.com/LinuxSecurityModule/kernel/blob/main/README.md
> > > >
> > > > Getting to the specifics of namespace related APIs, we've had a lot of
> > > > discussions about namespacing and my current opinion is that we need to sort
> > > > out if we want a userspace API at the LSM framework layer, or if we want to
> > > > do that at the individual LSM layer; there is a lot of nuance there and
> > > > while one option may seem like an obvious choice, we need some more
> > > > discussion and I need a chance to get caught up on the threads. Once we have
> > > > an API decision then we can start sorting out the implementation details
> > > > like the LSM blobs.
> > >
> > > I might be misunderstanding you but what you are talking about seems
> > > namespacing the LSM layer itself.
> > >
> > > But I cannot stress enough this is not at all what this patchset is
> > > doing. :)
> >
> > Likely also a misunderstanding on my end as I triage email/patches via phone.
> >
> > Regardless, the guidance in the doc I linked regarding the addition of new
> > LSM hooks would appear to apply here.
>
> FYI, I just sent an RFC to leverage this patch with Landlock:
> https://lore.kernel.org/all/20260312100444.2609563-1-mic@digikod.net/
I was working on an SELinux implementation of these hooks as well.
However, I noticed that when a mount namespace is created as anon
the security blob is allocated but never freed. The free_mnt_ns()
function only calls ns_common_free() if a mount namespace is not
marked as anon. This seems to be causing a memory leak.
^ permalink raw reply
* Re: [PATCH 1/3] backing_file: store user_path_file
From: Paul Moore @ 2026-03-18 20:00 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, linux-security-module, selinux, linux-fsdevel,
linux-unionfs, linux-erofs, Gao Xiang
In-Reply-To: <CAOQ4uxhfvS1SCkp504uDuBmgqSEBYaQDDVAm+JY=w_2fKLbQsQ@mail.gmail.com>
On Wed, Mar 18, 2026 at 8:07 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Mar 18, 2026 at 11:57 AM Christian Brauner <brauner@kernel.org> wrote:
> > On Mon, Mar 16, 2026 at 05:35:56PM -0400, Paul Moore wrote:
> > > From: Amir Goldstein <amir73il@gmail.com>
> > >
> > > Instead of storing the user_path, store an O_PATH file for the
> > > user_path with the original user file creds and a security context.
> > >
> > > The user_path_file is only exported as a const pointer and its refcnt
> > > is initialized to FILE_REF_DEAD, because it is not a refcounted object.
> > >
> > > The only caller of file_ref_init() is now open coded, so the helper
> > > is removed.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > Tested-by: Paul Moore <paul@paul-moore.com> (SELinux)
> > > Acked-by: Gao Xiang <hsiangkao@linux.alibaba.com> (EROFS)
> > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > > ---
> > > fs/backing-file.c | 20 ++++++++------
> > > fs/erofs/ishare.c | 6 ++--
> > > fs/file_table.c | 53 ++++++++++++++++++++++++++++--------
> > > fs/fuse/passthrough.c | 3 +-
> > > fs/internal.h | 5 ++--
> > > fs/overlayfs/dir.c | 3 +-
> > > fs/overlayfs/file.c | 1 +
> > > include/linux/backing-file.h | 29 ++++++++++++++++++--
> > > include/linux/file_ref.h | 10 -------
> > > 9 files changed, 90 insertions(+), 40 deletions(-)
...
> > > diff --git a/fs/file_table.c b/fs/file_table.c
> > > index aaa5faaace1e..b7dc94656c44 100644
> > > --- a/fs/file_table.c
> > > +++ b/fs/file_table.c
> > > @@ -56,24 +57,44 @@ struct backing_file {
> > >
> > > const struct path *backing_file_user_path(const struct file *f)
> > > {
> > > - return &backing_file(f)->user_path;
> > > + return &backing_file(f)->user_path_file.f_path;
> > > }
> > > EXPORT_SYMBOL_GPL(backing_file_user_path);
> > >
> > > -void backing_file_set_user_path(struct file *f, const struct path *path)
> > > +const struct file *backing_file_user_path_file(const struct file *f)
> > > {
> > > - backing_file(f)->user_path = *path;
> > > + return &backing_file(f)->user_path_file;
> > > +}
> > > +EXPORT_SYMBOL_GPL(backing_file_user_path_file);
> > > +
> > > +void backing_file_open_user_path(struct file *f, const struct path *path)
> >
> > I think this is a bad idea. This should return an error but still
> > WARN_ON(). Just make callers handle that error just like we do
> > everywhere else.
>
> OK.
That's good, I can remove the FMODE_OPENED sanity check in
selinux_file_mprotect() now.
> > > +{
> > > + /* open an O_PATH file to reference the user path - cannot fail */
> > > + WARN_ON(vfs_open(path, &backing_file(f)->user_path_file));
> > > +}
> > > +EXPORT_SYMBOL_GPL(backing_file_open_user_path);
> > > +
> > > +static void destroy_file(struct file *f)
> > > +{
> > > + security_file_free(f);
> > > + put_cred(f->f_cred);
> >
> > Note that calling destroy_file() in this way bypasses
> > security_file_release(). Presumably this doesn't matter because no LSM
> > does a security_alloc_file() for this but it adds a nother wrinkly into
> > the cleanup path.
The alloc_empty_backing_file() function will call init_file() on the
O_PATH file which in turnwill call security_file_alloc(). We depend
on that behavior as we need to set the creds properly on the file.
The tricky bit is that security_file_release() is currently only used
by IMA/EVM; it was created when we folded IMA/EVM into the LSM
framework, making them proper LSMs and cleaning up a lot of the hook
calls in the VFS. The commit description provides some info on the
hook:
IMA calculates at file close the new digest of the file content
and writes it to security.ima, so that appraisal at next file
access succeeds.
The new hook cannot return an error and cannot cause the operation
to be reverted.
As O_PATH files never go through the
security_file_open()/security_file_post_open() path, O_PATH files
should be ignored by IMA/EVM (which makes sense, as there is no data
or xattrs to measure, update, etc.). For that reason, skipping
security_file_release() should be okay in this particular case.
Other LSMs which allocate file specific state, e.g. AppArmor, in
security_file_alloc() should free that state in security_file_free()
which is handled in Amir's patch.
> This is for Paul to comment on.
> The way I see it, security_file_open() was not called on the user path file,
> so no reason to call security_file_release()?
>
> It is very much a possibility that LSM would want to allocate security
> context for the user path file during backing_file_mmap, when both files
> are available in context, so that later mprotect() will have this stored
> information in the user path file security context.
>
> But in this case, wouldn't security_file_free() be enough?
See my comments above. If we wanted to look into removing the
destroy_file() special case for the user_path file hanging off the
backing file I'm happy to work with you guys in the future to sort
that out, but I'd prefer to tackle that at a later date as it could
potentially have a much larger footprint than this patchset.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] lsm: export security_mmap_backing_file
From: Paul Moore @ 2026-03-18 18:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arnd Bergmann, James Morris, Serge E. Hallyn, Amir Goldstein,
Casey Schaufler, John Johansen, Christian Brauner, Mimi Zohar,
linux-security-module, linux-kernel
In-Reply-To: <fe689975-a8f1-4370-b99a-b3456182ab37@app.fastmail.com>
On Wed, Mar 18, 2026 at 1:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Mar 18, 2026, at 17:29, Paul Moore wrote:
> > On Wed, Mar 18, 2026 at 6:43 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> > Thanks Arnd. It looks like I'm going to need to respin this patchset
> > due to other changes, do you mind if I fold this patch into the
> > associated LSM patch when I do that, or would you prefer this as a
> > standalone fix?
>
> Please fold it into your patch for the next version.
Done with a thank you note added to the bottom of the patch description.
--
paul-moore.com
^ permalink raw reply
* [PATCH 7/7] lsm: Remove security_sb_mount and security_move_mount
From: Song Liu @ 2026-03-18 18:44 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260318184400.3502908-1-song@kernel.org>
Now that all LSMs have been converted to granular mount hooks,
remove the old hooks:
- security_sb_mount(): removed from lsm_hook_defs.h, security.h,
security.c, and its call in path_mount().
- security_move_mount(): removed and replaced by security_mount_move()
in do_move_mount(). All LSMs now use mount_move exclusively.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
fs/namespace.c | 6 +-----
include/linux/lsm_hook_defs.h | 4 ----
include/linux/security.h | 16 ---------------
kernel/bpf/bpf_lsm.c | 2 --
security/apparmor/lsm.c | 1 -
security/landlock/fs.c | 1 -
security/security.c | 38 -----------------------------------
security/selinux/hooks.c | 2 --
8 files changed, 1 insertion(+), 69 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index de33070e514a..ba5baccdde67 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4108,7 +4108,6 @@ int path_mount(const char *dev_name, const struct path *path,
const char *type_page, unsigned long flags, void *data_page)
{
unsigned int mnt_flags = 0, sb_flags;
- int ret;
/* Discard magic */
if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
@@ -4121,9 +4120,6 @@ int path_mount(const char *dev_name, const struct path *path,
if (flags & MS_NOUSER)
return -EINVAL;
- ret = security_sb_mount(dev_name, path, type_page, flags, data_page);
- if (ret)
- return ret;
if (!may_mount())
return -EPERM;
if (flags & SB_MANDLOCK)
@@ -4538,7 +4534,7 @@ static inline int vfs_move_mount(const struct path *from_path,
{
int ret;
- ret = security_move_mount(from_path, to_path);
+ ret = security_mount_move(from_path, to_path);
if (ret)
return ret;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 6bb67059fb43..95537574c40b 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -69,8 +69,6 @@ LSM_HOOK(int, 0, sb_remount, struct super_block *sb, void *mnt_opts)
LSM_HOOK(int, 0, sb_kern_mount, const struct super_block *sb)
LSM_HOOK(int, 0, sb_show_options, struct seq_file *m, struct super_block *sb)
LSM_HOOK(int, 0, sb_statfs, struct dentry *dentry)
-LSM_HOOK(int, 0, sb_mount, const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
LSM_HOOK(int, 0, sb_umount, struct vfsmount *mnt, int flags)
LSM_HOOK(int, 0, sb_pivotroot, const struct path *old_path,
const struct path *new_path)
@@ -79,8 +77,6 @@ LSM_HOOK(int, 0, sb_set_mnt_opts, struct super_block *sb, void *mnt_opts,
LSM_HOOK(int, 0, sb_clone_mnt_opts, const struct super_block *oldsb,
struct super_block *newsb, unsigned long kern_flags,
unsigned long *set_kern_flags)
-LSM_HOOK(int, 0, move_mount, const struct path *from_path,
- const struct path *to_path)
LSM_HOOK(int, 0, mount_bind, const struct path *from, const struct path *to,
bool recurse)
LSM_HOOK(int, 0, mount_new, struct fs_context *fc, const struct path *mp,
diff --git a/include/linux/security.h b/include/linux/security.h
index 6e31de9b3d68..3610a49304c6 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -372,8 +372,6 @@ int security_sb_remount(struct super_block *sb, void *mnt_opts);
int security_sb_kern_mount(const struct super_block *sb);
int security_sb_show_options(struct seq_file *m, struct super_block *sb);
int security_sb_statfs(struct dentry *dentry);
-int security_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data);
int security_sb_umount(struct vfsmount *mnt, int flags);
int security_sb_pivotroot(const struct path *old_path, const struct path *new_path);
int security_sb_set_mnt_opts(struct super_block *sb,
@@ -384,7 +382,6 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
struct super_block *newsb,
unsigned long kern_flags,
unsigned long *set_kern_flags);
-int security_move_mount(const struct path *from_path, const struct path *to_path);
int security_mount_bind(const struct path *from, const struct path *to,
bool recurse);
int security_mount_new(struct fs_context *fc, const struct path *mp,
@@ -818,13 +815,6 @@ static inline int security_sb_statfs(struct dentry *dentry)
return 0;
}
-static inline int security_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags,
- void *data)
-{
- return 0;
-}
-
static inline int security_sb_umount(struct vfsmount *mnt, int flags)
{
return 0;
@@ -852,12 +842,6 @@ static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
return 0;
}
-static inline int security_move_mount(const struct path *from_path,
- const struct path *to_path)
-{
- return 0;
-}
-
static inline int security_mount_bind(const struct path *from,
const struct path *to, bool recurse)
{
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 65235d70ee23..3e61c54f9b48 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -350,7 +350,6 @@ BTF_ID(func, bpf_lsm_release_secctx)
BTF_ID(func, bpf_lsm_sb_alloc_security)
BTF_ID(func, bpf_lsm_sb_eat_lsm_opts)
BTF_ID(func, bpf_lsm_sb_kern_mount)
-BTF_ID(func, bpf_lsm_sb_mount)
BTF_ID(func, bpf_lsm_sb_remount)
BTF_ID(func, bpf_lsm_sb_set_mnt_opts)
BTF_ID(func, bpf_lsm_sb_show_options)
@@ -383,7 +382,6 @@ BTF_ID(func, bpf_lsm_task_prctl)
BTF_ID(func, bpf_lsm_task_setscheduler)
BTF_ID(func, bpf_lsm_task_to_inode)
BTF_ID(func, bpf_lsm_userns_create)
-BTF_ID(func, bpf_lsm_move_mount)
BTF_ID(func, bpf_lsm_mount_bind)
BTF_ID(func, bpf_lsm_mount_new)
BTF_ID(func, bpf_lsm_mount_remount)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 7fe774535992..13a8049b1b59 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1713,7 +1713,6 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
LSM_HOOK_INIT(capget, apparmor_capget),
LSM_HOOK_INIT(capable, apparmor_capable),
- LSM_HOOK_INIT(move_mount, apparmor_move_mount),
LSM_HOOK_INIT(mount_bind, apparmor_mount_bind),
LSM_HOOK_INIT(mount_new, apparmor_mount_new),
LSM_HOOK_INIT(mount_remount, apparmor_mount_remount),
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 6e810550efcb..5f723a70baa4 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1857,7 +1857,6 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
LSM_HOOK_INIT(mount_reconfigure, hook_mount_reconfigure),
LSM_HOOK_INIT(mount_change_type, hook_mount_change_type),
LSM_HOOK_INIT(mount_move, hook_move_mount),
- LSM_HOOK_INIT(move_mount, hook_move_mount),
LSM_HOOK_INIT(sb_umount, hook_sb_umount),
LSM_HOOK_INIT(sb_remount, hook_sb_remount),
LSM_HOOK_INIT(sb_pivotroot, hook_sb_pivotroot),
diff --git a/security/security.c b/security/security.c
index 356ef228d5de..af95868af34d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1039,29 +1039,6 @@ int security_sb_statfs(struct dentry *dentry)
return call_int_hook(sb_statfs, dentry);
}
-/**
- * security_sb_mount() - Check permission for mounting a filesystem
- * @dev_name: filesystem backing device
- * @path: mount point
- * @type: filesystem type
- * @flags: mount flags
- * @data: filesystem specific data
- *
- * Check permission before an object specified by @dev_name is mounted on the
- * mount point named by @nd. For an ordinary mount, @dev_name identifies a
- * device if the file system type requires a device. For a remount
- * (@flags & MS_REMOUNT), @dev_name is irrelevant. For a loopback/bind mount
- * (@flags & MS_BIND), @dev_name identifies the pathname of the object being
- * mounted.
- *
- * Return: Returns 0 if permission is granted.
- */
-int security_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
-{
- return call_int_hook(sb_mount, dev_name, path, type, flags, data);
-}
-
/**
* security_sb_umount() - Check permission for unmounting a filesystem
* @mnt: mounted filesystem
@@ -1141,21 +1118,6 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
}
EXPORT_SYMBOL(security_sb_clone_mnt_opts);
-/**
- * security_move_mount() - Check permissions for moving a mount
- * @from_path: source mount point
- * @to_path: destination mount point
- *
- * Check permission before a mount is moved.
- *
- * Return: Returns 0 if permission is granted.
- */
-int security_move_mount(const struct path *from_path,
- const struct path *to_path)
-{
- return call_int_hook(move_mount, from_path, to_path);
-}
-
/**
* security_mount_bind() - Check permissions for a bind mount
* @from: source path
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 415b5541ab9e..446e9e242134 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7477,8 +7477,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts),
LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts),
- LSM_HOOK_INIT(move_mount, selinux_move_mount),
-
LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security),
LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as),
--
2.52.0
^ permalink raw reply related
* [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-03-18 18:43 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260318184400.3502908-1-song@kernel.org>
Replace tomoyo_sb_mount() with granular mount hooks. Each hook
reconstructs the MS_* flags expected by tomoyo_mount_permission()
using the original flags parameter where available.
Key changes:
- mount_bind: passes the pre-resolved source path to
tomoyo_mount_acl() via a new dev_path parameter, instead of
re-resolving dev_name via kern_path(). This eliminates a TOCTOU
vulnerability.
- mount_new, mount_remount, mount_reconfigure: use the original
mount(2) flags for policy matching.
- mount_move: passes pre-resolved paths for both source and
destination.
- mount_change_type: passes raw ms_flags directly.
Also removes the unused data_page parameter from
tomoyo_mount_permission().
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/tomoyo/common.h | 2 +-
security/tomoyo/mount.c | 31 +++++++++++++-------
security/tomoyo/tomoyo.c | 63 ++++++++++++++++++++++++++++++----------
3 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index 4f1704c911ef..e40441844eab 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -1013,7 +1013,7 @@ int tomoyo_mkdev_perm(const u8 operation, const struct path *path,
const unsigned int mode, unsigned int dev);
int tomoyo_mount_permission(const char *dev_name, const struct path *path,
const char *type, unsigned long flags,
- void *data_page);
+ const struct path *dev_path);
int tomoyo_open_control(const u8 type, struct file *file);
int tomoyo_path2_perm(const u8 operation, const struct path *path1,
const struct path *path2);
diff --git a/security/tomoyo/mount.c b/security/tomoyo/mount.c
index 322dfd188ada..82ffe7d02814 100644
--- a/security/tomoyo/mount.c
+++ b/security/tomoyo/mount.c
@@ -70,6 +70,7 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
* @dir: Pointer to "struct path".
* @type: Name of filesystem type.
* @flags: Mount options.
+ * @dev_path: Pre-resolved device/source path. Maybe NULL.
*
* Returns 0 on success, negative value otherwise.
*
@@ -78,11 +79,11 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
static int tomoyo_mount_acl(struct tomoyo_request_info *r,
const char *dev_name,
const struct path *dir, const char *type,
- unsigned long flags)
+ unsigned long flags,
+ const struct path *dev_path)
__must_hold_shared(&tomoyo_ss)
{
struct tomoyo_obj_info obj = { };
- struct path path;
struct file_system_type *fstype = NULL;
const char *requested_type = NULL;
const char *requested_dir_name = NULL;
@@ -134,13 +135,23 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r,
need_dev = 1;
}
if (need_dev) {
- /* Get mount point or device file. */
- if (!dev_name || kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
+ if (dev_path) {
+ /* Use pre-resolved path to avoid TOCTOU issues. */
+ obj.path1 = *dev_path;
+ path_get(&obj.path1);
+ } else if (!dev_name) {
error = -ENOENT;
goto out;
+ } else {
+ struct path path;
+
+ if (kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
+ error = -ENOENT;
+ goto out;
+ }
+ obj.path1 = path;
}
- obj.path1 = path;
- requested_dev_name = tomoyo_realpath_from_path(&path);
+ requested_dev_name = tomoyo_realpath_from_path(&obj.path1);
if (!requested_dev_name) {
error = -ENOENT;
goto out;
@@ -173,7 +184,7 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r,
if (fstype)
put_filesystem(fstype);
kfree(requested_type);
- /* Drop refcount obtained by kern_path(). */
+ /* Drop refcount obtained by kern_path() or path_get(). */
if (obj.path1.dentry)
path_put(&obj.path1);
return error;
@@ -186,13 +197,13 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r,
* @path: Pointer to "struct path".
* @type: Name of filesystem type. Maybe NULL.
* @flags: Mount options.
- * @data_page: Optional data. Maybe NULL.
+ * @dev_path: Pre-resolved device/source path. Maybe NULL.
*
* Returns 0 on success, negative value otherwise.
*/
int tomoyo_mount_permission(const char *dev_name, const struct path *path,
const char *type, unsigned long flags,
- void *data_page)
+ const struct path *dev_path)
{
struct tomoyo_request_info r;
int error;
@@ -236,7 +247,7 @@ int tomoyo_mount_permission(const char *dev_name, const struct path *path,
if (!type)
type = "<NULL>";
idx = tomoyo_read_lock();
- error = tomoyo_mount_acl(&r, dev_name, path, type, flags);
+ error = tomoyo_mount_acl(&r, dev_name, path, type, flags, dev_path);
tomoyo_read_unlock(idx);
return error;
}
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index c66e02ed8ee3..ac84e1f03d5e 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -6,6 +6,8 @@
*/
#include <linux/lsm_hooks.h>
+#include <linux/fs_context.h>
+#include <uapi/linux/mount.h>
#include <uapi/linux/lsm.h>
#include "common.h"
@@ -398,21 +400,47 @@ static int tomoyo_path_chroot(const struct path *path)
return tomoyo_path_perm(TOMOYO_TYPE_CHROOT, path, NULL);
}
-/**
- * tomoyo_sb_mount - Target for security_sb_mount().
- *
- * @dev_name: Name of device file. Maybe NULL.
- * @path: Pointer to "struct path".
- * @type: Name of filesystem type. Maybe NULL.
- * @flags: Mount options.
- * @data: Optional data. Maybe NULL.
- *
- * Returns 0 on success, negative value otherwise.
- */
-static int tomoyo_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
+static int tomoyo_mount_bind(const struct path *from, const struct path *to,
+ bool recurse)
+{
+ unsigned long flags = MS_BIND | (recurse ? MS_REC : 0);
+
+ return tomoyo_mount_permission(NULL, to, NULL, flags, from);
+}
+
+static int tomoyo_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ /* Use original MS_* flags for policy matching */
+ return tomoyo_mount_permission(fc->source, mp, fc->fs_type->name,
+ flags, NULL);
+}
+
+static int tomoyo_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ /* Use original MS_* flags for policy matching */
+ return tomoyo_mount_permission(NULL, mp, NULL, flags, NULL);
+}
+
+static int tomoyo_mount_reconfigure(const struct path *mp,
+ unsigned int mnt_flags,
+ unsigned long flags)
+{
+ /* Use original MS_* flags for policy matching */
+ return tomoyo_mount_permission(NULL, mp, NULL, flags, NULL);
+}
+
+static int tomoyo_mount_change_type(const struct path *mp, int ms_flags)
+{
+ return tomoyo_mount_permission(NULL, mp, NULL, ms_flags, NULL);
+}
+
+static int tomoyo_move_mount(const struct path *from_path,
+ const struct path *to_path)
{
- return tomoyo_mount_permission(dev_name, path, type, flags, data);
+ return tomoyo_mount_permission(NULL, to_path, NULL, MS_MOVE,
+ from_path);
}
/**
@@ -576,7 +604,12 @@ static struct security_hook_list tomoyo_hooks[] __ro_after_init = {
LSM_HOOK_INIT(path_chmod, tomoyo_path_chmod),
LSM_HOOK_INIT(path_chown, tomoyo_path_chown),
LSM_HOOK_INIT(path_chroot, tomoyo_path_chroot),
- LSM_HOOK_INIT(sb_mount, tomoyo_sb_mount),
+ LSM_HOOK_INIT(mount_bind, tomoyo_mount_bind),
+ LSM_HOOK_INIT(mount_new, tomoyo_mount_new),
+ LSM_HOOK_INIT(mount_remount, tomoyo_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, tomoyo_mount_reconfigure),
+ LSM_HOOK_INIT(mount_change_type, tomoyo_mount_change_type),
+ LSM_HOOK_INIT(mount_move, tomoyo_move_mount),
LSM_HOOK_INIT(sb_umount, tomoyo_sb_umount),
LSM_HOOK_INIT(sb_pivotroot, tomoyo_sb_pivotroot),
LSM_HOOK_INIT(socket_bind, tomoyo_socket_bind),
--
2.52.0
^ permalink raw reply related
* [PATCH 5/7] landlock: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-03-18 18:43 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260318184400.3502908-1-song@kernel.org>
Replace hook_sb_mount() with granular mount hooks. Landlock denies
all mount operations for sandboxed processes regardless of flags,
so all new hooks share a common hook_mount_deny() helper. The
mount_move hook reuses hook_move_mount().
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/landlock/fs.c | 40 ++++++++++++++++++++++++++++++++++++----
1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index e764470f588c..6e810550efcb 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1417,9 +1417,7 @@ static void log_fs_change_topology_dentry(
* inherit these new constraints. Anyway, for backward compatibility reasons,
* a dedicated user space option would be required (e.g. as a ruleset flag).
*/
-static int hook_sb_mount(const char *const dev_name,
- const struct path *const path, const char *const type,
- const unsigned long flags, void *const data)
+static int hook_mount_deny(const struct path *const path)
{
size_t handle_layer;
const struct landlock_cred_security *const subject =
@@ -1433,6 +1431,35 @@ static int hook_sb_mount(const char *const dev_name,
return -EPERM;
}
+static int hook_mount_bind(const struct path *const from,
+ const struct path *const to, bool recurse)
+{
+ return hook_mount_deny(to);
+}
+
+static int hook_mount_new(struct fs_context *fc, const struct path *const mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return hook_mount_deny(mp);
+}
+
+static int hook_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return hook_mount_deny(mp);
+}
+
+static int hook_mount_reconfigure(const struct path *const mp,
+ unsigned int mnt_flags, unsigned long flags)
+{
+ return hook_mount_deny(mp);
+}
+
+static int hook_mount_change_type(const struct path *const mp, int ms_flags)
+{
+ return hook_mount_deny(mp);
+}
+
static int hook_move_mount(const struct path *const from_path,
const struct path *const to_path)
{
@@ -1824,7 +1851,12 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
LSM_HOOK_INIT(inode_free_security_rcu, hook_inode_free_security_rcu),
LSM_HOOK_INIT(sb_delete, hook_sb_delete),
- LSM_HOOK_INIT(sb_mount, hook_sb_mount),
+ LSM_HOOK_INIT(mount_bind, hook_mount_bind),
+ LSM_HOOK_INIT(mount_new, hook_mount_new),
+ LSM_HOOK_INIT(mount_remount, hook_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, hook_mount_reconfigure),
+ LSM_HOOK_INIT(mount_change_type, hook_mount_change_type),
+ LSM_HOOK_INIT(mount_move, hook_move_mount),
LSM_HOOK_INIT(move_mount, hook_move_mount),
LSM_HOOK_INIT(sb_umount, hook_sb_umount),
LSM_HOOK_INIT(sb_remount, hook_sb_remount),
--
2.52.0
^ permalink raw reply related
* [PATCH 4/7] selinux: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-03-18 18:43 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260318184400.3502908-1-song@kernel.org>
Replace selinux_mount() with granular mount hooks, preserving the
same permission checks:
- mount_bind, mount_new, mount_change_type: FILE__MOUNTON
- mount_remount, mount_reconfigure: FILESYSTEM__REMOUNT
- mount_move: FILE__MOUNTON (reuses selinux_move_mount)
The flags and data parameters are unused by SELinux.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/selinux/hooks.c | 47 ++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d8224ea113d1..415b5541ab9e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2778,19 +2778,37 @@ static int selinux_sb_statfs(struct dentry *dentry)
return superblock_has_perm(cred, dentry->d_sb, FILESYSTEM__GETATTR, &ad);
}
-static int selinux_mount(const char *dev_name,
- const struct path *path,
- const char *type,
- unsigned long flags,
- void *data)
+static int selinux_mount_bind(const struct path *from, const struct path *to,
+ bool recurse)
{
- const struct cred *cred = current_cred();
+ return path_has_perm(current_cred(), to, FILE__MOUNTON);
+}
- if (flags & MS_REMOUNT)
- return superblock_has_perm(cred, path->dentry->d_sb,
- FILESYSTEM__REMOUNT, NULL);
- else
- return path_has_perm(cred, path, FILE__MOUNTON);
+static int selinux_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return path_has_perm(current_cred(), mp, FILE__MOUNTON);
+}
+
+static int selinux_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags,
+ void *data)
+{
+ return superblock_has_perm(current_cred(), fc->root->d_sb,
+ FILESYSTEM__REMOUNT, NULL);
+}
+
+static int selinux_mount_reconfigure(const struct path *mp,
+ unsigned int mnt_flags,
+ unsigned long flags)
+{
+ return superblock_has_perm(current_cred(), mp->dentry->d_sb,
+ FILESYSTEM__REMOUNT, NULL);
+}
+
+static int selinux_mount_change_type(const struct path *mp, int ms_flags)
+{
+ return path_has_perm(current_cred(), mp, FILE__MOUNTON);
}
static int selinux_move_mount(const struct path *from_path,
@@ -7449,7 +7467,12 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
LSM_HOOK_INIT(sb_kern_mount, selinux_sb_kern_mount),
LSM_HOOK_INIT(sb_show_options, selinux_sb_show_options),
LSM_HOOK_INIT(sb_statfs, selinux_sb_statfs),
- LSM_HOOK_INIT(sb_mount, selinux_mount),
+ LSM_HOOK_INIT(mount_bind, selinux_mount_bind),
+ LSM_HOOK_INIT(mount_new, selinux_mount_new),
+ LSM_HOOK_INIT(mount_remount, selinux_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, selinux_mount_reconfigure),
+ LSM_HOOK_INIT(mount_change_type, selinux_mount_change_type),
+ LSM_HOOK_INIT(mount_move, selinux_move_mount),
LSM_HOOK_INIT(sb_umount, selinux_umount),
LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts),
LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts),
--
2.52.0
^ permalink raw reply related
* [PATCH 3/7] apparmor: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-03-18 18:43 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260318184400.3502908-1-song@kernel.org>
Replace AppArmor's monolithic apparmor_sb_mount() with granular
mount hooks.
Key changes:
- mount_bind: uses the pre-resolved struct path from VFS instead of
re-resolving dev_name via kern_path(), eliminating a TOCTOU
vulnerability. aa_bind_mount() now takes a struct path instead of
a string for the source.
- mount_new, mount_remount: receive the original mount(2) flags and
data parameters for policy matching via match_mnt_flags() and
AA_MNT_CONT_MATCH data matching.
- mount_reconfigure: handles MS_REMOUNT|MS_BIND (mount attribute
reconfiguration) which was previously handled as a remount.
- mount_move: reuses apparmor_move_mount() which already handles
pre-resolved paths.
- mount_change_type: propagation type changes.
aa_move_mount_old() is removed since move mounts now go through
security_mount_move() with pre-resolved struct path pointers for
both the old mount(2) and new move_mount(2) APIs.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/apparmor/include/mount.h | 5 +-
security/apparmor/lsm.c | 99 ++++++++++++++++++++++++-------
security/apparmor/mount.c | 37 ++----------
3 files changed, 83 insertions(+), 58 deletions(-)
diff --git a/security/apparmor/include/mount.h b/security/apparmor/include/mount.h
index 46834f828179..088e2f938cc1 100644
--- a/security/apparmor/include/mount.h
+++ b/security/apparmor/include/mount.h
@@ -31,16 +31,13 @@ int aa_remount(const struct cred *subj_cred,
int aa_bind_mount(const struct cred *subj_cred,
struct aa_label *label, const struct path *path,
- const char *old_name, unsigned long flags);
+ const struct path *old_path, bool recurse);
int aa_mount_change_type(const struct cred *subj_cred,
struct aa_label *label, const struct path *path,
unsigned long flags);
-int aa_move_mount_old(const struct cred *subj_cred,
- struct aa_label *label, const struct path *path,
- const char *old_name);
int aa_move_mount(const struct cred *subj_cred,
struct aa_label *label, const struct path *from_path,
const struct path *to_path);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 48a8655af11e..7fe774535992 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -13,6 +13,7 @@
#include <linux/mm.h>
#include <linux/mman.h>
#include <linux/mount.h>
+#include <linux/fs_context.h>
#include <linux/namei.h>
#include <linux/ptrace.h>
#include <linux/ctype.h>
@@ -697,34 +698,83 @@ static int apparmor_uring_sqpoll(void)
}
#endif /* CONFIG_IO_URING */
-static int apparmor_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
+static int apparmor_mount_bind(const struct path *from, const struct path *to,
+ bool recurse)
{
struct aa_label *label;
int error = 0;
bool needput;
- flags &= ~AA_MS_IGNORE_MASK;
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_bind_mount(current_cred(), label, to, from,
+ recurse);
+ __end_current_label_crit_section(label, needput);
+ return error;
+}
+
+static int apparmor_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ /* flags and data are from the original mount(2) call */
label = __begin_current_label_crit_section(&needput);
- if (!unconfined(label)) {
- if (flags & MS_REMOUNT)
- error = aa_remount(current_cred(), label, path, flags,
- data);
- else if (flags & MS_BIND)
- error = aa_bind_mount(current_cred(), label, path,
- dev_name, flags);
- else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE |
- MS_UNBINDABLE))
- error = aa_mount_change_type(current_cred(), label,
- path, flags);
- else if (flags & MS_MOVE)
- error = aa_move_mount_old(current_cred(), label, path,
- dev_name);
- else
- error = aa_new_mount(current_cred(), label, dev_name,
- path, type, flags, data);
- }
+ if (!unconfined(label))
+ error = aa_new_mount(current_cred(), label, fc->source,
+ mp, fc->fs_type->name, flags, data);
+ __end_current_label_crit_section(label, needput);
+
+ return error;
+}
+
+static int apparmor_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags,
+ void *data)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ /* flags and data are from the original mount(2) call */
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_remount(current_cred(), label, mp, flags, data);
+ __end_current_label_crit_section(label, needput);
+
+ return error;
+}
+
+static int apparmor_mount_reconfigure(const struct path *mp,
+ unsigned int mnt_flags,
+ unsigned long flags)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ /* flags are from the original mount(2) call */
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_remount(current_cred(), label, mp, flags, NULL);
+ __end_current_label_crit_section(label, needput);
+
+ return error;
+}
+
+static int apparmor_mount_change_type(const struct path *mp, int ms_flags)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_mount_change_type(current_cred(), label, mp,
+ ms_flags);
__end_current_label_crit_section(label, needput);
return error;
@@ -1664,7 +1714,12 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
LSM_HOOK_INIT(capable, apparmor_capable),
LSM_HOOK_INIT(move_mount, apparmor_move_mount),
- LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
+ LSM_HOOK_INIT(mount_bind, apparmor_mount_bind),
+ LSM_HOOK_INIT(mount_new, apparmor_mount_new),
+ LSM_HOOK_INIT(mount_remount, apparmor_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, apparmor_mount_reconfigure),
+ LSM_HOOK_INIT(mount_move, apparmor_move_mount),
+ LSM_HOOK_INIT(mount_change_type, apparmor_mount_change_type),
LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 523570aa1a5a..38b40e16014f 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -418,25 +418,17 @@ int aa_remount(const struct cred *subj_cred,
}
int aa_bind_mount(const struct cred *subj_cred,
- struct aa_label *label, const struct path *path,
- const char *dev_name, unsigned long flags)
+ struct aa_label *label, const struct path *path,
+ const struct path *old_path, bool recurse)
{
struct aa_profile *profile;
char *buffer = NULL, *old_buffer = NULL;
- struct path old_path;
+ unsigned long flags = MS_BIND | (recurse ? MS_REC : 0);
int error;
AA_BUG(!label);
AA_BUG(!path);
-
- if (!dev_name || !*dev_name)
- return -EINVAL;
-
- flags &= MS_REC | MS_BIND;
-
- error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
- if (error)
- return error;
+ AA_BUG(!old_path);
buffer = aa_get_buffer(false);
old_buffer = aa_get_buffer(false);
@@ -445,12 +437,11 @@ int aa_bind_mount(const struct cred *subj_cred,
goto out;
error = fn_for_each_confined(label, profile,
- match_mnt(subj_cred, profile, path, buffer, &old_path,
+ match_mnt(subj_cred, profile, path, buffer, old_path,
old_buffer, NULL, flags, NULL, false));
out:
aa_put_buffer(buffer);
aa_put_buffer(old_buffer);
- path_put(&old_path);
return error;
}
@@ -514,24 +505,6 @@ int aa_move_mount(const struct cred *subj_cred,
return error;
}
-int aa_move_mount_old(const struct cred *subj_cred, struct aa_label *label,
- const struct path *path, const char *orig_name)
-{
- struct path old_path;
- int error;
-
- if (!orig_name || !*orig_name)
- return -EINVAL;
- error = kern_path(orig_name, LOOKUP_FOLLOW, &old_path);
- if (error)
- return error;
-
- error = aa_move_mount(subj_cred, label, &old_path, path);
- path_put(&old_path);
-
- return error;
-}
-
int aa_new_mount(const struct cred *subj_cred, struct aa_label *label,
const char *dev_name, const struct path *path,
const char *type, unsigned long flags, void *data)
--
2.52.0
^ permalink raw reply related
* [PATCH 2/7] apparmor: Remove redundant MS_MGC_MSK stripping in apparmor_sb_mount
From: Song Liu @ 2026-03-18 18:43 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260318184400.3502908-1-song@kernel.org>
path_mount() already strips the magic number from flags before
calling security_sb_mount(), so this check in apparmor_sb_mount()
is a no-op. Remove it.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/apparmor/lsm.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c1d42fc72fdb..48a8655af11e 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -704,10 +704,6 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
int error = 0;
bool needput;
- /* Discard magic */
- if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
- flags &= ~MS_MGC_MSK;
-
flags &= ~AA_MS_IGNORE_MASK;
label = __begin_current_label_crit_section(&needput);
--
2.52.0
^ permalink raw reply related
* [PATCH 1/7] lsm: Add granular mount hooks to replace security_sb_mount
From: Song Liu @ 2026-03-18 18:43 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260318184400.3502908-1-song@kernel.org>
Add six new LSM hooks for mount operations:
- mount_bind(from, to, recurse): bind mount with pre-resolved
struct path for source and destination.
- mount_new(fc, mp, mnt_flags, flags, data): new mount, called after
mount options are parsed. The flags and data parameters carry the
original mount(2) flags and data for LSMs that need them (AppArmor,
Tomoyo).
- mount_remount(fc, mp, mnt_flags, flags, data): filesystem remount,
called after mount options are parsed into the fs_context.
- mount_reconfigure(mp, mnt_flags, flags): mount flag reconfiguration
(MS_REMOUNT|MS_BIND path).
- mount_move(from, to): move mount with pre-resolved paths.
- mount_change_type(mp, ms_flags): propagation type changes.
These replace the monolithic security_sb_mount() which conflates
multiple distinct operations into a single hook, and suffers from
TOCTOU issues where LSMs re-resolve string-based dev_name via
kern_path().
The mount_move hook is added alongside the existing move_mount hook.
During the transition, LSMs register for both hooks. The move_mount
hook will be removed once all LSMs have been converted.
Some LSMs, such as apparmor and tomoyo, audit the original input passed
in the mount syscall. To keep the same behavior, argument data and flags
are passed in do_* functions. These can be removed if these LSMs no
longer need these information.
All new hooks are registered as sleepable BPF LSM hooks.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
fs/namespace.c | 35 ++++++++++--
include/linux/lsm_hook_defs.h | 12 ++++
include/linux/security.h | 50 +++++++++++++++++
kernel/bpf/bpf_lsm.c | 7 +++
security/security.c | 101 ++++++++++++++++++++++++++++++++++
5 files changed, 199 insertions(+), 6 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 854f4fc66469..de33070e514a 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2875,6 +2875,10 @@ static int do_change_type(const struct path *path, int ms_flags)
if (!type)
return -EINVAL;
+ err = security_mount_change_type(path, ms_flags);
+ if (err)
+ return err;
+
guard(namespace_excl)();
err = may_change_propagation(mnt);
@@ -3007,6 +3011,10 @@ static int do_loopback(const struct path *path, const char *old_name,
if (err)
return err;
+ err = security_mount_bind(&old_path, path, recurse);
+ if (err)
+ return err;
+
if (mnt_ns_loop(old_path.dentry))
return -EINVAL;
@@ -3319,7 +3327,8 @@ static void mnt_warn_timestamp_expiry(const struct path *mountpoint,
* superblock it refers to. This is triggered by specifying MS_REMOUNT|MS_BIND
* to mount(2).
*/
-static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
+static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags,
+ unsigned long flags)
{
struct super_block *sb = path->mnt->mnt_sb;
struct mount *mnt = real_mount(path->mnt);
@@ -3334,6 +3343,10 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
if (!can_change_locked_flags(mnt, mnt_flags))
return -EPERM;
+ ret = security_mount_reconfigure(path, mnt_flags, flags);
+ if (ret)
+ return ret;
+
/*
* We're only checking whether the superblock is read-only not
* changing it, so only take down_read(&sb->s_umount).
@@ -3357,7 +3370,7 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
* on it - tough luck.
*/
static int do_remount(const struct path *path, int sb_flags,
- int mnt_flags, void *data)
+ int mnt_flags, void *data, unsigned long flags)
{
int err;
struct super_block *sb = path->mnt->mnt_sb;
@@ -3384,6 +3397,9 @@ static int do_remount(const struct path *path, int sb_flags,
fc->oldapi = true;
err = parse_monolithic_mount_data(fc, data);
+ if (!err)
+ err = security_mount_remount(fc, path, mnt_flags, flags,
+ data);
if (!err) {
down_write(&sb->s_umount);
err = -EPERM;
@@ -3713,6 +3729,10 @@ static int do_move_mount_old(const struct path *path, const char *old_name)
if (err)
return err;
+ err = security_mount_move(&old_path, path);
+ if (err)
+ return err;
+
return do_move_mount(&old_path, path, 0);
}
@@ -3791,7 +3811,7 @@ static int do_new_mount_fc(struct fs_context *fc, const struct path *mountpoint,
*/
static int do_new_mount(const struct path *path, const char *fstype,
int sb_flags, int mnt_flags,
- const char *name, void *data)
+ const char *name, void *data, unsigned long flags)
{
struct file_system_type *type;
struct fs_context *fc;
@@ -3835,6 +3855,9 @@ static int do_new_mount(const struct path *path, const char *fstype,
err = parse_monolithic_mount_data(fc, data);
if (!err && !mount_capable(fc))
err = -EPERM;
+
+ if (!err)
+ err = security_mount_new(fc, path, mnt_flags, flags, data);
if (!err)
err = do_new_mount_fc(fc, path, mnt_flags);
@@ -4146,9 +4169,9 @@ int path_mount(const char *dev_name, const struct path *path,
SB_I_VERSION);
if ((flags & (MS_REMOUNT | MS_BIND)) == (MS_REMOUNT | MS_BIND))
- return do_reconfigure_mnt(path, mnt_flags);
+ return do_reconfigure_mnt(path, mnt_flags, flags);
if (flags & MS_REMOUNT)
- return do_remount(path, sb_flags, mnt_flags, data_page);
+ return do_remount(path, sb_flags, mnt_flags, data_page, flags);
if (flags & MS_BIND)
return do_loopback(path, dev_name, flags & MS_REC);
if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
@@ -4157,7 +4180,7 @@ int path_mount(const char *dev_name, const struct path *path,
return do_move_mount_old(path, dev_name);
return do_new_mount(path, type_page, sb_flags, mnt_flags, dev_name,
- data_page);
+ data_page, flags);
}
int do_mount(const char *dev_name, const char __user *dir_name,
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..6bb67059fb43 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -81,6 +81,18 @@ LSM_HOOK(int, 0, sb_clone_mnt_opts, const struct super_block *oldsb,
unsigned long *set_kern_flags)
LSM_HOOK(int, 0, move_mount, const struct path *from_path,
const struct path *to_path)
+LSM_HOOK(int, 0, mount_bind, const struct path *from, const struct path *to,
+ bool recurse)
+LSM_HOOK(int, 0, mount_new, struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+LSM_HOOK(int, 0, mount_remount, struct fs_context *fc,
+ const struct path *mp, int mnt_flags, unsigned long flags,
+ void *data)
+LSM_HOOK(int, 0, mount_reconfigure, const struct path *mp,
+ unsigned int mnt_flags, unsigned long flags)
+LSM_HOOK(int, 0, mount_move, const struct path *from_path,
+ const struct path *to_path)
+LSM_HOOK(int, 0, mount_change_type, const struct path *mp, int ms_flags)
LSM_HOOK(int, -EOPNOTSUPP, dentry_init_security, struct dentry *dentry,
int mode, const struct qstr *name, const char **xattr_name,
struct lsm_context *cp)
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..6e31de9b3d68 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -385,6 +385,17 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
unsigned long kern_flags,
unsigned long *set_kern_flags);
int security_move_mount(const struct path *from_path, const struct path *to_path);
+int security_mount_bind(const struct path *from, const struct path *to,
+ bool recurse);
+int security_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data);
+int security_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data);
+int security_mount_reconfigure(const struct path *mp, unsigned int mnt_flags,
+ unsigned long flags);
+int security_mount_move(const struct path *from_path,
+ const struct path *to_path);
+int security_mount_change_type(const struct path *mp, int ms_flags);
int security_dentry_init_security(struct dentry *dentry, int mode,
const struct qstr *name,
const char **xattr_name,
@@ -847,6 +858,45 @@ static inline int security_move_mount(const struct path *from_path,
return 0;
}
+static inline int security_mount_bind(const struct path *from,
+ const struct path *to, bool recurse)
+{
+ return 0;
+}
+
+static inline int security_mount_new(struct fs_context *fc,
+ const struct path *mp, int mnt_flags,
+ unsigned long flags, void *data)
+{
+ return 0;
+}
+
+static inline int security_mount_remount(struct fs_context *fc,
+ const struct path *mp, int mnt_flags,
+ unsigned long flags, void *data)
+{
+ return 0;
+}
+
+static inline int security_mount_reconfigure(const struct path *mp,
+ unsigned int mnt_flags,
+ unsigned long flags)
+{
+ return 0;
+}
+
+static inline int security_mount_move(const struct path *from_path,
+ const struct path *to_path)
+{
+ return 0;
+}
+
+static inline int security_mount_change_type(const struct path *mp,
+ int ms_flags)
+{
+ return 0;
+}
+
static inline int security_path_notify(const struct path *path, u64 mask,
unsigned int obj_type)
{
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 0c4a0c8e6f70..65235d70ee23 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -383,6 +383,13 @@ BTF_ID(func, bpf_lsm_task_prctl)
BTF_ID(func, bpf_lsm_task_setscheduler)
BTF_ID(func, bpf_lsm_task_to_inode)
BTF_ID(func, bpf_lsm_userns_create)
+BTF_ID(func, bpf_lsm_move_mount)
+BTF_ID(func, bpf_lsm_mount_bind)
+BTF_ID(func, bpf_lsm_mount_new)
+BTF_ID(func, bpf_lsm_mount_remount)
+BTF_ID(func, bpf_lsm_mount_reconfigure)
+BTF_ID(func, bpf_lsm_mount_move)
+BTF_ID(func, bpf_lsm_mount_change_type)
BTF_SET_END(sleepable_lsm_hooks)
BTF_SET_START(untrusted_lsm_hooks)
diff --git a/security/security.c b/security/security.c
index 67af9228c4e9..356ef228d5de 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1156,6 +1156,107 @@ int security_move_mount(const struct path *from_path,
return call_int_hook(move_mount, from_path, to_path);
}
+/**
+ * security_mount_bind() - Check permissions for a bind mount
+ * @from: source path
+ * @to: destination mount point
+ * @recurse: whether this is a recursive bind mount
+ *
+ * Check permission before a bind mount is performed. Called with the
+ * source path already resolved, eliminating TOCTOU issues with
+ * string-based dev_name in security_sb_mount().
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mount_bind(const struct path *from, const struct path *to,
+ bool recurse)
+{
+ return call_int_hook(mount_bind, from, to, recurse);
+}
+
+/**
+ * security_mount_new() - Check permissions for a new mount
+ * @fc: filesystem context with parsed options
+ * @mp: mount point path
+ * @mnt_flags: mount flags (MNT_*)
+ * @flags: original mount flags (MS_*, used by AppArmor/Tomoyo)
+ * @data: filesystem specific data (used by AppArmor)
+ *
+ * Check permission before a new filesystem is mounted. Called after
+ * mount options are parsed, providing access to the fs_context.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return call_int_hook(mount_new, fc, mp, mnt_flags, flags, data);
+}
+
+/**
+ * security_mount_remount() - Check permissions for a remount
+ * @fc: filesystem context with parsed options
+ * @mp: mount point path
+ * @mnt_flags: mount flags (MNT_*)
+ * @flags: original mount flags (MS_*, used by AppArmor/Tomoyo)
+ * @data: filesystem specific data (used by AppArmor)
+ *
+ * Check permission before a filesystem is remounted. Called after
+ * mount options are parsed, providing access to the fs_context.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return call_int_hook(mount_remount, fc, mp, mnt_flags, flags, data);
+}
+
+/**
+ * security_mount_reconfigure() - Check permissions for mount reconfiguration
+ * @mp: mount point path
+ * @mnt_flags: new mount flags (MNT_*)
+ * @flags: original mount flags (MS_*, used by AppArmor/Tomoyo)
+ *
+ * Check permission before mount flags are reconfigured (MS_REMOUNT|MS_BIND).
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mount_reconfigure(const struct path *mp, unsigned int mnt_flags,
+ unsigned long flags)
+{
+ return call_int_hook(mount_reconfigure, mp, mnt_flags, flags);
+}
+
+/**
+ * security_mount_move() - Check permissions for moving a mount
+ * @from_path: source mount path
+ * @to_path: destination mount point path
+ *
+ * Check permission before a mount is moved.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mount_move(const struct path *from_path,
+ const struct path *to_path)
+{
+ return call_int_hook(mount_move, from_path, to_path);
+}
+
+/**
+ * security_mount_change_type() - Check permissions for propagation changes
+ * @mp: mount point path
+ * @ms_flags: propagation flags (MS_SHARED, MS_PRIVATE, etc.)
+ *
+ * Check permission before mount propagation type is changed.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mount_change_type(const struct path *mp, int ms_flags)
+{
+ return call_int_hook(mount_change_type, mp, ms_flags);
+}
+
/**
* security_path_notify() - Check if setting a watch is allowed
* @path: file path
--
2.52.0
^ permalink raw reply related
* [PATCH 0/7] lsm: Replace security_sb_mount with granular mount hooks
From: Song Liu @ 2026-03-18 18:43 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
This series replaces the monolithic security_sb_mount() hook with
per-operation mount hooks, addressing two main issues:
1. TOCTOU: security_sb_mount() receives dev_name as a string, which
LSMs like AppArmor and Tomoyo re-resolve via kern_path(). The new
hooks pass pre-resolved struct path pointers where possible (bind
mount, move mount), eliminating the double-resolution.
2. Conflation: security_sb_mount() handles bind, new mount, remount,
move, propagation changes, and mount reconfiguration through a
single hook, requiring LSMs to dispatch on flags internally. The
new hooks are called at the operation level with appropriate
context.
The new hooks are:
mount_bind - bind mount (pre-resolved source path)
mount_new - new filesystem mount (with fs_context)
mount_remount - filesystem remount (with fs_context)
mount_reconfigure - mount flag reconfiguration (MS_REMOUNT|MS_BIND)
mount_move - move mount (pre-resolved paths)
mount_change_type - propagation type changes
mount_new and mount_remount are called after parse_monolithic_mount_data(),
so LSMs have access to the fs_context with parsed mount options. They also
receive the original mount(2) flags and data pointer for LSMs (AppArmor,
Tomoyo) that need them for policy matching.
The series also replaces security_move_mount() with the new mount_move
hook, unifying the old mount(2) MS_MOVE path with the move_mount(2)
syscall path.
All existing LSM behaviors are preserved:
AppArmor: same policy matching, TOCTOU fixed for bind/move
SELinux: same permission checks (FILE__MOUNTON, FILESYSTEM__REMOUNT)
Landlock: same deny-all for sandboxed processes
Tomoyo: same policy matching, TOCTOU fixed for bind/move, unused
data_page parameter removed
This work is inspired by earlier discussions:
[1] https://lore.kernel.org/bpf/20251127005011.1872209-1-song@kernel.org/
[2] https://lore.kernel.org/linux-security-module/20250708230504.3994335-1-song@kernel.org/
Song Liu (7):
lsm: Add granular mount hooks to replace security_sb_mount
apparmor: Remove redundant MS_MGC_MSK stripping in apparmor_sb_mount
apparmor: Convert from sb_mount to granular mount hooks
selinux: Convert from sb_mount to granular mount hooks
landlock: Convert from sb_mount to granular mount hooks
tomoyo: Convert from sb_mount to granular mount hooks
lsm: Remove security_sb_mount and security_move_mount
fs/namespace.c | 41 +++++++---
include/linux/lsm_hook_defs.h | 14 +++-
include/linux/security.h | 56 +++++++++++---
kernel/bpf/bpf_lsm.c | 7 +-
security/apparmor/include/mount.h | 5 +-
security/apparmor/lsm.c | 102 ++++++++++++++++++-------
security/apparmor/mount.c | 37 ++--------
security/landlock/fs.c | 41 ++++++++--
security/security.c | 119 +++++++++++++++++++++++-------
security/selinux/hooks.c | 49 ++++++++----
security/tomoyo/common.h | 2 +-
security/tomoyo/mount.c | 31 +++++---
security/tomoyo/tomoyo.c | 63 ++++++++++++----
13 files changed, 406 insertions(+), 161 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [PATCH v6 3/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-18 17:52 UTC (permalink / raw)
To: Justin Suess
Cc: Sebastian Andrzej Siewior, Günther Noack, John Johansen,
Tingmao Wang, Kuniyuki Iwashima, Jann Horn, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross,
Tahera Fahimi
In-Reply-To: <abrWSwS5urQpvbPb@suesslenovo>
On Wed, Mar 18, 2026 at 12:43:55PM -0400, Justin Suess wrote:
> On Wed, Mar 18, 2026 at 05:26:20PM +0100, Mickaël Salaün wrote:
> > On Wed, Mar 18, 2026 at 04:05:59PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2026-03-18 10:14:52 [-0400], Justin Suess wrote:
> > > > Sebastian,
> > > Justin,
> > >
> > > > In short: dom_other is a pointer to a landlock-owned refcounted struct.
> > > …
> > > >
> > > > But we copy the domain pointer, which points to a landlock allocated
> > > > and controlled object.
> > >
> > > and this is not going away while we are here and preempted after
> > > dropping the lock? (if the landlock policy is updated/ changed/ …)
> >
> > I agree with Sebastian, this is a bug, see my original proposal:
> > https://lore.kernel.org/all/20260217.lievaS8eeng8@digikod.net/
> Mickaël,
>
> Just to make sure we're speaking of the same thing (I spotted a bug
> shortly after replying to Sebastian).
>
> This is a potential UAF if the dom_other is freed before the access
> check takes place correct?
Yes
>
> dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> unix_state_unlock(other);
>
> unmask_scoped_access(subject->domain, dom_other, &layer_masks,
> fs_resolve_unix.fs);
>
> If the dom_other->usage reaches zero, then the domain could be
> freed after the unix_state_unlock while we're checking access??
>
> (I guess I assumed the sock_hold on the @other would prevent the task
> @other belongs to from being freed.)
>
> Would it be better to move the access check under the unix_state_lock or
> to acquire another reference to the ruleset (or something else)?
Because the unmask_scoped_access() only read a bounded array, it's
simpler to unlock just after.
The other alternatives would be to use an RCU lock or a new reference
but I don't think it's worth it.
>
> (Good catch Sebastian sorry for the confusion.)
>
> Justin
^ permalink raw reply
* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Frederick Lawler @ 2026-03-18 17:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Kumar Kartikeya Dwivedi, Paul Moore, James Morris,
Serge E. Hallyn, Eric Paris, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Shuah Khan, Mickaël Salaün,
Günther Noack, LKML, LSM List, audit, bpf,
open list:KERNEL SELFTEST FRAMEWORK, kernel-team
In-Reply-To: <CAADnVQJCJaLjGCwjCX9VtT615c4kn7v5bxviUmGFVj=uTtUHMg@mail.gmail.com>
Hi Alexei,
On Tue, Mar 17, 2026 at 06:15:59PM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 16, 2026 at 7:44 PM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > On Wed, 11 Mar 2026 at 22:31, Frederick Lawler <fred@cloudflare.com> wrote:
> > >
> > > The motivation behind the change is to give BPF LSM developers the
> > > ability to report accesses via the audit subsystem much like how LSMs
> > > operate today.
>
> Sure, but bpf lsm-s don't need to follow such conventions.
> audit is nothing but a message passing from kernel to user space
> and done in a very inefficient way by wrapping strings into skb/netlink.
> bpf progs can do this message passing already via various ways:
> perfbuf, ringbuf, streams.
> Teach your user space to consume one of them.
Audit provides additional functionality by keeping messages close to the
syscall, and in-line with other messages for that syscall. Aside from
that, BPF is arguably too flexible. I'm sure it's already understood,
but the idea here is to reduce bespoke logging implementations and reduce
attack surface for violation reporting. Audit is already provided by the
kernel and a well leveraged pipeline.
WRT to performance characteristics, would you rather see this
implementation leverage those maps in a way to reduce that, background
workers, or something else? This is something we've considered, but left out
of RFC to collect more opinion on perf considerations.
Thanks for bring this up. I forgot to include that in the RFC cover, sorry.
Best,
Fred
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Chris Fenner @ 2026-03-18 17:36 UTC (permalink / raw)
To: Mimi Zohar
Cc: Jarkko Sakkinen, linux-integrity, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <dca994cd0ed11a262d4022c4984984460ba06a78.camel@linux.ibm.com>
Apologies if my long message derailed this discussion. I meant to
support Mimi's concern here and project a future vision where
TCG_TPM2_HMAC doesn't conflict with other features.
More concisely, I think that:
> tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled
is not a compelling argument for removing TPM as an RNG source,
because TCG_TPM2_HMAC is known to have poor performance already
anyway.
Thanks
Chris
On Thu, Mar 5, 2026 at 7:37 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2026-03-03 at 23:32 +0200, Jarkko Sakkinen wrote:
> > On Fri, Feb 20, 2026 at 10:30:26AM -0800, Chris Fenner wrote:
> > > My conclusion about TCG_TPM2_HMAC after [1] and [2] was that
> > > TPM2_TCG_HMAC doesn't (or didn't at the time) actually solve the
> > > threat model it claims to (active interposer adversaries), while
> > > dramatically increasing the cost of many kernel TPM activities beyond
> > > the amount that would have been required to just solve for
> > > passive/bus-sniffer interposer adversaries. The added symmetric crypto
> > > required to secure a TPM transaction is almost not noticeable; the big
> > > performance problem is the re-bootstrapping of the session with ECDH
> > > for every command.
> > >
> > > My primary concern at that time was, essentially, that TCG_TPM2_HMAC
> > > punts on checking that the key that was used to secure the session was
> > > actually resident in a real TPM and not just an interposer adversary.
> > > I wrote up my understanding at
> > > https://www.dlp.rip/decorative-cryptography, for anyone who wants a
> > > long-form opinionated take :).
> > >
> > > Unless I'm wrong, or TCG_TPM2_HMAC has changed dramatically since
> > > August, I don't think "TPM2_TCG_HMAC makes this too costly" is a
> > > compelling reason to make a security decision. (There could be other
> > > reasons to make choices about whether to use the TPM as a source of
> > > randomness in the kernel! This just isn't one IMHO.)
> > >
> > > The version of TCG_TPM2_HMAC that I'd like to see someday would be one
> > > that fully admits that its threat model is only passive interposers,
> > > and sets up one session upon startup and ContextSaves/ContextLoads it
> > > back into the TPM as needed in order to secure parameter encryption
> > > for e.g., GetRandom() and Unseal() calls.
> >
> > Neither agreeing nor disagreeing but this patch set clearly does not
> > move forward and I spent already enough energy for this. For better
> > ideas the patches are available in queue branch.
>
> Jarkko, you totally ignored my comments below. I object to your removing the
> TPM trusted-keys RNG support.
>
> Mimi
>
> >
> > High-level takes don't move anything forward (or backward), sorry.
> >
> > >
> > > [1]: https://lore.kernel.org/linux-integrity/CAMigqh2nwuRRxaLyOJ+QaTJ+XGmkQj=rMj5K9GP1bCcXp2OsBQ@mail.gmail.com/
> > > [2]: https://lore.kernel.org/linux-integrity/20250825203223.629515-1-jarkko@kernel.org/
> > >
> > > Thanks
> > > Chris
> > >
> > > On Fri, Feb 20, 2026 at 10:04 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > >
> > > > [Cc: Chris Fenner, Jonathan McDowell, Roberto]
> > > >
> > > > On Sun, 2026-01-25 at 21:25 +0200, Jarkko Sakkinen wrote:
> > > > > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> > > > > use should be pooled rather than directly used. This both reduces
> > > > > latency and improves its predictability.
> > > >
> > > > If the concern is the latency of encrypting the bus session, please remember
> > > > that:
> > > >
> > > > - Not all environments expose the TPM bus to sniffing.
> > > > - The current TPM trusted keys design is based on TPM RNG, but already allows it
> > > > to be replaced with the kernel RNG via the "trusted_rng=kernel" boot command
> > > > line option.
> > > > - The proposed patch removes that possibility for no reason.
> > > >
> > > > Mimi & Elaine
> > > >
> > > >
> >
> > BR, Jarkko
^ permalink raw reply
* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Frederick Lawler @ 2026-03-18 17:34 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: Paul Moore, James Morris, Serge E. Hallyn, Eric Paris,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Shuah Khan, Mickaël Salaün, Günther Noack,
linux-kernel, linux-security-module, audit, bpf, linux-kselftest,
kernel-team
In-Reply-To: <CAP01T77VyW=5SHDvM3HXPNHaxRdzs8H__MOh2zx1dQ6STeAUtg@mail.gmail.com>
Hi Kumar,
On Tue, Mar 17, 2026 at 03:43:36AM +0100, Kumar Kartikeya Dwivedi wrote:
> On Wed, 11 Mar 2026 at 22:31, Frederick Lawler <fred@cloudflare.com> wrote:
> > 3. All bpf_audit_log_*() functions are destructive
> >
> > The audit subsystem allows for AUDIT_FAIL_PANIC to be set when the
> > subsystem can detect that missing events. Further, some call paths may
> > invoke a BUG_ON(). Therefore all the functions are marked destructive.
>
> I think the first part makes sense (i.e., the policy simply configured
> the system to panic on failure).
> However, in the second case, if the program is somehow able to trigger
> BUG_ON() relied upon for internal invariants, it would be considered
> broken.
> I tried grepping through and didn't find anything that would cause
> this, hence the whole thing about BUG_ON() in the cover letter only
> adds to confusion.
> Please drop it or describe cases which you were concerned about.
>
bpf_audit_log_cause()
-> audit_log_untrustedstring()
-> audit_log_n_untrustedstring()
-> audit_log_n_hex()
Was the primary call chain I was referring to, and yes this relies on
internal invariant of the SKB existing. I can remove from cover.
> >
> > 4. Functions are callable once per bpf_audit_context
> >
> > The rationale for this was to prevent abuse. Logs with repeated fields
> > are not helpful, and may not be handled by user space audit coherently.
> >
>
> This rationale feels weak. What abuse are we talking about?
> The LSM program is already written by a trusted entity.
>
I learned through off-list discussions that there's "undocumented" or
"unspoken" rules of log formatting and how auditd expects logs. Without
knowing exactly what these rules are, I can't provide any stronger
arguments other than to reduce duplication of fields in messages, or
exclude wrappers that supply unverified data.
WRT to unverified data, the socket wrappers accepting a netinf int.
In that case, the variable is at least verified via lookup to
report the interface in which it belongs. I left out wrapping
LSM_AUDIT_DATA_ANONINODE due to the passing of arbitrary string,
LSM_AUDIT_DATA_IPC arbitrary int. Others like for infiniband,
lockdown, nlmsgtype, were left out due to specificity, and not general
purpose usage.
Some of the rationale behind this is _not_ trusting BPF LSM authors as
much due to BPF LSM flexibility and accommodating user space audit.
Best,
Fred
^ permalink raw reply
* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Casey Schaufler @ 2026-03-18 17:09 UTC (permalink / raw)
To: Feng Yang, paul, jmorris, serge
Cc: linux-security-module, linux-kernel, Casey Schaufler
In-Reply-To: <20260318061925.134954-1-yangfeng59949@163.com>
On 3/17/2026 11:19 PM, Feng Yang wrote:
> From: Feng Yang <yangfeng@kylinos.cn>
>
> After hooking the following BPF program:
> SEC("lsm/xfrm_decode_session")
> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> {
> return 1; // Any non-zero value
> }
> Subsequent packet transmission triggers will cause a kernel panic:
LSM hooks that use or provide secids cannot be stacked. That is,
you can't provide a BPF LSM hook and an SELinux LSM hook and expect
correct behavior. Your proposed "fix" removes a legitimate check.
>
> [ 112.838874] ------------[ cut here ]------------
> [ 112.838895] kernel BUG at security/security.c:5282!
> [ 112.838902] invalid opcode: 0000 [#1] PREEMPT SMP PTI
> [ 112.838905] CPU: 5 PID: 4962 Comm: test Kdump: loaded Not tainted 6.19.0-rc5-gae23bc81ddf7 #2 PREEMPT(full)
> [ 112.838907] Source Version: 55e2f799c748c8e195569363edbd1d6a4159675a
> [ 112.838908] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> [ 112.838909] RIP: 0010:security_skb_classify_flow+0x3f/0x50
> [ 112.838914] Code: 85 db 74 28 49 89 fc 48 8d 6e 14 eb 08 48 8b 1b 48 85 db 74 17 31 d2 48 8b 43 18 48 89 ee 4c 89 e7 e8 05 33 86 00 85 c0 74 e3 <0f> 0b 5b 5d 41 5c c3 cc cc cc cc 66 0f 1f 44 00 00 90 90 90 90 90
> [ 112.838915] RSP: 0018:ffffc28400200b10 EFLAGS: 00010202
> [ 112.838918] RAX: 0000000000000001 RBX: ffffffff91d346d8 RCX: 0000000000000000
> [ 112.838919] RDX: ffffa0890f5eaf80 RSI: 0000000000000001 RDI: ffffa0890f5eaf80
> [ 112.838920] RBP: ffffc28400200d04 R08: 00000000000000c7 R09: 0000000000000002
> [ 112.838922] R10: 0000000000000000 R11: 000000000000000f R12: ffffa089086dedc0
> [ 112.838923] R13: ffffc28400200cf0 R14: ffffa08901ab2000 R15: 0000000000000000
> [ 112.838926] FS: 00007fb087dd2680(0000) GS:ffffa0891ba80000(0000) knlGS:0000000000000000
> [ 112.838927] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 112.838929] CR2: 00007fb087d1b940 CR3: 0000000107520006 CR4: 00000000000706e0
> [ 112.838930] Call Trace:
> [ 112.838931] <IRQ>
> [ 112.838933] icmp_route_lookup.constprop.0+0xd7/0x460
> [ 112.838941] ? switch_hrtimer_base+0x135/0x180
> [ 112.838944] ? update_sg_lb_stats+0x9c/0x440
> [ 112.838949] __icmp_send+0x3d3/0x740
> [ 112.838952] ? __udp4_lib_rcv+0x427/0x6f0
> [ 112.838955] __udp4_lib_rcv+0x427/0x6f0
> [ 112.838957] ip_protocol_deliver_rcu+0xb7/0x170
> [ 112.838960] ip_local_deliver_finish+0x76/0xa0
> [ 112.838961] __netif_receive_skb_one_core+0x89/0xa0
> [ 112.838967] process_backlog+0x95/0x140
> [ 112.838969] __napi_poll+0x2b/0x1c0
> [ 112.838971] net_rx_action+0x2aa/0x3a0
> [ 112.838972] ? swake_up_one+0x41/0x70
> [ 112.838974] ? kvm_sched_clock_read+0x11/0x20
> [ 112.838977] handle_softirqs+0xe3/0x2e0
> [ 112.838980] do_softirq+0x43/0x60
> [ 112.838982] </IRQ>
> [ 112.838982] <TASK>
> [ 112.838983] __local_bh_enable_ip+0x68/0x70
> [ 112.838985] __dev_queue_xmit+0x1c4/0x820
> [ 112.838987] ? nf_hook_slow+0x45/0xd0
> [ 112.838989] ip_finish_output2+0x1da/0x4a0
> [ 112.838992] ip_send_skb+0x86/0x90
> [ 112.838994] udp_send_skb+0x15e/0x380
> [ 112.838996] udp_sendmsg+0xb9a/0xf80
> [ 112.838998] ? __pfx_ip_generic_getfrag+0x10/0x10
> [ 112.839003] ? __sys_sendto+0x1e4/0x210
> [ 112.839005] __sys_sendto+0x1e4/0x210
> [ 112.839007] ? __handle_mm_fault+0x2fc/0x6c0
> [ 112.839013] __x64_sys_sendto+0x24/0x30
> [ 112.839014] do_syscall_64+0x5f/0x270
> [ 112.839017] entry_SYSCALL_64_after_hwframe+0x76/0xe0
> [ 112.839020] RIP: 0033:0x7fb087cfdb17
> [ 112.839021] Code: 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 80 3d 55 c8 0c 00 00 41 89 ca 74 10 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 71 c3 55 48 83 ec 30 44 89 4c 24 2c 4c 89 44
> [ 112.839023] RSP: 002b:00007ffea64704e8 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
> [ 112.839025] RAX: ffffffffffffffda RBX: 00007ffea6470638 RCX: 00007fb087cfdb17
> [ 112.839026] RDX: 0000000000000008 RSI: 00007ffea64704f8 RDI: 0000000000000003
> [ 112.839027] RBP: 00007ffea6470520 R08: 00007ffea6470500 R09: 0000000000000010
> [ 112.839029] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
> [ 112.839030] R13: 00007ffea6470648 R14: 0000000000403df0 R15: 00007fb087e15000
> [ 112.839032] </TASK>
>
> This BUG_ON was first mentioned in [1], but I could not find any explanatory record of why this check is needed.
>
> [1] https://lore.kernel.org/all/Pine.LNX.4.64.0607122149070.573@d.namei/
>
> In the existing LSM_HOOK_INIT(xfrm_decode_session, selinux_xfrm_decode_session),
> when the `ckall` parameter of the `selinux_xfrm_decode_session` function is 0,
> it can only return 0 and will not trigger BUG_ON.
> Therefore, remove the BUG_ON check to fix this issue.
>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Closes: https://lore.kernel.org/all/4c4d04ba.6c12b.19c039b69e6.Coremail.kaiyanm@hust.edu.cn/
> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> ---
> security/security.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e9..198f650070da 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4991,10 +4991,7 @@ int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
>
> void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic)
> {
> - int rc = call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid,
> - 0);
> -
> - BUG_ON(rc);
> + call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid, 0);
> }
> EXPORT_SYMBOL(security_skb_classify_flow);
> #endif /* CONFIG_SECURITY_NETWORK_XFRM */
^ permalink raw reply
* Re: [PATCH] lsm: export security_mmap_backing_file
From: Arnd Bergmann @ 2026-03-18 17:17 UTC (permalink / raw)
To: Paul Moore, Arnd Bergmann
Cc: James Morris, Serge E. Hallyn, Amir Goldstein, Casey Schaufler,
John Johansen, Christian Brauner, Mimi Zohar,
linux-security-module, linux-kernel
In-Reply-To: <CAHC9VhQoawCxVSFGY1whF_pwfH0FHrT7QFC6cgWeFV2XApC6hg@mail.gmail.com>
On Wed, Mar 18, 2026, at 17:29, Paul Moore wrote:
> On Wed, Mar 18, 2026 at 6:43 AM Arnd Bergmann <arnd@kernel.org> wrote:
> Thanks Arnd. It looks like I'm going to need to respin this patchset
> due to other changes, do you mind if I fold this patch into the
> associated LSM patch when I do that, or would you prefer this as a
> standalone fix?
Please fold it into your patch for the next version.
Arnd
^ permalink raw reply
* Re: [PATCH v6 2/9] landlock: use mem_is_zero() in is_layer_masks_allowed()
From: Mickaël Salaün @ 2026-03-18 16:52 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima
In-Reply-To: <20260315222150.121952-3-gnoack3000@gmail.com>
Subject should be "landlock: Use..."
On Sun, Mar 15, 2026 at 11:21:43PM +0100, Günther Noack wrote:
> This is equivalent, but expresses the intent a bit clearer.
>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> security/landlock/fs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index a03ec664c78e..97065d51685a 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -564,7 +564,7 @@ static void test_no_more_access(struct kunit *const test)
>
> static bool is_layer_masks_allowed(const struct layer_access_masks *masks)
> {
> - return !memchr_inv(&masks->access, 0, sizeof(masks->access));
> + return mem_is_zero(&masks->access, sizeof(masks->access));
Thanks
> }
>
> /*
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH v6 9/9] landlock: Document FS access right for pathname UNIX sockets
From: Mickaël Salaün @ 2026-03-18 16:54 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Justin Suess, linux-security-module, Tingmao Wang,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima
In-Reply-To: <20260315222150.121952-10-gnoack3000@gmail.com>
Please always add some minimal description.
Also, as already requested, could you run the check-linux.sh all on each
patch? That would avoid me to fix things like the date (which would now
be OK because of the new patch in my next branch, but still).
On Sun, Mar 15, 2026 at 11:21:50PM +0100, Günther Noack wrote:
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> Documentation/userspace-api/landlock.rst | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 13134bccdd39..e60ebd07c5cc 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -77,7 +77,8 @@ to be explicit about the denied-by-default access rights.
> LANDLOCK_ACCESS_FS_MAKE_SYM |
> LANDLOCK_ACCESS_FS_REFER |
> LANDLOCK_ACCESS_FS_TRUNCATE |
> - LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + LANDLOCK_ACCESS_FS_IOCTL_DEV |
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> .handled_access_net =
> LANDLOCK_ACCESS_NET_BIND_TCP |
> LANDLOCK_ACCESS_NET_CONNECT_TCP,
> @@ -127,6 +128,11 @@ version, and only use the available subset of access rights:
> /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
> ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> LANDLOCK_SCOPE_SIGNAL);
> + __attribute__((fallthrough));
Case 6 should be handled too:
case 6 ... 8:
> + case 7:
> + case 8:
> + /* Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX for ABI < 9 */
> + ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_RESOLVE_UNIX;
> }
>
> This enables the creation of an inclusive ruleset that will contain our rules.
> @@ -685,6 +691,13 @@ enforce Landlock rulesets across all threads of the calling process
> using the ``LANDLOCK_RESTRICT_SELF_TSYNC`` flag passed to
> sys_landlock_restrict_self().
>
> +Pathname UNIX sockets (ABI < 9)
> +-------------------------------
> +
> +Starting with the Landlock ABI version 9, it is possible to restrict
> +connections to pathname UNIX domain sockets (:manpage:`unix(7)`) using
> +the new ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX`` right.
> +
> .. _kernel_support:
>
> Kernel support
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH v6 7/9] landlock/selftests: Check that coredump sockets stay unrestricted
From: Mickaël Salaün @ 2026-03-18 16:53 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima
In-Reply-To: <20260315222150.121952-8-gnoack3000@gmail.com>
On Sun, Mar 15, 2026 at 11:21:48PM +0100, Günther Noack wrote:
> Even when a process is restricted with the new
> LANDLOCK_ACCESS_FS_RESOLVE_SOCKET right, the kernel can continue
LANDLOCK_ACCESS_FS_RESOLVE_UNIX (twice)
> writing its coredump to the configured coredump socket.
>
> In the test, we create a local server and rewire the system to write
> coredumps into it. We then create a child process within a Landlock
> domain where LANDLOCK_ACCESS_FS_RESOLVE_SOCKET is restricted and make
> the process crash. The test uses SO_PEERCRED to check that the
> connecting client process is the expected one.
>
> Includes a fix by Mickaël Salaün for setting the EUID to 0 (see [1]).
>
> Link[1]: https://lore.kernel.org/all/20260218.ohth8theu8Yi@digikod.net/
> Suggested-by: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> tools/testing/selftests/landlock/fs_test.c | 141 +++++++++++++++++++++
> 1 file changed, 141 insertions(+)
^ permalink raw reply
* Re: [PATCH v6 6/9] landlock/selftests: Audit test for LANDLOCK_ACCESS_FS_RESOLVE_UNIX
From: Mickaël Salaün @ 2026-03-18 16:53 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima
In-Reply-To: <20260315222150.121952-7-gnoack3000@gmail.com>
On Sun, Mar 15, 2026 at 11:21:47PM +0100, Günther Noack wrote:
> Add an audit test to check that Landlock denials from
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX result in audit logs in the expected
> format. (There is one audit test for each filesystem access right, so
> we should add one for LANDLOCK_ACCESS_FS_RESOLVE_UNIX as well.)
>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> tools/testing/selftests/landlock/fs_test.c | 43 +++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index fdbb024da774..4198148e172f 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -7538,7 +7538,8 @@ static const __u64 access_fs_16 =
> LANDLOCK_ACCESS_FS_MAKE_SYM |
> LANDLOCK_ACCESS_FS_REFER |
> LANDLOCK_ACCESS_FS_TRUNCATE |
> - LANDLOCK_ACCESS_FS_IOCTL_DEV;
> + LANDLOCK_ACCESS_FS_IOCTL_DEV |
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX;
The variable access_fs_16 contains 16 access rights. The idea was to
not need to change this variable with new FS access rights because the
tests should not change. I guess that was not a good choice because
new tests like audit_layout1.resolve_unix may not change the result of
existing tests (and then improve coverage). This variable can then be
replaced with ACCESS_ALL (in a dedicated patch please). If we need
exceptions we can mask the problematic access rights (see
audit_layout1.ioctl_dev).
> /* clang-format on */
>
> TEST_F(audit_layout1, execute_read)
> @@ -7983,6 +7984,46 @@ TEST_F(audit_layout1, ioctl_dev)
> EXPECT_EQ(1, records.domain);
> }
>
> +TEST_F(audit_layout1, resolve_unix)
> +{
> + struct audit_records records;
> + const char *const path = "sock";
> + int srv_fd, cli_fd, status;
> + pid_t child_pid;
> +
> + srv_fd = set_up_named_unix_server(_metadata, SOCK_STREAM, path);
> +
> + child_pid = fork();
> + ASSERT_LE(0, child_pid);
> + if (!child_pid) {
> + drop_access_rights(_metadata,
> + &(struct landlock_ruleset_attr){
> + .handled_access_fs = access_fs_16,
> + });
> +
> + cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> + ASSERT_LE(0, cli_fd);
> + EXPECT_EQ(EACCES,
> + test_connect_named_unix(_metadata, cli_fd, path));
> +
> + EXPECT_EQ(0, close(cli_fd));
> + _exit(_metadata->exit_code);
> + }
> +
> + ASSERT_EQ(child_pid, waitpid(child_pid, &status, 0));
> + EXPECT_EQ(1, WIFEXITED(status));
> + EXPECT_EQ(EXIT_SUCCESS, WEXITSTATUS(status));
> +
> + EXPECT_EQ(0, matches_log_fs_extra(_metadata, self->audit_fd,
> + "fs\\.resolve_unix", path, NULL));
> +
> + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> + EXPECT_EQ(0, records.access);
> + EXPECT_EQ(1, records.domain);
> +
> + EXPECT_EQ(0, close(srv_fd));
> +}
> +
> TEST_F(audit_layout1, mount)
> {
> struct audit_records records;
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH v6 5/9] landlock/selftests: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
From: Mickaël Salaün @ 2026-03-18 16:53 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Justin Suess, Tingmao Wang, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima
In-Reply-To: <20260315222150.121952-6-gnoack3000@gmail.com>
The subject's prefix is swapped, it should be "selftests/landlock".
On Sun, Mar 15, 2026 at 11:21:46PM +0100, Günther Noack wrote:
> * Extract common helpers from an existing IOCTL test that
> also uses pathname unix(7) sockets.
> * These tests use the common scoped domains fixture which is also used
> in other Landlock scoping tests and which was used in Tingmao Wang's
> earlier patch set in [1].
>
> These tests exercise the cross product of the following scenarios:
>
> * Stream connect(), Datagram connect(), Datagram sendmsg() and
> Seqpacket connect().
> * Child-to-parent and parent-to-child communication
> * The Landlock policy configuration as listed in the scoped_domains
> fixture.
> * In the default variant, Landlock domains are only placed where
> prescribed in the fixture.
> * In the "ALL_DOMAINS" variant, Landlock domains are also placed in
> the places where the fixture says to omit them, but with a
> LANDLOCK_RULE_PATH_BENEATH that allows connection.
>
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Cc: Mickaël Salaün <mic@digikod.net>
> Link[1]: https://lore.kernel.org/all/53b9883648225d5a08e82d2636ab0b4fda003bc9.1767115163.git.m@maowtm.org/
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> tools/testing/selftests/landlock/fs_test.c | 392 ++++++++++++++++++++-
> 1 file changed, 376 insertions(+), 16 deletions(-)
^ permalink raw reply
* Re: [PATCH v6 3/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-18 16:52 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Tingmao Wang, Justin Suess,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260315222150.121952-4-gnoack3000@gmail.com>
On Sun, Mar 15, 2026 at 11:21:44PM +0100, Günther Noack wrote:
> * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> controls the look up operations for named UNIX domain sockets. The
lookup
> resolution happens during connect() and sendmsg() (depending on
> socket type).
> * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> LSM hook. Make policy decisions based on the new access rights
> * Increment the Landlock ABI version.
> * Minor test adaptions to keep the tests working.
adaptations
> * Document the design rationale for scoped access rights,
> and cross-reference it from the header documentation.
>
> With this access right, access is granted if either of the following
> conditions is met:
>
> * The target socket's filesystem path was allow-listed using a
> LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> * The target socket was created in the same Landlock domain in which
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
>
> In case of a denial, connect() and sendmsg() return EACCES, which is
> the same error as it is returned if the user does not have the write
> bit in the traditional Unix file system permissions of that file.
UNIX
>
> Document the (possible future) interaction between scoped flags and
> other access rights in struct landlock_ruleset_attr, and summarize the
> rationale, as discussed in code review leading up to [2].
>
> This feature was created with substantial discussion and input from
> Justin Suess, Tingmao Wang and Mickaël Salaün.
>
> Cc: Tingmao Wang <m@maowtm.org>
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Kuniyuki Iwashima <kuniyu@google.com>
> Suggested-by: Jann Horn <jannh@google.com>
> Link[1]: https://github.com/landlock-lsm/linux/issues/36
> Link[2]: https://lore.kernel.org/all/20260205.8531e4005118@gnoack.org/
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> Documentation/security/landlock.rst | 40 +++++++
> include/uapi/linux/landlock.h | 19 ++++
> security/landlock/access.h | 2 +-
> security/landlock/audit.c | 1 +
> security/landlock/fs.c | 110 ++++++++++++++++++-
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 2 +-
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 5 +-
> 9 files changed, 176 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
> index 3e4d4d04cfae..4bbe250a6829 100644
> --- a/Documentation/security/landlock.rst
> +++ b/Documentation/security/landlock.rst
> @@ -89,6 +89,46 @@ this is required to keep access controls consistent over the whole system, and
> this avoids unattended bypasses through file descriptor passing (i.e. confused
> deputy attack).
>
> +.. _scoped-flags-interaction:
> +
> +Interaction between scoped flags and other access rights
> +--------------------------------------------------------
> +
> +The ``scoped`` flags in ``struct landlock_ruleset_attr`` restrict the
> +use of *outgoing* IPC from the created Landlock domain, while they
> +permit reaching out to IPC endpoints *within* the created Landlock
> +domain.
> +
> +In the future, scoped flags *may* interact with other access rights,
> +e.g. so that abstract UNIX sockets can be allow-listed by name, or so
> +that signals can be allow-listed by signal number or target process.
> +
> +When introducing ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX``, we defined it to
> +implicitly have the same scoping semantics as a
> +``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` flag would have: connecting to
> +UNIX sockets within the same domain (where
> +``LANDLOCK_ACCESS_FS_RESOLVE_UNIX`` is used) is unconditionally
> +allowed.
> +
> +The reasoning is:
> +
> +* Like other IPC mechanisms, connecting to named UNIX sockets in the
> + same domain should be expected and harmless. (If needed, users can
> + further refine their Landlock policies with nested domains or by
> + restricting ``LANDLOCK_ACCESS_FS_MAKE_SOCK``.)
> +* We reserve the option to still introduce
> + ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` in the future. (This would
> + be useful if we wanted to have a Landlock rule to permit IPC access
> + to other Landlock domains.)
> +* But we can postpone the point in time when users have to deal with
> + two interacting flags visible in the userspace API. (In particular,
> + it is possible that it won't be needed in practice, in which case we
> + can avoid the second flag altogether.)
> +* If we *do* introduce ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` in the
> + future, setting this scoped flag in a ruleset does *not reduce* the
> + restrictions, because access within the same scope is already
> + allowed based on ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX``.
> +
> Tests
> =====
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f88fa1f68b77..751e3c143cba 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -248,6 +248,24 @@ struct landlock_net_port_attr {
> *
> * This access right is available since the fifth version of the Landlock
> * ABI.
> + * - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX: Look up pathname UNIX domain sockets
> + * (:manpage:`unix(7)`). On UNIX domain sockets, this restricts both calls to
> + * :manpage:`connect(2)` as well as calls to :manpage:`sendmsg(2)` with an
> + * explicit recipient address.
> + *
> + * This access right only applies to connections to UNIX server sockets which
> + * were created outside of the newly created Landlock domain (e.g. from within
> + * a parent domain or from an unrestricted process). Newly created UNIX
> + * servers within the same Landlock domain continue to be accessible. In this
> + * regard, %LANDLOCK_ACCESS_RESOLVE_UNIX has the same semantics as the
LANDLOCK_ACCESS_FS_RESOLVE_UNIX
> + * ``LANDLOCK_SCOPE_*`` flags.
> + *
> + * If a resolve attempt is denied, the operation returns an ``EACCES`` error,
> + * in line with other filesystem access rights (but different to denials for
> + * abstract UNIX domain sockets).
This access right is available since the ninth version of the Landlock ABI.
> + *
> + * The rationale for this design is described in
> + * :ref:`Documentation/security/landlock.rst <scoped-flags-interaction>`.
> *
> * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
> * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
> @@ -333,6 +351,7 @@ struct landlock_net_port_attr {
> #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
> #define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15)
> +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX (1ULL << 16)
> /* clang-format on */
>
> /**
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 42c95747d7bd..89dc8e7b93da 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -34,7 +34,7 @@
> LANDLOCK_ACCESS_FS_IOCTL_DEV)
> /* clang-format on */
>
> -typedef u16 access_mask_t;
> +typedef u32 access_mask_t;
This change and the underlying implications are not explained in the
commit message, especially regarding the stack delta.
>
> /* Makes sure all filesystem access rights can be stored. */
> static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index 60ff217ab95b..8d0edf94037d 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
> [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
> };
>
> static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 97065d51685a..0486f5ab06c9 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -27,6 +27,7 @@
> #include <linux/lsm_hooks.h>
> #include <linux/mount.h>
> #include <linux/namei.h>
> +#include <linux/net.h>
> #include <linux/path.h>
> #include <linux/pid.h>
> #include <linux/rcupdate.h>
> @@ -36,6 +37,7 @@
> #include <linux/types.h>
> #include <linux/wait_bit.h>
> #include <linux/workqueue.h>
> +#include <net/af_unix.h>
> #include <uapi/linux/fiemap.h>
> #include <uapi/linux/landlock.h>
>
> @@ -314,7 +316,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> /* clang-format on */
>
> /*
> @@ -1557,6 +1560,110 @@ static int hook_path_truncate(const struct path *const path)
> return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> }
>
> +/**
> + * unmask_scoped_access - Remove access right bits in @masks in all layers
> + * where @client and @server have the same domain
> + *
> + * This does the same as domain_is_scoped(), but unmasks bits in @masks.
> + * It can not return early as domain_is_scoped() does.
I'd like a summary of your previous excellent explanation of
unmask_scoped_access() in this comment.
> + *
> + * @client: Client domain
> + * @server: Server domain
> + * @masks: Layer access masks to unmask
> + * @access: Access bit that controls scoping
> + */
> +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> + const struct landlock_ruleset *const server,
> + struct layer_access_masks *const masks,
> + const access_mask_t access)
> +{
> + int client_layer, server_layer;
> + const struct landlock_hierarchy *client_walker, *server_walker;
> +
> + /* This should not happen. */
> + if (WARN_ON_ONCE(!client))
> + return;
> +
> + /* Server has no Landlock domain; nothing to clear. */
> + if (!server)
> + return;
> +
Please also copy the BUILD_BUG_ON() from domain_is_scoped().
> + client_layer = client->num_layers - 1;
> + client_walker = client->hierarchy;
> + server_layer = server->num_layers - 1;
> + server_walker = server->hierarchy;
> +
> + /*
> + * Clears the access bits at all layers where the client domain is the
> + * same as the server domain. We start the walk at min(client_layer,
> + * server_layer). The layer bits until there can not be cleared because
> + * either the client or the server domain is missing.
> + */
> + for (; client_layer > server_layer; client_layer--)
> + client_walker = client_walker->parent;
> +
> + for (; server_layer > client_layer; server_layer--)
> + server_walker = server_walker->parent;
> +
> + for (; client_layer >= 0; client_layer--) {
> + if (masks->access[client_layer] & access &&
> + client_walker == server_walker)
> + masks->access[client_layer] &= ~access;
> +
> + client_walker = client_walker->parent;
> + server_walker = server_walker->parent;
> + }
> +}
> +
> +static int hook_unix_find(const struct path *const path, struct sock *other,
> + int flags)
> +{
> + const struct landlock_ruleset *dom_other;
> + const struct landlock_cred_security *subject;
> + struct layer_access_masks layer_masks;
> + struct landlock_request request = {};
> + static const struct access_masks fs_resolve_unix = {
> + .fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> + };
> +
> + /* Lookup for the purpose of saving coredumps is OK. */
> + if (unlikely(flags & SOCK_COREDUMP))
> + return 0;
> +
> + /* Access to the same (or a lower) domain is always allowed. */
This comment is related to the unmask_scoped_access() call.
> + subject = landlock_get_applicable_subject(current_cred(),
> + fs_resolve_unix, NULL);
> +
> + if (!subject)
> + return 0;
> +
> + if (!landlock_init_layer_masks(subject->domain, fs_resolve_unix.fs,
> + &layer_masks, LANDLOCK_KEY_INODE))
This case is not possible because landlock_get_applicable_subject()
already check it. Other hooks just ignore the returned value in this
case.
> + return 0;
> +
> + /* Checks the layers in which we are connecting within the same domain. */
> + unix_state_lock(other);
> + if (unlikely(sock_flag(other, SOCK_DEAD) || !other->sk_socket ||
> + !other->sk_socket->file)) {
> + unix_state_unlock(other);
> + return 0;
Is it safe to not return -ECONNREFUSED?
> + }
> + dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> + unix_state_unlock(other);
> +
> + unmask_scoped_access(subject->domain, dom_other, &layer_masks,
> + fs_resolve_unix.fs);
dom_other is not safe to use without the lock.
> +
> + /* Checks the connections to allow-listed paths. */
> + if (is_access_to_paths_allowed(subject->domain, path,
> + fs_resolve_unix.fs, &layer_masks,
> + &request, NULL, 0, NULL, NULL, NULL))
> + return 0;
> +
> + landlock_log_denial(subject, &request);
> + return -EACCES;
> +}
> +
> /* File hooks */
>
> /**
> @@ -1834,6 +1941,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(path_unlink, hook_path_unlink),
> LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
> LSM_HOOK_INIT(path_truncate, hook_path_truncate),
> + LSM_HOOK_INIT(unix_find, hook_unix_find),
>
> LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
> LSM_HOOK_INIT(file_open, hook_file_open),
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index eb584f47288d..b454ad73b15e 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -19,7 +19,7 @@
> #define LANDLOCK_MAX_NUM_LAYERS 16
> #define LANDLOCK_MAX_NUM_RULES U32_MAX
>
> -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV
> +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_RESOLVE_UNIX
> #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 3b33839b80c7..a6e23657f3ce 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -166,7 +166,7 @@ static const struct file_operations ruleset_fops = {
> * If the change involves a fix that requires userspace awareness, also update
> * the errata documentation in Documentation/userspace-api/landlock.rst .
> */
> -const int landlock_abi_version = 8;
> +const int landlock_abi_version = 9;
>
> /**
> * sys_landlock_create_ruleset - Create a new ruleset
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 0fea236ef4bd..30d37234086c 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -76,7 +76,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(9, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 968a91c927a4..b318627e7561 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -575,9 +575,10 @@ TEST_F_FORK(layout1, inval)
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
>
> -#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV
> +#define ACCESS_LAST LANDLOCK_ACCESS_FS_RESOLVE_UNIX
>
> #define ACCESS_ALL ( \
> ACCESS_FILE | \
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH v6 1/9] lsm: Add LSM hook security_unix_find
From: Mickaël Salaün @ 2026-03-18 16:51 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Paul Moore, James Morris, Serge E . Hallyn,
Tingmao Wang, Justin Suess, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima,
Simon Horman, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <20260315222150.121952-2-gnoack3000@gmail.com>
On Sun, Mar 15, 2026 at 11:21:42PM +0100, Günther Noack wrote:
> From: Justin Suess <utilityemal77@gmail.com>
>
> Add a LSM hook security_unix_find.
Add an LSM hook...
>
> This hook is called to check the path of a named unix socket before a
UNIX socket
> connection is initiated. The peer socket may be inspected as well.
>
> Why existing hooks are unsuitable:
>
> Existing socket hooks, security_unix_stream_connect(),
> security_unix_may_send(), and security_socket_connect() don't provide
> TOCTOU-free / namespace independent access to the paths of sockets.
>
> (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> This requires another path lookup. A change in the path between the
> two lookups will cause a TOCTOU bug.
>
> (2) We cannot use the struct path from the listening socket, because it
> may be bound to a path in a different namespace than the caller,
> resulting in a path that cannot be referenced at policy creation time.
>
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> include/linux/lsm_hook_defs.h | 5 +++++
> include/linux/security.h | 11 +++++++++++
> net/unix/af_unix.c | 13 ++++++++++---
> security/security.c | 20 ++++++++++++++++++++
> 4 files changed, 46 insertions(+), 3 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox