* Re: [PATCH v10 9/9] selftests/landlock: Add tests for invalid use of quiet flag
From: Mickaël Salaün @ 2026-06-08 22:41 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <99587e6f737a425d18fda31649edf304f74f3567.1780272022.git.m@maowtm.org>
On Mon, Jun 01, 2026 at 01:00:43AM +0100, Tingmao Wang wrote:
> Make sure that these calls return EINVAL.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes in v4:
> - New patch
>
> tools/testing/selftests/landlock/base_test.c | 57 ++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 84e91fcaa1b2..af9ad822a444 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -526,4 +526,61 @@ TEST(cred_transfer)
> EXPECT_EQ(EACCES, errno);
> }
>
> +TEST(useless_quiet_rule)
> +{
> + struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_fs = LANDLOCK_ACCESS_FS_READ_DIR,
> + .quiet_access_fs = 0,
The other .quiet_* fields should also be tested.
> + };
> + struct landlock_path_beneath_attr path_beneath_attr = {
> + .allowed_access = LANDLOCK_ACCESS_FS_READ_DIR,
> + };
> + int ruleset_fd, root_fd;
> +
> + drop_caps(_metadata);
> + ruleset_fd =
> + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> + ASSERT_LE(0, ruleset_fd);
> +
> + root_fd = open("/", O_PATH | O_CLOEXEC);
> + ASSERT_LE(0, root_fd);
> + path_beneath_attr.parent_fd = root_fd;
> + ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
> + &path_beneath_attr,
> + LANDLOCK_ADD_RULE_QUIET));
> + ASSERT_EQ(EINVAL, errno);
> +
> + /* Check that the rule had not been added. */
> + ASSERT_EQ(0, close(root_fd));
> + enforce_ruleset(_metadata, ruleset_fd);
> + ASSERT_EQ(0, close(ruleset_fd));
> +
> + ASSERT_EQ(-1, open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC));
> + ASSERT_EQ(EACCES, errno);
> +}
> +
> +TEST(invalid_quiet_bits_1)
> +{
> + struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_fs = LANDLOCK_ACCESS_FS_READ_DIR,
> + .quiet_access_fs = LANDLOCK_ACCESS_FS_WRITE_FILE,
ditto
> + };
> +
> + ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr,
> + sizeof(ruleset_attr), 0));
> + ASSERT_EQ(EINVAL, errno);
> +}
> +
> +TEST(invalid_quiet_bits_2)
> +{
> + struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_fs = LANDLOCK_ACCESS_FS_READ_DIR,
> + .quiet_access_fs = 1ULL << 63,
ditto
> + };
> +
> + ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr,
> + sizeof(ruleset_attr), 0));
> + ASSERT_EQ(EINVAL, errno);
> +}
> +
> TEST_HARNESS_MAIN
> --
> 2.54.0
>
^ permalink raw reply
* Re: [PATCH v10 2/9] landlock: Add API support and docs for the quiet flags
From: Mickaël Salaün @ 2026-06-08 22:41 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <3f622c8e06638383d3dbf00b7bc78c61bfd584dd.1780272022.git.m@maowtm.org>
On Mon, Jun 01, 2026 at 01:00:36AM +0100, Tingmao Wang wrote:
> Adds the UAPI for the quiet flags feature (but not the implementation
> yet).
>
> Even though currently LANDLOCK_ADD_RULE_QUIET only affects audit
> logging, in the future this can also be used as part of a supervisor
> mechanism, where it will also suppress denial notifications on a
> per-object basis. Thus the name is deliberately generic, as opposed to
> e.g. LANDLOCK_ADD_RULE_LOG_QUIET.
>
> According to pahole, even after adding the struct access_masks quiet_masks
> in struct landlock_hierarchy, the u32 log_* bitfield still only has a size
> of 2 bytes, so there's minimal wasted space.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes in v9:
> - Move a mistakenly included hunk into patch 1
> - Doc change for sys_landlock_create_ruleset to add missing
> "quiet_scoped | scoped == scoped" requirement.
> - Doc changes for struct landlock_ruleset_attr, and re-wrap added bits
> wider to stay consistent with the existing block.
> - Other style changes from suggestions
> - Added mention of this flag in the audit section of
> Documentation/admin-guide/LSM/landlock.rst
> - Added a block for this new flag to the "Previous limitations" section
> in Documentation/userspace-api/landlock.rst
>
> Changes in v8:
> - The new Landlock ABI version is now v10 as a result of rebase.
> - Allocate a rule_flags in hook_unix_find() and pass to
> is_access_to_paths_allowed().
>
> Changes in v6:
> - Fix typo in doc
>
> Changes in v5:
> - Doc fixes.
> - Fix build failure without CONFIG_AUDIT / CONFIG_INET (reported by Justin
> Suess)
>
> Changes in v4:
> - Minor update to this commit message.
> - Fix minor formatting
>
> Changes in v3:
> - Updated docs from Mickaël's suggestions.
>
> Changes in v2:
> - Per suggestion, added support for quieting only certain access bits,
> controlled by extra quiet_access_* fields in the ruleset_attr.
> - Added docs for the extra fields and made updates to doc changes in v1.
> In particular, call out that the effect of LANDLOCK_ADD_RULE_QUIET is
> independent from the access bits passed in rule_attr
> - landlock_add_rule will return -EINVAL when LANDLOCK_ADD_RULE_QUIET is
> used but the ruleset does not have any quiet access bits set for the
> given rule type.
> - ABI version bump to v8
> - Syntactic and comment changes per suggestion.
>
> Documentation/admin-guide/LSM/landlock.rst | 9 ++-
> Documentation/userspace-api/landlock.rst | 11 +++
> include/uapi/linux/landlock.h | 61 +++++++++++++++++
> security/landlock/domain.h | 5 ++
> security/landlock/fs.c | 4 +-
> security/landlock/fs.h | 2 +-
> security/landlock/net.c | 5 +-
> security/landlock/net.h | 5 +-
> security/landlock/ruleset.c | 12 +++-
> security/landlock/ruleset.h | 12 +++-
> security/landlock/syscalls.c | 71 +++++++++++++++-----
> tools/testing/selftests/landlock/base_test.c | 2 +-
> 12 files changed, 168 insertions(+), 31 deletions(-)
>
> diff --git a/Documentation/admin-guide/LSM/landlock.rst b/Documentation/admin-guide/LSM/landlock.rst
> index 9923874e2156..ccc32dad1d1c 100644
> --- a/Documentation/admin-guide/LSM/landlock.rst
> +++ b/Documentation/admin-guide/LSM/landlock.rst
> @@ -19,8 +19,10 @@ Audit
> Denied access requests are logged by default for a sandboxed program if `audit`
> is enabled. This default behavior can be changed with the
> sys_landlock_restrict_self() flags (cf.
> -Documentation/userspace-api/landlock.rst). Landlock logs can also be masked
> -thanks to audit rules. Landlock can generate 2 audit record types.
> +Documentation/userspace-api/landlock.rst), or suppressed on a per-object
> +basis by using ``LANDLOCK_ADD_RULE_QUIET`` (ABI 10+). Landlock logs can
> +also be masked thanks to audit rules. Landlock can generate 2 audit
> +record types.
>
> Record types
> ------------
> @@ -172,7 +174,8 @@ If you get spammed with audit logs related to Landlock, this is either an
> attack attempt or a bug in the security policy. We can put in place some
> filters to limit noise with two complementary ways:
>
> -- with sys_landlock_restrict_self()'s flags if we can fix the sandboxed
> +- with sys_landlock_restrict_self()'s flags, or
> + ``LANDLOCK_ADD_RULE_QUIET`` (ABI 10+) if we can fix the sandboxed
> programs,
> - or with audit rules (see :manpage:`auditctl(8)`).
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 45861fa75685..138d504cb498 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -722,6 +722,17 @@ Starting with the Landlock ABI version 9, it is possible to restrict
> connections to pathname UNIX domain sockets (:manpage:`unix(7)`) using
> the new ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX`` right.
>
> +Quiet rule flag (ABI < 10)
> +-----------------------------------------
The line should be the same number of characters as the title.
> +
> +Starting with the Landlock ABI version 10, it is possible to selectively
> +suppress audit logs for specific denied accesses on a per-object basis with
> +the ``LANDLOCK_ADD_RULE_QUIET`` flag of sys_landlock_add_rule(), in
> +combination with the ``quiet_access_fs`` and ``quiet_access_net`` fields of
> +struct landlock_ruleset_attr. It is also now possible to suppress audit logs
> +for scope accesses via the ``quiet_scoped`` field of struct
> +landlock_ruleset_attr.
This doc should also quickly explain what happen when two rules with the
same object/inode are added with or without the flag.
> +
> .. _kernel_support:
>
> Kernel support
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index b147223efc97..90a0752b61bf 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -32,6 +32,19 @@
> * *handle* a wide range or all access rights that they know about at build time
> * (and that they have tested with a kernel that supported them all).
> *
> + * @quiet_access_fs and @quiet_access_net are bitmasks of actions for which a
> + * denial by this layer will not trigger an audit log if the corresponding
> + * object (or its children, for filesystem rules) is marked with the "quiet" bit
> + * via %LANDLOCK_ADD_RULE_QUIET, even if logging would normally take place per
> + * landlock_restrict_self() flags. @quiet_scoped is similar, except that it
> + * does not require marking any objects as quiet - if the ruleset is created
> + * with any bits set in @quiet_scoped, then denial of such scoped resources will
> + * not trigger any log. These 3 fields are available since Landlock ABI version
> + * 10.
> + *
> + * @quiet_access_fs, @quiet_access_net and @quiet_scoped must be a subset of
> + * @handled_access_fs, @handled_access_net and @scoped respectively.
> + *
> * This structure can grow in future Landlock versions.
> */
> struct landlock_ruleset_attr {
> @@ -51,6 +64,21 @@ struct landlock_ruleset_attr {
> * resources (e.g. IPCs).
> */
> __u64 scoped;
> + /**
> + * @quiet_access_fs: Bitmask of filesystem actions which should not be
> + * logged if per-object quiet flag is set.
> + */
> + __u64 quiet_access_fs;
> + /**
> + * @quiet_access_net: Bitmask of network actions which should not be
> + * logged if per-object quiet flag is set.
> + */
> + __u64 quiet_access_net;
> + /**
> + * @quiet_scoped: Bitmask of scoped actions which should not be
> + * logged.
> + */
> + __u64 quiet_scoped;
> };
>
> /**
> @@ -69,6 +97,39 @@ struct landlock_ruleset_attr {
> #define LANDLOCK_CREATE_RULESET_ERRATA (1U << 1)
> /* clang-format on */
>
> +/**
> + * DOC: landlock_add_rule_flags
> + *
> + * **Flags**
> + *
> + * %LANDLOCK_ADD_RULE_QUIET
> + * Together with the quiet_* fields in struct landlock_ruleset_attr,
> + * this flag controls whether Landlock will log audit messages when
> + * access to the objects covered by this rule is denied by this layer.
> + *
> + * If audit logging is enabled, when Landlock denies an access, it will
> + * suppress the audit log if all of the following are true:
Still a lot of "audit log"...
> + *
> + * - this layer is the innermost layer that denied the access;
> + * - all accesses denied by this layer are part of the quiet_* fields
> + * in the related struct landlock_ruleset_attr;
> + * - the object (or one of its parents, for filesystem rules) is
> + * marked as "quiet" via %LANDLOCK_ADD_RULE_QUIET.
> + *
> + * Because logging is only suppressed by a layer if the layer denies
> + * access, a sandboxed program cannot use this flag to "hide" access
> + * denials, without denying itself the access in the first place.
> + *
> + * The effect of this flag does not depend on the value of
> + * allowed_access in the passed in rule_attr. When this flag is
> + * present, the caller is also allowed to pass in an empty
> + * allowed_access.
> + */
> +
> +/* clang-format off */
> +#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
> +/* clang-format on */
> +
> /**
> * DOC: landlock_restrict_self_flags
> *
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index af100a8cd939..9f560f3c3bd1 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -111,6 +111,11 @@ struct landlock_hierarchy {
> * %LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON. Set to false by default.
> */
> log_new_exec : 1;
> + /**
> + * @quiet_masks: Bitmasks of access that should be quieted (i.e. not
> + * logged) if the related object is marked as quiet.
> + */
> + struct access_masks quiet_masks;
> #endif /* CONFIG_AUDIT */
> };
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index f7c1bc64de20..cc0852f77311 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -325,7 +325,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> */
> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> const struct path *const path,
> - access_mask_t access_rights)
> + access_mask_t access_rights, const int flags)
const u32 flags (see syscall argument)
> {
> int err;
> struct landlock_id id = {
> @@ -346,7 +346,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> if (IS_ERR(id.key.object))
> return PTR_ERR(id.key.object);
> mutex_lock(&ruleset->lock);
> - err = landlock_insert_rule(ruleset, id, access_rights);
> + err = landlock_insert_rule(ruleset, id, access_rights, flags);
> mutex_unlock(&ruleset->lock);
> /*
> * No need to check for an error because landlock_insert_rule()
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index bf9948941f2f..cb7e654933ac 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -126,6 +126,6 @@ __init void landlock_add_fs_hooks(void);
>
> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> const struct path *const path,
> - access_mask_t access_hierarchy);
> + access_mask_t access_hierarchy, const int flags);
const u32 flags
>
> #endif /* _SECURITY_LANDLOCK_FS_H */
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index 981a362c24db..71868289748a 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -20,7 +20,8 @@
> #include "ruleset.h"
>
> int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> - const u16 port, access_mask_t access_rights)
> + const u16 port, access_mask_t access_rights,
> + const int flags)
const u32 flags
> {
> int err;
> const struct landlock_id id = {
> @@ -35,7 +36,7 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> ~landlock_get_net_access_mask(ruleset, 0);
>
> mutex_lock(&ruleset->lock);
> - err = landlock_insert_rule(ruleset, id, access_rights);
> + err = landlock_insert_rule(ruleset, id, access_rights, flags);
> mutex_unlock(&ruleset->lock);
>
> return err;
> diff --git a/security/landlock/net.h b/security/landlock/net.h
> index 09960c237a13..72c47f4d6803 100644
> --- a/security/landlock/net.h
> +++ b/security/landlock/net.h
> @@ -16,7 +16,8 @@
> __init void landlock_add_net_hooks(void);
>
> int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> - const u16 port, access_mask_t access_rights);
> + const u16 port, access_mask_t access_rights,
> + const int flags);
const u32 flags
> #else /* IS_ENABLED(CONFIG_INET) */
> static inline void landlock_add_net_hooks(void)
> {
> @@ -24,7 +25,7 @@ static inline void landlock_add_net_hooks(void)
>
> static inline int
> landlock_append_net_rule(struct landlock_ruleset *const ruleset, const u16 port,
> - access_mask_t access_rights)
> + access_mask_t access_rights, const int flags)
const u32 flags
> {
> return -EAFNOSUPPORT;
> }
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 91948e406e69..f01c3e14e55d 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -21,6 +21,7 @@
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/workqueue.h>
> +#include <uapi/linux/landlock.h>
>
> #include "access.h"
> #include "domain.h"
> @@ -255,6 +256,7 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
> if (WARN_ON_ONCE(this->layers[0].level != 0))
> return -EINVAL;
> this->layers[0].access |= (*layers)[0].access;
> + this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
> return 0;
> }
>
> @@ -305,12 +307,15 @@ static void build_check_layer(void)
> /* @ruleset must be locked by the caller. */
> int landlock_insert_rule(struct landlock_ruleset *const ruleset,
> const struct landlock_id id,
> - const access_mask_t access)
> + const access_mask_t access, const int flags)
const u32 flags
> {
> struct landlock_layer layers[] = { {
> .access = access,
> /* When @level is zero, insert_rule() extends @ruleset. */
> .level = 0,
> + .flags = {
> + .quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
> + },
> } };
>
> build_check_layer();
> @@ -351,6 +356,7 @@ static int merge_tree(struct landlock_ruleset *const dst,
> return -EINVAL;
>
> layers[0].access = walker_rule->layers[0].access;
> + layers[0].flags = walker_rule->layers[0].flags;
>
> err = insert_rule(dst, id, &layers, ARRAY_SIZE(layers));
> if (err)
> @@ -581,6 +587,10 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
> if (err)
> return ERR_PTR(err);
>
> +#ifdef CONFIG_AUDIT
> + new_dom->hierarchy->quiet_masks = ruleset->quiet_masks;
> +#endif /* CONFIG_AUDIT */
> +
> return no_free_ptr(new_dom);
> }
>
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index f80ca487d125..d54bdb925e96 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -156,8 +156,8 @@ struct landlock_ruleset {
> * @work_free: Enables to free a ruleset within a lockless
> * section. This is only used by
> * landlock_put_ruleset_deferred() when @usage reaches zero.
> - * The fields @lock, @usage, @num_rules, @num_layers and
> - * @access_masks are then unused.
> + * The fields @lock, @usage, @num_rules, @num_layers, @quiet_masks
> + * and @access_masks are then unused.
> */
> struct work_struct work_free;
> struct {
> @@ -183,6 +183,12 @@ struct landlock_ruleset {
> * non-merged ruleset (i.e. not a domain).
> */
> u32 num_layers;
> + /**
> + * @quiet_masks: Stores the quiet flags for an unmerged
> + * ruleset. For a merged domain, this is stored in each
> + * layer's struct landlock_hierarchy instead.
> + */
> + struct access_masks quiet_masks;
> /**
> * @access_masks: Contains the subset of filesystem and
> * network actions that are restricted by a ruleset.
> @@ -213,7 +219,7 @@ DEFINE_FREE(landlock_put_ruleset, struct landlock_ruleset *,
>
> int landlock_insert_rule(struct landlock_ruleset *const ruleset,
> const struct landlock_id id,
> - const access_mask_t access);
> + const access_mask_t access, const int flags);
const u32 flags
>
> struct landlock_ruleset *
> landlock_merge_ruleset(struct landlock_ruleset *const parent,
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index d45469d5d464..08b6045d6926 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -105,8 +105,11 @@ static void build_check_abi(void)
> ruleset_size = sizeof(ruleset_attr.handled_access_fs);
> ruleset_size += sizeof(ruleset_attr.handled_access_net);
> ruleset_size += sizeof(ruleset_attr.scoped);
> + ruleset_size += sizeof(ruleset_attr.quiet_access_fs);
> + ruleset_size += sizeof(ruleset_attr.quiet_access_net);
> + ruleset_size += sizeof(ruleset_attr.quiet_scoped);
> BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
> - BUILD_BUG_ON(sizeof(ruleset_attr) != 24);
> + BUILD_BUG_ON(sizeof(ruleset_attr) != 48);
>
> path_beneath_size = sizeof(path_beneath_attr.allowed_access);
> path_beneath_size += sizeof(path_beneath_attr.parent_fd);
> @@ -193,6 +196,9 @@ const int landlock_abi_version = 10;
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small
> * @size;
> + * - %EINVAL: quiet_access_fs, quiet_access_net, or quiet_scoped is not a
> + * subset of the corresponding handled_access_fs, handled_access_net, or
> + * scoped;
> * - %E2BIG: @attr or @size inconsistencies;
> * - %EFAULT: @attr or @size inconsistencies;
> * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
> @@ -249,6 +255,21 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
> if ((ruleset_attr.scoped | LANDLOCK_MASK_SCOPE) != LANDLOCK_MASK_SCOPE)
> return -EINVAL;
>
> + /*
> + * Check that quiet masks are subsets of the respective handled masks.
> + * Because of the checks above this is sufficient to also ensure that
> + * the quiet masks are valid access masks.
> + */
> + if ((ruleset_attr.quiet_access_fs | ruleset_attr.handled_access_fs) !=
> + ruleset_attr.handled_access_fs)
> + return -EINVAL;
> + if ((ruleset_attr.quiet_access_net | ruleset_attr.handled_access_net) !=
> + ruleset_attr.handled_access_net)
> + return -EINVAL;
> + if ((ruleset_attr.quiet_scoped | ruleset_attr.scoped) !=
> + ruleset_attr.scoped)
> + return -EINVAL;
> +
> /* Checks arguments and transforms to kernel struct. */
> ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs,
> ruleset_attr.handled_access_net,
> @@ -256,6 +277,10 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
> if (IS_ERR(ruleset))
> return PTR_ERR(ruleset);
>
> + ruleset->quiet_masks.fs = ruleset_attr.quiet_access_fs;
> + ruleset->quiet_masks.net = ruleset_attr.quiet_access_net;
> + ruleset->quiet_masks.scope = ruleset_attr.quiet_scoped;
> +
> /* Creates anonymous FD referring to the ruleset. */
> ruleset_fd = anon_inode_getfd("[landlock-ruleset]", &ruleset_fops,
> ruleset, O_RDWR | O_CLOEXEC);
> @@ -320,7 +345,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
> }
>
> static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
> - const void __user *const rule_attr)
> + const void __user *const rule_attr, int flags)
const u32 flags
> {
> struct landlock_path_beneath_attr path_beneath_attr;
> struct path path;
> @@ -335,9 +360,10 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
>
> /*
> * Informs about useless rule: empty allowed_access (i.e. deny rules)
> - * are ignored in path walks.
> + * are ignored in path walks. However, the rule is not useless if it
> + * is there to hold a quiet flag.
> */
> - if (!path_beneath_attr.allowed_access)
> + if (!flags && !path_beneath_attr.allowed_access)
> return -ENOMSG;
>
> /* Checks that allowed_access matches the @ruleset constraints. */
> @@ -345,6 +371,10 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
> if ((path_beneath_attr.allowed_access | mask) != mask)
> return -EINVAL;
>
> + /* Checks for useless quiet flag. */
> + if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.fs)
> + return -EINVAL;
> +
> /* Gets and checks the new rule. */
> err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
> if (err)
> @@ -352,13 +382,13 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
>
> /* Imports the new rule. */
> err = landlock_append_fs_rule(ruleset, &path,
> - path_beneath_attr.allowed_access);
> + path_beneath_attr.allowed_access, flags);
> path_put(&path);
> return err;
> }
>
> static int add_rule_net_port(struct landlock_ruleset *ruleset,
> - const void __user *const rule_attr)
> + const void __user *const rule_attr, int flags)
const u32 flags
> {
> struct landlock_net_port_attr net_port_attr;
> int res;
> @@ -371,9 +401,10 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
>
> /*
> * Informs about useless rule: empty allowed_access (i.e. deny rules)
> - * are ignored by network actions.
> + * are ignored by network actions. However, the rule is not useless
> + * if it is there to hold a quiet flag.
> */
> - if (!net_port_attr.allowed_access)
> + if (!flags && !net_port_attr.allowed_access)
> return -ENOMSG;
>
> /* Checks that allowed_access matches the @ruleset constraints. */
> @@ -381,13 +412,17 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> if ((net_port_attr.allowed_access | mask) != mask)
> return -EINVAL;
>
> + /* Checks for useless quiet flag. */
> + if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.net)
> + return -EINVAL;
> +
> /* Denies inserting a rule with port greater than 65535. */
> if (net_port_attr.port > U16_MAX)
> return -EINVAL;
>
> /* Imports the new rule. */
> return landlock_append_net_rule(ruleset, net_port_attr.port,
> - net_port_attr.allowed_access);
> + net_port_attr.allowed_access, flags);
> }
>
> /**
> @@ -398,7 +433,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> * @rule_type: Identify the structure type pointed to by @rule_attr:
> * %LANDLOCK_RULE_PATH_BENEATH or %LANDLOCK_RULE_NET_PORT.
> * @rule_attr: Pointer to a rule (matching the @rule_type).
> - * @flags: Must be 0.
> + * @flags: Must be 0 or %LANDLOCK_ADD_RULE_QUIET.
> *
> * This system call enables to define a new rule and add it to an existing
> * ruleset.
> @@ -408,20 +443,25 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> * - %EAFNOSUPPORT: @rule_type is %LANDLOCK_RULE_NET_PORT but TCP/IP is not
> * supported by the running kernel;
> - * - %EINVAL: @flags is not 0;
> + * - %EINVAL: @flags is not valid;
> * - %EINVAL: The rule accesses are inconsistent (i.e.
> * &landlock_path_beneath_attr.allowed_access or
> * &landlock_net_port_attr.allowed_access is not a subset of the ruleset
> * handled accesses)
> * - %EINVAL: &landlock_net_port_attr.port is greater than 65535;
> + * - %EINVAL: LANDLOCK_ADD_RULE_QUIET is passed but the ruleset has no
> + * quiet access bits set for the corresponding rule type.
> * - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access is
> - * 0);
> + * 0) and no flags;
> * - %EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
> * member of @rule_attr is not a file descriptor as expected;
> * - %EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
> * @rule_attr is not the expected file descriptor type;
> * - %EPERM: @ruleset_fd has no write access to the underlying ruleset;
> * - %EFAULT: @rule_attr was not a valid address.
> + *
> + * .. kernel-doc:: include/uapi/linux/landlock.h
> + * :identifiers: landlock_add_rule_flags
> */
> SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
> const enum landlock_rule_type, rule_type,
> @@ -432,8 +472,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
> if (!is_initialized())
> return -EOPNOTSUPP;
>
> - /* No flag for now. */
> - if (flags)
> + if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
> return -EINVAL;
>
> /* Gets and checks the ruleset. */
> @@ -443,9 +482,9 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
>
> switch (rule_type) {
> case LANDLOCK_RULE_PATH_BENEATH:
> - return add_rule_path_beneath(ruleset, rule_attr);
> + return add_rule_path_beneath(ruleset, rule_attr, flags);
> case LANDLOCK_RULE_NET_PORT:
> - return add_rule_net_port(ruleset, rule_attr);
> + return add_rule_net_port(ruleset, rule_attr, flags);
> default:
> return -EINVAL;
> }
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 6c8113c2ded1..84e91fcaa1b2 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -201,7 +201,7 @@ TEST(add_rule_checks_ordering)
> ASSERT_LE(0, ruleset_fd);
>
> /* Checks invalid flags. */
> - ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 1));
> + ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 100));
> ASSERT_EQ(EINVAL, errno);
>
> /* Checks invalid ruleset FD. */
> --
> 2.54.0
>
^ permalink raw reply
* Re: [PATCH v6 2/4] security: ima: introduce IMA_INIT_LATE_SYNC option
From: Mimi Zohar @ 2026-06-08 17:40 UTC (permalink / raw)
To: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity
Cc: paul, roberto.sassu, noodles, jarkko, sudeep.holla, jmorris,
serge, dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260605144325.434436-3-yeoreum.yun@arm.com>
On Fri, 2026-06-05 at 15:43 +0100, Yeoreum Yun wrote:
> To generate the boot_aggregate log in the IMA subsystem with
> TPM PCR values, the TPM driver must be built as built-in and
> must be probed before the IMA subsystem is initialized.
>
> However, when the TPM device operates over the FF-A protocol using
> the CRB interface, probing fails and returns -EPROBE_DEFER if
> the tpm_crb_ffa device — an FF-A device that provides the communication
> interface to the tpm_crb driver — has not yet been probed.
>
> To ensure the TPM device operating over the FF-A protocol with
> the CRB interface is probed before IMA initialization,
> the following conditions must be met:
>
> 1. The corresponding ffa_device must be registered,
> which is done via ffa_init().
>
> 2. The tpm_crb_driver must successfully probe this device via
> tpm_crb_ffa_init().
>
> 3. The tpm_crb driver using CRB over FF-A can then
> be probed successfully. (See crb_acpi_add() and
> tpm_crb_ffa_init() for reference.)
>
> Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init()
> are all registered with device_initcall, which means
> crb_acpi_driver_init() may be invoked before ffa_init() and
> tpm_crb_ffa_init() are completed.
>
> When this occurs, probing the TPM device is deferred.
> However, the deferred probe can happen after the IMA subsystem
> has already been initialized, since IMA initialization is performed
> during late_initcall, and deferred_probe_initcall() is performed
> at the same level.
>
> And the similar situation is reported on TPM devices attached on SPI
> bus[0].
>
> To resolve this, introduce IMA_INIT_LATE_SYNC option to initialise
> IMA at late_inicall_sync so that IMA is initialized with the TPM
> device probed deferred.
>
> When this option is enabled, modules that access files in the
> initramfs through usermode helper calls such as request_module()
> during initcall must not be built-in. Otherwise, IMA may miss
> measuring those files [1].
>
> Link: https://lore.kernel.org/all/aYXEepLhUouN5f99@earth.li/ [0]
> Link: https://lore.kernel.org/all/2b3782398cc17ce9d355490a0c42ebce9120a9ae.camel@linux.ibm.com/ [1]
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> security/integrity/ima/Kconfig | 10 ++++++++++
> security/integrity/ima/ima_main.c | 4 ++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 862fbee2b174..75f71401fba3 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -332,4 +332,14 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
> If set to the default value of 0, an extra half page of memory for those
> additional measurements will be allocated.
>
> +config IMA_INIT_LATE_SYNC
> + bool "Initialise IMA at late_initcall_sync"
> + default n
> + help
> + This option initialises IMA at late_initcall_sync for platforms
> + where TPM device probing is deferred.
> + When this option is enabled, modules that access files in the
> + initramfs through usermode helper calls such as request_module()
> + during initcall must not be built-in. Otherwise, IMA may miss
> + file measurements for them.
> endif
I fixed the merge conflict with the "ima: Exporting and deleting IMA measurement
records from kernel memory" patch set. These patches are now queued in next-
integrity-testing awaiting Paul's Ack.
Mimi
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5cea53fc36df..1cfae4b83dc5 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1337,5 +1337,9 @@ DEFINE_LSM(ima) = {
> .order = LSM_ORDER_LAST,
> .blobs = &ima_blob_sizes,
> /* Start IMA after the TPM is available */
> +#ifndef CONFIG_IMA_INIT_LATE_SYNC
> .initcall_late = init_ima,
> +#else
> + .initcall_late_sync = init_ima,
> +#endif
> };
^ permalink raw reply
* Re: [PATCH v6 1/4] security: lsm: allow LSMs to register for late_initcall_sync init
From: Paul Moore @ 2026-06-08 17:24 UTC (permalink / raw)
To: Mimi Zohar
Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
roberto.sassu, noodles, jarkko, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <9b499a2a3101cadcfdcd6fc32289f54df10fea80.camel@linux.ibm.com>
On Mon, Jun 8, 2026 at 12:52 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2026-06-05 at 15:43 +0100, Yeoreum Yun wrote:
> > There are situations where LSMs have dependencies that might mean they
> > want to be initialised later in the boot process, to ensure those
> > dependencies are available. In particular there are some TPM setups (Arm
> > FF-A devices, SPI attached TPMs) required by IMA which are not
> > guaranteed to be initialised for regular initcall_late.
> >
> > Add an initcall_late_sync option that can be used in these situations.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>
> Cc: Paul Moore <paul@paul-moore.com>
>
> Jarkko has already queued 4/4. Paul, can we get an Ack from you?
It's in my queue, but with the merge window opening next week, this is
definitely not suitable for the upcoming merge window, so it's not
near the top of my review list.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v6 1/4] security: lsm: allow LSMs to register for late_initcall_sync init
From: Mimi Zohar @ 2026-06-08 16:52 UTC (permalink / raw)
To: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity
Cc: paul, roberto.sassu, noodles, jarkko, sudeep.holla, jmorris,
serge, dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260605144325.434436-2-yeoreum.yun@arm.com>
On Fri, 2026-06-05 at 15:43 +0100, Yeoreum Yun wrote:
> There are situations where LSMs have dependencies that might mean they
> want to be initialised later in the boot process, to ensure those
> dependencies are available. In particular there are some TPM setups (Arm
> FF-A devices, SPI attached TPMs) required by IMA which are not
> guaranteed to be initialised for regular initcall_late.
>
> Add an initcall_late_sync option that can be used in these situations.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Cc: Paul Moore <paul@paul-moore.com>
Jarkko has already queued 4/4. Paul, can we get an Ack from you?
Mimi
^ permalink raw reply
* Re: [PATCH v7 00/12] ima: Exporting and deleting IMA measurement records from kernel memory
From: Roberto Sassu @ 2026-06-08 16:22 UTC (permalink / raw)
To: Mimi Zohar, corbet, skhan, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <22debae414a07a3cbdb62e723dfb737d6d4bd693.camel@linux.ibm.com>
On Mon, 2026-06-08 at 12:21 -0400, Mimi Zohar wrote:
> On Fri, 2026-06-05 at 19:22 +0200, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > Introduction
> > ============
> >
> > The IMA measurements list is currently stored in the kernel memory.
> > Memory occupation grows linearly with the number of records, and can
> > become a problem especially in environments with reduced resources.
> >
> > While there is an advantage in keeping the IMA measurements list in
> > kernel memory, so that it is always available for reading from the
> > securityfs interfaces, storing it elsewhere would make it possible to
> > free precious memory for other kernel usage.
> >
> > The IMA measurements list needs to be retained and safely stored for new
> > attestation servers to validate it. Assuming the IMA measurements list
> > is properly saved, storing it outside the kernel does not introduce
> > security issues, since its integrity is anyway protected by the TPM.
> >
> > Hence, the new IMA staging mechanism is introduced to export IMA
> > measurements to user space and delete them from kernel space.
> >
> > Staging consists in atomically moving the current measurements list to a
> > temporary list, so that measurements can be deleted afterwards. The
> > staging operation locks the hot path (racing with addition of new
> > measurements) for a very short time, only for swapping the list
> > pointers. Deletion of the measurements instead is done locklessly, away
> > from the hot path.
> >
> > There are two flavors of the staging mechanism. In the staging with
> > prompt, all current measurements are staged, read and deleted upon
> > confirmation. In the staging and deleting flavor, N measurements are
> > staged from the beginning of the current measurements list and
> > immediately deleted without confirmation.
> >
> >
> > Usage
> > =====
> >
> > The IMA staging mechanism can be enabled from the kernel configuration
> > with the CONFIG_IMA_STAGING option. This option prevents inadvertently
> > removing the IMA measurement list on systems which do not properly save
> > it.
> >
> > If the option is enabled, IMA duplicates the current securityfs
> > measurements interfaces (both binary and ASCII), by adding the _staged
> > file suffix. Both the original and the staging interfaces gain the write
> > permission for the root user and group, but require the process to have
> > CAP_SYS_ADMIN set.
> >
> > The staging mechanism supports two flavors.
> >
> > Staging with prompt
> > ~~~~~~~~~~~~~~~~~~~
> >
> > The current measurement list is moved to a temporary staging area,
> > allowing it to be saved to external storage, before being deleted upon
> > confirmation.
> >
> > This staging process is achieved with the following steps.
> >
> > 1. echo A > <_staged interface>: the user requests IMA to stage the
> > entire measurements list;
> > 2. cat <_staged interface>: the user reads the staged measurements;
> > 3. echo D > <_staged interface>: the user requests IMA to delete
> > staged measurements.
> >
> > Staging and deleting
> > ~~~~~~~~~~~~~~~~~~~~
> >
> > N measurements are staged to a temporary staging area, and immediately
> > deleted without further confirmation.
> >
> > This staging process is achieved with the following steps.
> >
> > 1. cat <original interface>: the user reads the current measurements
> > list and determines what the value N for staging should be;
> > 2. echo N > <original interface>: the user requests IMA to delete N
> > measurements from the current measurements list.
> >
> >
> > Management of Staged Measurements
> > =================================
> >
> > Since with the staging mechanism measurement records are removed from
> > the kernel, the staged measurements need to be saved in a storage and
> > concatenated together, so that they can be presented during remote
> > attestation as if staging was never done. This task can be accomplished
> > by a remote attestation agent modified to support staging, or a system
> > service.
> >
> >
> > Patch set content
> > =================
> >
> > Patches 1-8 are preparatory patches to quickly replace the hash table,
> > maintain separate counters for the different measurements list types,
> > mediate access to the measurements list interface, and simplify the staging
> > patches.
> >
> > Patch 9 introduces the staging with prompt flavor. Patch 10 makes it
> > possible to flush the hash table when deleting all the staged measurements.
> > Patch 11 introduces the staging and deleting flavor. Patch 12 adds the
> > documentation of the staging mechanism.
> >
> >
> > Changelog
> > =========
> >
> > v6:
> > - Make ima_extend_list_mutex as static since it is not needed anymore by
> > ima_dump_measurement_list() (suggested by Mimi)
> > - Export ima_flush_htable in patch 11 instead of 10 (suggested by Mimi)
> > - Add clarification in the documentation regarding a proactive remote
> > attestation agent, and storing all the measurements in the storage
> > (suggested by Mimi)
>
> Roberto, thank you for making these and all the other changes. The patch set is
> now queued in next-integrity.
Perfect, thank you!
Roberto
^ permalink raw reply
* Re: [PATCH v7 00/12] ima: Exporting and deleting IMA measurement records from kernel memory
From: Mimi Zohar @ 2026-06-08 16:21 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260605172236.2042045-1-roberto.sassu@huaweicloud.com>
On Fri, 2026-06-05 at 19:22 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduction
> ============
>
> The IMA measurements list is currently stored in the kernel memory.
> Memory occupation grows linearly with the number of records, and can
> become a problem especially in environments with reduced resources.
>
> While there is an advantage in keeping the IMA measurements list in
> kernel memory, so that it is always available for reading from the
> securityfs interfaces, storing it elsewhere would make it possible to
> free precious memory for other kernel usage.
>
> The IMA measurements list needs to be retained and safely stored for new
> attestation servers to validate it. Assuming the IMA measurements list
> is properly saved, storing it outside the kernel does not introduce
> security issues, since its integrity is anyway protected by the TPM.
>
> Hence, the new IMA staging mechanism is introduced to export IMA
> measurements to user space and delete them from kernel space.
>
> Staging consists in atomically moving the current measurements list to a
> temporary list, so that measurements can be deleted afterwards. The
> staging operation locks the hot path (racing with addition of new
> measurements) for a very short time, only for swapping the list
> pointers. Deletion of the measurements instead is done locklessly, away
> from the hot path.
>
> There are two flavors of the staging mechanism. In the staging with
> prompt, all current measurements are staged, read and deleted upon
> confirmation. In the staging and deleting flavor, N measurements are
> staged from the beginning of the current measurements list and
> immediately deleted without confirmation.
>
>
> Usage
> =====
>
> The IMA staging mechanism can be enabled from the kernel configuration
> with the CONFIG_IMA_STAGING option. This option prevents inadvertently
> removing the IMA measurement list on systems which do not properly save
> it.
>
> If the option is enabled, IMA duplicates the current securityfs
> measurements interfaces (both binary and ASCII), by adding the _staged
> file suffix. Both the original and the staging interfaces gain the write
> permission for the root user and group, but require the process to have
> CAP_SYS_ADMIN set.
>
> The staging mechanism supports two flavors.
>
> Staging with prompt
> ~~~~~~~~~~~~~~~~~~~
>
> The current measurement list is moved to a temporary staging area,
> allowing it to be saved to external storage, before being deleted upon
> confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. echo A > <_staged interface>: the user requests IMA to stage the
> entire measurements list;
> 2. cat <_staged interface>: the user reads the staged measurements;
> 3. echo D > <_staged interface>: the user requests IMA to delete
> staged measurements.
>
> Staging and deleting
> ~~~~~~~~~~~~~~~~~~~~
>
> N measurements are staged to a temporary staging area, and immediately
> deleted without further confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. cat <original interface>: the user reads the current measurements
> list and determines what the value N for staging should be;
> 2. echo N > <original interface>: the user requests IMA to delete N
> measurements from the current measurements list.
>
>
> Management of Staged Measurements
> =================================
>
> Since with the staging mechanism measurement records are removed from
> the kernel, the staged measurements need to be saved in a storage and
> concatenated together, so that they can be presented during remote
> attestation as if staging was never done. This task can be accomplished
> by a remote attestation agent modified to support staging, or a system
> service.
>
>
> Patch set content
> =================
>
> Patches 1-8 are preparatory patches to quickly replace the hash table,
> maintain separate counters for the different measurements list types,
> mediate access to the measurements list interface, and simplify the staging
> patches.
>
> Patch 9 introduces the staging with prompt flavor. Patch 10 makes it
> possible to flush the hash table when deleting all the staged measurements.
> Patch 11 introduces the staging and deleting flavor. Patch 12 adds the
> documentation of the staging mechanism.
>
>
> Changelog
> =========
>
> v6:
> - Make ima_extend_list_mutex as static since it is not needed anymore by
> ima_dump_measurement_list() (suggested by Mimi)
> - Export ima_flush_htable in patch 11 instead of 10 (suggested by Mimi)
> - Add clarification in the documentation regarding a proactive remote
> attestation agent, and storing all the measurements in the storage
> (suggested by Mimi)
Roberto, thank you for making these and all the other changes. The patch set is
now queued in next-integrity.
Mimi
^ permalink raw reply
* RE: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration()
From: Benoit HOUYERE @ 2026-06-08 14:46 UTC (permalink / raw)
To: Serge E. Hallyn, Jarkko Sakkinen
Cc: linux-integrity@vger.kernel.org, Frédéric Jouen,
Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM,
Laurent CHARPENTIER
In-Reply-To: <aMzSyCQks3NlMhPI@mail.hallyn.com>
Hello Serge and Jarkko,
We have detected a regression with this fix. Indeed, it miss one zero on TPM_LONG_LONG. Initial value was 300000 and not 30000.
> + {TPM2_CC_CREATE_PRIMARY, 30000},
> + {TPM2_CC_CREATE, 30000},
> + {TPM2_CC_CREATE_LOADED, 30000},
> +enum tpm2_durations {
> TPM2_DURATION_SHORT = 20,
> - TPM2_DURATION_MEDIUM = 750,
> TPM2_DURATION_LONG = 2000,
> - TPM2_DURATION_LONG_LONG = 300000,
> TPM2_DURATION_DEFAULT = 120000,
> };
Best Regards, Cordialement, Cordialmente, Hälsningar, 最好的问候, Mit besten Grüßen, 真心を込めて, 진심으로
Benoit HOUYERE | Tel: +33 6 14 22 81 30
TPM specialist
-----Original Message-----
From: Serge E. Hallyn <serge@hallyn.com>
Sent: Friday, September 19, 2025 5:49 AM
To: Jarkko Sakkinen <jarkko@kernel.org>
Cc: linux-integrity@vger.kernel.org; Frédéric Jouen <fjouen@sealsq.com>; Peter Huewe <peterhuewe@gmx.de>; Jason Gunthorpe <jgg@ziepe.ca>; James Bottomley <James.Bottomley@hansenpartnership.com>; Mimi Zohar <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn <serge@hallyn.com>; open list <linux-kernel@vger.kernel.org>; open list:KEYS-TRUSTED <keyrings@vger.kernel.org>; open list:SECURITY SUBSYSTEM <linux-security-module@vger.kernel.org>
Subject: Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration()
On Thu, Sep 18, 2025 at 10:30:18PM +0300, Jarkko Sakkinen wrote:
> The current shenanigans for duration calculation introduce too much
> complexity for a trivial problem, and further the code is hard to
> patch and maintain.
>
> Address these issues with a flat look-up table, which is easy to
> understand and patch. If leaf driver specific patching is required in
> future, it is easy enough to make a copy of this table during driver
> initialization and add the chip parameter back.
>
> 'chip->duration' is retained for TPM 1.x.
>
> As the first entry for this new behavior address TCG spec update
> mentioned in this issue:
>
> https://github.com/raspberrypi/linux/issues/7054
>
> Therefore, for TPM_SelfTest the duration is set to 3000 ms.
>
> This does not categorize a as bug, given that this is introduced to
> the spec after the feature was originally made.
>
> Cc: Frédéric Jouen <fjouen@sealsq.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
fwiw (which shouldn't be much) looks good to me, but two questions, one here and one below.
First, it looks like in the existing code it is possible for a tpm2 chip to set its own timeouts and then set the TPM_CHIP_FLAG_HAVE_TIMEOUTS flag to avoid using the defaults, but I don't see anything using that in-tree. Is it possible that there are out of tree drivers that will be sabotaged here? Or am I misunderstanding that completely?
> ---
> v2:
> - Add the missing msec_to_jiffies() calls.
> - Drop redundant stuff.
> ---
> drivers/char/tpm/tpm-interface.c | 2 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/char/tpm/tpm2-cmd.c | 127 ++++++++-----------------------
> include/linux/tpm.h | 5 +-
> 4 files changed, 37 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c
> b/drivers/char/tpm/tpm-interface.c
> index b71725827743..c9f173001d0e 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -52,7 +52,7 @@ MODULE_PARM_DESC(suspend_pcr, unsigned long
> tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal) {
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> - return tpm2_calc_ordinal_duration(chip, ordinal);
> + return tpm2_calc_ordinal_duration(ordinal);
> else
> return tpm1_calc_ordinal_duration(chip, ordinal); } diff --git
> a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> 7bb87fa5f7a1..2726bd38e5ac 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -299,7 +299,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32
> property_id, ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
> int tpm2_auto_startup(struct tpm_chip *chip); void
> tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type); -unsigned
> long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> +unsigned long tpm2_calc_ordinal_duration(u32 ordinal);
> int tpm2_probe(struct tpm_chip *chip); int
> tpm2_get_cc_attrs_tbl(struct tpm_chip *chip); int tpm2_find_cc(struct
> tpm_chip *chip, u32 cc); diff --git a/drivers/char/tpm/tpm2-cmd.c
> b/drivers/char/tpm/tpm2-cmd.c index 524d802ede26..7d77f6fbc152 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -28,120 +28,57 @@ static struct tpm2_hash tpm2_hash_map[] = {
>
> int tpm2_get_timeouts(struct tpm_chip *chip) {
> - /* Fixed timeouts for TPM2 */
> chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
> chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
> chip->timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C);
> chip->timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D);
> -
> - /* PTP spec timeouts */
> - chip->duration[TPM_SHORT] = msecs_to_jiffies(TPM2_DURATION_SHORT);
> - chip->duration[TPM_MEDIUM] = msecs_to_jiffies(TPM2_DURATION_MEDIUM);
> - chip->duration[TPM_LONG] = msecs_to_jiffies(TPM2_DURATION_LONG);
> -
> - /* Key creation commands long timeouts */
> - chip->duration[TPM_LONG_LONG] =
> - msecs_to_jiffies(TPM2_DURATION_LONG_LONG);
> -
> chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
> -
> return 0;
> }
>
> -/**
> - * tpm2_ordinal_duration_index() - returns an index to the chip
> duration table
> - * @ordinal: TPM command ordinal.
> - *
> - * The function returns an index to the chip duration table
> - * (enum tpm_duration), that describes the maximum amount of
> - * time the chip could take to return the result for a particular ordinal.
> - *
> - * The values of the MEDIUM, and LONG durations are taken
> - * from the PC Client Profile (PTP) specification (750, 2000 msec)
> - *
> - * LONG_LONG is for commands that generates keys which empirically
> takes
> - * a longer time on some systems.
> - *
> - * Return:
> - * * TPM_MEDIUM
> - * * TPM_LONG
> - * * TPM_LONG_LONG
> - * * TPM_UNDEFINED
> +/*
> + * Contains the maximum durations in milliseconds for TPM2 commands.
> */
> -static u8 tpm2_ordinal_duration_index(u32 ordinal) -{
> - switch (ordinal) {
> - /* Startup */
> - case TPM2_CC_STARTUP: /* 144 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_SELF_TEST: /* 143 */
> - return TPM_LONG;
> -
> - case TPM2_CC_GET_RANDOM: /* 17B */
> - return TPM_LONG;
> -
> - case TPM2_CC_SEQUENCE_UPDATE: /* 15C */
> - return TPM_MEDIUM;
> - case TPM2_CC_SEQUENCE_COMPLETE: /* 13E */
> - return TPM_MEDIUM;
> - case TPM2_CC_EVENT_SEQUENCE_COMPLETE: /* 185 */
> - return TPM_MEDIUM;
> - case TPM2_CC_HASH_SEQUENCE_START: /* 186 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_VERIFY_SIGNATURE: /* 177 */
> - return TPM_LONG_LONG;
> -
> - case TPM2_CC_PCR_EXTEND: /* 182 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_HIERARCHY_CONTROL: /* 121 */
> - return TPM_LONG;
> - case TPM2_CC_HIERARCHY_CHANGE_AUTH: /* 129 */
> - return TPM_LONG;
> -
> - case TPM2_CC_GET_CAPABILITY: /* 17A */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_NV_READ: /* 14E */
> - return TPM_LONG;
> -
> - case TPM2_CC_CREATE_PRIMARY: /* 131 */
> - return TPM_LONG_LONG;
> - case TPM2_CC_CREATE: /* 153 */
> - return TPM_LONG_LONG;
> - case TPM2_CC_CREATE_LOADED: /* 191 */
> - return TPM_LONG_LONG;
> -
> - default:
> - return TPM_UNDEFINED;
> - }
> -}
> +static const struct {
> + unsigned long ordinal;
> + unsigned long duration;
> +} tpm2_ordinal_duration_map[] = {
> + {TPM2_CC_STARTUP, 750},
> + {TPM2_CC_SELF_TEST, 3000},
I assume you intended to increase TPM2_CC_SELF_TEST from 2000 to 3000 here? But it's not mentioned in the commit, so making sure...
> + {TPM2_CC_GET_RANDOM, 2000},
> + {TPM2_CC_SEQUENCE_UPDATE, 750},
> + {TPM2_CC_SEQUENCE_COMPLETE, 750},
> + {TPM2_CC_EVENT_SEQUENCE_COMPLETE, 750},
> + {TPM2_CC_HASH_SEQUENCE_START, 750},
> + {TPM2_CC_VERIFY_SIGNATURE, 30000},
> + {TPM2_CC_PCR_EXTEND, 750},
> + {TPM2_CC_HIERARCHY_CONTROL, 2000},
> + {TPM2_CC_HIERARCHY_CHANGE_AUTH, 2000},
> + {TPM2_CC_GET_CAPABILITY, 750},
> + {TPM2_CC_NV_READ, 2000},
> + {TPM2_CC_CREATE_PRIMARY, 30000},
> + {TPM2_CC_CREATE, 30000},
> + {TPM2_CC_CREATE_LOADED, 30000},
> +};
>
> /**
> - * tpm2_calc_ordinal_duration() - calculate the maximum command duration
> - * @chip: TPM chip to use.
> + * tpm2_calc_ordinal_duration() - Calculate the maximum command
> + duration
> * @ordinal: TPM command ordinal.
> *
> - * The function returns the maximum amount of time the chip could
> take
> - * to return the result for a particular ordinal in jiffies.
> - *
> - * Return: A maximal duration time for an ordinal in jiffies.
> + * Returns the maximum amount of time the chip is expected by kernel
> + to
> + * take in jiffies.
> */
> -unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32
> ordinal)
> +unsigned long tpm2_calc_ordinal_duration(u32 ordinal)
> {
> - unsigned int index;
> + int i;
>
> - index = tpm2_ordinal_duration_index(ordinal);
> + for (i = 0; i < ARRAY_SIZE(tpm2_ordinal_duration_map); i++)
> + if (ordinal == tpm2_ordinal_duration_map[i].ordinal)
> + return msecs_to_jiffies(tpm2_ordinal_duration_map[i].duration);
>
> - if (index != TPM_UNDEFINED)
> - return chip->duration[index];
> - else
> - return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> + return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> }
>
> -
> struct tpm2_pcr_read_out {
> __be32 update_cnt;
> __be32 pcr_selects_cnt;
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h index
> b0e9eb5ef022..dc0338a783f3 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -228,10 +228,11 @@ enum tpm2_timeouts {
> TPM2_TIMEOUT_B = 4000,
> TPM2_TIMEOUT_C = 200,
> TPM2_TIMEOUT_D = 30,
> +};
> +
> +enum tpm2_durations {
> TPM2_DURATION_SHORT = 20,
> - TPM2_DURATION_MEDIUM = 750,
> TPM2_DURATION_LONG = 2000,
> - TPM2_DURATION_LONG_LONG = 300000,
> TPM2_DURATION_DEFAULT = 120000,
> };
>
> --
> 2.39.5
^ permalink raw reply
* Re: [PATCH] keys: allow request-key path to be configured via Kconfig
From: Gary Guo @ 2026-06-08 10:30 UTC (permalink / raw)
To: Jarkko Sakkinen, Gary Guo
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <aiZMT5UYhVDlLVHq@kernel.org>
On Mon Jun 8, 2026 at 5:59 AM BST, Jarkko Sakkinen wrote:
> On Mon, Jun 08, 2026 at 07:50:03AM +0300, Jarkko Sakkinen wrote:
>> On Sun, Jun 07, 2026 at 02:49:27PM +0100, Gary Guo wrote:
>> > From: Gary Guo <gary@garyguo.net>
>> >
>> > Some Linux distributions (e.g. NixOS) does not have /sbin present, and they
>> > currently carry patches to replace /sbin/request-key to some other path.
>>
>> Sorry but no configuration for introducing API divergence.
What is the API divergence here? Distros can already patch the kernel or place a
different binary there, so I don't see what's being gained on not allowing to
change this with Kconfig.
Also to note, the actual binary being called can already be swapped out by
CONFIG_STATIC_USERMODEHELPER_PATH, although for the NixOS this is not the proper
mechanism as it affects coredump too which isn't a fixed path binary in /sbin.
This is really just for distros to be able to configure where /sbin is located.
Given usr merge and (some distros) bin/sbin merge, the canonical path of
request-key binary is very likely not /sbin/request-key anymore, so it seems to
make sense to me to allow this to be changed rather than always go through
compatibility symlinks.
How about a something like CONFIG_DEFAULT_USERMODEHELPER_PATH which defaults to
/sbin, and then request-key uses that concatenated with "/request-key"?
>
> Not sure right now but one option might kernel command-line. Then it is
> known at run-time, can be signed etc. Compiled value has no identity in
> the same way.
>
> And I don't care if NixOS has such a problem as I've not have any stake
> making of those decisions.
>
> You really should explain why it makes sense to have such feature i.e.,
> why is it useful. And if NixOS considered, why is it useful for NixOS.
>
> This all should be in the commit message.
Sure, if you need some more detailed explaination on how NixOS works.
NixOS intentionally not use FHS directory paths, so packages refers to their
dependencies via cryptographical hashes in rather than fixed paths for build-time
known dependencies, and themselves also live in a path with hashes in them (so
two different versions of the same package are in different paths). E.g.
/nix/store/wjzk0s5dwk0i7swh3rmh1pl10k6v1w6d-keyutils-1.6.3/bin/request-key
The full system is also built as a package with all installed binaries in
$pkg/sw/bin, configuration in $pkg/etc, etc.. The current system is symlinked to
/run/current-system, and when a new system is activated this symlink is swapped
out and therefore all paths are updated atomically. For request-key, this is
symlinked to
/run/current-system/sw/bin/request-key
NixOS carries a patch which uses this path instead of /sbin (which does not
exist on NixOS at all).
The motivation is really "be able to switch /sbin". I feel that all the above are
secondary motivations and is too verbose to include in the commit message.
I am not a maintainer for NixOS's kernel; I use NixOS and just want to develop
kernel and test out kernel on my machines without having to patch them.
Best,
Gary
^ permalink raw reply
* Re: [PATCH] keys: prevent slab cache merging for key_jar
From: Vlastimil Babka (SUSE) @ 2026-06-08 7:20 UTC (permalink / raw)
To: Jarkko Sakkinen, Mohammed EL Kadiri
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Kees Cook, Vlastimil Babka, keyrings, linux-security-module,
linux-hardening, linux-kernel
In-Reply-To: <aiZDlSTqaQTcvLBl@kernel.org>
On 6/8/26 06:22, Jarkko Sakkinen wrote:
> On Thu, Jun 04, 2026 at 01:50:34PM +0100, Mohammed EL Kadiri wrote:
>> The key_jar slab cache holds struct key objects containing cryptographic
>> keys, authentication tokens, and keyring linkage. This cache currently
>> lacks merge prevention, allowing the SLUB allocator to merge it with
>> other similarly-sized caches.
>>
>> On a default Ubuntu 6.17.0-23-generic system, key_jar has 5 aliases,
>> meaning 5 unrelated object types share its slab pages. struct key is
>> 224 bytes, placed in 256-byte slabs alongside biovec-16, maple_node,
>> ip6_dst_cache, task_delay_info, and kmalloc-256 users.
>>
>> Cross-cache heap exploitation is a well-documented attack class
>> (CVE-2022-29582, CVE-2022-2588, CVE-2021-22555) where slab cache
>> merging enables type confusion between unrelated kernel objects. A
>> use-after-free in any subsystem sharing slab pages with key_jar could
>> allow an attacker to reclaim a freed slot as a struct key, or corrupt
>> an existing key through a dangling pointer to a different type.
>>
>> Add SLAB_NO_MERGE to ensure key_jar receives dedicated slab pages,
>> eliminating cross-cache attacks targeting struct key. The memory
>> overhead is minimal: with 32 objects per slab page and typical key
>> usage bounded by system keyring size, the cost of dedicated pages is
>> negligible. There is zero performance impact on the allocation hot
>> path.
>>
>> This follows the precedent set by skbuff_head_cache (net/core/skbuff.c)
>> which uses SLAB_NO_MERGE for similar isolation requirements.
I just realized this part is somewhat misleading, because it's done there
for performance reasons, so I wouldn't say "similar".
>
> ~/work/kernel.org/jarkko/linux-tpmdd master*
> ❯ git log --oneline -1 d0bf7d5759c1d89fb013aa41cca5832e00b9632a
> d0bf7d5759c1 mm/slab: introduce kmem_cache flag SLAB_NO_MERGE
>
> ~/work/kernel.org/jarkko/linux-tpmdd master*
> ❯ git describe --contains d0bf7d5759c1d89fb013aa41cca5832e00b9632a
> v6.5-rc1~137^2^3~1
>
> So we could probably forward to stable's starting from v6.6+ if that
> is necessary / makes sense?
It won't hurt, but I doubt it's "necessary" per stable rules. But stable
maintainers ignore those themselves anyway, so whatever.
> It's not a bug fix but kind of still I think would be a change that
> stable kernels are better off with it than without it.
>
> What do you think?
Won't object.
>> Signed-off-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
>> ---
>> security/keys/key.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/security/keys/key.c b/security/keys/key.c
>> index 3bbdde778631..592b65cf8539 100644
>> --- a/security/keys/key.c
>> +++ b/security/keys/key.c
>> @@ -1275,7 +1275,7 @@ void __init key_init(void)
>> {
>> /* allocate a slab in which we can store keys */
>> key_jar = kmem_cache_create("key_jar", sizeof(struct key),
>> - 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
>> + 0, SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_NO_MERGE, NULL);
>>
>> /* add the special key types */
>> list_add_tail(&key_type_keyring.link, &key_types_list);
>> --
>> 2.43.0
>>
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Yeoreum Yun @ 2026-06-08 7:15 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <aiZJAR3-JACaDcwR@kernel.org>
On Mon, Jun 08, 2026 at 07:45:53AM +0300, Jarkko Sakkinen wrote:
> On Fri, Jun 05, 2026 at 03:43:25PM +0100, Yeoreum Yun wrote:
> > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in")
> > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> >
> > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > initialises IMA at the late_initcall_sync level, so this change is no
> > longer required.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> > 1 file changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> > index 99f1c1e5644b..025c4d4b17ca 100644
> > --- a/drivers/char/tpm/tpm_crb_ffa.c
> > +++ b/drivers/char/tpm/tpm_crb_ffa.c
> > @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> > */
> > int tpm_crb_ffa_init(void)
> > {
> > - int ret = 0;
> > -
> > - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> > - ret = ffa_register(&tpm_crb_ffa_driver);
> > - if (ret) {
> > - tpm_crb_ffa = ERR_PTR(-ENODEV);
> > - return ret;
> > - }
> > - }
> > -
> > if (!tpm_crb_ffa)
> > - ret = -ENOENT;
> > + return -ENOENT;
> >
> > if (IS_ERR_VALUE(tpm_crb_ffa))
> > - ret = -ENODEV;
> > + return -ENODEV;
> >
> > - return ret;
> > + return 0;
> > }
> > EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
> >
> > @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> > .id_table = tpm_crb_ffa_device_id,
> > };
> >
> > -#ifdef MODULE
> > module_ffa_driver(tpm_crb_ffa_driver);
> > -#endif
> >
> > MODULE_AUTHOR("Arm");
> > MODULE_DESCRIPTION("TPM CRB FFA driver");
> > --
> > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> >
>
> Is this different I applied?
>
> If yes, I'll swap (if mandatory).
No difference. Thanks! ;)
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* [PATCH] apparmor: check label build before no_new_privs test
From: Ruoyu Wang @ 2026-06-08 6:36 UTC (permalink / raw)
To: John Johansen
Cc: Paul Moore, James Morris, Serge E . Hallyn, apparmor,
linux-security-module, linux-kernel, Ruoyu Wang
aa_change_profile() builds a replacement label with
fn_label_build_in_scope() before the no_new_privs subset check. The build
helper can fail and return NULL or an ERR_PTR, but the result was passed
to aa_label_is_unconfined_subset() before the existing IS_ERR_OR_NULL()
check.
Reuse the existing target-label build failure handling immediately after
the build. This preserves the current audit handling while preventing the
subset helper from dereferencing an invalid label.
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
security/apparmor/domain.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index f02bf770f6385..6748ac74b060b 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -1527,6 +1527,8 @@ int aa_change_profile(const char *fqname, int flags)
new = fn_label_build_in_scope(label, profile, GFP_KERNEL,
aa_get_label(target),
aa_get_label(&profile->label));
+ if (IS_ERR_OR_NULL(new))
+ goto build_fail;
/*
* no new privs prevents domain transitions that would
* reduce restrictions.
@@ -1545,16 +1547,8 @@ int aa_change_profile(const char *fqname, int flags)
/* only transition profiles in the current ns */
if (stack)
new = aa_label_merge(label, target, GFP_KERNEL);
- if (IS_ERR_OR_NULL(new)) {
- info = "failed to build target label";
- if (!new)
- error = -ENOMEM;
- else
- error = PTR_ERR(new);
- new = NULL;
- perms.allow = 0;
- goto audit;
- }
+ if (IS_ERR_OR_NULL(new))
+ goto build_fail;
error = aa_replace_current_label(new);
} else {
if (new) {
@@ -1566,6 +1560,17 @@ int aa_change_profile(const char *fqname, int flags)
aa_set_current_onexec(target, stack);
}
+ goto audit;
+
+build_fail:
+ info = "failed to build target label";
+ if (!new)
+ error = -ENOMEM;
+ else
+ error = PTR_ERR(new);
+ new = NULL;
+ perms.allow = 0;
+
audit:
error = fn_for_each_in_scope(label, profile,
aa_audit_file(subj_cred,
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths
From: Jarkko Sakkinen @ 2026-06-08 5:42 UTC (permalink / raw)
To: Shaomin Chen
Cc: keyrings, linux-security-module, linux-kernel, David Howells,
Paul Moore, James Morris, Serge E. Hallyn
In-Reply-To: <aiZTIzGg_peDVo1M@kernel.org>
On Mon, Jun 08, 2026 at 08:29:11AM +0300, Jarkko Sakkinen wrote:
> On Mon, Jun 08, 2026 at 06:10:23AM +0300, Jarkko Sakkinen wrote:
> > On Mon, Jun 08, 2026 at 06:06:21AM +0300, Jarkko Sakkinen wrote:
> > > On Tue, May 26, 2026 at 10:48:38AM +0800, Shaomin Chen wrote:
> > > > keyctl_instantiate_key_common() reads request_key_auth from the assumed
> > > > auth key before copying an instantiation payload from userspace. The copy
> > > > can fault and sleep. If the request completes and revokes the auth key in
> > > > that window, the auth payload can be detached and freed before the
> > > > instantiate path uses it again.
> > > >
> > > > A request-key helper reproducer can trigger this race. One helper child
> > > > blocks in KEYCTL_INSTANTIATE_IOV while the original helper instantiates the
> > > > requested key and returns. KASAN then reports a use-after-free from the
> > > > stale request_key_auth payload in keyctl_instantiate_key_common().
> > > >
> > > > Give request_key_auth payloads a refcount. Take a payload reference while
> > >
> > > Please, name concrete things accurately. I.e. 'usage' in this case. If
> > > you have a name, use it instead of obfuscating generalizations.
> > >
> > > > authkey->sem stabilizes the payload and revocation state. Hold that
> > > > reference across the instantiate and reject paths. Drop the auth key
> > > > owning reference from revoke and destroy.
> > > >
> > > > Reported-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > > Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@gmail.com
> > > > Signed-off-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > > ---
> > > > include/keys/request_key_auth-type.h | 2 ++
> > > > security/keys/internal.h | 2 ++
> > > > security/keys/keyctl.c | 24 +++++++++++++++-----
> > > > security/keys/request_key_auth.c | 33 ++++++++++++++++++++++++++--
> > > > 4 files changed, 53 insertions(+), 8 deletions(-)
> > >
> > > So first, couple of things.
> > >
> > > I'm not going to test not that well documented involving OOT driver.
> >
> > Oops, sorry typo. "not that well documented reproducer" :-)
> >
> > But it is cool we just then need to draw the picture.
>
> I think I got this:
>
> A: request_key() B: KEYCTL_INSTANTIATE_IOV
> ---------------- -------------------------
> create auth key
> store rka in auth key
> wait for helper
> get auth key
> load rka from auth key
> copy user payload
> sleep on #PF
> helper completed
> detach and free rka
> destroy auth key
> wake up
> use rka->target_key
> **USE-AFTER-FREE**
>
> So nothing really complicated here, is there?
Send v2 with the code changes that I proposed as we want to the change
as ergonomic as possible.
Use this as the commit message:
keys: Pin request_key_auth payload in instantiate paths
A: request_key() B: KEYCTL_INSTANTIATE_IOV
---------------- -------------------------
create auth key
store rka in auth key
wait for helper
get auth key
load rka from auth key
copy user payload
sleep on #PF
helper completed
detach and free rka
destroy auth key
wake up
use rka->target_key
**USE-AFTER-FREE**
Give request_key_auth payloads a refcount. Take a payload reference while
authkey->sem stabilizes the payload and revocation state. Hold that
reference across the instantiate and reject paths. Drop the auth key
owning reference from revoke and destroy.
[jarkko: Replaced the first two paragraphs of text with a concurrency scenario.]
And it includes also the remark at the end.
BR, Jarkko
^ permalink raw reply
* security_inode_follow_link: KASAN UAF localization report
From: David Maximiliano Hermitte @ 2026-06-08 5:31 UTC (permalink / raw)
To: paul
Cc: jmorris, serge, viro, brauner, jack, linux-security-module,
linux-fsdevel, linux-kernel, David Maximiliano Hermitte
Hello,
I reproduced this issue locally in a QEMU/TCG VM and I can confirm a valid BEFORE signal.
Summary of the local evidence:
- Reproducer started: yes
- KASAN seen: yes
- use-after-free seen: yes
- target function seen: security_inode_follow_link
- target file seen: security/security.c
- Call Trace seen: yes
- RIP seen: yes
- BEFORE validation: true
At this point I am treating this as a localization report, not as a final patch submission.
The trace points to the security_inode_follow_link / link-follow path. I would prefer not to guess the final fix, since I do not yet have a validated AFTER patch for this issue.
I can provide the reproducer evidence and retest any proposed patch if helpful.
Thanks,
David
^ permalink raw reply
* Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths
From: Jarkko Sakkinen @ 2026-06-08 5:29 UTC (permalink / raw)
To: Shaomin Chen
Cc: keyrings, linux-security-module, linux-kernel, David Howells,
Paul Moore, James Morris, Serge E. Hallyn
In-Reply-To: <aiYynOxFk3LuK4LA@kernel.org>
On Mon, Jun 08, 2026 at 06:10:23AM +0300, Jarkko Sakkinen wrote:
> On Mon, Jun 08, 2026 at 06:06:21AM +0300, Jarkko Sakkinen wrote:
> > On Tue, May 26, 2026 at 10:48:38AM +0800, Shaomin Chen wrote:
> > > keyctl_instantiate_key_common() reads request_key_auth from the assumed
> > > auth key before copying an instantiation payload from userspace. The copy
> > > can fault and sleep. If the request completes and revokes the auth key in
> > > that window, the auth payload can be detached and freed before the
> > > instantiate path uses it again.
> > >
> > > A request-key helper reproducer can trigger this race. One helper child
> > > blocks in KEYCTL_INSTANTIATE_IOV while the original helper instantiates the
> > > requested key and returns. KASAN then reports a use-after-free from the
> > > stale request_key_auth payload in keyctl_instantiate_key_common().
> > >
> > > Give request_key_auth payloads a refcount. Take a payload reference while
> >
> > Please, name concrete things accurately. I.e. 'usage' in this case. If
> > you have a name, use it instead of obfuscating generalizations.
> >
> > > authkey->sem stabilizes the payload and revocation state. Hold that
> > > reference across the instantiate and reject paths. Drop the auth key
> > > owning reference from revoke and destroy.
> > >
> > > Reported-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@gmail.com
> > > Signed-off-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > ---
> > > include/keys/request_key_auth-type.h | 2 ++
> > > security/keys/internal.h | 2 ++
> > > security/keys/keyctl.c | 24 +++++++++++++++-----
> > > security/keys/request_key_auth.c | 33 ++++++++++++++++++++++++++--
> > > 4 files changed, 53 insertions(+), 8 deletions(-)
> >
> > So first, couple of things.
> >
> > I'm not going to test not that well documented involving OOT driver.
>
> Oops, sorry typo. "not that well documented reproducer" :-)
>
> But it is cool we just then need to draw the picture.
I think I got this:
A: request_key() B: KEYCTL_INSTANTIATE_IOV
---------------- -------------------------
create auth key
store rka in auth key
wait for helper
get auth key
load rka from auth key
copy user payload
sleep on #PF
helper completed
detach and free rka
destroy auth key
wake up
use rka->target_key
**USE-AFTER-FREE**
So nothing really complicated here, is there?
`
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: allow request-key path to be configured via Kconfig
From: Jarkko Sakkinen @ 2026-06-08 4:59 UTC (permalink / raw)
To: Gary Guo
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <aiZJ94eugtNHcILD@kernel.org>
On Mon, Jun 08, 2026 at 07:50:03AM +0300, Jarkko Sakkinen wrote:
> On Sun, Jun 07, 2026 at 02:49:27PM +0100, Gary Guo wrote:
> > From: Gary Guo <gary@garyguo.net>
> >
> > Some Linux distributions (e.g. NixOS) does not have /sbin present, and they
> > currently carry patches to replace /sbin/request-key to some other path.
>
> Sorry but no configuration for introducing API divergence.
Not sure right now but one option might kernel command-line. Then it is
known at run-time, can be signed etc. Compiled value has no identity in
the same way.
And I don't care if NixOS has such a problem as I've not have any stake
making of those decisions.
You really should explain why it makes sense to have such feature i.e.,
why is it useful. And if NixOS considered, why is it useful for NixOS.
This all should be in the commit message.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: allow request-key path to be configured via Kconfig
From: Jarkko Sakkinen @ 2026-06-08 4:49 UTC (permalink / raw)
To: Gary Guo
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260607134928.2832202-1-gary@kernel.org>
On Sun, Jun 07, 2026 at 02:49:27PM +0100, Gary Guo wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Some Linux distributions (e.g. NixOS) does not have /sbin present, and they
> currently carry patches to replace /sbin/request-key to some other path.
Sorry but no configuration for introducing API divergence.
>
> Follow the way modprobe handles this by making this a Kconfig option which
> defaults to the current /sbin/request-key.
>
> Also changed "char const" to "const char" as checkpatch complains
> otherwise.
>
> Link: https://github.com/NixOS/nixpkgs/blob/6b316287bae2ee04c9b93c8c858d930fd07d7338/pkgs/os-specific/linux/kernel/request-key-helper.patch
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
> I did not update mentions of /sbin/request-key in documentation and
> elsewhere, as "/sbin/request-key" is concise while "request-key UMH" is
> more mouthful and less clear.
>
> Number of distros that doesn't have /sbin is limited so I think it wouldn't
> create much confusion. Similarly, there are a lot of places where
> /sbin/modprobe is mentioned despite it is technically configurable.
> ---
> security/keys/Kconfig | 9 +++++++++
> security/keys/request_key.c | 2 +-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index f4510d8cb485..ee3c3d85fc03 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -40,6 +40,15 @@ config KEYS_REQUEST_CACHE
> key. Pathwalk will call multiple methods for each dentry traversed
> (permission, d_revalidate, lookup, getxattr, getacl, ...).
>
> +config REQUEST_KEY_PATH
> + string "Path to the request-key binary"
> + default "/sbin/request-key"
> + help
> + Path of the request-key usermode helper binary.
> +
> + This program is invoked by the kernel when the kernel is asked for
> + a key that it doesn't have immediately available.
> +
> config PERSISTENT_KEYRINGS
> bool "Enable register of persistent per-UID keyrings"
> help
> diff --git a/security/keys/request_key.c b/security/keys/request_key.c
> index a7673ad86d18..ac8f9d1a87ad 100644
> --- a/security/keys/request_key.c
> +++ b/security/keys/request_key.c
> @@ -117,7 +117,7 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
> */
> static int call_sbin_request_key(struct key *authkey, void *aux)
> {
> - static char const request_key[] = "/sbin/request-key";
> + static const char request_key[] = CONFIG_REQUEST_KEY_PATH;
> struct request_key_auth *rka = get_request_key_auth(authkey);
> const struct cred *cred = current_cred();
> key_serial_t prkey, sskey;
>
> base-commit: 6e845bcb78c95af935094040bd4edc3c2b6dd784
> --
> 2.54.0
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH next] keys: Replace strcpy(derived_buf, "AUTH_KEY") with strscpy(..., HASH_SIZE)
From: Jarkko Sakkinen @ 2026-06-08 4:46 UTC (permalink / raw)
To: david.laight.linux
Cc: Kees Cook, linux-hardening, Arnd Bergmann, keyrings,
linux-integrity, linux-kernel, linux-security-module,
David Howells, James Morris, Mimi Zohar, Paul Moore,
Serge E. Hallyn
In-Reply-To: <20260606202633.5018-9-david.laight.linux@gmail.com>
On Sat, Jun 06, 2026 at 09:26:03PM +0100, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> derived_buf is guaranteed to be HASH_SIZE - and it is more than enough.
> The strscpy() degenerates into an memcpy() (as did the strcpy()).
> Do the same for the associated "ENC_KEY" copy.
>
> Removes a possibly unbounded strcpy().
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> This is one of a group of patches that remove potentially unbounded
> strcpy() calls.
>
> They are mostly replaced by strscpy() or, when strlen() has just been
> called, with memcpy() (usually including the '\0').
>
> Calls with copy string literals into arrays are left unchanged.
> They are safe and easily detected as such.
>
> The changes were made by getting the compiler to detect the calls and
> then fixing the code by hand.
>
> Note that all the changes are only compile tested.
>
> Some Makefiles were changed to allow files to contain strcpy().
> As well as 'difficult to fix' files, this included 'show' functions
> as they really need to use sysfs_emit() or seq_printf().
>
> All the patches are being sent individually to avoid very long cc lists.
> Apologies for the terse commit messages and likely unexpected tags.
> (There are about 100 patches in total.)
>
> security/keys/encrypted-keys/encrypted.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 56b531587a1e..59cb77b237b3 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -343,9 +343,9 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
> return -ENOMEM;
>
> if (key_type)
> - strcpy(derived_buf, "AUTH_KEY");
> + strscpy(derived_buf, "AUTH_KEY", HASH_SIZE);
> else
> - strcpy(derived_buf, "ENC_KEY");
> + strscpy(derived_buf, "ENC_KEY", HASH_SIZE);
>
> memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
> master_keylen);
> --
> 2.39.5
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 4:45 UTC (permalink / raw)
To: Yeoreum Yun
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260605144325.434436-5-yeoreum.yun@arm.com>
On Fri, Jun 05, 2026 at 03:43:25PM +0100, Yeoreum Yun wrote:
> commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in")
> probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
>
> However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> initialises IMA at the late_initcall_sync level, so this change is no
> longer required.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> 1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> index 99f1c1e5644b..025c4d4b17ca 100644
> --- a/drivers/char/tpm/tpm_crb_ffa.c
> +++ b/drivers/char/tpm/tpm_crb_ffa.c
> @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> */
> int tpm_crb_ffa_init(void)
> {
> - int ret = 0;
> -
> - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> - ret = ffa_register(&tpm_crb_ffa_driver);
> - if (ret) {
> - tpm_crb_ffa = ERR_PTR(-ENODEV);
> - return ret;
> - }
> - }
> -
> if (!tpm_crb_ffa)
> - ret = -ENOENT;
> + return -ENOENT;
>
> if (IS_ERR_VALUE(tpm_crb_ffa))
> - ret = -ENODEV;
> + return -ENODEV;
>
> - return ret;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
>
> @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> .id_table = tpm_crb_ffa_device_id,
> };
>
> -#ifdef MODULE
> module_ffa_driver(tpm_crb_ffa_driver);
> -#endif
>
> MODULE_AUTHOR("Arm");
> MODULE_DESCRIPTION("TPM CRB FFA driver");
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>
Is this different I applied?
If yes, I'll swap (if mandatory).
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: prevent slab cache merging for key_jar
From: Jarkko Sakkinen @ 2026-06-08 4:27 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Mohammed EL Kadiri, David Howells, Paul Moore, James Morris,
Serge E . Hallyn, Kees Cook, Vlastimil Babka, keyrings,
linux-security-module, linux-hardening, linux-kernel
In-Reply-To: <19fb9d38-fe02-4653-bbad-58806e55ca84@kernel.org>
On Fri, Jun 05, 2026 at 02:16:00PM +0200, Vlastimil Babka (SUSE) wrote:
> On 6/4/26 14:50, Mohammed EL Kadiri wrote:
> > The key_jar slab cache holds struct key objects containing cryptographic
> > keys, authentication tokens, and keyring linkage. This cache currently
> > lacks merge prevention, allowing the SLUB allocator to merge it with
> > other similarly-sized caches.
> >
> > On a default Ubuntu 6.17.0-23-generic system, key_jar has 5 aliases,
> > meaning 5 unrelated object types share its slab pages. struct key is
> > 224 bytes, placed in 256-byte slabs alongside biovec-16, maple_node,
> > ip6_dst_cache, task_delay_info, and kmalloc-256 users.
> >
> > Cross-cache heap exploitation is a well-documented attack class
> > (CVE-2022-29582, CVE-2022-2588, CVE-2021-22555) where slab cache
> > merging enables type confusion between unrelated kernel objects. A
> > use-after-free in any subsystem sharing slab pages with key_jar could
> > allow an attacker to reclaim a freed slot as a struct key, or corrupt
> > an existing key through a dangling pointer to a different type.
> >
> > Add SLAB_NO_MERGE to ensure key_jar receives dedicated slab pages,
> > eliminating cross-cache attacks targeting struct key. The memory
> > overhead is minimal: with 32 objects per slab page and typical key
> > usage bounded by system keyring size, the cost of dedicated pages is
> > negligible. There is zero performance impact on the allocation hot
> > path.
> >
> > This follows the precedent set by skbuff_head_cache (net/core/skbuff.c)
> > which uses SLAB_NO_MERGE for similar isolation requirements.
> >
> > Signed-off-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
>
> Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Thank you.
has been applied
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: prevent slab cache merging for key_jar
From: Jarkko Sakkinen @ 2026-06-08 4:22 UTC (permalink / raw)
To: Mohammed EL Kadiri
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Kees Cook, Vlastimil Babka, keyrings, linux-security-module,
linux-hardening, linux-kernel
In-Reply-To: <20260604125034.13757-1-med08elkadiri@gmail.com>
On Thu, Jun 04, 2026 at 01:50:34PM +0100, Mohammed EL Kadiri wrote:
> The key_jar slab cache holds struct key objects containing cryptographic
> keys, authentication tokens, and keyring linkage. This cache currently
> lacks merge prevention, allowing the SLUB allocator to merge it with
> other similarly-sized caches.
>
> On a default Ubuntu 6.17.0-23-generic system, key_jar has 5 aliases,
> meaning 5 unrelated object types share its slab pages. struct key is
> 224 bytes, placed in 256-byte slabs alongside biovec-16, maple_node,
> ip6_dst_cache, task_delay_info, and kmalloc-256 users.
>
> Cross-cache heap exploitation is a well-documented attack class
> (CVE-2022-29582, CVE-2022-2588, CVE-2021-22555) where slab cache
> merging enables type confusion between unrelated kernel objects. A
> use-after-free in any subsystem sharing slab pages with key_jar could
> allow an attacker to reclaim a freed slot as a struct key, or corrupt
> an existing key through a dangling pointer to a different type.
>
> Add SLAB_NO_MERGE to ensure key_jar receives dedicated slab pages,
> eliminating cross-cache attacks targeting struct key. The memory
> overhead is minimal: with 32 objects per slab page and typical key
> usage bounded by system keyring size, the cost of dedicated pages is
> negligible. There is zero performance impact on the allocation hot
> path.
>
> This follows the precedent set by skbuff_head_cache (net/core/skbuff.c)
> which uses SLAB_NO_MERGE for similar isolation requirements.
>
~/work/kernel.org/jarkko/linux-tpmdd master*
❯ git log --oneline -1 d0bf7d5759c1d89fb013aa41cca5832e00b9632a
d0bf7d5759c1 mm/slab: introduce kmem_cache flag SLAB_NO_MERGE
~/work/kernel.org/jarkko/linux-tpmdd master*
❯ git describe --contains d0bf7d5759c1d89fb013aa41cca5832e00b9632a
v6.5-rc1~137^2^3~1
So we could probably forward to stable's starting from v6.6+ if that
is necessary / makes sense?
It's not a bug fix but kind of still I think would be a change that
stable kernels are better off with it than without it.
What do you think?
> Signed-off-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
> ---
> security/keys/key.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 3bbdde778631..592b65cf8539 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -1275,7 +1275,7 @@ void __init key_init(void)
> {
> /* allocate a slab in which we can store keys */
> key_jar = kmem_cache_create("key_jar", sizeof(struct key),
> - 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> + 0, SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_NO_MERGE, NULL);
>
> /* add the special key types */
> list_add_tail(&key_type_keyring.link, &key_types_list);
> --
> 2.43.0
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v5 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 4:01 UTC (permalink / raw)
To: Yeoreum Yun
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <ah7TAk3iItltddzT@e129823.arm.com>
On Tue, Jun 02, 2026 at 01:56:34PM +0100, Yeoreum Yun wrote:
> > On Mon, Jun 01, 2026 at 03:27:49PM +0100, Yeoreum Yun wrote:
> > > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's build_in")
> > > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> > >
> > > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > > initialises IMA at the late_initcall_sync level, so this change is no
> > > longer required.
> > >
> > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> Might be rb tag?. Thanks!
>
> --
> Sincerely,
> Yeoreum Yun
SOB is enough information as it is a small change. I pushed it to
for-next-tpm.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v4 3/3] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 3:54 UTC (permalink / raw)
To: Sudeep Holla
Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
paul, zohar, roberto.sassu, noodles, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260602-vague-proficient-gibbon-b005c0@sudeepholla>
On Tue, Jun 02, 2026 at 10:57:25AM +0100, Sudeep Holla wrote:
> On Tue, Jun 02, 2026 at 04:50:42AM +0300, Jarkko Sakkinen wrote:
> > On Mon, Jun 01, 2026 at 09:54:08AM +0100, Sudeep Holla wrote:
> > > On Mon, Jun 01, 2026 at 08:17:13AM +0100, Yeoreum Yun wrote:
> > > > Hi Jarkko,
> > > >
> > > > Sorry for late answer.
> > > >
> > > > > On Mon, May 25, 2026 at 08:54:04AM +0100, Yeoreum Yun wrote:
> > > > > > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's build_in")
> > > > > > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> > > > > >
> > > > > > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > > > > > initialises IMA at the late_initcall_sync level, so this change is no
> > > > > > longer required.
> > > > > >
> > > > > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > > > > > ---
> > > > > > drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> > > > > > 1 file changed, 3 insertions(+), 15 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > index 99f1c1e5644b..025c4d4b17ca 100644
> > > > > > --- a/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > +++ b/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> > > > > > */
> > > > > > int tpm_crb_ffa_init(void)
> > > > > > {
> > > > > > - int ret = 0;
> > > > > > -
> > > > > > - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> > > > > > - ret = ffa_register(&tpm_crb_ffa_driver);
> > > > > > - if (ret) {
> > > > > > - tpm_crb_ffa = ERR_PTR(-ENODEV);
> > > > > > - return ret;
> > > > > > - }
> > > > > > - }
> > > > > > -
> > > > > > if (!tpm_crb_ffa)
> > > > > > - ret = -ENOENT;
> > > > > > + return -ENOENT;
> > > > > >
> > > > > > if (IS_ERR_VALUE(tpm_crb_ffa))
> > > > > > - ret = -ENODEV;
> > > > > > + return -ENODEV;
> > > > > >
> > > > > > - return ret;
> > > > > > + return 0;
> > > > > > }
> > > > > > EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
> > > > > >
> > > > > > @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> > > > > > .id_table = tpm_crb_ffa_device_id,
> > > > > > };
> > > > > >
> > > > > > -#ifdef MODULE
> > > > > > module_ffa_driver(tpm_crb_ffa_driver);
> > > > > > -#endif
> > > > > >
> > > > > > MODULE_AUTHOR("Arm");
> > > > > > MODULE_DESCRIPTION("TPM CRB FFA driver");
> > > > > > --
> > > > > > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> > > > > >
> > > > >
> > > > > How we would sync up this patch? Through which tree etc.
> > > >
> > > > IMHO, the IMA relevant thing would be into IMA tree,
> > > > However I think this patch would be much easier to sync into Sudeep's
> > > > FF-A tree where ff-a initilisation is reverted to device_initcall
> > > > unless you're uncomfortable.
> > > >
> > > > For this, It might be better to split this patch from this series
> > > > since by above and defer probe of ff-a would make a register failure
> > > > of registering tpm_crb_ffa driver which is built-in.
> > > >
> > > > @Sudeep what do you think?
> > > >
> > >
> > > IIRC, there is/was no dependency between these and FF-A patches that are
> > > queued in terms of build. I agree there may be dependency to get all the
> > > functionality but we can resort to linux-next for that. FF-A is not enabled
> > > in the defconfig, so anyone working on FF-A + TPM must enable then and can
> > > rely on -next IMHO.
> > >
> > > That said, I have already sent PR for FF-A to SoC team and it is already
> > > queued for v7.2. I don't have any other plans unless they are fixes.
> >
> > Works for me.
> >
>
> Sorry if I was not clear. I haven't included any TPM patches in this series
> as part of my FF-A pull request queued for v7.2. So I was asking to route this
> via your tree.
It's now in 'for-next-tpm'. Just pushed.
>
> --
> Regards,
> Sudeep
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: Use acquire when reading state in keyring search
From: Jarkko Sakkinen @ 2026-06-08 3:48 UTC (permalink / raw)
To: Gui-Dong Han
Cc: dhowells, keyrings, ebiggers, linux-security-module, linux-kernel,
baijiaju1990
In-Reply-To: <CALbr=LYaca4+=hTsZJORQfrtzyUAGo8c+4ZEgkzgkFWQDWNJBA@mail.gmail.com>
On Tue, Jun 02, 2026 at 05:42:26PM +0800, Gui-Dong Han wrote:
> On Sat, May 30, 2026 at 9:02 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Fri, May 29, 2026 at 11:34:06AM +0800, Gui-Dong Han wrote:
> > > The negative-key race fix added release/acquire ordering for key use.
> > >
> > > Publish payload before state; read state before payload.
> > >
> > > keyring_search_iterator() still uses READ_ONCE() before match callbacks.
> > > An asymmetric match callback calls asymmetric_key_ids(), which reads
> > > key->payload.data[asym_key_ids].
> > >
> > > Use key_read_state() there to complete that ordering.
> >
> > OK, so... I'm having a bit trouble understanding the exact concurrency
> > scenario you're trying to describe despite I think I get the fix itself
> > i.e. it is not pairing with mark_key_instantiated?
>
> Yes, it is intended to pair with mark_key_instantiated().
OK, right. I'll apply this.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths
From: Jarkko Sakkinen @ 2026-06-08 3:10 UTC (permalink / raw)
To: Shaomin Chen
Cc: keyrings, linux-security-module, linux-kernel, David Howells,
Paul Moore, James Morris, Serge E. Hallyn
In-Reply-To: <aiYxqTyfHwrDOTCs@kernel.org>
On Mon, Jun 08, 2026 at 06:06:21AM +0300, Jarkko Sakkinen wrote:
> On Tue, May 26, 2026 at 10:48:38AM +0800, Shaomin Chen wrote:
> > keyctl_instantiate_key_common() reads request_key_auth from the assumed
> > auth key before copying an instantiation payload from userspace. The copy
> > can fault and sleep. If the request completes and revokes the auth key in
> > that window, the auth payload can be detached and freed before the
> > instantiate path uses it again.
> >
> > A request-key helper reproducer can trigger this race. One helper child
> > blocks in KEYCTL_INSTANTIATE_IOV while the original helper instantiates the
> > requested key and returns. KASAN then reports a use-after-free from the
> > stale request_key_auth payload in keyctl_instantiate_key_common().
> >
> > Give request_key_auth payloads a refcount. Take a payload reference while
>
> Please, name concrete things accurately. I.e. 'usage' in this case. If
> you have a name, use it instead of obfuscating generalizations.
>
> > authkey->sem stabilizes the payload and revocation state. Hold that
> > reference across the instantiate and reject paths. Drop the auth key
> > owning reference from revoke and destroy.
> >
> > Reported-by: Shaomin Chen <eeesssooo020@gmail.com>
> > Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@gmail.com
> > Signed-off-by: Shaomin Chen <eeesssooo020@gmail.com>
> > ---
> > include/keys/request_key_auth-type.h | 2 ++
> > security/keys/internal.h | 2 ++
> > security/keys/keyctl.c | 24 +++++++++++++++-----
> > security/keys/request_key_auth.c | 33 ++++++++++++++++++++++++++--
> > 4 files changed, 53 insertions(+), 8 deletions(-)
>
> So first, couple of things.
>
> I'm not going to test not that well documented involving OOT driver.
Oops, sorry typo. "not that well documented reproducer" :-)
But it is cool we just then need to draw the picture.
BR, Jarkko
^ 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