* [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 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 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
* 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
* 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 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 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 bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Alexei Starovoitov @ 2026-03-18 20:55 UTC (permalink / raw)
To: Frederick Lawler
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: <abrlvEjQKUdAOcw5@CMGLRV3>
On Wed, Mar 18, 2026 at 10:49 AM Frederick Lawler <fred@cloudflare.com> wrote:
>
> 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.
Sorry, I don't buy any of these arguments.
Nack.
^ permalink raw reply
* Re: linux-next: Tree for Mar 18 (landlock docs)
From: Randy Dunlap @ 2026-03-18 22:12 UTC (permalink / raw)
To: Mark Brown, Linux Next Mailing List
Cc: Linux Kernel Mailing List, Dan Cojocaru, Mickaël Salaün,
git, linux-security-module
In-Reply-To: <abrYTGOZRH9oP2GQ@sirena.org.uk>
On 3/18/26 9:52 AM, Mark Brown wrote:
> Hi all,
>
> Changes since 20260317:
>
linux-next/Documentation/userspace-api/landlock.rst:198: ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 105 supplied.
^^^^^^^^^^^^^^^^^^^^^^^^^^
.. code-block:: c
__u32 restrict_flags =
Please insert a blank line between ".. code-block:: c"
and the code.
d37d3347aa59 ("landlock: Expand restrict flags example for ABI version 8")
--
~Randy
^ permalink raw reply
* Re: [PATCH v2 1/2] kthread: remove kthread_exit()
From: Steven Rostedt @ 2026-03-18 23:12 UTC (permalink / raw)
To: David Laight
Cc: Christian Brauner, Linus Torvalds, linux-kernel, linux-modules,
linux-nfs, bpf, kunit-dev, linux-doc, linux-trace-kernel, netfs,
io-uring, audit, rcu, kvm, virtualization, netdev, linux-mm,
linux-security-module, Christian Loehle, linux-fsdevel
In-Reply-To: <20260311104736.51b53405@pumpkin>
On Wed, 11 Mar 2026 10:47:36 +0000
David Laight <david.laight.linux@gmail.com> wrote:
> > -#define module_put_and_kthread_exit(code) kthread_exit(code)
> > +#define module_put_and_kthread_exit(code) do_exit(code)
>
> I'm intrigued...
> How does that actually know to do the module_put()?
> (I know it does one - otherwise my driver wouldn't unload.)
It's in the !CONFIG_MODULES section. No module_put() necessary. Only the
kthread_exit (do_exit) is needed.
-- Steve
^ permalink raw reply
* Re: [PATCH v6] backing_file: store user_path_file
From: Paul Moore @ 2026-03-18 23:46 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs
In-Reply-To: <20260318131258.1457101-1-amir73il@gmail.com>
On Wed, Mar 18, 2026 at 9:13 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> 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 file_ref_init() helper was changed to accept the FILE_REF_ constant
> instead of the fake +1 integer count.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>
> Christian,
>
> My v5 patch was sent by Paul along with his LSM/selinux pataches [1].
> Here are the changes you requested.
>
> I removed the ACKs and Tested-by because of the changes.
>
> Thanks,
> Amir.
>
> Changes since v5:
> - Restore file_ref_init() helper without refcnt -1 offset
> - Future proofing errors from backing_file_open_user_path()
>
> [1] https://lore.kernel.org/r/20260316213606.374109-6-paul@paul-moore.com/
>
> fs/backing-file.c | 26 ++++++++++--------
> fs/erofs/ishare.c | 13 +++++++--
> 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 | 4 +--
> 9 files changed, 103 insertions(+), 34 deletions(-)
Still works for me. I'm going to update lsm/stable-7.0 with this
patch so we can get some more linux-next testing.
Tested-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* 社長のNG行動
From: 神田／ナレッジリンク @ 2026-03-18 23:40 UTC (permalink / raw)
To: linux-security-module
お世話になります。ナレッジリンクセミナー事務局です。
「社員のモチベーションを上げようと声をかける」
「部下の相談に乗り、一緒に悩んであげる」
「現場のトラブルに、自ら先頭に立って対応する」
もし社長がこれらを率先しているようであれば、
残念ながら、その組織の成長はそこで止まります。
社長のその“優しさ”が、社員の甘えを生み、責任感を奪い、
「指示待ち人間」を量産する装置になっているからです。
4,800社の経営者が衝撃を受けた、
良かれと思ってやってしまう「社長のNG行動」の正体。
組織を劇的に変えるための、
オンラインセミナーを開催いたします。
1つでも心当たりがあれば、一度ご視聴ください。
>>視聴予約はこちら
https://knowledge-corp.jp/shikigaku5/
----------------------------------------------
テーマ ： それ、危険です 『社長のNG行動』
〜 その行動が、組織崩壊を招く 〜
日 程 ： 3月19日（木）13:00〜15:00 残り14枠
4月14日（火）13:00〜15:00
4月21日（火）13:00〜15:00
※どちらの日程も内容は同じ
会 場 ：Zoom開催
定 員 ：先着100名（費用は不要）
----------------------------------------------
※経営層の方限定です
なぜ、社員は「言われたこと」しかやらないのか。
なぜ、次世代のリーダー候補が育たないのか。
それは、能力の問題ではなく、
良かれと思って続けている「社長の配慮」こそが、
組織成長を止める、最大のボトルネックかもしれません。
本セミナーでは、4,800社以上が導入した
独自の組織論「識学」に基づき、社長の「NG行動」と
真の経営者へ脱皮するためのマインドセットを伝授します。
【セミナー内容(一部抜粋)】
○ NG行動3選
○ なぜ優秀なNo.2や部長が育たないのか
○ マネジメントスタイルの変革について
○ 導入企業の事例
「管理職が育ったら任せる」ではなく「任せるから育つ」
という思考の逆転を提言。
現場から「冷たくなった」と思われることを恐れず、
機能的な階層構造（仕組み）を作ることで
結果として社員全員を守り、
利益を最大化させる道筋を明示します。
「優しさ」で人を動かすのではなく、
「正しさ」で組織を動かす。
音声やお顔が表に出ることはございませんので
お気軽にご視聴ください。
>>視聴予約はこちら
https://knowledge-corp.jp/shikigaku5/
-----------------------
一般社団法人 ナレッジリンク
東京都千代田区神田小川町1-8-3
電話：03-5256-7638
セミナーのご案内が不要な方は大変残念ではございますが、
下記URLより手続き下さいませ。
メール配信のワンクリック解除はこちら
https://fc-knowledgelink-corp.jp/mail/
^ permalink raw reply
* Re: [PATCH v2 7/10] security: Hornet LSM
From: Blaise Boscaccy @ 2026-03-19 0:18 UTC (permalink / raw)
To: Paul Moore, Jonathan Corbet, James Morris, Serge E. Hallyn,
Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
Fan Wu, Ryan Foster, linux-security-module, linux-doc,
linux-kernel, bpf
In-Reply-To: <a4ffe6eac1f9963e1bb092b4a6096154@paul-moore.com>
Paul Moore <paul@paul-moore.com> writes:
> On Feb 27, 2026 Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
>>
>> This adds the Hornet Linux Security Module which provides enhanced
>> signature verification and data validation for eBPF programs. This
>> allows users to continue to maintain an invariant that all code
>> running inside of the kernel has actually been signed and verified, by
>> the kernel.
>>
>> This effort builds upon the currently excepted upstream solution. It
>> further hardens it by providing deterministic, in-kernel checking of
>> map hashes to solidify auditing along with preventing TOCTOU attacks
>> against lskel map hashes.
>>
>> Target map hashes are passed in via PKCS#7 signed attributes. Hornet
>> determines the extent which the eBFP program is signed and defers to
>> other LSMs for policy decisions.
>>
>> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
>> Nacked-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> ---
>> Documentation/admin-guide/LSM/Hornet.rst | 310 ++++++++++++++++++++++
>> Documentation/admin-guide/LSM/index.rst | 1 +
>> MAINTAINERS | 9 +
>> include/linux/oid_registry.h | 3 +
>> include/uapi/linux/lsm.h | 1 +
>> security/Kconfig | 3 +-
>> security/Makefile | 1 +
>> security/hornet/Kconfig | 11 +
>> security/hornet/Makefile | 7 +
>> security/hornet/hornet.asn1 | 13 +
>> security/hornet/hornet_lsm.c | 323 +++++++++++++++++++++++
>> 11 files changed, 681 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/admin-guide/LSM/Hornet.rst
>> create mode 100644 security/hornet/Kconfig
>> create mode 100644 security/hornet/Makefile
>> create mode 100644 security/hornet/hornet.asn1
>> create mode 100644 security/hornet/hornet_lsm.c
>>
>> diff --git a/Documentation/admin-guide/LSM/Hornet.rst b/Documentation/admin-guide/LSM/Hornet.rst
>> new file mode 100644
>> index 000000000000..0dd4c03b8a7e
>> --- /dev/null
>> +++ b/Documentation/admin-guide/LSM/Hornet.rst
>> @@ -0,0 +1,310 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +======
>> +Hornet
>> +======
>> +
>> +Hornet is a Linux Security Module that provides extensible signature
>> +verification for eBPF programs. This is selectable at build-time with
>> +``CONFIG_SECURITY_HORNET``.
>> +
>> +Overview
>> +========
>> +
>> +Hornet addresses concerns from users who require strict audit trails and
>> +verification guarantees for eBPF programs, especially in
>> +security-sensitive environments. Many production systems need assurance
>> +that only authorized, unmodified eBPF programs are loaded into the
>> +kernel. Hornet provides this assurance through cryptographic signature
>> +verification.
>> +
>> +When an eBPF program is loaded via the ``bpf()`` syscall, Hornet
>> +verifies a PKCS#7 signature attached to the program instructions. The
>> +signature is checked against the kernel's secondary keyring using the
>> +existing kernel cryptographic infrastructure. In addition to signing the
>> +program bytecode, Hornet supports signing SHA-256 hashes of associated
>> +BPF maps, enabling integrity verification of map contents at load time
>> +and at runtime.
>> +
>> +After verification, Hornet classifies the program into one of the
>> +following integrity states and passes the result to a downstream LSM hook
>> +(``bpf_prog_load_post_integrity``), allowing other security modules to
>> +make policy decisions based on the verification outcome:
>> +
>> +``LSM_INT_VERDICT_OK``
>> + The program signature and all map hashes verified successfully.
>> +
>> +``LSM_INT_VERDICT_UNSIGNED``
>> + No signature was provided with the program.
>> +
>> +``LSM_INT_VERDICT_PARTIALSIG``
>> + The program signature verified, but the signing certificate is not
>> + trusted in the secondary keyring ...
>
> Do you think there is value in separating this case out from _PARTIALSIG?
> Maybe a LSM_INT_VERDICT_UNKNOWNKEY?
>
Yeah we can break these out a bit with this one and the VERDICT_FAULT
discussed below. Looking at this we, may want to add a VERDICT_UNEXPECTED
or something akin to that to handle the runtime check failure or any
other discrepancies.
>> + ... or the signature did not contain
>> + hornet map hash data.
>> +
>> +``LSM_INT_VERDICT_BADSIG``
>> + The signature or a map hash failed verification.
>> +
>> +Hornet itself does not enforce a policy on whether unsigned or partially
>> +signed programs should be rejected. It delegates that decision to
>> +downstream LSMs via the ``bpf_prog_load_post_integrity`` hook, making it
>> +a composable building block in a larger security architecture.
>> +
>> +Use Cases
>> +=========
>> +
>> +- **Locked-down production environments**: Ensure only eBPF programs
>> + signed by a trusted authority can be loaded, preventing unauthorized
>> + or tampered programs from running in the kernel.
>> +
>> +- **Audit and compliance**: Provide cryptographic evidence that loaded
>> + eBPF programs match their expected build artifacts, supporting
>> + compliance requirements in regulated industries.
>> +
>> +- **Supply chain integrity**: Verify that eBPF programs and their
>> + associated map data have not been modified since they were built and
>> + signed, protecting against supply chain attacks.
>> +
>> +Threat Model
>> +============
>> +
>> +Hornet protects against the following threats:
>> +
>> +- **Unauthorized eBPF program loading**: Programs that have not been
>> + signed by a trusted key will be reported as unsigned or badly signed.
>> +
>> +- **Tampering with program instructions**: Any modification to the eBPF
>> + bytecode after signing will cause signature verification to fail.
>> +
>> +- **Tampering with map data**: When map hashes are included in the
>> + signature, Hornet verifies that frozen BPF maps match their expected
>> + SHA-256 hashes at load time. Maps are also re-verified before program
>> + execution via ``BPF_PROG_RUN``.
>> +
>> +Hornet does **not** protect against:
>> +
>> +- Compromise of the signing key itself.
>> +- Attacks that occur after a program has been loaded and verified.
>> +- Programs loaded by the kernel itself (kernel-internal loads bypass
>> + the ``BPF_PROG_RUN`` map check).
>> +
>> +Known Limitations
>> +=================
>> +
>> +- Hornet requires programs to use :doc:`light skeletons
>> + </bpf/libbpf/libbpf_naming_convention>` (lskels) for the signing
>> + workflow, as the tooling operates on lskel-generated headers.
>> +
>> +- A maximum of 64 maps per program can be tracked for hash
>> + verification.
>> +
>> +- Map hash verification requires the maps to be frozen before loading.
>> + Maps that are not frozen at load time will cause verification to fail
>> + when their hashes are included in the signature.
>> +
>> +- Hornet relies on the kernel's secondary keyring
>> + (``VERIFY_USE_SECONDARY_KEYRING``) for certificate trust. Keys must
>> + be provisioned into this keyring before programs can be verified.
>
> I would add a bullet point describing the SHA256 limitation. If I
> understand things correctly this restriction comes from the core BPF
> code and not Hornet itself, so it would be nice to have this documented
> as it isn't immediately clear when looking only at the Hornet code.
>
Sure we can do that. Yes that's correct. The hashing algorithm is hard
coded and baked into bpf subsystem.
>> +Configuration
>> +=============
>> +
>> +Build Configuration
>> +-------------------
>> +
>> +Enable Hornet by setting the following kernel configuration option::
>> +
>> + CONFIG_SECURITY_HORNET=y
>> +
>> +This option is found under :menuselection:`Security options --> Hornet
>> +support` and depends on ``CONFIG_SECURITY``.
>> +
>> +When enabled, Hornet is included in the default LSM initialization order
>> +and will appear in ``/sys/kernel/security/lsm``.
>> +
>> +Architecture
>> +============
>> +
>> +Signature Verification Flow
>> +---------------------------
>> +
>> +The following describes what happens when a userspace program calls
>> +``bpf(BPF_PROG_LOAD, ...)`` with a signature attached:
>> +
>> +1. The ``bpf_prog_load_integrity`` LSM hook is invoked.
>> +
>> +2. Hornet reads the signature from the userspace buffer specified by
>> + ``attr->signature`` (with length ``attr->signature_size``).
>> +
>> +3. The PKCS#7 signature is verified against the program instructions
>> + using ``verify_pkcs7_signature()`` with the kernel's secondary
>> + keyring.
>> +
>> +4. The PKCS#7 message is parsed and its trust chain is validated via
>> + ``validate_pkcs7_trust()``.
>> +
>> +5. Hornet extracts the authenticated attribute identified by
>> + ``OID_hornet_data`` (OID ``2.25.316487325684022475439036912669789383960``)
>> + from the PKCS#7 message. This attribute contains an ASN.1-encoded set
>> + of map index/hash pairs.
>> +
>> +6. For each map hash entry, Hornet retrieves the corresponding BPF map
>> + via its file descriptor, confirms it is frozen, computes its SHA-256
>> + hash, and compares it against the signed hash.
>> +
>> +7. The resulting integrity verdict is passed to the
>> + ``bpf_prog_load_post_integrity`` hook so that downstream LSMs can
>> + enforce policy.
>> +
>> +Runtime Map Verification
>> +------------------------
>> +
>> +When ``bpf(BPF_PROG_RUN, ...)`` is called from userspace, Hornet
>> +re-verifies the hashes of all maps associated with the program. This
>> +ensures that map contents have not been modified between program load
>> +and execution. If any map hash no longer matches, the ``BPF_PROG_RUN``
>> +command is denied.
>> +
>> +Userspace Interface
>> +-------------------
>> +
>> +Signatures are passed to the kernel through fields in ``union bpf_attr``
>> +when using the ``BPF_PROG_LOAD`` command:
>> +
>> +``signature``
>> + A pointer to a userspace buffer containing the PKCS#7 signature.
>> +
>> +``signature_size``
>> + The size of the signature buffer in bytes.
>> +
>> +ASN.1 Schema
>> +------------
>> +
>> +Map hashes are encoded as a signed attribute in the PKCS#7 message using
>> +the following ASN.1 schema::
>> +
>> + HornetData ::= SET OF Map
>> +
>> + Map ::= SEQUENCE {
>> + index INTEGER,
>> + sha OCTET STRING
>> + }
>> +
>> +Each ``Map`` entry contains the index of the map in the program's
>> +``fd_array`` and its expected SHA-256 hash. A zero-length ``sha`` field
>> +indicates that the map at that index should be skipped during
>> +verification.
>> +
>> +Tooling
>> +=======
>> +
>> +Helper scripts and a signature generation tool are provided in
>> +``scripts/hornet/`` to support the development of signed eBPF light
>> +skeletons.
>> +
>> +gen_sig
>> +-------
>> +
>> +``gen_sig`` is a C program (using OpenSSL) that creates a PKCS#7
>> +signature over eBPF program instructions and optionally includes
>> +SHA-256 hashes of BPF maps as signed attributes.
>> +
>> +Usage::
>> +
>> + gen_sig --data <instructions.bin> \
>> + --cert <signer.crt> \
>> + --key <signer.key> \
>> + [--pass <passphrase>] \
>> + --out <signature.p7b> \
>> + [--add <mapfile.bin>:<index> ...]
>> +
>> +``--data``
>> + Path to the binary file containing eBPF program instructions to sign.
>> +
>> +``--cert``
>> + Path to the signing certificate (PEM or DER format).
>> +
>> +``--key``
>> + Path to the private key (PEM or DER format).
>> +
>> +``--pass``
>> + Optional passphrase for the private key.
>> +
>> +``--out``
>> + Path to write the output PKCS#7 signature.
>> +
>> +``--add``
>> + Attach a map hash as a signed attribute. The argument is a path to a
>> + binary map file followed by a colon and the map's index in the
>> + ``fd_array``. This option may be specified multiple times.
>> +
>> +extract-skel.sh
>> +---------------
>> +
>> +Extracts a named field from an autogenerated eBPF lskel header file.
>> +Used internally by other helper scripts.
>> +
>> +extract-insn.sh
>> +---------------
>> +
>> +Extracts the eBPF program instructions (``opts_insn``) from an lskel
>> +header into a binary file suitable for signing with ``gen_sig``.
>> +
>> +extract-map.sh
>> +--------------
>> +
>> +Extracts the map data (``opts_data``) from an lskel header into a
>> +binary file suitable for hashing with ``gen_sig``.
>> +
>> +write-sig.sh
>> +------------
>> +
>> +Replaces the signature data in an lskel header with a new signature
>> +from a binary file. This is used to embed a freshly generated signature
>> +back into the header after signing.
>> +
>> +Signing Workflow
>> +================
>> +
>> +A typical workflow for building and signing an eBPF light skeleton is:
>> +
>> +1. **Compile the eBPF program**::
>> +
>> + clang -O2 -target bpf -c program.bpf.c -o program.bpf.o
>> +
>> +2. **Generate the light skeleton header** using ``bpftool``::
>> +
>> + bpftool gen skeleton -S program.bpf.o > loader.h
>> +
>> +3. **Extract instructions and map data** from the generated header::
>> +
>> + scripts/hornet/extract-insn.sh loader.h > insn.bin
>> + scripts/hornet/extract-map.sh loader.h > map.bin
>> +
>> +4. **Generate the signature** with ``gen_sig``::
>> +
>> + scripts/hornet/gen_sig \
>> + --key signing_key.pem \
>> + --cert signing_key.x509 \
>> + --data insn.bin \
>> + --add map.bin:0 \
>> + --out sig.bin
>> +
>> +5. **Embed the signature** back into the header::
>> +
>> + scripts/hornet/write-sig.sh loader.h sig.bin > signed_loader.h
>> +
>> +6. **Build the loader program** using the signed header::
>> +
>> + cc -o loader loader.c -lbpf
>> +
>> +The resulting loader program will pass the embedded signature to the
>> +kernel when loading the eBPF program, enabling Hornet to verify it.
>> +
>> +Testing
>> +=======
>> +
>> +Self-tests are provided in ``tools/testing/selftests/hornet/``. The test
>> +suite builds a minimal eBPF program (``trivial.bpf.c``), signs it using
>> +the workflow described above, and verifies that the signed program loads
>> +successfully.
>> diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst
>> index b44ef68f6e4d..57f6e9fbe5fd 100644
>> --- a/Documentation/admin-guide/LSM/index.rst
>> +++ b/Documentation/admin-guide/LSM/index.rst
>> @@ -49,3 +49,4 @@ subdirectories.
>> SafeSetID
>> ipe
>> landlock
>> + Hornet
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 55af015174a5..6e91234a9ba4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11682,6 +11682,15 @@ S: Maintained
>> F: Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
>> F: drivers/iio/pressure/mprls0025pa*
>>
>> +HORNET SECURITY MODULE
>> +M: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
>> +L: linux-security-module@vger.kernel.org
>> +S: Supported
>> +T: git https://github.com/blaiseboscaccy/hornet.git
>> +F: Documentation/admin-guide/LSM/Hornet.rst
>> +F: scripts/hornet/
>> +F: security/hornet/
>> +
>> HP BIOSCFG DRIVER
>> M: Jorge Lopez <jorge.lopez2@hp.com>
>> L: platform-driver-x86@vger.kernel.org
>> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
>> index ebce402854de..bf852715aaea 100644
>> --- a/include/linux/oid_registry.h
>> +++ b/include/linux/oid_registry.h
>> @@ -150,6 +150,9 @@ enum OID {
>> OID_id_ml_dsa_65, /* 2.16.840.1.101.3.4.3.18 */
>> OID_id_ml_dsa_87, /* 2.16.840.1.101.3.4.3.19 */
>>
>> + /* Hornet LSM */
>> + OID_hornet_data, /* 2.25.316487325684022475439036912669789383960 */
>> +
>> OID__NR
>> };
>>
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> index 938593dfd5da..2ff9bcdd551e 100644
>> --- a/include/uapi/linux/lsm.h
>> +++ b/include/uapi/linux/lsm.h
>> @@ -65,6 +65,7 @@ struct lsm_ctx {
>> #define LSM_ID_IMA 111
>> #define LSM_ID_EVM 112
>> #define LSM_ID_IPE 113
>> +#define LSM_ID_HORNET 114
>>
>> /*
>> * LSM_ATTR_XXX definitions identify different LSM attributes
>> diff --git a/security/Kconfig b/security/Kconfig
>> index 6a4393fce9a1..283c4a103209 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -230,6 +230,7 @@ source "security/safesetid/Kconfig"
>> source "security/lockdown/Kconfig"
>> source "security/landlock/Kconfig"
>> source "security/ipe/Kconfig"
>> +source "security/hornet/Kconfig"
>>
>> source "security/integrity/Kconfig"
>>
>> @@ -274,7 +275,7 @@ config LSM
>> default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,ipe,bpf" if DEFAULT_SECURITY_APPARMOR
>> default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,ipe,bpf" if DEFAULT_SECURITY_TOMOYO
>> default "landlock,lockdown,yama,loadpin,safesetid,ipe,bpf" if DEFAULT_SECURITY_DAC
>> - default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,ipe,bpf"
>> + default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,ipe,hornet,bpf"
>> help
>> A comma-separated list of LSMs, in initialization order.
>> Any LSMs left off this list, except for those with order
>> diff --git a/security/Makefile b/security/Makefile
>> index 4601230ba442..b68cb56e419b 100644
>> --- a/security/Makefile
>> +++ b/security/Makefile
>> @@ -26,6 +26,7 @@ obj-$(CONFIG_CGROUPS) += device_cgroup.o
>> obj-$(CONFIG_BPF_LSM) += bpf/
>> obj-$(CONFIG_SECURITY_LANDLOCK) += landlock/
>> obj-$(CONFIG_SECURITY_IPE) += ipe/
>> +obj-$(CONFIG_SECURITY_HORNET) += hornet/
>>
>> # Object integrity file lists
>> obj-$(CONFIG_INTEGRITY) += integrity/
>> diff --git a/security/hornet/Kconfig b/security/hornet/Kconfig
>> new file mode 100644
>> index 000000000000..19406aa237ac
>> --- /dev/null
>> +++ b/security/hornet/Kconfig
>> @@ -0,0 +1,11 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +config SECURITY_HORNET
>> + bool "Hornet support"
>> + depends on SECURITY
>> + default n
>> + help
>> + This selects Hornet.
>> + Further information can be found in
>> + Documentation/admin-guide/LSM/Hornet.rst.
>> +
>> + If you are unsure how to answer this question, answer N.
>> diff --git a/security/hornet/Makefile b/security/hornet/Makefile
>> new file mode 100644
>> index 000000000000..26b6f954f762
>> --- /dev/null
>> +++ b/security/hornet/Makefile
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +obj-$(CONFIG_SECURITY_HORNET) := hornet.o
>> +
>> +hornet-y := hornet.asn1.o \
>> + hornet_lsm.o \
>> +
>> +$(obj)/hornet.asn1.o: $(obj)/hornet.asn1.c $(obj)/hornet.asn1.h
>> diff --git a/security/hornet/hornet.asn1 b/security/hornet/hornet.asn1
>> new file mode 100644
>> index 000000000000..c8d47b16b65d
>> --- /dev/null
>> +++ b/security/hornet/hornet.asn1
>> @@ -0,0 +1,13 @@
>> +-- SPDX-License-Identifier: BSD-3-Clause
>> +--
>> +-- Copyright (C) 2009 IETF Trust and the persons identified as authors
>> +-- of the code
>> +--
>> +-- https://www.rfc-editor.org/rfc/rfc5652#section-3
>> +
>> +HornetData ::= SET OF Map
>> +
>> +Map ::= SEQUENCE {
>> + index INTEGER ({ hornet_map_index }),
>> + sha OCTET STRING ({ hornet_map_hash })
>> +} ({ hornet_next_map })
>> diff --git a/security/hornet/hornet_lsm.c b/security/hornet/hornet_lsm.c
>> new file mode 100644
>> index 000000000000..6c821d6441fb
>> --- /dev/null
>> +++ b/security/hornet/hornet_lsm.c
>> @@ -0,0 +1,323 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Hornet Linux Security Module
>> + *
>> + * Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
>> + *
>> + * Copyright (C) 2026 Microsoft Corporation
>> + */
>> +
>> +#include <linux/lsm_hooks.h>
>> +#include <uapi/linux/lsm.h>
>> +#include <linux/bpf.h>
>> +#include <linux/verification.h>
>> +#include <crypto/public_key.h>
>> +#include <linux/module_signature.h>
>> +#include <crypto/pkcs7.h>
>> +#include <linux/sort.h>
>> +#include <linux/asn1_decoder.h>
>> +#include <linux/oid_registry.h>
>> +#include "hornet.asn1.h"
>> +
>> +#define MAX_USED_MAPS 64
>> +
>> +struct hornet_maps {
>> + bpfptr_t fd_array;
>> +};
>> +
>> +struct hornet_parse_context {
>> + int indexes[MAX_USED_MAPS];
>> + bool skips[MAX_USED_MAPS];
>> + unsigned char hashes[SHA256_DIGEST_SIZE * MAX_USED_MAPS];
>> + int hash_count;
>> +};
>
> I might include a brief comment at near the top of this file referencing
> the hash algorithm limitation in the Hornet docs, otherwise someone is
> surely going to advocate for hash agility improvements at some point.
>
Good idea.
>> +struct hornet_prog_security_struct {
>> + bool checked[MAX_USED_MAPS];
>> + unsigned char hashes[SHA256_DIGEST_SIZE * MAX_USED_MAPS];
>> +};
>> +
>> +struct hornet_map_security_struct {
>> + bool checked;
>> + int index;
>> +};
>> +
>> +struct lsm_blob_sizes hornet_blob_sizes __ro_after_init = {
>> + .lbs_bpf_map = sizeof(struct hornet_map_security_struct),
>> + .lbs_bpf_prog = sizeof(struct hornet_prog_security_struct),
>> +};
>> +
>> +static inline struct hornet_prog_security_struct *
>> +hornet_bpf_prog_security(struct bpf_prog *prog)
>> +{
>> + return prog->aux->security + hornet_blob_sizes.lbs_bpf_prog;
>> +}
>> +
>> +static inline struct hornet_map_security_struct *
>> +hornet_bpf_map_security(struct bpf_map *map)
>> +{
>> + return map->security + hornet_blob_sizes.lbs_bpf_map;
>> +}
>> +
>> +static int hornet_verify_hashes(struct hornet_maps *maps,
>> + struct hornet_parse_context *ctx,
>> + struct bpf_prog *prog)
>> +{
>> + int map_fd;
>> + u32 i;
>> + struct bpf_map *map;
>> + int err = 0;
>> + unsigned char hash[SHA256_DIGEST_SIZE];
>> + struct hornet_prog_security_struct *security = hornet_bpf_prog_security(prog);
>> + struct hornet_map_security_struct *map_security;
>> +
>> + for (i = 0; i < ctx->hash_count; i++) {
>> + if (ctx->skips[i]) {
>> + security->checked[i] = false;
>
> I'm not going to argue against an explicit false assignement here, but
> as a FYI, when the LSM framework allocates the various object blobs it
> (re)sets the blob memory to zero via kzalloc(). Even if/when the LSM
> framwork moves to some other allocation scheme we will still need to keep
> that reset-to-zero behavior.
>
> The same applies to the BPF map blobs.
>
Yeah, I'm fine with that change.
>> + continue;
>> + }
>> +
>> + err = copy_from_bpfptr_offset(&map_fd, maps->fd_array,
>> + ctx->indexes[i] * sizeof(map_fd),
>> + sizeof(map_fd));
>> + if (err < 0)
>> + return LSM_INT_VERDICT_BADSIG;
>> +
>> + CLASS(fd, f)(map_fd);
>> + if (fd_empty(f))
>> + return LSM_INT_VERDICT_BADSIG;
>> + if (unlikely(fd_file(f)->f_op != &bpf_map_fops))
>> + return LSM_INT_VERDICT_BADSIG;
>
> I'm wondering if it is worth defining a generic LSM_INT_VERDICT_FAULT
> verdict to indicate a system error when verifying the integrity rather
> than a bad signature. Yes, the enforcement action will likely be the
> same, but it might help when debugging or chasing forensic data.
>
I like that. At this point there was actually a signature that was
verified against the progam's instruction making VERDICT_BAD_SIG
misleading.
>> + map = fd_file(f)->private_data;
>> + if (!map->frozen)
>> + return LSM_INT_VERDICT_BADSIG;
>> +
>> + map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
>> +
>> + err = memcmp(hash, &ctx->hashes[i * SHA256_DIGEST_SIZE],
>> + SHA256_DIGEST_SIZE);
>> + if (err)
>> + return LSM_INT_VERDICT_BADSIG;
>> +
>> + security->checked[i] = true;
>> + memcpy(&security->hashes[i * SHA256_DIGEST_SIZE], hash, SHA256_DIGEST_SIZE);
>> + map_security = hornet_bpf_map_security(map);
>> + map_security->checked = true;
>> + map_security->index = i;
>> + }
>> + return LSM_INT_VERDICT_OK;
>> +}
>> +
>> +int hornet_next_map(void *context, size_t hdrlen,
>> + unsigned char tag,
>> + const void *value, size_t vlen)
>> +{
>> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
>> +
>> + ctx->hash_count++;
>
> Do we need a check here to ensure that ctx->hash_count doesn't exceed
> MAX_USED_MAPS? If not here, where do we ensure we don't blow past
> MAX_USED_MAPS?
>
Good idea.
> What does Hornet do if the number of hashed maps is greater then
> MAX_USED_MAPS? I'm guessing we would want it to return an error and
> fail the load?
>
I'm thinking that's a degenerate case of VERDICT_BADSIG and we should
pass that to the downstream policy handler.
>> + return 0;
>> +}
>> +
>> +int hornet_map_index(void *context, size_t hdrlen,
>> + unsigned char tag,
>> + const void *value, size_t vlen)
>> +{
>> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
>> +
>> + if (vlen > 1)
>> + return -EINVAL;
>> +
>> + ctx->indexes[ctx->hash_count] = *(u8 *)value;
>> + return 0;
>> +}
>> +
>> +int hornet_map_hash(void *context, size_t hdrlen,
>> + unsigned char tag,
>> + const void *value, size_t vlen)
>> +
>> +{
>> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
>> +
>> + if (vlen != SHA256_DIGEST_SIZE && vlen != 0)
>> + return -EINVAL;
>> +
>> + if (vlen) {
>> + ctx->skips[ctx->hash_count] = false;
>> + memcpy(&ctx->hashes[ctx->hash_count * SHA256_DIGEST_SIZE], value, vlen);
>> + } else
>> + ctx->skips[ctx->hash_count] = true;
>> +
>> + return 0;
>> +}
>> +
>> +static int hornet_check_program(struct bpf_prog *prog, union bpf_attr *attr,
>> + struct bpf_token *token, bool is_kernel)
>> +{
>> + struct hornet_maps maps = {0};
>> + bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
>> + struct pkcs7_message *msg;
>> + struct hornet_parse_context *ctx;
>> + void *sig;
>> + int err;
>> + const void *authattrs;
>> + size_t authattrs_len;
>> +
>> + if (!attr->signature)
>> + return LSM_INT_VERDICT_UNSIGNED;
>> +
>> + ctx = kzalloc(sizeof(struct hornet_parse_context), GFP_KERNEL);
>> + if (!ctx)
>> + return -ENOMEM;
>
> I think I mentioned this previously, but let me repeat myself in case I
> didn't ... we don't want to mix LSM_INT_VERDICT enums and errno values
> in the return value. Yes, you can probably get away with it in the
> majority of cases, but I worry it is a problem waiting to happen. I
> count only four parameters right now, so adding a verdict enum pointer
> shouldn't be too difficult.
>
Sounds good. I can refactor that.
>> + maps.fd_array = make_bpfptr(attr->fd_array, is_kernel);
>> + sig = kzalloc(attr->signature_size, GFP_KERNEL);
>> + if (!sig) {
>> + err = -ENOMEM;
>> + goto out;
>> + }
>> + err = copy_from_bpfptr(sig, usig, attr->signature_size);
>> + if (err != 0)
>> + goto cleanup_sig;
>> +
>> + err = verify_pkcs7_signature(prog->insnsi, prog->len * sizeof(struct bpf_insn),
>> + sig, attr->signature_size, VERIFY_USE_SECONDARY_KEYRING,
>> + VERIFYING_BPF_SIGNATURE, NULL, NULL);
>> + if (err < 0) {
>> + err = LSM_INT_VERDICT_BADSIG;
>> + goto cleanup_sig;
>> + }
>> +
>> + msg = pkcs7_parse_message(sig, attr->signature_size);
>> + if (IS_ERR(msg)) {
>> + err = LSM_INT_VERDICT_BADSIG;
>> + goto cleanup_sig;
>> + }
>> +
>> + if (validate_pkcs7_trust(msg, VERIFY_USE_SECONDARY_KEYRING)) {
>> + err = LSM_INT_VERDICT_PARTIALSIG;
>> + goto cleanup_msg;
>> + }
>> + if (pkcs7_get_authattr(msg, OID_hornet_data,
>> + &authattrs, &authattrs_len) == -ENODATA) {
>> + err = LSM_INT_VERDICT_PARTIALSIG;
>> + goto cleanup_msg;
>> + }
>> +
>> + err = asn1_ber_decoder(&hornet_decoder, ctx, authattrs, authattrs_len);
>> + if (err < 0 || authattrs == NULL) {
>> + err = LSM_INT_VERDICT_PARTIALSIG;
>> + goto cleanup_msg;
>> + }
>> + err = hornet_verify_hashes(&maps, ctx, prog);
>> +
>> +cleanup_msg:
>> + pkcs7_free_message(msg);
>> +cleanup_sig:
>> + kfree(sig);
>> +out:
>> + kfree(ctx);
>> + return err;
>> +}
>> +
>> +static const struct lsm_id hornet_lsmid = {
>> + .name = "hornet",
>> + .id = LSM_ID_HORNET,
>> +};
>> +
>> +static int hornet_bpf_prog_load_integrity(struct bpf_prog *prog, union bpf_attr *attr,
>> + struct bpf_token *token, bool is_kernel)
>> +{
>> + int result = hornet_check_program(prog, attr, token, is_kernel);
>
> Can you explain a bit why we check for the kernel flag in hornet_bpf(),
> but not here? It may be that a brief comment in hornet_bpf() explaining
> the kernel flag exception would be helpful.
>
I can add a comment. The gist of this is that in hornet_bpf(), anything
that had originated from kernel space we assume has already been
checked, in some form or another, so we don't bother checking the
intergity of any maps. In this function, hornet doesn't make
any opinion on that and delegates that to the downstream policy
enforcement.
>> + if (result < 0)
>> + return result;
>> +
>> + return security_bpf_prog_load_post_integrity(prog, attr, token, is_kernel,
>> + &hornet_lsmid, result);
>> +}
>> +
>> +static int hornet_verify_map(struct bpf_prog *prog, int index)>> +{
>> + unsigned char hash[SHA256_DIGEST_SIZE];
>> + int i;
>> + struct bpf_map *map;
>> + struct hornet_prog_security_struct *security = hornet_bpf_prog_security(prog);
>> + struct hornet_map_security_struct *map_security;
>> +
>> + if (!security->checked[index])
>> + return 0;
>> +
>> + for (i = 0; i < prog->aux->used_map_cnt; i++) {
>> + map = prog->aux->used_maps[i];
>> + map_security = hornet_bpf_map_security(map);
>> + if (map_security->index != index)
>> + continue;
>> +
>> + if (!map->frozen)
>> + return -EINVAL;
>
> Unless there is serious tampering going on we should never see an
> unfrozen map here, yes?
Someone could write a custom loader program in userspace that doesn't
freeze the map. Having the map frozen to check the hash wasn't enforced
in the BPF subsystem until very recently and it's still only enforced in
the syscall handler, not the actual hash-checking itself.
>
> We probably also want to use a return value other than -EINVAL as this
> is a access/permission denial. I would think -EACCES or -EPERM would be
> more appropriate.
>
I'm fine with that.
>> + map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
>> + if (memcmp(hash, &security->hashes[index * SHA256_DIGEST_SIZE],
>> + SHA256_DIGEST_SIZE) != 0)
>
> Presumably this is just being extra careful?
>
Yes. There are some small windows where someone could engage in some
shennagians between loader verification and when the program is actually
run. The check here verifies that the hashes are seeing now still match up
with the hashes we calculated from the uapi fd array.
>> + return -EINVAL;
>
> See above, -EACCES or -EPERM is likely a better choice here.
>
I'm fine with that.
>> + else
>> + return 0;
>> + }
>> + return -EINVAL;
>
> See above.
>
>> +}
>> +
>> +static int hornet_check_prog_maps(u32 ufd)
>> +{
>> + CLASS(fd, f)(ufd);
>> + struct bpf_prog *prog;
>> + int i, result = 0;
>> +
>> + if (fd_empty(f))
>> + return -EBADF;
>> + if (fd_file(f)->f_op != &bpf_prog_fops)
>> + return -EINVAL;
>> +
>> + prog = fd_file(f)->private_data;
>> +
>> + mutex_lock(&prog->aux->used_maps_mutex);
>> + if (!prog->aux->used_map_cnt)
>> + goto out;
>> +
>> + for (i = 0; i < prog->aux->used_map_cnt; i++) {
>> + result = hornet_verify_map(prog, i);
>> + if (result)
>> + goto out;
>> + }
>> +out:
>> + mutex_unlock(&prog->aux->used_maps_mutex);
>> + return result;
>> +}
>> +
>> +static int hornet_bpf(int cmd, union bpf_attr *attr, unsigned int size, bool kernel)
>> +{
>> + if (cmd != BPF_PROG_RUN)
>> + return 0;
>> + if (kernel)
>> + return 0;
>> +
>> + return hornet_check_prog_maps(attr->test.prog_fd);
>> +}
>> +
>> +static struct security_hook_list hornet_hooks[] __ro_after_init = {
>> + LSM_HOOK_INIT(bpf_prog_load_integrity, hornet_bpf_prog_load_integrity),
>> + LSM_HOOK_INIT(bpf, hornet_bpf),
>> +};
>> +
>> +static int __init hornet_init(void)
>> +{
>> + pr_info("Hornet: eBPF signature verification enabled\n");
>> + security_add_hooks(hornet_hooks, ARRAY_SIZE(hornet_hooks), &hornet_lsmid);
>> + return 0;
>> +}
>> +
>> +DEFINE_LSM(hornet) = {
>> + .id = &hornet_lsmid,
>> + .blobs = &hornet_blob_sizes,
>> + .init = hornet_init,
>> +};
>> --
>> 2.52.0
>
> --
> paul-moore.com
Thanks Paul.
-blaise
^ permalink raw reply
* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-03-19 2:22 UTC (permalink / raw)
To: casey
Cc: jmorris, linux-kernel, linux-security-module, paul, serge,
yangfeng59949
In-Reply-To: <df35542e-d58f-47db-8a4f-92698281a69a@schaufler-ca.com>
On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
> 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.
I'm very sorry, I didn't quite understand what you meant.
Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.
Therefore, is it the expected behavior that simply using
SEC("lsm/xfrm_decode_session")
int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
return -1;
}
would cause a kernel panic? If not, and if the BUG_ON check is still necessary,
then does it mean we need to modify the return value validation logic in the BPF
verifier to ensure that only BPF programs returning 0 are accepted for this hook?
Thanks.
^ permalink raw reply
* Re: [PATCH v1] security/safesetid: fix comment and error handling
From: Micah Morton @ 2026-03-19 3:27 UTC (permalink / raw)
To: yanwei.gao; +Cc: paul, linux-security-module
In-Reply-To: <CAJ-EccNLTuj74vKsNQFnOSXgjdyRpUrFVARsuqn6VNatQu33WA@mail.gmail.com>
On Wed, Mar 18, 2026 at 1:49 PM Micah Morton <mortonm@chromium.org> wrote:
>
> 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?
I guess securityfs_remove() just returns right away if the
'policy_dir' pointer IS_ERR, so either way is fine. I don't really see
a reason to change this though.. seems better to err on the side of
calling the securityfs_remove() function even if its currently a
no-op.
>
> > }
> >
> > uid_policy_file = securityfs_create_file("uid_allowlist_policy", 0600,
> > --
> > 2.43.5
> >
^ permalink raw reply
* [PATCH] apparmor: Fix string overrun due to missing termination
From: Daniel J Blueman @ 2026-03-19 6:24 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel,
stable, Daniel J Blueman
When booting Ubuntu 26.04 with Linux 7.0-rc4 on an ARM64 Qualcomm
Snapdragon X1 we see a string buffer overrun:
BUG: KASAN: slab-out-of-bounds in aa_dfa_match (security/apparmor/match.c:535)
Read of size 1 at addr ffff0008901cc000 by task snap-update-ns/2120
CPU: 5 UID: 60578 PID: 2120 Comm: snap-update-ns Not tainted 7.0.0-rc4+ #22 PREEMPTLAZY
Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN60WW 09/11/2025
Call trace:
show_stack (arch/arm64/kernel/stacktrace.c:501) (C)
dump_stack_lvl (lib/dump_stack.c:122)
print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
kasan_report (mm/kasan/report.c:597)
__asan_report_load1_noabort (mm/kasan/report_generic.c:378)
aa_dfa_match (security/apparmor/match.c:535)
match_mnt_path_str (security/apparmor/mount.c:244 security/apparmor/mount.c:336)
match_mnt (security/apparmor/mount.c:371)
aa_bind_mount (security/apparmor/mount.c:447 (discriminator 4))
apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
security_sb_mount (security/security.c:1062 (discriminator 31))
path_mount (fs/namespace.c:4101)
__arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
do_el0_svc (arch/arm64/kernel/syscall.c:152)
el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
el0t_64_sync (arch/arm64/kernel/entry.S:596)
Allocated by task 2120:
kasan_save_stack (mm/kasan/common.c:58)
kasan_save_track (./arch/arm64/include/asm/current.h:19 mm/kasan/common.c:70 mm/kasan/common.c:79)
kasan_save_alloc_info (mm/kasan/generic.c:571)
__kasan_kmalloc (mm/kasan/common.c:419)
__kmalloc_noprof (./include/linux/kasan.h:263 mm/slub.c:5260 mm/slub.c:5272)
aa_get_buffer (security/apparmor/lsm.c:2201)
aa_bind_mount (security/apparmor/mount.c:442)
apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
security_sb_mount (security/security.c:1062 (discriminator 31))
path_mount (fs/namespace.c:4101)
__arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
do_el0_svc (arch/arm64/kernel/syscall.c:152)
el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
el0t_64_sync (arch/arm64/kernel/entry.S:596)
The buggy address belongs to the object at ffff0008901ca000
which belongs to the cache kmalloc-rnd-06-8k of size 8192
The buggy address is located 0 bytes to the right of
allocated 8192-byte region [ffff0008901ca000, ffff0008901cc000)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9101c8
head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:-1 pincount:0
flags: 0x8000000000000040(head|zone=2)
page_type: f5(slab)
raw: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
raw: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
head: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
head: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
head: 8000000000000003 fffffdffe2407201 fffffdffffffffff 00000000ffffffff
head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff0008901cbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff0008901cbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff0008901cc000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff0008901cc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff0008901cc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
This was introduced by previous incorrect conversion from strcpy(). Fix it
by adding the missing terminator.
Signed-off-by: Daniel J Blueman <daniel@quora.org>
Fixes: 93d4dbdc8da0 ("apparmor: Replace deprecated strcpy in d_namespace_path")
---
security/apparmor/path.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index 65a0ca5cc1bd..2494e8101538 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -164,14 +164,16 @@ static int d_namespace_path(const struct path *path, char *buf, char **name,
}
out:
- /* Append "/" to directory paths, except for root "/" which
- * already ends in a slash.
+ /* Append "/" to directory paths and reterminate string, except for
+ * root "/" which already ends in a slash.
*/
if (!error && isdir) {
bool is_root = (*name)[0] == '/' && (*name)[1] == '\0';
- if (!is_root)
+ if (!is_root) {
buf[aa_g_path_max - 2] = '/';
+ buf[aa_g_path_max - 1] = '\0';
+ }
}
return error;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] apparmor: Fix string overrun due to missing termination
From: Greg KH @ 2026-03-19 6:40 UTC (permalink / raw)
To: Daniel J Blueman
Cc: John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Thorsten Blum, apparmor, linux-security-module, linux-kernel,
stable
In-Reply-To: <20260319062433.17648-1-daniel@quora.org>
On Thu, Mar 19, 2026 at 02:24:32PM +0800, Daniel J Blueman wrote:
> When booting Ubuntu 26.04 with Linux 7.0-rc4 on an ARM64 Qualcomm
> Snapdragon X1 we see a string buffer overrun:
>
> BUG: KASAN: slab-out-of-bounds in aa_dfa_match (security/apparmor/match.c:535)
> Read of size 1 at addr ffff0008901cc000 by task snap-update-ns/2120
>
> CPU: 5 UID: 60578 PID: 2120 Comm: snap-update-ns Not tainted 7.0.0-rc4+ #22 PREEMPTLAZY
> Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN60WW 09/11/2025
> Call trace:
> show_stack (arch/arm64/kernel/stacktrace.c:501) (C)
> dump_stack_lvl (lib/dump_stack.c:122)
> print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
> kasan_report (mm/kasan/report.c:597)
> __asan_report_load1_noabort (mm/kasan/report_generic.c:378)
> aa_dfa_match (security/apparmor/match.c:535)
> match_mnt_path_str (security/apparmor/mount.c:244 security/apparmor/mount.c:336)
> match_mnt (security/apparmor/mount.c:371)
> aa_bind_mount (security/apparmor/mount.c:447 (discriminator 4))
> apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
> security_sb_mount (security/security.c:1062 (discriminator 31))
> path_mount (fs/namespace.c:4101)
> __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
> invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
> el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
> do_el0_svc (arch/arm64/kernel/syscall.c:152)
> el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
> el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
> el0t_64_sync (arch/arm64/kernel/entry.S:596)
>
> Allocated by task 2120:
> kasan_save_stack (mm/kasan/common.c:58)
> kasan_save_track (./arch/arm64/include/asm/current.h:19 mm/kasan/common.c:70 mm/kasan/common.c:79)
> kasan_save_alloc_info (mm/kasan/generic.c:571)
> __kasan_kmalloc (mm/kasan/common.c:419)
> __kmalloc_noprof (./include/linux/kasan.h:263 mm/slub.c:5260 mm/slub.c:5272)
> aa_get_buffer (security/apparmor/lsm.c:2201)
> aa_bind_mount (security/apparmor/mount.c:442)
> apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
> security_sb_mount (security/security.c:1062 (discriminator 31))
> path_mount (fs/namespace.c:4101)
> __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
> invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
> el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
> do_el0_svc (arch/arm64/kernel/syscall.c:152)
> el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
> el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
> el0t_64_sync (arch/arm64/kernel/entry.S:596)
>
> The buggy address belongs to the object at ffff0008901ca000
> which belongs to the cache kmalloc-rnd-06-8k of size 8192
> The buggy address is located 0 bytes to the right of
> allocated 8192-byte region [ffff0008901ca000, ffff0008901cc000)
>
> The buggy address belongs to the physical page:
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9101c8
> head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:-1 pincount:0
> flags: 0x8000000000000040(head|zone=2)
> page_type: f5(slab)
> raw: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
> raw: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
> head: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
> head: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
> head: 8000000000000003 fffffdffe2407201 fffffdffffffffff 00000000ffffffff
> head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff0008901cbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff0008901cbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff0008901cc000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff0008901cc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff0008901cc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>
> This was introduced by previous incorrect conversion from strcpy(). Fix it
> by adding the missing terminator.
>
> Signed-off-by: Daniel J Blueman <daniel@quora.org>
> Fixes: 93d4dbdc8da0 ("apparmor: Replace deprecated strcpy in d_namespace_path")
> ---
> security/apparmor/path.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/security/apparmor/path.c b/security/apparmor/path.c
> index 65a0ca5cc1bd..2494e8101538 100644
> --- a/security/apparmor/path.c
> +++ b/security/apparmor/path.c
> @@ -164,14 +164,16 @@ static int d_namespace_path(const struct path *path, char *buf, char **name,
> }
>
> out:
> - /* Append "/" to directory paths, except for root "/" which
> - * already ends in a slash.
> + /* Append "/" to directory paths and reterminate string, except for
> + * root "/" which already ends in a slash.
> */
> if (!error && isdir) {
> bool is_root = (*name)[0] == '/' && (*name)[1] == '\0';
>
> - if (!is_root)
> + if (!is_root) {
> buf[aa_g_path_max - 2] = '/';
> + buf[aa_g_path_max - 1] = '\0';
> + }
> }
>
> return error;
> --
> 2.53.0
>
>
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
^ permalink raw reply
* Re: [apparmor][PATCH] apparmor: fix incorrect success return value in unpack_tag_headers()
From: Massimiliano Pellizzer @ 2026-03-19 8:23 UTC (permalink / raw)
To: John Johansen; +Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <779f2d5b-a5af-4137-a1ee-78dc9fed58e1@canonical.com>
On Wed, Mar 18, 2026 at 6:53 AM John Johansen
<john.johansen@canonical.com> wrote:
>
> On 2/10/26 09:21, Massimiliano Pellizzer wrote:
> > unpack_tag_headers() returns `true` (1) on success instead of 0.
> > Since it's caller unpack_tags() checks the return value with
> > `if (error)`, a non-zero success value is incorrectly treated as
> > a failure, causing tag header unpacking to always even if the data
> > is well-formed.
> >
> > Change the success return in unpack_tag_headers() from `true` to 0.
> >
> > Fixes: 3d28e2397af7 ("apparmor: add support loading per permission tagging")
> > Signed-off-by: Massimiliano Pellizzer <mpellizzer.dev@gmail.com>
>
> sorry, my reply to this seems to have failed. This was pulled in for the
> 7.0 PR
>
> Acked-by: John Johansen <john.johansen@canonical.com>
>
>
> > ---
> > security/apparmor/policy_unpack.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> > index dc908e1f5a88..221208788025 100644
> > --- a/security/apparmor/policy_unpack.c
> > +++ b/security/apparmor/policy_unpack.c
> > @@ -825,7 +825,7 @@ static int unpack_tag_headers(struct aa_ext *e, struct aa_tags_struct *tags)
> > tags->hdrs.size = size;
> > tags->hdrs.table = hdrs;
> > AA_DEBUG(DEBUG_UNPACK, "headers %ld size %d", (long) hdrs, size);
> > - return true;
> > + return 0;
> >
> > fail:
> > kfree_sensitive(hdrs);
>
Hello JJ,
I don't see this patch being part of v7.0-rc4:
$ git --no-pager log --grep "apparmor: fix incorrect success return
value in unpack"
I don't see the change applied to the apparmor-next branch either
(https://gitlab.com/apparmor/apparmor-kernel/-/blob/apparmor-next/security/apparmor/policy_unpack.c).
---
Massimiliano Pellizzer
^ permalink raw reply
* [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 12:46 UTC (permalink / raw)
To: Christian Brauner
Cc: Al Viro, Miklos Szeredi, Paul Moore, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
Skip setting them for O_PATH file, so that the O_PATH file could be
opened with a negative dentry.
This is not something that a user should be able to do, but vfs code,
such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
before instantiating the dentry.
Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
Christian,
This patch fixes the syzbot report [1] that the
backing_file_user_path_file() patch [2] introduces.
This is not the only possible fix, but it is the cleanest one IMO.
There is a small risk in introducing a state of an O_PATH file with
NULL f_inode, but I (and the bots that I asked) did not find any
obvious risk in this state.
Note that specifically, the user path inode is accessed via d_inode()
and not via file_inode(), which makes this safe for file_user_inode()
callers.
BTW, I missed this regression with the original patch because I
only ran the quick overlayfs sanity test.
Now I ran a full quick fstest cycle and verified that the O_TMPFILE
test case is covered and that the bug is detected.
WDYT?
Thanks,
Amir.
[1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
[2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
fs/open.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 91f1139591abe..2004a8c0d9c97 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
path_get(&f->f_path);
f->f_inode = inode;
- f->f_mapping = inode->i_mapping;
- f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
- f->f_sb_err = file_sample_sb_err(f);
if (unlikely(f->f_flags & O_PATH)) {
f->f_mode = FMODE_PATH | FMODE_OPENED;
@@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
return 0;
}
+ f->f_mapping = inode->i_mapping;
+ f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
+ f->f_sb_err = file_sample_sb_err(f);
+
if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
i_readcount_inc(inode);
} else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Christian Brauner @ 2026-03-19 13:13 UTC (permalink / raw)
To: Amir Goldstein
Cc: Al Viro, Miklos Szeredi, Paul Moore, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <20260319124616.1544415-1-amir73il@gmail.com>
On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> Skip setting them for O_PATH file, so that the O_PATH file could be
> opened with a negative dentry.
>
> This is not something that a user should be able to do, but vfs code,
> such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> before instantiating the dentry.
>
> Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>
> Christian,
>
> This patch fixes the syzbot report [1] that the
> backing_file_user_path_file() patch [2] introduces.
>
> This is not the only possible fix, but it is the cleanest one IMO.
> There is a small risk in introducing a state of an O_PATH file with
> NULL f_inode, but I (and the bots that I asked) did not find any
> obvious risk in this state.
>
> Note that specifically, the user path inode is accessed via d_inode()
> and not via file_inode(), which makes this safe for file_user_inode()
> callers.
>
> BTW, I missed this regression with the original patch because I
> only ran the quick overlayfs sanity test.
>
> Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> test case is covered and that the bug is detected.
>
> WDYT?
>
> Thanks,
> Amir.
>
> [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
>
> fs/open.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index 91f1139591abe..2004a8c0d9c97 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
>
> path_get(&f->f_path);
> f->f_inode = inode;
> - f->f_mapping = inode->i_mapping;
> - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> - f->f_sb_err = file_sample_sb_err(f);
>
> if (unlikely(f->f_flags & O_PATH)) {
> f->f_mode = FMODE_PATH | FMODE_OPENED;
> @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> return 0;
> }
>
> + f->f_mapping = inode->i_mapping;
> + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> + f->f_sb_err = file_sample_sb_err(f);
> +
> if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> i_readcount_inc(inode);
> } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
I think this is really ugly and I'm really unhappy that we should adjust
initialization of generic vfs code for this. My preference is to push
the pain into the backing file stuff. And my ultimate preference is for
this backing file stuff to be removed again for a simple struct path.
We're working around some more fundamental cleanup here imho.
^ permalink raw reply
* Re: [PATCH 12/61] quota: Prefer IS_ERR_OR_NULL over manual NULL check
From: Jan Kara @ 2026-03-19 14:13 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Jan Kara
In-Reply-To: <20260310-b4-is_err_or_null-v1-12-bd63b656022d@avm.de>
On Tue 10-03-26 12:48:38, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
>
> To: Jan Kara <jack@suse.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
Thanks for the patch but frankly I find the original variant clearer wrt
what is going on. So I prefer to keep the code as is.
Honza
> ---
> fs/quota/quota.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 33bacd70758007129e0375bab44d7431195ec441..2e09fc247d0cf45b9e83a4f8a0be7ea694c8c2a1 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -965,7 +965,7 @@ SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
> else
> drop_super_exclusive(sb);
> out:
> - if (pathp && !IS_ERR(pathp))
> + if (!IS_ERR_OR_NULL(pathp))
> path_put(pathp);
> return ret;
> }
>
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Mimi Zohar @ 2026-03-19 14:28 UTC (permalink / raw)
To: Chris Fenner
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: <CAMigqh2kW_srDgnOs+1t=sk9Q9jJgaQVswO0ZRHhV-6zrOAZ6A@mail.gmail.com>
On Wed, 2026-03-18 at 10:36 -0700, Chris Fenner wrote:
> 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.
Agreed. Thanks, Chris! FYI, we raised concerns about IMA performance with the
TPM HMAC and encrypted feature while it was being developed. James had some
ideas, at the time, as to how to resolve the performance issue for IMA. Yet it
was upstreamed without those changes and with CONFIG_TCG_TPM2_HMAC enabled by
default on x86 systems.
Jarkko has queued this patch in the "queue" branch, without indicating whether
it will eventually be upstreamed or not.
Mimi
^ permalink raw reply
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 14:50 UTC (permalink / raw)
To: Christian Brauner, Paul Moore
Cc: Al Viro, Miklos Szeredi, Gao Xiang, linux-security-module,
selinux, linux-erofs, linux-fsdevel, linux-unionfs,
syzbot+f34aab278bf5d664e2be
In-Reply-To: <20260319-unentbehrlich-reitturnier-f78d9fb01230@brauner>
On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > Skip setting them for O_PATH file, so that the O_PATH file could be
> > opened with a negative dentry.
> >
> > This is not something that a user should be able to do, but vfs code,
> > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > before instantiating the dentry.
> >
> > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Christian,
> >
> > This patch fixes the syzbot report [1] that the
> > backing_file_user_path_file() patch [2] introduces.
> >
> > This is not the only possible fix, but it is the cleanest one IMO.
> > There is a small risk in introducing a state of an O_PATH file with
> > NULL f_inode, but I (and the bots that I asked) did not find any
> > obvious risk in this state.
> >
> > Note that specifically, the user path inode is accessed via d_inode()
> > and not via file_inode(), which makes this safe for file_user_inode()
> > callers.
> >
> > BTW, I missed this regression with the original patch because I
> > only ran the quick overlayfs sanity test.
> >
> > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > test case is covered and that the bug is detected.
> >
> > WDYT?
> >
> > Thanks,
> > Amir.
> >
> > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> >
> > fs/open.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/open.c b/fs/open.c
> > index 91f1139591abe..2004a8c0d9c97 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> >
> > path_get(&f->f_path);
> > f->f_inode = inode;
> > - f->f_mapping = inode->i_mapping;
> > - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > - f->f_sb_err = file_sample_sb_err(f);
> >
> > if (unlikely(f->f_flags & O_PATH)) {
> > f->f_mode = FMODE_PATH | FMODE_OPENED;
> > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > return 0;
> > }
> >
> > + f->f_mapping = inode->i_mapping;
> > + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > + f->f_sb_err = file_sample_sb_err(f);
> > +
> > if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > i_readcount_inc(inode);
> > } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
>
> I think this is really ugly and I'm really unhappy that we should adjust
> initialization of generic vfs code for this. My preference is to push
> the pain into the backing file stuff. And my ultimate preference is for
> this backing file stuff to be removed again for a simple struct path.
> We're working around some more fundamental cleanup here imho.
Fair enough, we can rip the entire thing from vfs if you don't like it.
The user path file can be opened and stored internally by selinux
without adding all the associated risks in vfs.
Paul,
Please see compile tested code at:
https://github.com/amir73il/linux/commits/user_path_file/
Feel free to use my patch as is or reorder/squash/rebase as you see fit.
Feel free to attribute my contribution any way you see fit, just pls cc me for
review when you post v2.
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH v6] backing_file: store user_path_file
From: Amir Goldstein @ 2026-03-19 14:53 UTC (permalink / raw)
To: Paul Moore
Cc: Christian Brauner, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs
In-Reply-To: <CAHC9VhTKvOzMS2ZyawE=qBN-EpyucfxvwcHZYjXF5zugRhE2rw@mail.gmail.com>
On Thu, Mar 19, 2026 at 12:47 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Mar 18, 2026 at 9:13 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > 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 file_ref_init() helper was changed to accept the FILE_REF_ constant
> > instead of the fake +1 integer count.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Christian,
> >
> > My v5 patch was sent by Paul along with his LSM/selinux pataches [1].
> > Here are the changes you requested.
> >
> > I removed the ACKs and Tested-by because of the changes.
> >
> > Thanks,
> > Amir.
> >
> > Changes since v5:
> > - Restore file_ref_init() helper without refcnt -1 offset
> > - Future proofing errors from backing_file_open_user_path()
> >
> > [1] https://lore.kernel.org/r/20260316213606.374109-6-paul@paul-moore.com/
> >
> > fs/backing-file.c | 26 ++++++++++--------
> > fs/erofs/ishare.c | 13 +++++++--
> > 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 | 4 +--
> > 9 files changed, 103 insertions(+), 34 deletions(-)
>
> Still works for me. I'm going to update lsm/stable-7.0 with this
> patch so we can get some more linux-next testing.
>
> Tested-by: Paul Moore <paul@paul-moore.com>
>
Paul,
As you saw, syzbot found a nasty bug in this patch and it is too hard
to fix it without introducing more hazards.
Therefore, per Christian's request I am withdrawing this patch.
Please see compile tested alternative solution for selinux without
intrusive vfs change at:
https://github.com/amir73il/linux/commits/user_path_file/
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Paul Moore @ 2026-03-19 15:54 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, Al Viro, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <CAOQ4uxiS1uM=mn4q6CQfganba1XcqyXYmXfQceWdfUVRFK_Pvg@mail.gmail.com>
On Thu, Mar 19, 2026 at 10:50 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
> > On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > > Skip setting them for O_PATH file, so that the O_PATH file could be
> > > opened with a negative dentry.
> > >
> > > This is not something that a user should be able to do, but vfs code,
> > > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > > before instantiating the dentry.
> > >
> > > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >
> > > Christian,
> > >
> > > This patch fixes the syzbot report [1] that the
> > > backing_file_user_path_file() patch [2] introduces.
> > >
> > > This is not the only possible fix, but it is the cleanest one IMO.
> > > There is a small risk in introducing a state of an O_PATH file with
> > > NULL f_inode, but I (and the bots that I asked) did not find any
> > > obvious risk in this state.
> > >
> > > Note that specifically, the user path inode is accessed via d_inode()
> > > and not via file_inode(), which makes this safe for file_user_inode()
> > > callers.
> > >
> > > BTW, I missed this regression with the original patch because I
> > > only ran the quick overlayfs sanity test.
> > >
> > > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > > test case is covered and that the bug is detected.
> > >
> > > WDYT?
> > >
> > > Thanks,
> > > Amir.
> > >
> > > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> > >
> > > fs/open.c | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/open.c b/fs/open.c
> > > index 91f1139591abe..2004a8c0d9c97 100644
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> > >
> > > path_get(&f->f_path);
> > > f->f_inode = inode;
> > > - f->f_mapping = inode->i_mapping;
> > > - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > - f->f_sb_err = file_sample_sb_err(f);
> > >
> > > if (unlikely(f->f_flags & O_PATH)) {
> > > f->f_mode = FMODE_PATH | FMODE_OPENED;
> > > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > > return 0;
> > > }
> > >
> > > + f->f_mapping = inode->i_mapping;
> > > + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > + f->f_sb_err = file_sample_sb_err(f);
> > > +
> > > if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > > i_readcount_inc(inode);
> > > } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
> >
> > I think this is really ugly and I'm really unhappy that we should adjust
> > initialization of generic vfs code for this. My preference is to push
> > the pain into the backing file stuff. And my ultimate preference is for
> > this backing file stuff to be removed again for a simple struct path.
> > We're working around some more fundamental cleanup here imho.
>
> Fair enough, we can rip the entire thing from vfs if you don't like it.
> The user path file can be opened and stored internally by selinux
> without adding all the associated risks in vfs.
>
> Paul,
>
> Please see compile tested code at:
> https://github.com/amir73il/linux/commits/user_path_file/
No. Definitely no. Ignoring the fact that there is no reason we
should pushing this into the LSM, doing it in this way means it is
very likely that each LSM wanting to provide mmap/mprotect controls on
overlayfs will have to create a new O_PATH file. No.
... and let me preemptively comment that this doesn't belong in the
LSM framework either.
As Christian already mentioned, this really needs to be addressed in
the backing file code, please do it there.
--
paul-moore.com
^ 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