* Re: [RFC PATCH v1 11/11] landlock: Add documentation for capability and namespace restrictions
From: Justin Suess @ 2026-04-23 16:08 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Christian Brauner, Günther Noack, Paul Moore,
Serge E . Hallyn, Lennart Poettering, Mikhail Ivanov,
Nicolas Bouchinet, Shervin Oloumi, Tingmao Wang, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module
In-Reply-To: <aeo7m6LCE0Pi_O-V@suesslenovo>
On Thu, Apr 23, 2026 at 12:01:08PM -0400, Justin Suess wrote:
> On Thu, Apr 23, 2026 at 03:51:32PM +0200, Mickaël Salaün wrote:
> > On Thu, Mar 12, 2026 at 10:48:42AM -0400, Justin Suess wrote:
> > > On Thu, Mar 12, 2026 at 11:04:44AM +0100, Mickaël Salaün wrote:
> > > > Document the two new Landlock permission categories in the userspace
> > > > API guide, admin guide, and kernel security documentation.
> > > >
> > > > The userspace API guide adds sections on capability restriction
> > > > (LANDLOCK_PERM_CAPABILITY_USE with LANDLOCK_RULE_CAPABILITY), namespace
> > > > restriction (LANDLOCK_PERM_NAMESPACE_ENTER with LANDLOCK_RULE_NAMESPACE
> > > > covering creation via unshare/clone and entry via setns), and the
> > > > backward-compatible degradation pattern for ABI < 9. A table documents
> > > > the per-namespace-type capability requirements for both creation and
> > > > entry.
> > > >
> > > > The admin guide adds the new perm.namespace_enter and
> > > > perm.capability_use audit blocker names with their object identification
> > > > fields (namespace_type, namespace_inum, capability).
> > > >
> > > > The kernel security documentation adds a "Ruleset restriction models"
> > > > section defining the three models (handled_access_*, handled_perm,
> > > > scoped), their coverage and compatibility properties, and the criteria
> > > > for choosing between them for future features. It also documents
> > > > composability with user namespaces and adds kernel-doc references for
> > > > the new capability and namespace headers.
> > > >
> > > > Cc: Christian Brauner <brauner@kernel.org>
> > > > Cc: Günther Noack <gnoack@google.com>
> > > > Cc: Paul Moore <paul@paul-moore.com>
> > > > Cc: Serge E. Hallyn <serge@hallyn.com>
> > > > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > > > ---
> > > > Documentation/admin-guide/LSM/landlock.rst | 19 ++-
> > > > Documentation/security/landlock.rst | 80 ++++++++++-
> > > > Documentation/userspace-api/landlock.rst | 156 ++++++++++++++++++++-
> > > > 3 files changed, 245 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/Documentation/admin-guide/LSM/landlock.rst b/Documentation/admin-guide/LSM/landlock.rst
> > > > index 9923874e2156..99c6a599ce9e 100644
> > > > --- a/Documentation/admin-guide/LSM/landlock.rst
> > > > +++ b/Documentation/admin-guide/LSM/landlock.rst
> > > > @@ -6,7 +6,7 @@ Landlock: system-wide management
> > > > ================================
> > > >
> > > > :Author: Mickaël Salaün
> > > > -:Date: January 2026
> > > > +:Date: March 2026
> > > >
> > > > Landlock can leverage the audit framework to log events.
> > > >
> > > > @@ -59,14 +59,25 @@ AUDIT_LANDLOCK_ACCESS
> > > > - scope.abstract_unix_socket - Abstract UNIX socket connection denied
> > > > - scope.signal - Signal sending denied
> > > >
> > > > + **perm.*** - Permission restrictions (ABI 9+):
> > > > + - perm.namespace_enter - Namespace entry was denied (creation via
> > > > + :manpage:`unshare(2)` / :manpage:`clone(2)` or joining via
> > > > + :manpage:`setns(2)`);
> > > > + ``namespace_type`` indicates the type (hex CLONE_NEW* bitmask),
> > > > + ``namespace_inum`` identifies the target namespace for
> > > > + :manpage:`setns(2)` operations
> > > > + - perm.capability_use - Capability use was denied;
> > > > + ``capability`` indicates the capability number
> > > > +
> > > > Multiple blockers can appear in a single event (comma-separated) when
> > > > multiple access rights are missing. For example, creating a regular file
> > > > in a directory that lacks both ``make_reg`` and ``refer`` rights would show
> > > > ``blockers=fs.make_reg,fs.refer``.
> > > >
> > > > - The object identification fields (path, dev, ino for filesystem; opid,
> > > > - ocomm for signals) depend on the type of access being blocked and provide
> > > > - context about what resource was involved in the denial.
> > > > + The object identification fields depend on the type of access being blocked:
> > > > + ``path``, ``dev``, ``ino`` for filesystem; ``opid``, ``ocomm`` for signals;
> > > > + ``namespace_type`` and ``namespace_inum`` for namespace operations;
> > > > + ``capability`` for capability use.
> > > >
> > > >
> > > > AUDIT_LANDLOCK_DOMAIN
> > > > diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
> > > > index 3e4d4d04cfae..cd3d640ca5c9 100644
> > > > --- a/Documentation/security/landlock.rst
> > > > +++ b/Documentation/security/landlock.rst
> > > > @@ -7,7 +7,7 @@ Landlock LSM: kernel documentation
> > > > ==================================
> > > >
> > > > :Author: Mickaël Salaün
> > > > -:Date: September 2025
> > > > +:Date: March 2026
> > > >
> > > > Landlock's goal is to create scoped access-control (i.e. sandboxing). To
> > > > harden a whole system, this feature should be available to any process,
> > > > @@ -89,6 +89,72 @@ this is required to keep access controls consistent over the whole system, and
> > > > this avoids unattended bypasses through file descriptor passing (i.e. confused
> > > > deputy attack).
> > > >
> > > > +Composability with user namespaces
> > > > +----------------------------------
> > > > +
> > > > +Landlock domain-based scoping and the kernel's user namespace-based capability
> > > > +scoping enforce isolation over independent hierarchies. Landlock checks domain
> > > > +ancestry; the kernel's ``ns_capable()`` checks user namespace ancestry. These
> > > > +hierarchies are orthogonal: Landlock enforcement is deterministic with respect
> > > > +to its own configuration, regardless of namespace or capability state, and vice
> > > > +versa. This orthogonality is a design invariant that must hold for all new
> > > > +scoped features.
> > > The last sentence on orthogonality may better belong under the restriction
> > > model section for scoped access rights. I assume that future scopes must
> > > also be deterministic with respect to landlock's configuration as well,
> > > not just user namespaces.
> >
> > Correct
> >
> > > > +
> > > > +Ruleset restriction models
> > > > +--------------------------
> > > +1
> > >
> > > This section is very helpful for aligning new features with a particular
> > > model.
> >
> > Thanks
> >
> > >
> > > > +
> > > > +Landlock provides three restriction models, each with different coverage
> > > > +and compatibility properties.
> > > Maybe add:
> > >
> > > Each restriction model below corresponds to one or more fields of
> > > ``struct landlock_ruleset_attr``.
> >
> > Ok
> >
> > >
> > > > +
> > > > +Access rights (``handled_access_*``)
> > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +Access rights control **enumerated operations on kernel objects**
> > > > +identified by a rule key (a file hierarchy or a network port). Each
> > > > +``handled_access_*`` field declares a set of access rights that the
> > > > +ruleset restricts. Multiple access rights share a single rule type.
> > > > +Operations for which no access right exists yet remain uncontrolled;
> > > > +new rights are added incrementally across ABI versions.
> > > > +
> > > > +Permissions (``handled_perm``)
> > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +Permissions control **broad operations enforced at single kernel
> > > > +chokepoints**, achieving complete deny-by-default coverage. Each
> > > > +``LANDLOCK_PERM_*`` flag maps to its own rule type. When a ruleset
> > > > +handles a permission, all instances of that operation are denied unless
> > > > +explicitly allowed by a rule. New kernel values (new ``CAP_*``
> > > > +capabilities, new ``CLONE_NEW*`` namespace types) are automatically
> > > > +denied without any Landlock update.
> > > > +
> > > > +Each permission flag names a single gateway operation whose control
> > > > +transitively covers an open-ended set of downstream operations: for
> > > > +example, exercising a capability enables privileged operations across
> > > > +many subsystems; entering a namespace enables gaining capabilities in a
> > > > +new context.
> > > > +
> > > > +Permission rules identify what to allow using constants defined by other
> > > > +kernel subsystems (``CAP_*``, ``CLONE_NEW*``). Unknown values are
> > > > +silently ignored because deny-by-default ensures they are denied anyway.
> > > > +In contrast, unknown ``LANDLOCK_PERM_*`` flags in ``handled_perm`` are
> > > > +rejected (``-EINVAL``), since Landlock owns that namespace.
> > > > +
> > > > +Scopes (``scoped``)
> > > > +~~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +Scopes restrict **cross-domain interactions** categorically, without
> > > > +rules. Setting a scope flag (e.g. ``LANDLOCK_SCOPE_SIGNAL``) denies the
> > > > +operation to targets outside the Landlock domain or its children. Like
> > > > +permissions, scopes provide complete coverage of the controlled
> > > > +operation.
> > > > +
> > > > +When adding new Landlock features, new operations on existing rule types
> > > > +extend the corresponding ``handled_access_*`` field (e.g. a new
> > > > +filesystem operation extends ``handled_access_fs``). A new object
> > > > +category with multiple fine-grained operations would use a new
> > > > +``handled_access_*`` field. New rule types that control a single
> > > > +chokepoint operation use ``handled_perm``.
> > > > +
> > > > Tests
> > > > =====
> > > >
> > > > @@ -110,6 +176,18 @@ Filesystem
> > > > .. kernel-doc:: security/landlock/fs.h
> > > > :identifiers:
> > > >
> > > > +Namespace
> > > > +---------
> > > > +
> > > > +.. kernel-doc:: security/landlock/ns.h
> > > > + :identifiers:
> > > > +
> > > > +Capability
> > > > +----------
> > > > +
> > > > +.. kernel-doc:: security/landlock/cap.h
> > > > + :identifiers:
> > > > +
> > > > Process credential
> > > > ------------------
> > > >
> > > > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> > > > index 13134bccdd39..238d30a18162 100644
> > > > --- a/Documentation/userspace-api/landlock.rst
> > > > +++ b/Documentation/userspace-api/landlock.rst
> > > > @@ -8,7 +8,7 @@ Landlock: unprivileged access control
> > > > =====================================
> > > >
> > > > :Author: Mickaël Salaün
> > > > -:Date: January 2026
> > > > +:Date: March 2026
> > > >
> > > > The goal of Landlock is to enable restriction of ambient rights (e.g. global
> > > > filesystem or network access) for a set of processes. Because Landlock
> > > > @@ -33,7 +33,7 @@ A Landlock rule describes an action on an object which the process intends to
> > > > perform. A set of rules is aggregated in a ruleset, which can then restrict
> > > > the thread enforcing it, and its future children.
> > > >
> > > > -The two existing types of rules are:
> > > > +The existing types of rules are:
> > > >
> > > > Filesystem rules
> > > > For these rules, the object is a file hierarchy,
> > > > @@ -44,6 +44,14 @@ Network rules (since ABI v4)
> > > > For these rules, the object is a TCP port,
> > > > and the related actions are defined with `network access rights`.
> > > >
> > > > +Capability rules (since ABI v9)
> > > > + For these rules, the object is a set of Linux capabilities,
> > > > + and the related actions are defined with `permission flags`.
> > > > +
> > > > +Namespace rules (since ABI v9)
> > > > + For these rules, the object is a set of namespace types,
> > > > + and the related actions are defined with `permission flags`.
> > > > +
> > > > Defining and enforcing a security policy
> > > > ----------------------------------------
> > > >
> > > > @@ -84,6 +92,9 @@ to be explicit about the denied-by-default access rights.
> > > > .scoped =
> > > > LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> > > > LANDLOCK_SCOPE_SIGNAL,
> > > > + .handled_perm =
> > > > + LANDLOCK_PERM_CAPABILITY_USE |
> > > > + LANDLOCK_PERM_NAMESPACE_ENTER,
> > > > };
> > > >
> > > > Because we may not know which kernel version an application will be executed
> > > > @@ -127,6 +138,12 @@ version, and only use the available subset of access rights:
> > > > /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
> > > > ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> > > > LANDLOCK_SCOPE_SIGNAL);
> > > > + __attribute__((fallthrough));
> > > > + case 6:
> > > > + case 7:
> > > > + case 8:
> > > > + /* Removes permission support for ABI < 9 */
> > > > + ruleset_attr.handled_perm = 0;
> > > > }
> > > >
> > > > This enables the creation of an inclusive ruleset that will contain our rules.
> > > > @@ -191,6 +208,42 @@ number for a specific action: HTTPS connections.
> > > > err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
> > > > &net_port, 0);
> > > >
> > > > +For capability access-control, we can add rules that allow specific
> > > > +capabilities. For instance, to allow ``CAP_SYS_CHROOT`` (so the sandboxed
> > > > +process can call :manpage:`chroot(2)` inside a user namespace):
> > > > +
> > > > +.. code-block:: c
> > > > +
> > > > + struct landlock_capability_attr cap_attr = {
> > > > + .allowed_perm = LANDLOCK_PERM_CAPABILITY_USE,
> > > > + .capabilities = (1ULL << CAP_SYS_CHROOT),
> > > > + };
> > > > +
> > > > + err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_CAPABILITY,
> > > > + &cap_attr, 0);
> > > > +
> > > > +For namespace access-control, we can add rules that allow entering specific
> > > > +namespace types (creating them via :manpage:`unshare(2)` / :manpage:`clone(2)`
> > > > +or joining them via :manpage:`setns(2)`). For instance, to allow creating user
> > > > +namespaces (which grants all capabilities inside the new namespace):
> > > > +
> > > > +.. code-block:: c
> > > > +
> > > > + struct landlock_namespace_attr ns_attr = {
> > > > + .allowed_perm = LANDLOCK_PERM_NAMESPACE_ENTER,
> > > > + .namespace_types = CLONE_NEWUSER,
> > > > + };
> > > > +
> > > > + err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NAMESPACE,
> > > > + &ns_attr, 0);
> > > > +
> > > > +Together, these two rules allow an unprivileged process to create a user
> > > > +namespace and call :manpage:`chroot(2)` inside it, while denying all other
> > > > +capabilities and namespace types. User namespace creation is the one operation
> > > > +that does not require ``CAP_SYS_ADMIN``, so no capability rule is needed for it.
> > > > +See `Capability and namespace restrictions`_ for details on capability
> > > > +requirements.
> > > > +
> > > > When passing a non-zero ``flags`` argument to ``landlock_restrict_self()``, a
> > > > similar backwards compatibility check is needed for the restrict flags
> > > > (see sys_landlock_restrict_self() documentation for available flags):
> > > > @@ -354,10 +407,87 @@ The operations which can be scoped are:
> > > > A :manpage:`sendto(2)` on a socket which was previously connected will not
> > > > be restricted. This works for both datagram and stream sockets.
> > > >
> > > > -IPC scoping does not support exceptions via :manpage:`landlock_add_rule(2)`.
> > > > +Scoping does not support exceptions via :manpage:`landlock_add_rule(2)`.
> > > > If an operation is scoped within a domain, no rules can be added to allow access
> > > > to resources or processes outside of the scope.
> > > >
> > > > +Capability and namespace restrictions
> > > > +-------------------------------------
> > > > +
> > > > +See Documentation/security/landlock.rst for the design rationale behind
> > > > +the permission model (``handled_perm``) and how it differs from access
> > > > +rights (``handled_access_*``) and scopes (``scoped``).
> > > > +When a process creates a user namespace, the kernel grants all capabilities
> > > > +within that namespace. While these capabilities cannot directly bypass Landlock
> > > > +restrictions (Landlock enforces access controls independently of capability
> > > > +checks), they open kernel code paths that are normally unreachable to
> > > > +unprivileged users and may contain exploitable bugs.
> > > > +
> > > > +Landlock provides two complementary permissions to address this.
> > > > +``LANDLOCK_PERM_CAPABILITY_USE`` restricts which capabilities a process can use,
> > > > +even when it holds them. ``LANDLOCK_PERM_NAMESPACE_ENTER`` restricts which
> > > > +namespace types a process can create (via :manpage:`unshare(2)` or
> > > > +:manpage:`clone(2)`) or join (via :manpage:`setns(2)`). After creating a user
> > > > +namespace, the granted capabilities are scoped to namespaces owned by that user
> > > > +namespace or its descendants; to exercise a capability such as
> > > > +``CAP_NET_ADMIN``, the process must create a namespace of the corresponding type
> > > > +(e.g., a network namespace). Configuring both permissions together provides
> > > > +full coverage: ``LANDLOCK_PERM_CAPABILITY_USE`` restricts which capabilities are
> > > > +available, while ``LANDLOCK_PERM_NAMESPACE_ENTER`` restricts the namespaces in
> > > > +which they can be used.
> > > Maybe add a section on the what this does versus PR_SET_NO_NEW_PRIVS.
> >
> > Hmm, what do you mean? What would be the link with this part?
> PR_SET_NO_NEW_PRIVS prevents gaining of privileges through execution,
> including capabilities (i.e setcap command, not just setuid/gid).
> So they're adjacent at least.
>
> Some users might not want to set NNP because they want to execute
> a binary with w/ CAP_BPF file capabilities set for instance. But
> they don't need CAP_SYS_ADMIN or whatever for their usecase.
>
Bad example sorry. They need CAP_SYS_ADMIN to make the ruleset. But this
point still applies for other caps or if they drop CAP_SYS_ADMIN after
applying the ruleset.
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Jonathan McDowell @ 2026-04-23 17:02 UTC (permalink / raw)
To: Mimi Zohar
Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will, noodles, sebastianene
In-Reply-To: <2866f7679fe6933de667ce74ae68bd4ea9198e2a.camel@linux.ibm.com>
On Thu, Apr 23, 2026 at 10:48:49AM -0400, Mimi Zohar wrote:
>On Thu, 2026-04-23 at 15:03 +0100, Jonathan McDowell wrote:
>> On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
>> > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
>> > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
>> > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
>> > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
>> > > > > > > > > > > Hi Mimi,
>> > > > > > > > > > >
>> > > > > > > > > > > > On Wed, 2026-04-22 at 17:24 +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.
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > To resolve this, call ima_init() again at late_inicall_sync level
>> > > > > > > > > > > > > so that let IMA not miss TPM PCR value when generating boot_aggregate
>> > > > > > > > > > > > > log though TPM device presents in the system.
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>> > > > > > > > > > > >
>> > > > > > > > > > > > A lot of change for just detecting whether ima_init() is being called on
>> > > > > > > > > > > > late_initcall or late_initcall_sync(), without any explanation for all the other
>> > > > > > > > > > > > changes (e.g. ima_init_core).
>> > > > > > > > > > > >
>> > > > > > > > > > > > Please just limit the change to just calling ima_init() twice.
>> > > > > > > > > > >
>> > > > > > > > > > > My concern is that ima_update_policy_flags() will be called
>> > > > > > > > > > > when ima_init() is deferred -- not initialised anything.
>> > > > > > > > > > > though functionally, it might be okay however,
>> > > > > > > > > > > I think ima_update_policy_flags() and notifier should work after ima_init()
>> > > > > > > > > > > works logically.
>> > > > > > > > > > >
>> > > > > > > > > > > This change I think not much quite a lot. just wrapper ima_init() with
>> > > > > > > > > > > ima_init_core() with some error handling.
>> > > > > > > > > > >
>> > > > > > > > > > > Am I missing something?
>> > > > > > > > > >
>> > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
>> > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
>> > > > > > > > > >
>> > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
>> > > > > > > > > > it by caller of ima_init().
>> > > > > > > > >
>> > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip. On failure,
>> > > > > > > > > instead of going into TPM-bypass mode, return immediately. There are no calls
>> > > > > > > > > to anything else. Just call ima_init() a second time.
>> > > > > > > >
>> > > > > > > > I’m not fully convinced this is sufficient.
>> > > > > > > >
>> > > > > > > > What I meant is the case where ima_init() fails due to other
>> > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
>> > > > > > >
>> > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
>> > > > > > > available at late_initcall. This would be classified as a bug fix and would be
>> > > > > > > backported. No other changes should be included in this patch.
>> > > > > >
>> > > > > > Okay.
>> > > > > >
>> > > > > > > >
>> > > > > > > > I’d also like to ask again whether it is fine to call
>> > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
>> > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
>> > > > > > > > logically questionable to do so when ima_init() has not completed.
>> > > > > > >
>> > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
>> > > > > > > is a TPM or goes into TPM-bypass mode.
>> > > > > > >
>> > > > > > > >
>> > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
>> > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
>> > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
>> > > > > > > > it seems more appropriate to handle this state in the caller of
>> > > > > > > > ima_init(), rather than inside ima_init() itself.
>> > > > > > >
>> > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
>> > > > > > > bypass mode. Please don't make any other changes to the existing IMA behavior
>> > > > > > > and hide it here behind the late_initcall_sync change.
>> > > > > >
>> > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
>> > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
>> > > > > > get failed with "TPM-bypass mode".
>> > > > > >
>> > > > > > I see then, I'll make a patch simpler then.
>> > > > >
>> > > > > But I think in case of below situation:
>> > > > > - late_initcall's first ima_init() is deferred.
>> > > > > - late_initcall_sync try again but failed and try again with
>> > > > > CONFIG_IMA_DEFAULT_HASH.
>> > > > >
>> > > > > I would like to sustain init_ima_core to reduce the same code repeat
>> > > > > in late_initcall_sync.
>> > > >
>> > > > I think what Mimi's proposing is:
>> > > >
>> > > > If we're in late_initcall, and the TPM isn't available, return
>> > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
>> > > >
>> > > > If we're in late_initcall_sync, either we're already initialised, so do
>> > > > return and nothing, or run through the entire flow, even if the TPM
>> > > > isn't unavailable.
>> > > >
>> > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
>> > > > and b) for the sync mode, if we've already done the init at
>> > > > non-sync.
>> > >
>> > > Thanks, Jonathan. That is exactly what I'm suggesting. Any other changes
>> > > should not be included in this patch. Since Yeoreum is not hearing me, feel
>> > > free to post a patch.
>> >
>> > I see. so what you need to is this only
>> > If it looks good to you. I'll send it at v3.
>>
>> FWIW, I pulled the tpm_default_chip check out a level to account for the
>> extra init you mentioned, and have the following (completely untested or
>> compiled, but gives the approach):
>
>Thanks, Jonathan! It looks good. Similarly untested/compiled.
FWIW, it does compile.
>Emitting a message on failure to initialize IMA at late_initcall is good, but
>the attestation service won't know. Could you somehow differentiate between the
>late_initcall and late_initcall_sync boot_aggregate records?
Are you thinking "boot_aggregate" and "boot_aggregate_late" or similar
as the "filename" on the entries, just so it's clear when we did the
init in the log, or something else?
J.
--
/-\ | 101 things you can't have too much
|@/ Debian GNU/Linux Developer | of : 39 - silver bullets.
\- |
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-23 17:13 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will, noodles, sebastianene
In-Reply-To: <aepQwcY523aukAvw@earth.li>
On Thu, 2026-04-23 at 18:02 +0100, Jonathan McDowell wrote:
> > > > > >
> > > > > > I think what Mimi's proposing is:
> > > > > >
> > > > > > If we're in late_initcall, and the TPM isn't available, return
> > > > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > > > >
> > > > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > > > return and nothing, or run through the entire flow, even if the TPM
> > > > > > isn't unavailable.
> > > > > >
> > > > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > > > and b) for the sync mode, if we've already done the init at
> > > > > > non-sync.
> > > > >
> > > > > Thanks, Jonathan. That is exactly what I'm suggesting. Any other changes
> > > > > should not be included in this patch. Since Yeoreum is not hearing me, feel
> > > > > free to post a patch.
> > > >
> > > > I see. so what you need to is this only
> > > > If it looks good to you. I'll send it at v3.
> > >
> > > FWIW, I pulled the tpm_default_chip check out a level to account for the
> > > extra init you mentioned, and have the following (completely untested or
> > > compiled, but gives the approach):
> >
> > Thanks, Jonathan! It looks good. Similarly untested/compiled.
>
> FWIW, it does compile.
>
> > Emitting a message on failure to initialize IMA at late_initcall is good, but
> > the attestation service won't know. Could you somehow differentiate between the
> > late_initcall and late_initcall_sync boot_aggregate records?
>
> Are you thinking "boot_aggregate" and "boot_aggregate_late" or similar
> as the "filename" on the entries, just so it's clear when we did the
> init in the log, or something else?
Perfect!
Mimi
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-23 18:01 UTC (permalink / raw)
To: Yeoreum Yun, Jonathan McDowell
Cc: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will, noodles, sebastianene
In-Reply-To: <aeotq8nPVu4wvEx5@e129823.arm.com>
On Thu, 2026-04-23 at 15:33 +0100, Yeoreum Yun wrote:
> Hi Jonathan,
>
> > * # Be careful, this email looks suspicious; * Out of Character: The sender is exhibiting a significant deviation from their usual behavior, this may indicate that their account has been compromised. Be extra cautious before opening links or attachments. *
> > On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
> > > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> > > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > > Hi Mimi,
> > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, 2026-04-22 at 17:24 +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.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > To resolve this, call ima_init() again at late_inicall_sync level
> > > > > > > > > > > > > > so that let IMA not miss TPM PCR value when generating boot_aggregate
> > > > > > > > > > > > > > log though TPM device presents in the system.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > > > > > > > > > > > >
> > > > > > > > > > > > > A lot of change for just detecting whether ima_init() is being called on
> > > > > > > > > > > > > late_initcall or late_initcall_sync(), without any explanation for all the other
> > > > > > > > > > > > > changes (e.g. ima_init_core).
> > > > > > > > > > > > >
> > > > > > > > > > > > > Please just limit the change to just calling ima_init() twice.
> > > > > > > > > > > >
> > > > > > > > > > > > My concern is that ima_update_policy_flags() will be called
> > > > > > > > > > > > when ima_init() is deferred -- not initialised anything.
> > > > > > > > > > > > though functionally, it might be okay however,
> > > > > > > > > > > > I think ima_update_policy_flags() and notifier should work after ima_init()
> > > > > > > > > > > > works logically.
> > > > > > > > > > > >
> > > > > > > > > > > > This change I think not much quite a lot. just wrapper ima_init() with
> > > > > > > > > > > > ima_init_core() with some error handling.
> > > > > > > > > > > >
> > > > > > > > > > > > Am I missing something?
> > > > > > > > > > >
> > > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > > > > >
> > > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > > > > > it by caller of ima_init().
> > > > > > > > > >
> > > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip. On failure,
> > > > > > > > > > instead of going into TPM-bypass mode, return immediately. There are no calls
> > > > > > > > > > to anything else. Just call ima_init() a second time.
> > > > > > > > >
> > > > > > > > > I’m not fully convinced this is sufficient.
> > > > > > > > >
> > > > > > > > > What I meant is the case where ima_init() fails due to other
> > > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > > > > >
> > > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > > > > > available at late_initcall. This would be classified as a bug fix and would be
> > > > > > > > backported. No other changes should be included in this patch.
> > > > > > >
> > > > > > > Okay.
> > > > > > >
> > > > > > > > >
> > > > > > > > > I’d also like to ask again whether it is fine to call
> > > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > > > > > logically questionable to do so when ima_init() has not completed.
> > > > > > > >
> > > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > > > > > is a TPM or goes into TPM-bypass mode.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > > > > > it seems more appropriate to handle this state in the caller of
> > > > > > > > > ima_init(), rather than inside ima_init() itself.
> > > > > > > >
> > > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > > > > > bypass mode. Please don't make any other changes to the existing IMA behavior
> > > > > > > > and hide it here behind the late_initcall_sync change.
> > > > > > >
> > > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > > > > > get failed with "TPM-bypass mode".
> > > > > > >
> > > > > > > I see then, I'll make a patch simpler then.
> > > > > >
> > > > > > But I think in case of below situation:
> > > > > > - late_initcall's first ima_init() is deferred.
> > > > > > - late_initcall_sync try again but failed and try again with
> > > > > > CONFIG_IMA_DEFAULT_HASH.
> > > > > >
> > > > > > I would like to sustain init_ima_core to reduce the same code repeat
> > > > > > in late_initcall_sync.
> > > > >
> > > > > I think what Mimi's proposing is:
> > > > >
> > > > > If we're in late_initcall, and the TPM isn't available, return
> > > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > > >
> > > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > > return and nothing, or run through the entire flow, even if the TPM
> > > > > isn't unavailable.
> > > > >
> > > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > > and b) for the sync mode, if we've already done the init at
> > > > > non-sync.
> > > >
> > > > Thanks, Jonathan. That is exactly what I'm suggesting. Any other changes
> > > > should not be included in this patch. Since Yeoreum is not hearing me, feel
> > > > free to post a patch.
> > >
> > > I see. so what you need to is this only
> > > If it looks good to you. I'll send it at v3.
> >
> > FWIW, I pulled the tpm_default_chip check out a level to account for the
> > extra init you mentioned, and have the following (completely untested or
> > compiled, but gives the approach):
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index d48bf0ad26f4..88fe105b7f00 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -166,6 +166,7 @@ enum lsm_order {
> > * @initcall_fs: LSM callback for fs_initcall setup, optional
> > * @initcall_device: LSM callback for device_initcall() setup, optional
> > * @initcall_late: LSM callback for late_initcall() setup, optional
> > + * @initcall_late_sync: LSM callback for late_initcall_sync() setup, optional
> > */
> > struct lsm_info {
> > const struct lsm_id *id;
> > @@ -181,6 +182,7 @@ struct lsm_info {
> > int (*initcall_fs)(void);
> > int (*initcall_device)(void);
> > int (*initcall_late)(void);
> > + int (*initcall_late_sync)(void);
> > };
> > #define DEFINE_LSM(lsm) \
> > diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> > index a2f34f2d8ad7..a60dfb8316d8 100644
> > --- a/security/integrity/ima/ima_init.c
> > +++ b/security/integrity/ima/ima_init.c
> > @@ -119,10 +119,6 @@ int __init ima_init(void)
> > {
> > int rc;
> > - ima_tpm_chip = tpm_default_chip();
> > - if (!ima_tpm_chip)
> > - pr_info("No TPM chip found, activating TPM-bypass!\n");
> > -
> > rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
> > if (rc)
> > return rc;
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 1d6229b156fb..b60a85fa803a 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -1237,7 +1237,7 @@ static int ima_kernel_module_request(char *kmod_name)
> > #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
> > -static int __init init_ima(void)
> > +static int __init init_ima(bool sync)
> > {
> > int error;
> > @@ -1247,6 +1247,19 @@ static int __init init_ima(void)
> > return 0;
> > }
> > + /* If we found the TPM during our first attempt, nothing further to do */
> > + if (sync && ima_tpm_chip)
> > + return 0;
> > +
> > + ima_tpm_chip = tpm_default_chip();
> > + if (!ima_tpm_chip && !sync) {
> > + pr_debug("TPM not available, will try later\n");
> > + return -EPROBE_DEFER;
> > + }
> > +
> > + if (!ima_tpm_chip)
> > + pr_info("No TPM chip found, activating TPM-bypass!\n");
> > +
> > ima_appraise_parse_cmdline();
> > ima_init_template_list();
> > hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > @@ -1274,6 +1287,16 @@ static int __init init_ima(void)
> > return error;
> > }
> > +static int __init init_ima_late(void)
> > +{
> > + return init_ima(false);
> > +}
> > +
> > +static int __init init_ima_late_sync(void)
> > +{
> > + return init_ima(true);
> > +}
> > +
> > static struct security_hook_list ima_hooks[] __ro_after_init = {
> > LSM_HOOK_INIT(bprm_check_security, ima_bprm_check),
> > LSM_HOOK_INIT(bprm_creds_for_exec, ima_bprm_creds_for_exec),
> > @@ -1319,6 +1342,7 @@ DEFINE_LSM(ima) = {
> > .init = init_ima_lsm,
> > .order = LSM_ORDER_LAST,
> > .blobs = &ima_blob_sizes,
> > - /* Start IMA after the TPM is available */
> > - .initcall_late = init_ima,
> > + /* Ensure we start IMA after the TPM is available */
> > + .initcall_late = init_ima_late,
> > + .initcall_late_sync = init_ima_late_sync,
> > };
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index 573e2a7250c4..4e5c59beb82a 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -547,13 +547,22 @@ device_initcall(security_initcall_device);
> > * security_initcall_late - Run the LSM late initcalls
> > */
> > static int __init security_initcall_late(void)
> > +{
> > + return lsm_initcall(late);
> > +}
> > +late_initcall(security_initcall_late);
> > +
> > +/**
> > + * security_initcall_late_sync - Run the LSM late initcalls sync
> > + */
> > +static int __init security_initcall_late_sync(void)
> > {
> > int rc;
> > - rc = lsm_initcall(late);
> > + rc = lsm_initcall(late_sync);
> > lsm_pr_dbg("all enabled LSMs fully activated\n");
> > call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
> > return rc;
> > }
> > -late_initcall(security_initcall_late);
> > +late_initcall_sync(security_initcall_late_sync);
>
> I'm fine this. but are we talking about "ima_init()" not "init_ima()"?
Having two functions named ima_init() and init_ima() is really confusing. At
least with this patch, init_ima() will be replaced with init_ima_late() and
init_ima_sync().
> Because of this, I've fixuated and make a long stupid speaking myself.
The commit 0e0546eabcd6 ("firmware: arm_ffa: Change initcall level of ffa_init()
to rootfs_initcall") patch description was really well written. I'm really sad
that it needs to be reverted.
The TPM not being initialized before IMA, has been an issue for a really long
time. Hopefully this patch will safely fix it, not only for you, but for others
as well.
>
> If this seems good to Mimi, I don't care who send it.
> But If you're going to send this, could you includes 2 and 3 too?
Once this patch is ready, we can create a topic branch to coordinate upstreaming
the remaining patches.
thanks!
Mimi
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-23 18:13 UTC (permalink / raw)
To: Mimi Zohar
Cc: Jonathan McDowell, linux-security-module, linux-kernel,
linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
roberto.sassu, dmitry.kasatkin, eric.snowberg, jarkko, jgg,
sudeep.holla, maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, will, noodles, sebastianene
In-Reply-To: <e4e242ae5533d5762a3647186a178764881bf9ff.camel@linux.ibm.com>
> On Thu, 2026-04-23 at 15:33 +0100, Yeoreum Yun wrote:
> > Hi Jonathan,
> >
> > > * # Be careful, this email looks suspicious; * Out of Character: The sender is exhibiting a significant deviation from their usual behavior, this may indicate that their account has been compromised. Be extra cautious before opening links or attachments. *
> > > On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
> > > > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> > > > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > > > Hi Mimi,
> > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, 2026-04-22 at 17:24 +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.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > To resolve this, call ima_init() again at late_inicall_sync level
> > > > > > > > > > > > > > > so that let IMA not miss TPM PCR value when generating boot_aggregate
> > > > > > > > > > > > > > > log though TPM device presents in the system.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > A lot of change for just detecting whether ima_init() is being called on
> > > > > > > > > > > > > > late_initcall or late_initcall_sync(), without any explanation for all the other
> > > > > > > > > > > > > > changes (e.g. ima_init_core).
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Please just limit the change to just calling ima_init() twice.
> > > > > > > > > > > > >
> > > > > > > > > > > > > My concern is that ima_update_policy_flags() will be called
> > > > > > > > > > > > > when ima_init() is deferred -- not initialised anything.
> > > > > > > > > > > > > though functionally, it might be okay however,
> > > > > > > > > > > > > I think ima_update_policy_flags() and notifier should work after ima_init()
> > > > > > > > > > > > > works logically.
> > > > > > > > > > > > >
> > > > > > > > > > > > > This change I think not much quite a lot. just wrapper ima_init() with
> > > > > > > > > > > > > ima_init_core() with some error handling.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Am I missing something?
> > > > > > > > > > > >
> > > > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > > > > > >
> > > > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > > > > > > it by caller of ima_init().
> > > > > > > > > > >
> > > > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip. On failure,
> > > > > > > > > > > instead of going into TPM-bypass mode, return immediately. There are no calls
> > > > > > > > > > > to anything else. Just call ima_init() a second time.
> > > > > > > > > >
> > > > > > > > > > I’m not fully convinced this is sufficient.
> > > > > > > > > >
> > > > > > > > > > What I meant is the case where ima_init() fails due to other
> > > > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > > > > > >
> > > > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > > > > > > available at late_initcall. This would be classified as a bug fix and would be
> > > > > > > > > backported. No other changes should be included in this patch.
> > > > > > > >
> > > > > > > > Okay.
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I’d also like to ask again whether it is fine to call
> > > > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > > > > > > logically questionable to do so when ima_init() has not completed.
> > > > > > > > >
> > > > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > > > > > > is a TPM or goes into TPM-bypass mode.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > > > > > > it seems more appropriate to handle this state in the caller of
> > > > > > > > > > ima_init(), rather than inside ima_init() itself.
> > > > > > > > >
> > > > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > > > > > > bypass mode. Please don't make any other changes to the existing IMA behavior
> > > > > > > > > and hide it here behind the late_initcall_sync change.
> > > > > > > >
> > > > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > > > > > > get failed with "TPM-bypass mode".
> > > > > > > >
> > > > > > > > I see then, I'll make a patch simpler then.
> > > > > > >
> > > > > > > But I think in case of below situation:
> > > > > > > - late_initcall's first ima_init() is deferred.
> > > > > > > - late_initcall_sync try again but failed and try again with
> > > > > > > CONFIG_IMA_DEFAULT_HASH.
> > > > > > >
> > > > > > > I would like to sustain init_ima_core to reduce the same code repeat
> > > > > > > in late_initcall_sync.
> > > > > >
> > > > > > I think what Mimi's proposing is:
> > > > > >
> > > > > > If we're in late_initcall, and the TPM isn't available, return
> > > > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > > > >
> > > > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > > > return and nothing, or run through the entire flow, even if the TPM
> > > > > > isn't unavailable.
> > > > > >
> > > > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > > > and b) for the sync mode, if we've already done the init at
> > > > > > non-sync.
> > > > >
> > > > > Thanks, Jonathan. That is exactly what I'm suggesting. Any other changes
> > > > > should not be included in this patch. Since Yeoreum is not hearing me, feel
> > > > > free to post a patch.
> > > >
> > > > I see. so what you need to is this only
> > > > If it looks good to you. I'll send it at v3.
> > >
> > > FWIW, I pulled the tpm_default_chip check out a level to account for the
> > > extra init you mentioned, and have the following (completely untested or
> > > compiled, but gives the approach):
> > >
> > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > > index d48bf0ad26f4..88fe105b7f00 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -166,6 +166,7 @@ enum lsm_order {
> > > * @initcall_fs: LSM callback for fs_initcall setup, optional
> > > * @initcall_device: LSM callback for device_initcall() setup, optional
> > > * @initcall_late: LSM callback for late_initcall() setup, optional
> > > + * @initcall_late_sync: LSM callback for late_initcall_sync() setup, optional
> > > */
> > > struct lsm_info {
> > > const struct lsm_id *id;
> > > @@ -181,6 +182,7 @@ struct lsm_info {
> > > int (*initcall_fs)(void);
> > > int (*initcall_device)(void);
> > > int (*initcall_late)(void);
> > > + int (*initcall_late_sync)(void);
> > > };
> > > #define DEFINE_LSM(lsm) \
> > > diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> > > index a2f34f2d8ad7..a60dfb8316d8 100644
> > > --- a/security/integrity/ima/ima_init.c
> > > +++ b/security/integrity/ima/ima_init.c
> > > @@ -119,10 +119,6 @@ int __init ima_init(void)
> > > {
> > > int rc;
> > > - ima_tpm_chip = tpm_default_chip();
> > > - if (!ima_tpm_chip)
> > > - pr_info("No TPM chip found, activating TPM-bypass!\n");
> > > -
> > > rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
> > > if (rc)
> > > return rc;
> > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > index 1d6229b156fb..b60a85fa803a 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -1237,7 +1237,7 @@ static int ima_kernel_module_request(char *kmod_name)
> > > #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
> > > -static int __init init_ima(void)
> > > +static int __init init_ima(bool sync)
> > > {
> > > int error;
> > > @@ -1247,6 +1247,19 @@ static int __init init_ima(void)
> > > return 0;
> > > }
> > > + /* If we found the TPM during our first attempt, nothing further to do */
> > > + if (sync && ima_tpm_chip)
> > > + return 0;
> > > +
> > > + ima_tpm_chip = tpm_default_chip();
> > > + if (!ima_tpm_chip && !sync) {
> > > + pr_debug("TPM not available, will try later\n");
> > > + return -EPROBE_DEFER;
> > > + }
> > > +
> > > + if (!ima_tpm_chip)
> > > + pr_info("No TPM chip found, activating TPM-bypass!\n");
> > > +
> > > ima_appraise_parse_cmdline();
> > > ima_init_template_list();
> > > hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > > @@ -1274,6 +1287,16 @@ static int __init init_ima(void)
> > > return error;
> > > }
> > > +static int __init init_ima_late(void)
> > > +{
> > > + return init_ima(false);
> > > +}
> > > +
> > > +static int __init init_ima_late_sync(void)
> > > +{
> > > + return init_ima(true);
> > > +}
> > > +
> > > static struct security_hook_list ima_hooks[] __ro_after_init = {
> > > LSM_HOOK_INIT(bprm_check_security, ima_bprm_check),
> > > LSM_HOOK_INIT(bprm_creds_for_exec, ima_bprm_creds_for_exec),
> > > @@ -1319,6 +1342,7 @@ DEFINE_LSM(ima) = {
> > > .init = init_ima_lsm,
> > > .order = LSM_ORDER_LAST,
> > > .blobs = &ima_blob_sizes,
> > > - /* Start IMA after the TPM is available */
> > > - .initcall_late = init_ima,
> > > + /* Ensure we start IMA after the TPM is available */
> > > + .initcall_late = init_ima_late,
> > > + .initcall_late_sync = init_ima_late_sync,
> > > };
> > > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > > index 573e2a7250c4..4e5c59beb82a 100644
> > > --- a/security/lsm_init.c
> > > +++ b/security/lsm_init.c
> > > @@ -547,13 +547,22 @@ device_initcall(security_initcall_device);
> > > * security_initcall_late - Run the LSM late initcalls
> > > */
> > > static int __init security_initcall_late(void)
> > > +{
> > > + return lsm_initcall(late);
> > > +}
> > > +late_initcall(security_initcall_late);
> > > +
> > > +/**
> > > + * security_initcall_late_sync - Run the LSM late initcalls sync
> > > + */
> > > +static int __init security_initcall_late_sync(void)
> > > {
> > > int rc;
> > > - rc = lsm_initcall(late);
> > > + rc = lsm_initcall(late_sync);
> > > lsm_pr_dbg("all enabled LSMs fully activated\n");
> > > call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
> > > return rc;
> > > }
> > > -late_initcall(security_initcall_late);
> > > +late_initcall_sync(security_initcall_late_sync);
> >
> > I'm fine this. but are we talking about "ima_init()" not "init_ima()"?
>
> Having two functions named ima_init() and init_ima() is really confusing. At
> least with this patch, init_ima() will be replaced with init_ima_late() and
> init_ima_sync().
>
> > Because of this, I've fixuated and make a long stupid speaking myself.
>
> The commit 0e0546eabcd6 ("firmware: arm_ffa: Change initcall level of ffa_init()
> to rootfs_initcall") patch description was really well written. I'm really sad
> that it needs to be reverted.
>
> The TPM not being initialized before IMA, has been an issue for a really long
> time. Hopefully this patch will safely fix it, not only for you, but for others
> as well.
>
> >
> > If this seems good to Mimi, I don't care who send it.
> > But If you're going to send this, could you includes 2 and 3 too?
>
> Once this patch is ready, we can create a topic branch to coordinate upstreaming
> the remaining patches.
Sounds good. Once the patch is posted, I’ll review it as well.
Sorry again for the noise, and thanks for your patience ;)
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [PATCH v5 6/10] security: Hornet LSM
From: Paul Moore @ 2026-04-23 18:37 UTC (permalink / raw)
To: Blaise Boscaccy, Blaise Boscaccy, Jonathan Corbet, James Morris,
Serge E. Hallyn, Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
Fan Wu, Ryan Foster, Randy Dunlap, linux-security-module,
linux-doc, linux-kernel, bpf, Song Liu
In-Reply-To: <20260420212653.438685-7-bboscaccy@linux.microsoft.com>
On Apr 20, 2026 Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
>
> This adds the Hornet Linux Security Module which provides enhanced
> signature verification and data validation for eBPF programs. This
> allows users to continue to maintain an invariant that all code
> running inside of the kernel has actually been signed and verified, by
> the kernel.
>
> This effort builds upon the currently excepted upstream solution. It
> further hardens it by providing deterministic, in-kernel checking of
> map hashes to solidify auditing along with preventing TOCTOU attacks
> against lskel map hashes.
>
> Target map hashes are passed in via PKCS#7 signed attributes. Hornet
> determines the extent which the eBFP program is signed and defers to
> other LSMs for policy decisions.
>
> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> Nacked-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> ---
> Documentation/admin-guide/LSM/Hornet.rst | 321 +++++++++++++++++++++
> Documentation/admin-guide/LSM/index.rst | 1 +
> MAINTAINERS | 9 +
> include/linux/oid_registry.h | 3 +
> include/uapi/linux/lsm.h | 1 +
> security/Kconfig | 3 +-
> security/Makefile | 1 +
> security/hornet/Kconfig | 11 +
> security/hornet/Makefile | 7 +
> security/hornet/hornet.asn1 | 13 +
> security/hornet/hornet_lsm.c | 346 +++++++++++++++++++++++
> 11 files changed, 715 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/admin-guide/LSM/Hornet.rst
> create mode 100644 security/hornet/Kconfig
> create mode 100644 security/hornet/Makefile
> create mode 100644 security/hornet/hornet.asn1
> create mode 100644 security/hornet/hornet_lsm.c
While I think this is looking pretty reasonable, I think Fan had some
feedback which merits a reply. I also spotted some references to the
secondary keyring in the docs which need to be updated (below).
> diff --git a/Documentation/admin-guide/LSM/Hornet.rst b/Documentation/admin-guide/LSM/Hornet.rst
> new file mode 100644
> index 0000000000000..af5e9cd9d83a8
> --- /dev/null
> +++ b/Documentation/admin-guide/LSM/Hornet.rst
> @@ -0,0 +1,321 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +======
> +Hornet
> +======
> +
> +Hornet is a Linux Security Module that provides extensible signature
> +verification for eBPF programs. This is selectable at build-time with
> +``CONFIG_SECURITY_HORNET``.
> +
> +Overview
> +========
> +
> +Hornet addresses concerns from users who require strict audit trails and
> +verification guarantees for eBPF programs, especially in
> +security-sensitive environments. Many production systems need assurance
> +that only authorized, unmodified eBPF programs are loaded into the
> +kernel. Hornet provides this assurance through cryptographic signature
> +verification.
> +
> +When an eBPF program is loaded via the ``bpf()`` syscall, Hornet
> +verifies a PKCS#7 signature attached to the program instructions. The
> +signature is checked against the kernel's secondary keyring using the
This version now supports using the keyring specified in the bpf_attr
union (presumably for maximum compatibility with KP's signature scheme),
which is good, but the docs need to be updated.
See below, but I would probably make a note that LSMs providing
enforcement of BPF signatures will likely want to check what keyring was
used to verify the signature, e.g. a trusted keyring vs a user supplied
keyring.
> +existing kernel cryptographic infrastructure. In addition to signing the
> +program bytecode, Hornet supports signing SHA-256 hashes of associated
> +BPF maps, enabling integrity verification of map contents at load time
> +and at runtime.
> +
> +After verification, Hornet classifies the program into one of the
> +following integrity states and passes the result to a downstream LSM hook
> +(``bpf_prog_load_post_integrity``), allowing other security modules to
> +make policy decisions based on the verification outcome:
> +
> +``LSM_INT_VERDICT_OK``
> + The program signature and all map hashes verified successfully.
> +
> +``LSM_INT_VERDICT_UNSIGNED``
> + No signature was provided with the program.
> +
> +``LSM_INT_VERDICT_PARTIALSIG``
> + The program signature verified, but the signature did not contain
> + hornet map hash data.
> +
> +``LSM_INT_VERDICT_UNKNOWNKEY``
> + The signing certificate is not trusted in the secondary keyring,
Another secondary keyring mention.
> +``LSM_INT_VERDICT_FAULT``
> + A system error occured during verification.
> +
> +``LSM_INT_VERDICT_UNEXPECTED``
> + An unexpected map hash value was encountered.
> +
> +``LSM_INT_VERDICT_BADSIG``
> + The signature or a map hash failed verification.
> +
> +Hornet itself does not enforce a policy on whether unsigned or partially
> +signed programs should be rejected. It delegates that decision to
> +downstream LSMs via the ``bpf_prog_load_post_integrity`` hook, making it
> +a composable building block in a larger security architecture.
This might be a good place to document that in addition to the verdicts
described above, LSMs providing enforcement should also consider the
keyring used for verification.
> +Known Limitations
> +=================
> +
> +- Hornet requires programs to use :doc:`light skeletons
> + </bpf/libbpf/libbpf_naming_convention>` (lskels) for the signing
> + workflow, as the tooling operates on lskel-generated headers.
> +
> +- A maximum of 64 maps per program can be tracked for hash
> + verification.
> +
> +- Map hash verification requires the maps to be frozen before loading.
> + Maps that are not frozen at load time will cause verification to fail
> + when their hashes are included in the signature.
> +
> +- Hornet relies on the kernel's secondary keyring
> + (``VERIFY_USE_SECONDARY_KEYRING``) for certificate trust. Keys must
> + be provisioned into this keyring before programs can be verified.
... another spot.
> +- The only hashing algorithm available is SHA256 due to it be hardcoded
> + in the bpf subsystem.
...
> +Signature Verification Flow
> +---------------------------
> +
> +The following describes what happens when a userspace program calls
> +``bpf(BPF_PROG_LOAD, ...)`` with a signature attached:
> +
> +1. The ``bpf_prog_load_integrity`` LSM hook is invoked.
> +
> +2. Hornet reads the signature from the userspace buffer specified by
> + ``attr->signature`` (with length ``attr->signature_size``).
> +
> +3. The PKCS#7 signature is verified against the program instructions
> + using ``verify_pkcs7_signature()`` with the kernel's secondary
> + keyring.
I believe this is the last mention of the secondary keyring.
> +4. The PKCS#7 message is parsed and its trust chain is validated via
> + ``validate_pkcs7_trust()``.
> +
> +5. Hornet extracts the authenticated attribute identified by
> + ``OID_hornet_data`` (OID ``2.25.316487325684022475439036912669789383960``)
> + from the PKCS#7 message. This attribute contains an ASN.1-encoded set
> + of map index/hash pairs.
> +
> +6. For each map hash entry, Hornet retrieves the corresponding BPF map
> + via its file descriptor, confirms it is frozen, computes its SHA-256
> + hash, and compares it against the signed hash.
> +
> +7. The resulting integrity verdict is passed to the
> + ``bpf_prog_load_post_integrity`` hook so that downstream LSMs can
> + enforce policy.
--
paul-moore.com
^ permalink raw reply
* [PATCH v3] evm: terminate and bound the evm_xattrs read buffer
From: Pengpeng Hou @ 2026-04-23 15:30 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge Hallyn, linux-integrity,
linux-security-module, linux-kernel, pengpeng
In-Reply-To: <20260417223004.1-evm-xattrs-v2-pengpeng@iscas.ac.cn>
evm_read_xattrs() allocates size + 1 bytes, fills them from the list of
enabled xattrs, and then passes strlen(temp) to
simple_read_from_buffer(). When no configured xattrs are enabled, the
fill loop stores nothing and temp[0] remains uninitialized, so strlen()
reads beyond initialized memory.
Explicitly terminate the buffer after allocation, use snprintf() for
each formatted line, and pass the accumulated length, without risk of
truncation, to simple_read_from_buffer().
Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v2:
- adjust the changelog wording to mention why the accumulated length is
safe
- add the blank line after the allocation error path
- add a comment explaining why snprintf() cannot truncate in the fill loop
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index acd840461902..4baf5e23bc97 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -127,8 +127,8 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
size_t count, loff_t *ppos)
{
char *temp;
- int offset = 0;
- ssize_t rc, size = 0;
+ size_t offset = 0, size = 0;
+ ssize_t rc;
struct xattr_list *xattr;
if (*ppos != 0)
@@ -151,16 +151,22 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
return -ENOMEM;
}
+ temp[size] = '\0';
+
+ /*
+ * No truncation possible: size is computed over the same enabled
+ * xattrs under xattr_list_mutex, so offset never exceeds size.
+ */
list_for_each_entry(xattr, &evm_config_xattrnames, list) {
if (!xattr->enabled)
continue;
- sprintf(temp + offset, "%s\n", xattr->name);
- offset += strlen(xattr->name) + 1;
+ offset += snprintf(temp + offset, size + 1 - offset, "%s\n",
+ xattr->name);
}
mutex_unlock(&xattr_list_mutex);
- rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+ rc = simple_read_from_buffer(buf, count, ppos, temp, offset);
kfree(temp);
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH] tomoyo: reject short exec.envp[] names before suffix checks
From: Pengpeng Hou @ 2026-04-23 22:53 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Kentaro Takeda, Paul Moore, James Morris, Serge Hallyn,
linux-security-module, linux-kernel, pengpeng
In-Reply-To: <20260417073249.93906-1-pengpeng@iscas.ac.cn>
Hi Tetsuo,
Thanks for the explanation.
Agreed, I missed that the left-hand string is already guaranteed to be
safely dereferenced at that call site. I'll drop this patch.
Thanks,
Pengpeng
^ permalink raw reply
* [GIT PULL] AppArmor updates for 7.1
From: John Johansen @ 2026-04-23 23:53 UTC (permalink / raw)
To: Linus Torvalds; +Cc: open list:SECURITY SUBSYSTEM, LKLM
Hi Linus,
We ran into some issue with what was going to go in this cycles PR,
and I am still not happy with the revisions. So I am just going with
some bug fixes and 3 cleanups that have been queued most of the cycle.
This has been merge, built and regression tested, against your tree
from a couple of days ago.
There isn't any thing all that exciting in here, separating out the
changes.
+ Cleanups
- Use sysfs_emit in param_get_{audit,mode}
- Remove redundant if check in sk_peer_get_label
- Replace memcpy + NUL termination with kmemdup_nul in do_setattr
+ Bug Fixes
- Fix aa_dfa_unpack's error handling in aa_setup_dfa_engine
- Fix string overrun due to missing termination
- Fix wrong dentry in RENAME_EXCHANGE uid check
- fix unpack_tags to properly return error in failure cases
- fix dfa size check
- return error on namespace mismatch in verify_header
- use target task's context in apparmor_getprocattr()
Pull at your convenience -rc2 even later is fine, and if you want I
can just send the bug fixes up during the -rc2 window.
thanks
- john
The following changes since commit 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f:
Linux 7.0-rc1 (2026-02-22 13:18:59 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor tags/apparmor-pr-2026-04-23
for you to fetch changes up to 11b7df0952663f20ce72c9a22a3cf9278cf84db7:
apparmor/lsm: Fix aa_dfa_unpack's error handling in aa_setup_dfa_engine (2026-04-22 20:11:08 -0700)
----------------------------------------------------------------
+ Cleanups
- Use sysfs_emit in param_get_{audit,mode}
- Remove redundant if check in sk_peer_get_label
- Replace memcpy + NUL termination with kmemdup_nul in do_setattr
+ Bug Fixes
- Fix aa_dfa_unpack's error handling in aa_setup_dfa_engine
- Fix string overrun due to missing termination
- Fix wrong dentry in RENAME_EXCHANGE uid check
- fix unpack_tags to properly return error in failure cases
- fix dfa size check
- return error on namespace mismatch in verify_header
- use target task's context in apparmor_getprocattr()
----------------------------------------------------------------
Cengiz Can (1):
apparmor: use target task's context in apparmor_getprocattr()
Daniel J Blueman (1):
apparmor: Fix string overrun due to missing termination
Dudu Lu (1):
apparmor: Fix wrong dentry in RENAME_EXCHANGE uid check
GONG Ruiqi (1):
apparmor/lsm: Fix aa_dfa_unpack's error handling in aa_setup_dfa_engine
John Johansen (2):
apparmor: fix dfa size check
apparmor: fix unpack_tags to properly return error in failure cases
Massimiliano Pellizzer (1):
apparmor: return error on namespace mismatch in verify_header
Thorsten Blum (3):
apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr
apparmor: Remove redundant if check in sk_peer_get_label
apparmor: Use sysfs_emit in param_get_{audit,mode}
security/apparmor/lsm.c | 36 ++++++++++++++----------------------
security/apparmor/match.c | 2 +-
security/apparmor/path.c | 8 +++++---
security/apparmor/policy_unpack.c | 2 ++
4 files changed, 22 insertions(+), 26 deletions(-)
^ permalink raw reply
* Re: [PATCH RFC 1/3] LSM: add a flags field to the LSM hook definitions
From: Paul Moore @ 2026-04-24 1:19 UTC (permalink / raw)
To: Casey Schaufler, casey, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, selinux
In-Reply-To: <20260225192143.14448-2-casey@schaufler-ca.com>
On Feb 25, 2026 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Add a field for flags to the definition of LSM hooks. This allows
> for hooks to be identified at system initialization for special
> processing.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/bpf_lsm.h | 2 +-
> include/linux/lsm_hook_defs.h | 614 ++++++++++++++++++----------------
> include/linux/lsm_hooks.h | 4 +-
> kernel/bpf/bpf_lsm.c | 10 +-
> security/bpf/hooks.c | 2 +-
> security/security.c | 6 +-
> 6 files changed, 331 insertions(+), 307 deletions(-)
>
> diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
> index 643809cc78c3..d71ba8c87e79 100644
> --- a/include/linux/bpf_lsm.h
> +++ b/include/linux/bpf_lsm.h
> @@ -14,7 +14,7 @@
>
> #ifdef CONFIG_BPF_LSM
>
> -#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> +#define LSM_HOOK(RET, DEFAULT, FLAGS, NAME, ...) \
> RET bpf_lsm_##NAME(__VA_ARGS__);
> #include <linux/lsm_hook_defs.h>
> #undef LSM_HOOK
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..acda3a02da97 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -18,451 +18,475 @@
> * The macro LSM_HOOK is used to define the data structures required by
> * the LSM framework using the pattern:
> *
> - * LSM_HOOK(<return_type>, <default_value>, <hook_name>, args...)
> + * LSM_HOOK(<return_type>, <default_value>, <flags>, <single>,
> + * <hook_name>, args...)
> *
> * struct security_hook_heads {
> - * #define LSM_HOOK(RET, DEFAULT, NAME, ...) struct hlist_head NAME;
> + * #define LSM_HOOK(RET, DEFAULT, FLAGS, NAME, ...) struct hlist_head NAME;
> * #include <linux/lsm_hook_defs.h>
> * #undef LSM_HOOK
> * };
> */
> -LSM_HOOK(int, 0, binder_set_context_mgr, const struct cred *mgr)
> -LSM_HOOK(int, 0, binder_transaction, const struct cred *from,
> +LSM_HOOK(int, 0, 0, binder_set_context_mgr, const struct cred *mgr)
> +LSM_HOOK(int, 0, 0, binder_transaction, const struct cred *from,
> const struct cred *to)
I think adding a flag field to the LSM_HOOK() macro/definitions is a good
and useful addition, but I'd prefer if we created a LSM_FLAG_NONE #define
and used it here just so we could avoid the back-to-back 0's and do a bit
of self-documentation.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 2/3] LSM: Enforce exclusive hooks
From: Paul Moore @ 2026-04-24 1:19 UTC (permalink / raw)
To: Casey Schaufler, casey, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, selinux
In-Reply-To: <20260225192143.14448-3-casey@schaufler-ca.com>
On Feb 25, 2026 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> If an LSM hook is marked as exclusive via LSM_FLAG_EXCLUSIVE
> in lsm_hook_defs.h it will not be added to the set of hooks to
> be executed if an different LSM has already registered an
> exclusive hook.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/security.h | 2 ++
> security/lsm_init.c | 66 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..e3c137a1b30a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -2404,4 +2404,6 @@ static inline void security_initramfs_populated(void)
> }
> #endif /* CONFIG_SECURITY */
>
> +extern u64 lsm_exclusive_hooks;
We already have the 'lsm_exclusive' variable in lsm_init.c, don't create
another variable that does the same thing. If the scope of the existing
variable isn't what you need, change that.
Although to be honest, I'm not in love with what you're doing with the
variable anyway, more on that later in the review.
> #endif /* ! __LINUX_SECURITY_H */
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 05bd52e6b1f2..dc3c84387a7e 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -356,6 +356,70 @@ static int __init lsm_static_call_init(struct security_hook_list *hl)
> return -ENOSPC;
> }
>
> +/*
> + * Hooks that are restricted to use by a single security module.
> + *
> + * Secmark hooks have not been converted from secids to lsm_props
> + * due to space limitations in packet headers.
If this is a general purpose mechanism for all types of LSM hooks, please
don't put commentary in here about a single class of hook.
If this only reason for doing all of this is for secmark, just fix
secmark instead of going through all of this trouble.
> + * Conversions from a secid to a secctx are restricted to the
> + * single security module. All cases where there may be multiple
> + * modules providing the input data have been converted to use
> + * a lsm_prop instead of a secid.
Okay, yes, the paragraph above is true, I'm just not sure why it is
important here?
> + */
> +struct lsm_exclusive {
> + struct lsm_static_call *name;
> + char *namestr;
> + u32 flags;
> +};
> +
> +static __initdata struct lsm_exclusive lsm_exclusive_set[] = {
> +#define LSM_HOOK(RET, DEFAULT, FLAGS, NAME, ...) \
> + { .name = static_calls_table.NAME, .flags = FLAGS, .namestr = "" #NAME "" , },
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +};
> +u64 lsm_exclusive_hooks;
> +EXPORT_SYMBOL(lsm_exclusive_hooks);
Unless I missed something, we really shouldn't need to export this, why
did you need EXPORT_SYMBOL() here?
> +/**
> + * lsm_exclusive_hook_denial - Check if exclusive hook is in use
> + * @hook: the hook to check
> + *
> + * Check if the hook in question is restricted to a single using LSM,
> + * and if the LSM providing single LSM hooks is defined.
> + *
> + * Returns true if the hook is exclusive and they are already provided,
> + * false otherwise.
> + */
> +static bool __init lsm_exclusive_hook_denial(struct security_hook_list *hook)
> +{
> + int i;
> +
> + if (lsm_exclusive_hooks == hook->lsmid->id)
> + return false;
> +
> + for (i = 0; i < ARRAY_SIZE(lsm_exclusive_set); i++) {
> + if (!(lsm_exclusive_set[i].flags & LSM_FLAG_EXCLUSIVE))
> + continue;
The logic on this looks a bit odd. What is wrong with something like the
pseduo code below?
for (i = 0; ARRAY_SIZE(lsm_hooks); i++) {
if (lsm_hooks[i] == hook) {
if (lsm_hooks[i].flags & LSM_FLAG_EXCLUSIVE)
return true;
else
return false;
}
}
> + if (hook->scalls == lsm_exclusive_set[i].name) {
> + if (lsm_exclusive_hooks) {
> + if (lsm_debug)
> + lsm_pr("%s denied for %s.\n",
> + lsm_exclusive_set[i].namestr,
> + hook->lsmid->name);
The lsm_pr_dbg() macro exists for this very reason.
> + return true;
> + }
> + if (lsm_debug)
> + lsm_pr("Exclusive hooks limited to %s.\n",
> + hook->lsmid->name);
Same as above.
> + lsm_exclusive_hooks = hook->lsmid->id;
> + break;
> + }
> + }
> + return false;
> +}
>
> /**
> * security_add_hooks - Add a LSM's hooks to the LSM framework's hook lists
> * @hooks: LSM hooks to add
> @@ -371,6 +435,8 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>
> for (i = 0; i < count; i++) {
> hooks[i].lsmid = lsmid;
> + if (lsm_exclusive_hook_denial(&hooks[i]))
> + continue;
> if (lsm_static_call_init(&hooks[i]))
> panic("exhausted LSM callback slots with LSM %s\n",
> lsmid->name);
I don't think we'd want to simply skip over a hook registration if the
LSM doesn't have access to an exclusive hook. At the very least it risks
unexpected behavior in the LSM and at the worst it prevents the LSM from
properly enforcing it's security policy. There is a reason we have the
panic() call in the existing code if we are not able to register a hook.
The simpliest solution here would be to panic, like we do today.
However, if you want to get fancy and enable LSMs to optionally adjust
their behavior to cope with the loss of a hook callback (I'm looking at
your patch 3/3 now), come up with a mechanism to report back to the LSM
that one or more of the callbacks were not registered. One quick thought
would be to return a non-zero error code and add an indicator/bool/flag
to the security_hook_list that would be set by security_add_hooks().
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 3/3] LSM: Reserve use of secmarks
From: Paul Moore @ 2026-04-24 1:19 UTC (permalink / raw)
To: Casey Schaufler, casey, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, selinux
In-Reply-To: <20260225192143.14448-4-casey@schaufler-ca.com>
On Feb 25, 2026 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Use of "exclusive" LSM hooks are limited to the first LSM registering
> them. These hooks include those use to process network secmarks.
> The hooks required to process secmarks are flagged with LSM_FLAG_EXCLUSIVE
> in their definitions. This includes secid_to_secctx and secctx_to_secid,
> which are used by netfilter.
>
> The various LSMs that use secmarks are updated to detect whether
> they are providing exclusive hooks, and to eschew the use of secmarks
> if they are not.
>
> SELinux has a peculiar behavior in that it may decide that it
> must have network controls, but only after policy is loaded.
> This patch includes a warning should policy capability alwaysnetwork
> be set when another LSM holds the exclusive hooks. It has been
> suggested that SELinux would consider this a fatal condition, in
> which case a panic might be a favored, if draconian, option.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hook_defs.h | 12 +++++------
> include/linux/security.h | 1 +
> security/apparmor/lsm.c | 24 ++++++++++++++++------
> security/security.c | 15 ++++++++++++++
> security/selinux/hooks.c | 35 ++++++++++++++++++++++++--------
> security/selinux/ss/services.c | 3 +++
> security/smack/smack_lsm.c | 6 ++++--
> security/smack/smack_netfilter.c | 6 ++++++
> 8 files changed, 80 insertions(+), 22 deletions(-)
...
> diff --git a/include/linux/security.h b/include/linux/security.h
> index e3c137a1b30a..638436b9b748 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -326,6 +326,7 @@ int unregister_blocking_lsm_notifier(struct notifier_block *nb);
> extern int security_init(void);
> extern int early_security_init(void);
> extern u64 lsm_name_to_attr(const char *name);
> +extern u32 lsm_secmark_from_skb(struct sk_buff *skb, const u64 lsmid);
>
> /* Security operations */
> int security_binder_set_context_mgr(const struct cred *mgr);
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index a87cd60ed206..2ce0d9a73973 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1511,9 +1511,11 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
> static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> {
> struct aa_sk_ctx *ctx = aa_sock(sk);
> + u32 secmark;
> int error;
>
> - if (!skb->secmark)
> + secmark = lsm_secmark_from_skb(skb, LSM_ID_APPARMOR);
> + if (!secmark)
> return 0;
>
> /*
...
> diff --git a/security/security.c b/security/security.c
> index 25e7cfc96f20..754b16004677 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4509,6 +4509,21 @@ void security_inet_conn_established(struct sock *sk,
> }
> EXPORT_SYMBOL(security_inet_conn_established);
>
> +/**
> + * lsm_secmark_from_skb - secid for the specified LSM from the packet
> + * @skb: the packet
> + * @lsm: which LSM is asking
> + *
> + * If the specified LSM has use of the secmark, return its value.
> + * Otherwise, return the invalid secid 0.
> + */
> +u32 lsm_secmark_from_skb(struct sk_buff *skb, const u64 lsmid)
> +{
> + if (lsmid == lsm_exclusive_hooks)
> + return skb->secmark;
> + return 0;
> +}
Ooof. Not a fan. A better way to handle this would be to like I
mentioned in my comments on patch 2/3: have the LSM detect that it lost
the LSM_FLAG_EXCLUSIVE battle at callback registration time and do
whatever adjustments are necessary at init time. In a number of cases I
believe that simply not registering the callback will be sufficient.
If the only way to solve this is via runtime checks like this,
unfortunately my answer is going to be "no".
... and of course the proper way to solve this for secmark is to just
do the idr/xarray conversion for secmarks so every LSM can have their
own secmark. Yes, it does require some work, but to be honest I think
you've spent more time avoiding that work then it would be to just do
it. I'm growing increasing inclined to simply state that this is the
only solution I'm going to accept.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Paul Moore @ 2026-04-24 1:27 UTC (permalink / raw)
To: Mimi Zohar, roberto.sassu
Cc: Yeoreum Yun, Jonathan McDowell, linux-security-module,
linux-kernel, linux-integrity, linux-arm-kernel, kvmarm, jmorris,
serge, dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla,
maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, will, noodles, sebastianene
In-Reply-To: <aephL3MzYoqFGaT5@e129823.arm.com>
On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
>
> Sounds good. Once the patch is posted, I’ll review it as well.
> Sorry again for the noise, and thanks for your patience ;)
My apologies for not getting a chance to look at this patchset sooner.
This seems like an obvious, perhaps even stupid, question, but I have
to ask: if IMA can be properly initialized via late_initcall_sync(),
why not simply do the initialization in late_initcall_sync() and drop
the late_initcall() initialization?
Does any IMA functionality suffer if initialization waits until
late_initcall_sync()? If so, it seems non-critical if waiting until
_sync() is acceptable, as it appears in these patches/comments.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-24 5:57 UTC (permalink / raw)
To: Paul Moore
Cc: Mimi Zohar, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <CAHC9VhRQWHEWQ5NzOPiu8jtYv6UsRm8WVS4fd74AbkOcAd4y_g@mail.gmail.com>
Hi Paul,
> On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> >
> > Sounds good. Once the patch is posted, I’ll review it as well.
> > Sorry again for the noise, and thanks for your patience ;)
>
> My apologies for not getting a chance to look at this patchset sooner.
>
> This seems like an obvious, perhaps even stupid, question, but I have
> to ask: if IMA can be properly initialized via late_initcall_sync(),
> why not simply do the initialization in late_initcall_sync() and drop
> the late_initcall() initialization?
>
> Does any IMA functionality suffer if initialization waits until
> late_initcall_sync()? If so, it seems non-critical if waiting until
> _sync() is acceptable, as it appears in these patches/comments.
This is the way first patch did, and here is some discussion for this
(Might you have seen, but in case of you missed):
- https://lore.kernel.org/all/a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com/
Thanks.
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [PATCH v3] evm: terminate and bound the evm_xattrs read buffer
From: Roberto Sassu @ 2026-04-24 8:13 UTC (permalink / raw)
To: Pengpeng Hou, Mimi Zohar
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge Hallyn, linux-integrity,
linux-security-module, linux-kernel
In-Reply-To: <20260424070101.1-evm-v3-pengpeng@iscas.ac.cn>
On Thu, 2026-04-23 at 23:30 +0800, Pengpeng Hou wrote:
> evm_read_xattrs() allocates size + 1 bytes, fills them from the list of
> enabled xattrs, and then passes strlen(temp) to
> simple_read_from_buffer(). When no configured xattrs are enabled, the
> fill loop stores nothing and temp[0] remains uninitialized, so strlen()
> reads beyond initialized memory.
>
> Explicitly terminate the buffer after allocation, use snprintf() for
> each formatted line, and pass the accumulated length, without risk of
> truncation, to simple_read_from_buffer().
>
> Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Thanks
Roberto
> ---
> Changes since v2:
> - adjust the changelog wording to mention why the accumulated length is
> safe
> - add the blank line after the allocation error path
> - add a comment explaining why snprintf() cannot truncate in the fill loop
>
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index acd840461902..4baf5e23bc97 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -127,8 +127,8 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
> size_t count, loff_t *ppos)
> {
> char *temp;
> - int offset = 0;
> - ssize_t rc, size = 0;
> + size_t offset = 0, size = 0;
> + ssize_t rc;
> struct xattr_list *xattr;
>
> if (*ppos != 0)
> @@ -151,16 +151,22 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
> return -ENOMEM;
> }
>
> + temp[size] = '\0';
> +
> + /*
> + * No truncation possible: size is computed over the same enabled
> + * xattrs under xattr_list_mutex, so offset never exceeds size.
> + */
> list_for_each_entry(xattr, &evm_config_xattrnames, list) {
> if (!xattr->enabled)
> continue;
>
> - sprintf(temp + offset, "%s\n", xattr->name);
> - offset += strlen(xattr->name) + 1;
> + offset += snprintf(temp + offset, size + 1 - offset, "%s\n",
> + xattr->name);
> }
>
> mutex_unlock(&xattr_list_mutex);
> - rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
> + rc = simple_read_from_buffer(buf, count, ppos, temp, offset);
>
> kfree(temp);
>
^ permalink raw reply
* [PATCH] ima: Fix sigv3 signature handling for EVM_IMA_XATTR_DIGSIG
From: Kamlesh Kumar @ 2026-04-24 11:39 UTC (permalink / raw)
To: zohar, stefanb
Cc: linux-integrity, linux-security-module, linux-kernel,
Kamlesh Kumar
ima_get_hash_algo() only recognizes version 2 signatures when the xattr
type is EVM_IMA_XATTR_DIGSIG. Since sigv3 signatures also use
EVM_IMA_XATTR_DIGSIG as the xattr type, version 3 must be accepted as
well to correctly determine the hash algorithm.
Additionally, ima_validate_rule() does not include IMA_SIGV3_REQUIRED in
the allowed flags bitmask for MODULE_CHECK, KEXEC_KERNEL_CHECK, and
KEXEC_INITRAMFS_CHECK hook functions. As a result, policy rules with
"appraise_type=sigv3" are rejected for these functions.
Add version 3 to the accepted versions in ima_get_hash_algo() for
EVM_IMA_XATTR_DIGSIG, and add IMA_SIGV3_REQUIRED to the allowed flags
for MODULE_CHECK, KEXEC_KERNEL_CHECK, and KEXEC_INITRAMFS_CHECK in
ima_validate_rule().
Signed-off-by: Kamlesh Kumar <kam@juniper.net>
---
security/integrity/ima/ima_appraise.c | 5 +++--
security/integrity/ima/ima_policy.c | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index de963b9f3634..2dd231567710 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -195,8 +195,9 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
return sig->hash_algo;
case EVM_IMA_XATTR_DIGSIG:
sig = (typeof(sig))xattr_value;
- if (sig->version != 2 || xattr_len <= sizeof(*sig)
- || sig->hash_algo >= HASH_ALGO__LAST)
+ if ((sig->version != 2 && sig->version != 3) ||
+ xattr_len <= sizeof(*sig) ||
+ sig->hash_algo >= HASH_ALGO__LAST)
return ima_hash_algo;
return sig->hash_algo;
case IMA_XATTR_DIGEST_NG:
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f7f940a76922..b1c010e8eb13 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1313,7 +1313,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
IMA_GID | IMA_EGID |
IMA_FGROUP | IMA_DIGSIG_REQUIRED |
IMA_PERMIT_DIRECTIO | IMA_MODSIG_ALLOWED |
- IMA_CHECK_BLACKLIST | IMA_VALIDATE_ALGOS))
+ IMA_CHECK_BLACKLIST | IMA_VALIDATE_ALGOS |
+ IMA_SIGV3_REQUIRED))
return false;
break;
base-commit: 82bbd447199ff1441031d2eaf9afe041550cf525
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v3 0/4] Fix IMA + TPM initialisation ordering issue
From: Jonathan McDowell @ 2026-04-24 13:23 UTC (permalink / raw)
To: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm
Cc: paul, jmorris, serge, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene, Yeoreum Yun
This is a slight reworking of the series from Yeoreum Yun, adding
functionality for IMA initialisation during the late_initcall_sync
stage. This solves the situation where the TPM is not fully registered
at the point IMA wants to initialise, avoiding a failure to correctly
extend TPM measurements from IMA. This has been observed on both Arm
FF-A and SPI attached TPM setups.
As part of this series we also revert the original changes made to the
FF-A driver to try and solve this problem.
(I have left Yeoreum credited in all the diffs except my rework of the
IMA piece. Yeoreum, please yell if you're not happy with this.)
Patch history
=============
from v2 to v3:
- Drop ff-a/pKVM diff (this seems to have a separate set of
discussion)
- Rework IMA delayed initialisation to avoid delaying when unnecessary
- Ensure IMA log clearly indicates when we've initialised late
from v1 to v2:
- add notifier to make ffa-driver pkvm initialised.
- modify to try initailisation again when IMA coudln't find proper TPM device.
- https://lore.kernel.org/all/20260417175759.3191279-1-yeoreum.yun@arm.com/#t
Jonathan McDowell (1):
security: ima: call ima_init() again at late_initcall_sync for defered
TPM
Yeoreum Yun (3):
lsm: Allow LSMs to register for late_initcall_sync init
Revert "tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in"
Revert "firmware: arm_ffa: Change initcall level of ffa_init() to
rootfs_initcall"
drivers/char/tpm/tpm_crb_ffa.c | 19 ++----------
drivers/firmware/arm_ffa/driver.c | 2 +-
include/linux/lsm_hooks.h | 2 ++
security/integrity/ima/ima.h | 3 +-
security/integrity/ima/ima_init.c | 25 ++++++++-------
security/integrity/ima/ima_main.c | 37 ++++++++++++++++++++---
security/integrity/ima/ima_template_lib.c | 3 +-
security/lsm_init.c | 13 ++++++--
8 files changed, 67 insertions(+), 37 deletions(-)
--
2.53.0
^ permalink raw reply
* [RFC PATCH v3 1/4] lsm: Allow LSMs to register for late_initcall_sync init
From: Jonathan McDowell @ 2026-04-24 13:24 UTC (permalink / raw)
To: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm
Cc: paul, jmorris, serge, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene, Yeoreum Yun
In-Reply-To: <cover.1777036497.git.noodles@meta.com>
From: Yeoreum Yun <yeoreum.yun@arm.com>
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.
[noodles: Split out from actual IMA changes]
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Signed-off-by: Jonathan McDowell <noodles@meta.com>
---
include/linux/lsm_hooks.h | 2 ++
security/lsm_init.c | 13 +++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index d48bf0ad26f4..88fe105b7f00 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -166,6 +166,7 @@ enum lsm_order {
* @initcall_fs: LSM callback for fs_initcall setup, optional
* @initcall_device: LSM callback for device_initcall() setup, optional
* @initcall_late: LSM callback for late_initcall() setup, optional
+ * @initcall_late_sync: LSM callback for late_initcall_sync() setup, optional
*/
struct lsm_info {
const struct lsm_id *id;
@@ -181,6 +182,7 @@ struct lsm_info {
int (*initcall_fs)(void);
int (*initcall_device)(void);
int (*initcall_late)(void);
+ int (*initcall_late_sync)(void);
};
#define DEFINE_LSM(lsm) \
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 573e2a7250c4..4e5c59beb82a 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -547,13 +547,22 @@ device_initcall(security_initcall_device);
* security_initcall_late - Run the LSM late initcalls
*/
static int __init security_initcall_late(void)
+{
+ return lsm_initcall(late);
+}
+late_initcall(security_initcall_late);
+
+/**
+ * security_initcall_late_sync - Run the LSM late initcalls sync
+ */
+static int __init security_initcall_late_sync(void)
{
int rc;
- rc = lsm_initcall(late);
+ rc = lsm_initcall(late_sync);
lsm_pr_dbg("all enabled LSMs fully activated\n");
call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
return rc;
}
-late_initcall(security_initcall_late);
+late_initcall_sync(security_initcall_late_sync);
--
2.53.0
^ permalink raw reply related
* [RFC PATCH v3 2/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Jonathan McDowell @ 2026-04-24 13:24 UTC (permalink / raw)
To: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm
Cc: paul, jmorris, serge, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene, Yeoreum Yun
In-Reply-To: <cover.1777036497.git.noodles@meta.com>
From: Jonathan McDowell <noodles@meta.com>
The Linux IMA (Integrity Measurement Architecture) subsystem used for
secure boot, file integrity, or remote attestation cannot be a loadable
module for few reasons listed below:
o Boot-Time Integrity: IMA’s main role is to measure and appraise files
before they are used. This includes measuring critical system files
during early boot (e.g., init, init scripts, login binaries). If IMA
were a module, it would be loaded too late to cover those.
o TPM Dependency: IMA integrates tightly with the TPM to record
measurements into PCRs. The TPM must be initialized early (ideally
before init_ima()), which aligns with IMA being built-in.
o Security Model: IMA is part of a Trusted Computing Base (TCB). Making
it a module would weaken the security model, as a potentially
compromised system could delay or tamper with its initialization.
IMA must be built-in to ensure it starts measuring from the earliest
possible point in boot which inturn implies TPM must be initialised and
ready to use before IMA.
Unfortunately some TPM drivers (such as Arm FF-A, or SPI attached TPM
devices) are not reliably available during the initcall_late stage,
resulting in a log error:
ima: No TPM chip found, activating TPM-bypass!
and no measurements into the TPM by IMA. We can avoid this by doing IMA
init in the initcall_late_sync stage, after the drivers have completed
their init + registration.
Rather than do this everywhere, and needlessly delay the initialisation
of IMA when there is no need to do so, we continue to try to initialise
at the earlier stage, only deferring to the later point if the TPM is
not available yet.
Signed-off-by: Jonathan McDowell <noodles@meta.com>
---
security/integrity/ima/ima.h | 3 +-
security/integrity/ima/ima_init.c | 25 ++++++++-------
security/integrity/ima/ima_main.c | 37 ++++++++++++++++++++---
security/integrity/ima/ima_template_lib.c | 3 +-
4 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 89ebe98ffc5e..b3677b403a5a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -65,6 +65,7 @@ extern struct ima_algo_desc *ima_algo_array __ro_after_init;
extern int ima_appraise;
extern struct tpm_chip *ima_tpm_chip;
extern const char boot_aggregate_name[];
+extern const char boot_aggregate_late_name[];
/* IMA event related data */
struct ima_event_data {
@@ -257,7 +258,7 @@ static inline void ima_measure_kexec_event(const char *event_name) {}
extern bool ima_canonical_fmt;
/* Internal IMA function definitions */
-int ima_init(void);
+int ima_init_core(bool late);
int ima_fs_init(void);
int ima_add_template_entry(struct ima_template_entry *entry, int violation,
const char *op, struct inode *inode,
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index a2f34f2d8ad7..5f335834a9bb 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -22,6 +22,7 @@
/* name for boot aggregate entry */
const char boot_aggregate_name[] = "boot_aggregate";
+const char boot_aggregate_late_name[] = "boot_aggregate_late";
struct tpm_chip *ima_tpm_chip;
/* Add the boot aggregate to the IMA measurement list and extend
@@ -39,17 +40,17 @@ struct tpm_chip *ima_tpm_chip;
* a different value.) Violations add a zero entry to the measurement
* list and extend the aggregate PCR value with ff...ff's.
*/
-static int __init ima_add_boot_aggregate(void)
+static int __init ima_add_boot_aggregate(bool late)
{
static const char op[] = "add_boot_aggregate";
const char *audit_cause = "ENOMEM";
struct ima_template_entry *entry;
struct ima_iint_cache tmp_iint, *iint = &tmp_iint;
- struct ima_event_data event_data = { .iint = iint,
- .filename = boot_aggregate_name };
+ struct ima_event_data event_data = { .iint = iint };
struct ima_max_digest_data hash;
struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
struct ima_digest_data, hdr);
+ const char *filename;
int result = -ENOMEM;
int violation = 0;
@@ -59,6 +60,12 @@ static int __init ima_add_boot_aggregate(void)
iint->ima_hash->algo = ima_hash_algo;
iint->ima_hash->length = hash_digest_size[ima_hash_algo];
+ if (late)
+ filename = boot_aggregate_late_name;
+ else
+ filename = boot_aggregate_name;
+ event_data.filename = filename;
+
/*
* With TPM 2.0 hash agility, TPM chips could support multiple TPM
* PCR banks, allowing firmware to configure and enable different
@@ -86,7 +93,7 @@ static int __init ima_add_boot_aggregate(void)
}
result = ima_store_template(entry, violation, NULL,
- boot_aggregate_name,
+ filename,
CONFIG_IMA_MEASURE_PCR_IDX);
if (result < 0) {
ima_free_template_entry(entry);
@@ -95,7 +102,7 @@ static int __init ima_add_boot_aggregate(void)
}
return 0;
err_out:
- integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, boot_aggregate_name, op,
+ integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, filename, op,
audit_cause, result, 0);
return result;
}
@@ -115,14 +122,10 @@ void __init ima_load_x509(void)
}
#endif
-int __init ima_init(void)
+int __init ima_init_core(bool late)
{
int rc;
- ima_tpm_chip = tpm_default_chip();
- if (!ima_tpm_chip)
- pr_info("No TPM chip found, activating TPM-bypass!\n");
-
rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
if (rc)
return rc;
@@ -140,7 +143,7 @@ int __init ima_init(void)
rc = ima_init_digests();
if (rc != 0)
return rc;
- rc = ima_add_boot_aggregate(); /* boot aggregate must be first entry */
+ rc = ima_add_boot_aggregate(late); /* boot aggregate must be first entry */
if (rc != 0)
return rc;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1d6229b156fb..0b93a286c0d3 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1237,7 +1237,7 @@ static int ima_kernel_module_request(char *kmod_name)
#endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
-static int __init init_ima(void)
+static int __init init_ima(bool late)
{
int error;
@@ -1247,10 +1247,26 @@ static int __init init_ima(void)
return 0;
}
+ /*
+ * If we found the TPM during our first attempt, or we know there's no
+ * TPM, nothing further to do
+ */
+ if (late && (ima_tpm_chip || !IS_ENABLED(CONFIG_TCG_TPM)))
+ return 0;
+
+ ima_tpm_chip = tpm_default_chip();
+ if (!ima_tpm_chip && !late && IS_ENABLED(CONFIG_TCG_TPM)) {
+ pr_debug("TPM not available, will try later\n");
+ return -EPROBE_DEFER;
+ }
+
+ if (!ima_tpm_chip)
+ pr_info("No TPM chip found, activating TPM-bypass!\n");
+
ima_appraise_parse_cmdline();
ima_init_template_list();
hash_setup(CONFIG_IMA_DEFAULT_HASH);
- error = ima_init();
+ error = ima_init_core(late);
if (error && strcmp(hash_algo_name[ima_hash_algo],
CONFIG_IMA_DEFAULT_HASH) != 0) {
@@ -1258,7 +1274,7 @@ static int __init init_ima(void)
hash_algo_name[ima_hash_algo], CONFIG_IMA_DEFAULT_HASH);
hash_setup_done = 0;
hash_setup(CONFIG_IMA_DEFAULT_HASH);
- error = ima_init();
+ error = ima_init_core(late);
}
if (error)
@@ -1274,6 +1290,16 @@ static int __init init_ima(void)
return error;
}
+static int __init init_ima_late(void)
+{
+ return init_ima(false);
+}
+
+static int __init init_ima_late_sync(void)
+{
+ return init_ima(true);
+}
+
static struct security_hook_list ima_hooks[] __ro_after_init = {
LSM_HOOK_INIT(bprm_check_security, ima_bprm_check),
LSM_HOOK_INIT(bprm_creds_for_exec, ima_bprm_creds_for_exec),
@@ -1319,6 +1345,7 @@ DEFINE_LSM(ima) = {
.init = init_ima_lsm,
.order = LSM_ORDER_LAST,
.blobs = &ima_blob_sizes,
- /* Start IMA after the TPM is available */
- .initcall_late = init_ima,
+ /* Ensure we start IMA after the TPM is available */
+ .initcall_late = init_ima_late,
+ .initcall_late_sync = init_ima_late_sync,
};
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 0e627eac9c33..8a89236f926c 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -363,7 +363,8 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
goto out;
}
- if ((const char *)event_data->filename == boot_aggregate_name) {
+ if ((const char *)event_data->filename == boot_aggregate_name ||
+ (const char *)event_data->filename == boot_aggregate_late_name) {
if (ima_tpm_chip) {
hash.hdr.algo = HASH_ALGO_SHA1;
result = ima_calc_boot_aggregate(hash_hdr);
--
2.53.0
^ permalink raw reply related
* [RFC PATCH v3 3/4] Revert "tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in"
From: Jonathan McDowell @ 2026-04-24 13:24 UTC (permalink / raw)
To: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm
Cc: paul, jmorris, serge, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene, Yeoreum Yun
In-Reply-To: <cover.1777036497.git.noodles@meta.com>
From: Yeoreum Yun <yeoreum.yun@arm.com>
This reverts commit 746d9e9f62a6e8ba0eba2b83fc61cfe7fa8797ce.
Now that IMA will retry in the late_initcall_sync level if the TPM is
not available at first, this change is no longer required.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Signed-off-by: Jonathan McDowell <noodles@meta.com>
---
drivers/char/tpm/tpm_crb_ffa.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 99f1c1e5644b..05f19c0ebf82 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -123,7 +123,6 @@ struct tpm_crb_ffa {
};
static struct tpm_crb_ffa *tpm_crb_ffa;
-static struct ffa_driver tpm_crb_ffa_driver;
static int tpm_crb_ffa_to_linux_errno(int errno)
{
@@ -177,23 +176,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 +394,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");
--
2.53.0
^ permalink raw reply related
* [RFC PATCH v3 4/4] Revert "firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"
From: Jonathan McDowell @ 2026-04-24 13:24 UTC (permalink / raw)
To: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm
Cc: paul, jmorris, serge, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene, Yeoreum Yun
In-Reply-To: <cover.1777036497.git.noodles@meta.com>
From: Yeoreum Yun <yeoreum.yun@arm.com>
This reverts commit 0e0546eabcd6c19765a8dbf5b5db3723e7b0ea75, which was
added to address ordering issues with the IMA LSM initialisation where
the TPM would not be fully ready by the time IMA wanted it. This has
been resolved within IMA by retrying setup during late_initcall_sync if
the TPM is not available at first.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Signed-off-by: Jonathan McDowell <noodles@meta.com>
---
drivers/firmware/arm_ffa/driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index f2f94d4d533e..01547c5c0e38 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -2106,7 +2106,7 @@ static int __init ffa_init(void)
kfree(drv_info);
return ret;
}
-rootfs_initcall(ffa_init);
+module_init(ffa_init);
static void __exit ffa_exit(void)
{
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 0/4] Firmware LSM hook
From: Jason Gunthorpe @ 2026-04-24 14:19 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Paul Moore, Roberto Sassu, KP Singh, Matt Bobrowski,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
Saeed Mahameed, Itay Avraham, Dave Jiang, Jonathan Cameron, bpf,
linux-kernel, linux-kselftest, linux-rdma, Chiara Meiohas,
Maher Sanalla, linux-security-module
In-Reply-To: <20260423140950.GE172828@unreal>
On Thu, Apr 23, 2026 at 05:09:50PM +0300, Leon Romanovsky wrote:
> > > Leon mentioned that different firmware revisions would have different
> > > parameters for a given opcode, and that one would need to inspect
> > > those parameters to properly filter the command. Is that not true, or
> > > am I misreading or misunderstanding Leon's comments?
> >
> > They are ABI stable, so there will be rules about future changes that
> > old software can follow to ignore or reject future things it doesn't
> > understand.
>
> It is wishful thinking and applicable only to mlx5 devices. No one
> promises that other devices follow same ABI rules.
Well, I will definately kick them out of fwctl if they don't.
Jason
^ permalink raw reply
* Re: [PATCH v2 0/4] Firmware LSM hook
From: Jason Gunthorpe @ 2026-04-24 14:36 UTC (permalink / raw)
To: Paul Moore
Cc: Leon Romanovsky, Roberto Sassu, KP Singh, Matt Bobrowski,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
Saeed Mahameed, Itay Avraham, Dave Jiang, Jonathan Cameron, bpf,
linux-kernel, linux-kselftest, linux-rdma, Chiara Meiohas,
Maher Sanalla, linux-security-module
In-Reply-To: <CAHC9VhQbpS9XpO6dWu7gOiX=ppjtAxnNBOFe6s5wjEZNpMRjgw@mail.gmail.com>
On Mon, Apr 20, 2026 at 08:58:09PM -0400, Paul Moore wrote:
> > > > > The access control point itself represents the requested
> > > > > operation. This is possible because the number of networking
> > > > > operations on a given packet is well defined and fairly limited; at a
> > > > > high level the packet is either being sent from the node, received by
> > > > > the node, or is passing through the node.
> > > >
> > > > I think we have the same split, fwctl send/recive analog is also very
> > > > limited.
> > >
> > > Sure, but I thought the goal was to enforce access controls on the
> > > firmware requests based on the opcodes/parameters contained within the
> > > firmware request blob/mailbox?
> >
> > Yes, that's the goal. It is the same as iptables being able to
> > identify that a send system call has a packet that is http or dns.
>
> I think we still have a disconnect here. A packet being a DNS or HTTP
> packet is different from an opcode. The opcode in the iptables isn't
> "DNS" or "HTTP" it is "INPUT", "OUTPUT", or "FORWARD".
I understand that
> Most LSMs will want to know who is initiating the firmware request
> (the subject), the requested operation/opcode (the action/verb), and
> the target of the request (the object, which in this case is likely
> the kernel or the device).
How is
system_u:object_r:httpd_packet_t:s0
A kernel or device? It is a label for packet contents. I also want a
label for packet contents.
> As I understand things, the action/verb is going to be the opcode
> within the firmware request. If you believe I'm wrong about this
> please help me understand why.
You could make that choice, I'm arguing we should not, and it should
be in the object side.
> > - op_X_t is the result of the classifier inspecting the RPC
> > packet. Admin tells the classifier to return op_X_t similar to
> > how --selctx does for iptables.
>
> I've tried to explain how this doesn't match with secmark, but I'm
> evidently doing a poor job.
Yeah, I don't get it at all, sorry. I fell you are making some very
nuanced distinction with HTTP being an object but the HTTP-equivilant
in fwctl is not an object, I can't follow it at all.
By that logic:
iptables -p 80 --string "GET"
Is an action, and it should get a unique action in the tuple.
> If you want to continue with the secmark comparisons it might be
> helpful to spend some time configuring secmark on a SELinux system,
> and writing policy for it, to see how it works.
I think I have a pretty good idea, you haven't said anything that
contradicts what I expect..
> certain action on an object. My concern with your example is that the
> object isn't what is actually being acted upon, it's the requested
> action.
Object is a label for the packet contents.
> The fwctl ioctl/API allows a user to act on a device, with the
> actual action being specified by the fwctl payload. From what I can
> see, the classifier's output is the action, not the object.
You can take that view, it is certainly one valid way to look at it.
But it is completely impractical.
I am arguing for the secmark like view where the content of the packet
decides the object label.
> > The same way secmark cannot pre-identify all the XXX_packet_t's.
>
> Once again, I think there is a disconnect or misunderstanding, on a
> SELinux system using secmark all of the packet types, e.g.
> "XXX_packet_t's", *are* pre-defined in the SELinux policy.
"Pre-defined" in a text files in user space controlled by the admin.
Admin can "pre-define" their fwctl ones too, what is the issue?
AFAICT the only debate here is you want to make fwctl's packet content
an action while allowing iptable's packet content to be an object.
Jason
^ permalink raw reply
* Re: [PATCH RFC 1/3] LSM: add a flags field to the LSM hook definitions
From: Casey Schaufler @ 2026-04-24 15:24 UTC (permalink / raw)
To: Paul Moore, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, selinux, Casey Schaufler
In-Reply-To: <c25833f447e6c00868b352cfea50d2b2@paul-moore.com>
On 4/23/2026 6:19 PM, Paul Moore wrote:
> On Feb 25, 2026 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Add a field for flags to the definition of LSM hooks. This allows
>> for hooks to be identified at system initialization for special
>> processing.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> include/linux/bpf_lsm.h | 2 +-
>> include/linux/lsm_hook_defs.h | 614 ++++++++++++++++++----------------
>> include/linux/lsm_hooks.h | 4 +-
>> kernel/bpf/bpf_lsm.c | 10 +-
>> security/bpf/hooks.c | 2 +-
>> security/security.c | 6 +-
>> 6 files changed, 331 insertions(+), 307 deletions(-)
>>
>> diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
>> index 643809cc78c3..d71ba8c87e79 100644
>> --- a/include/linux/bpf_lsm.h
>> +++ b/include/linux/bpf_lsm.h
>> @@ -14,7 +14,7 @@
>>
>> #ifdef CONFIG_BPF_LSM
>>
>> -#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
>> +#define LSM_HOOK(RET, DEFAULT, FLAGS, NAME, ...) \
>> RET bpf_lsm_##NAME(__VA_ARGS__);
>> #include <linux/lsm_hook_defs.h>
>> #undef LSM_HOOK
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index 8c42b4bde09c..acda3a02da97 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -18,451 +18,475 @@
>> * The macro LSM_HOOK is used to define the data structures required by
>> * the LSM framework using the pattern:
>> *
>> - * LSM_HOOK(<return_type>, <default_value>, <hook_name>, args...)
>> + * LSM_HOOK(<return_type>, <default_value>, <flags>, <single>,
>> + * <hook_name>, args...)
>> *
>> * struct security_hook_heads {
>> - * #define LSM_HOOK(RET, DEFAULT, NAME, ...) struct hlist_head NAME;
>> + * #define LSM_HOOK(RET, DEFAULT, FLAGS, NAME, ...) struct hlist_head NAME;
>> * #include <linux/lsm_hook_defs.h>
>> * #undef LSM_HOOK
>> * };
>> */
>> -LSM_HOOK(int, 0, binder_set_context_mgr, const struct cred *mgr)
>> -LSM_HOOK(int, 0, binder_transaction, const struct cred *from,
>> +LSM_HOOK(int, 0, 0, binder_set_context_mgr, const struct cred *mgr)
>> +LSM_HOOK(int, 0, 0, binder_transaction, const struct cred *from,
>> const struct cred *to)
> I think adding a flag field to the LSM_HOOK() macro/definitions is a good
> and useful addition, but I'd prefer if we created a LSM_FLAG_NONE #define
> and used it here just so we could avoid the back-to-back 0's and do a bit
> of self-documentation.
I had LSM_FLAG_NONE initially, but removed it when I saw the amount of code
churn it introduced. I'm happy to put it back.
^ permalink raw reply
* Re: [RFC PATCH v3 4/4] Revert "firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"
From: Sudeep Holla @ 2026-04-24 16:09 UTC (permalink / raw)
To: Jonathan McDowell
Cc: linux-security-module, linux-kernel, Sudeep Holla,
linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, jarkko, jgg,
maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, will, noodles, sebastianene, Yeoreum Yun
In-Reply-To: <2e7b4dc552b45ddf14cc43bc449cbebb4ade0027.1777036497.git.noodles@meta.com>
On Fri, Apr 24, 2026 at 02:24:42PM +0100, Jonathan McDowell wrote:
> From: Yeoreum Yun <yeoreum.yun@arm.com>
>
> This reverts commit 0e0546eabcd6c19765a8dbf5b5db3723e7b0ea75, which was
> added to address ordering issues with the IMA LSM initialisation where
> the TPM would not be fully ready by the time IMA wanted it. This has
> been resolved within IMA by retrying setup during late_initcall_sync if
> the TPM is not available at first.
>
Reviewed-by: Sudeep Holla <sudeep.holla@kernel.org>
--
Regards,
Sudeep
^ 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