* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Mimi Zohar @ 2026-01-16 16:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Coiby Xu, linux-integrity, Heiko Carstens, Roberto Sassu,
Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <CAMj1kXHsJNZoUEnbD1y=v4Ftuv9d2c08VckRV7ru4k4P83vZbQ@mail.gmail.com>
On Fri, 2026-01-16 at 14:18 +0100, Ard Biesheuvel wrote:
> On Fri, 16 Jan 2026 at 14:11, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Fri, 2026-01-16 at 10:41 +0100, Ard Biesheuvel wrote:
> > > On Thu, 15 Jan 2026 at 01:43, Coiby Xu <coxu@redhat.com> wrote:
> > > >
> > > > EVM and other LSMs need the ability to query the secure boot status of
> > > > the system, without directly calling the IMA arch_ima_get_secureboot
> > > > function. Refactor the secure boot status check into a general,
> > > > integrity-wide function named arch_integrity_get_secureboot.
> > > >
> > > > Define a new Kconfig option CONFIG_INTEGRITY_SECURE_BOOT, which is
> > > > automatically configured by the supported architectures. The existing
> > > > IMA_SECURE_AND_OR_TRUSTED_BOOT Kconfig loads the architecture specific
> > > > IMA policy based on the refactored secure boot status code.
> > > >
> > > > Reported-and-suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > Suggested-by: Roberto Sassu <roberto.sassu@huaweicloud.com>
> > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > ---
> > > > arch/arm64/Kconfig | 1 +
> > > > arch/powerpc/Kconfig | 1 +
> > > > arch/powerpc/kernel/Makefile | 2 +-
> > > > arch/powerpc/kernel/ima_arch.c | 5 --
> > > > arch/powerpc/kernel/integrity_sb_arch.c | 13 +++++
> > > > arch/s390/Kconfig | 1 +
> > > > arch/s390/kernel/Makefile | 1 +
> > > > arch/s390/kernel/ima_arch.c | 6 --
> > > > arch/s390/kernel/integrity_sb_arch.c | 9 +++
> > > > arch/x86/Kconfig | 1 +
> > > > arch/x86/include/asm/efi.h | 4 +-
> > > > arch/x86/platform/efi/efi.c | 2 +-
> > > > include/linux/ima.h | 7 +--
> > > > include/linux/integrity.h | 8 +++
> > > > security/integrity/Kconfig | 6 ++
> > > > security/integrity/Makefile | 3 +
> > > > security/integrity/efi_secureboot.c | 56 +++++++++++++++++++
> > > > security/integrity/ima/ima_appraise.c | 2 +-
> > > > security/integrity/ima/ima_efi.c | 47 +---------------
> > > > security/integrity/ima/ima_main.c | 4 +-
> > > > security/integrity/platform_certs/load_uefi.c | 2 +-
> > > > 21 files changed, 111 insertions(+), 70 deletions(-)
> > > > create mode 100644 arch/powerpc/kernel/integrity_sb_arch.c
> > > > create mode 100644 arch/s390/kernel/integrity_sb_arch.c
> > > > create mode 100644 security/integrity/efi_secureboot.c
> > > >
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index 93173f0a09c7..4c265b7386bb 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -2427,6 +2427,7 @@ config EFI
> > > > select EFI_STUB
> > > > select EFI_GENERIC_STUB
> > > > imply IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > > + imply INTEGRITY_SECURE_BOOT
> > >
> > > This allows both to be en/disabled individually, which I don't think
> > > is what we want. It also results in more churn across the
> > > arch-specific Kconfigs than needed.
> > >
> > > Wouldn't it be better if IMA_SECURE_AND_OR_TRUSTED_BOOT 'select'ed
> > > INTEGRITY_SECURE_BOOT in its Kconfig definition?
> >
> > As much as possible, EVM (and other LSMs) shouldn't be dependent on another LSM,
> > in this case IMA, being configured.
>
> Sure, but that is not my point.
>
> This arrangement allows for IMA_SECURE_AND_OR_TRUSTED_BOOT to be
> enabled without INTEGRITY_SECURE_BOOT, resulting in the stub
> implementation of arch_integrity_get_secureboot() being used, which
> always returns false.
I understand your concern, but instead of "select"ing INTEGRITY_SECURE_BOOT from
IMA_SECURE_AND_OR_TRUSTED_BOOT, how making IMA_SECURE_AND_OR_TRUSTED_BOOT
dependent on both IMA_ARCH_POLICY and INTEGRITY_SECURE_BOOT.
Including the "imply INTEGRITY_SECURE_BOOT" here in the arch Kconfig allows EVM
to query the secure boot state without relying on IMA_SECURE_AND_OR_TRUSTED_BOOT
being configured.
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Ard Biesheuvel @ 2026-01-16 17:27 UTC (permalink / raw)
To: Mimi Zohar
Cc: Coiby Xu, linux-integrity, Heiko Carstens, Roberto Sassu,
Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <97b69bc79a5d9246f7a399510908c7b95b2e95e7.camel@linux.ibm.com>
On Fri, 16 Jan 2026 at 17:39, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Fri, 2026-01-16 at 14:18 +0100, Ard Biesheuvel wrote:
> > On Fri, 16 Jan 2026 at 14:11, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Fri, 2026-01-16 at 10:41 +0100, Ard Biesheuvel wrote:
> > > > On Thu, 15 Jan 2026 at 01:43, Coiby Xu <coxu@redhat.com> wrote:
> > > > >
> > > > > EVM and other LSMs need the ability to query the secure boot status of
> > > > > the system, without directly calling the IMA arch_ima_get_secureboot
> > > > > function. Refactor the secure boot status check into a general,
> > > > > integrity-wide function named arch_integrity_get_secureboot.
> > > > >
> > > > > Define a new Kconfig option CONFIG_INTEGRITY_SECURE_BOOT, which is
> > > > > automatically configured by the supported architectures. The existing
> > > > > IMA_SECURE_AND_OR_TRUSTED_BOOT Kconfig loads the architecture specific
> > > > > IMA policy based on the refactored secure boot status code.
> > > > >
> > > > > Reported-and-suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > > Suggested-by: Roberto Sassu <roberto.sassu@huaweicloud.com>
> > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > > ---
> > > > > arch/arm64/Kconfig | 1 +
> > > > > arch/powerpc/Kconfig | 1 +
> > > > > arch/powerpc/kernel/Makefile | 2 +-
> > > > > arch/powerpc/kernel/ima_arch.c | 5 --
> > > > > arch/powerpc/kernel/integrity_sb_arch.c | 13 +++++
> > > > > arch/s390/Kconfig | 1 +
> > > > > arch/s390/kernel/Makefile | 1 +
> > > > > arch/s390/kernel/ima_arch.c | 6 --
> > > > > arch/s390/kernel/integrity_sb_arch.c | 9 +++
> > > > > arch/x86/Kconfig | 1 +
> > > > > arch/x86/include/asm/efi.h | 4 +-
> > > > > arch/x86/platform/efi/efi.c | 2 +-
> > > > > include/linux/ima.h | 7 +--
> > > > > include/linux/integrity.h | 8 +++
> > > > > security/integrity/Kconfig | 6 ++
> > > > > security/integrity/Makefile | 3 +
> > > > > security/integrity/efi_secureboot.c | 56 +++++++++++++++++++
> > > > > security/integrity/ima/ima_appraise.c | 2 +-
> > > > > security/integrity/ima/ima_efi.c | 47 +---------------
> > > > > security/integrity/ima/ima_main.c | 4 +-
> > > > > security/integrity/platform_certs/load_uefi.c | 2 +-
> > > > > 21 files changed, 111 insertions(+), 70 deletions(-)
> > > > > create mode 100644 arch/powerpc/kernel/integrity_sb_arch.c
> > > > > create mode 100644 arch/s390/kernel/integrity_sb_arch.c
> > > > > create mode 100644 security/integrity/efi_secureboot.c
> > > > >
> > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > > index 93173f0a09c7..4c265b7386bb 100644
> > > > > --- a/arch/arm64/Kconfig
> > > > > +++ b/arch/arm64/Kconfig
> > > > > @@ -2427,6 +2427,7 @@ config EFI
> > > > > select EFI_STUB
> > > > > select EFI_GENERIC_STUB
> > > > > imply IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > > > + imply INTEGRITY_SECURE_BOOT
> > > >
> > > > This allows both to be en/disabled individually, which I don't think
> > > > is what we want. It also results in more churn across the
> > > > arch-specific Kconfigs than needed.
> > > >
> > > > Wouldn't it be better if IMA_SECURE_AND_OR_TRUSTED_BOOT 'select'ed
> > > > INTEGRITY_SECURE_BOOT in its Kconfig definition?
> > >
> > > As much as possible, EVM (and other LSMs) shouldn't be dependent on another LSM,
> > > in this case IMA, being configured.
> >
> > Sure, but that is not my point.
> >
> > This arrangement allows for IMA_SECURE_AND_OR_TRUSTED_BOOT to be
> > enabled without INTEGRITY_SECURE_BOOT, resulting in the stub
> > implementation of arch_integrity_get_secureboot() being used, which
> > always returns false.
>
> I understand your concern, but instead of "select"ing INTEGRITY_SECURE_BOOT from
> IMA_SECURE_AND_OR_TRUSTED_BOOT, how making IMA_SECURE_AND_OR_TRUSTED_BOOT
> dependent on both IMA_ARCH_POLICY and INTEGRITY_SECURE_BOOT.
>
Given that INTEGRITY_SECURE_BOOT has no dependencies of its own,
afaict, selecting it is the least disruptive option, as otherwise,
existing configs will disable IMA_SECURE_AND_OR_TRUSTED_BOOT as the
kernel is being upgraded. But conceptually, I agree that they are
equivalent.
> Including the "imply INTEGRITY_SECURE_BOOT" here in the arch Kconfig allows EVM
> to query the secure boot state without relying on IMA_SECURE_AND_OR_TRUSTED_BOOT
> being configured.
Yes, I understand that this is the whole point of the exercise. But
'imply' should be used with care, and in this case, implying both from
CONFIG_EFI really makes little sense. INTEGRITY_SECURE_BOOT should be
selected by options that need the functionality, not 'implied' by
options that might provide it.
^ permalink raw reply
* Re: [PATCH] ima: Detect changes to files via kstat changes rather than i_version
From: Frederick Lawler @ 2026-01-16 21:25 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com>
On Mon, Jan 12, 2026 at 04:32:23PM -0600, Frederick Lawler wrote:
> Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> introduced a means to track change detection for an inode
> via ctime updates, opposed to setting kstat.change_cookie when
> calling into xfs_vn_getattr().
>
> This introduced a regression because IMA caches kstat.change_cookie
> to compare against an inode's i_version directly in
> integrity_inode_attrs_changed(), and thus could be out of date
> depending on how file systems increment i_version.
>
> To address this, require integrity_inode_attrs_changed() to query
> vfs_getattr_nosec() to compare the cached version against
> kstat.change_cookie directly. This ensures that when updates occur,
> we're accessing the same changed inode version on changes, and fallback
> to compare against an artificial version generated from kstat.ctime
> via integrity_ctime_guard() when there's no detected change
> to the kstat.change_cookie.
>
> This ensures that in the absence of i_version support for file systems,
> and in the absence of a kstat.change_cookie update, we ultimately have a
> unique-enough version to compare against.
>
> The exact implementation for integrity_ctime_guard() is to ensure that
> if tv_sec or tv_nsec are zero, there's some value to store back into
> struct integrity_inode_attributes.version. This also avoids the need to
> add additional storage and comparisons.
>
> Lastly, because EVM still relies on querying and caching a backing inode's
> i_version, the integrity_inode_attrs_changed() falls back to the
> original inode.i_version != cached comparison. This maintains the
> invariant that a re-evaluation in unknown change detection circumstances
> is required.
>
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> using multigrain ctime (as well as other file systems) for
> change detection in commit 1cf7e834a6fb ("xfs: switch to
> multigrain timestamps").
>
> Because file systems may implement i_version as they see fit, IMA
> caching may be behind as well as file systems that don't support/export
> i_version. Thus we're proposing to compare against the kstat.change_cookie
> directly to the cached version, and fall back to a ctime guard when
> that's not updated.
>
> EVM is largely left alone since there's no trivial way to query a file
> directly in the LSM call paths to obtain kstat.change_cookie &
> kstat.ctime to cache. Thus retains accessing i_version directly.
>
> Regression tests will be added to the Linux Test Project instead of
> selftest to help catch future file system changes that may impact
> future evaluation of IMA.
>
> I'd like this to be backported to at least 6.18 if possible.
>
> Below is a simplified test that demonstrates the issue:
>
> _fragment.config_
> CONFIG_XFS_FS=y
> CONFIG_OVERLAY_FS=y
> CONFIG_IMA=y
> CONFIG_IMA_WRITE_POLICY=y
> CONFIG_IMA_READ_POLICY=y
>
> _./test.sh_
>
> IMA_POLICY="/sys/kernel/security/ima/policy"
> TEST_BIN="/bin/date"
> MNT_BASE="/tmp/ima_test_root"
>
> mkdir -p "$MNT_BASE"
> mount -t tmpfs tmpfs "$MNT_BASE"
> mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
>
> dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> mkfs.xfs -q "$MNT_BASE/xfs.img"
> mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
>
> mount -t overlay overlay -o \
> "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> "$MNT_BASE/ovl"
>
> echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
>
> target_prog="$MNT_BASE/ovl/test_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
>
> audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
>
> if [[ "$audit_count" -eq 1 ]]; then
> echo "PASS: Found exactly 1 audit event."
> else
> echo "FAIL: Expected 1 audit event, but found $audit_count."
> exit 1
> fi
> ---
> Changes since RFC:
> - Remove calls to I_IS_VERSION()
> - Function documentation/comments
> - Abide IMA/EVM change detection fallback invariants
> - Combined ctime guard into version for attributes struct
> - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> ---
> include/linux/integrity.h | 42 +++++++++++++++++++++++++++++++++++----
> security/integrity/evm/evm_main.c | 5 ++---
> security/integrity/ima/ima_api.c | 11 +++++++---
> security/integrity/ima/ima_main.c | 15 +++++---------
> 4 files changed, 53 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index f5842372359be5341b6870a43b92e695e8fc78af..5eca8aa2769f9238c68bb40885ecc46910524f11 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -9,6 +9,7 @@
>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> +#include <linux/kernel.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -36,6 +37,14 @@ struct integrity_inode_attributes {
> dev_t dev;
> };
>
> +/*
> + * Wrapper to generate an artificial version for a file.
> + */
> +static inline u64 integrity_ctime_guard(struct kstat stat)
> +{
> + return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> +}
> +
> /*
> * On stacked filesystems the i_version alone is not enough to detect file data
> * or metadata change. Additional metadata is required.
> @@ -51,14 +60,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
>
> /*
> * On stacked filesystems detect whether the inode or its content has changed.
> + *
> + * Must be called in process context.
> */
> static inline bool
> integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> - const struct inode *inode)
> + struct file *file, struct inode *inode)
> {
> - return (inode->i_sb->s_dev != attrs->dev ||
> - inode->i_ino != attrs->ino ||
> - !inode_eq_iversion(inode, attrs->version));
> + struct kstat stat;
> +
> + might_sleep();
> +
> + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> + return true;
> +
> + /*
> + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> + * is not a good indicator of i_version support, this still retains
> + * the logic such that a re-evaluation should still occur for EVM, and
> + * only for IMA if vfs_getattr_nosec() fails.
> + */
> + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT))
> + return !IS_I_VERSION(inode) ||
> + !inode_eq_iversion(inode, attrs->version);
> +
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + return stat.change_cookie != attrs->version;
> +
> + if (stat.result_mask & STATX_CTIME)
> + return integrity_ctime_guard(stat) != attrs->version;
> +
> + return true;
> }
>
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> bool ret = false;
>
> if (iint) {
> - ret = (!IS_I_VERSION(metadata_inode) ||
> - integrity_inode_attrs_changed(&iint->metadata_inode,
> - metadata_inode));
> + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> + NULL, metadata_inode);
> if (ret)
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8d404ba4886e3b736c3bab29d1668..8096986f3689781d3cdf6595f330033782f9cc45 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + else if (stat.result_mask & STATX_CTIME)
> + i_version = integrity_ctime_guard(stat);
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5770cf691912aa912fc65280c59f5baac35dd725..3a4c32e254f925bba85cb91b63744ac142b3b049 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -22,6 +22,7 @@
> #include <linux/mount.h>
> #include <linux/mman.h>
> #include <linux/slab.h>
> +#include <linux/stat.h>
> #include <linux/xattr.h>
> #include <linux/ima.h>
> #include <linux/fs.h>
> @@ -191,18 +192,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
>
> mutex_lock(&iint->mutex);
> if (atomic_read(&inode->i_writecount) == 1) {
> - struct kstat stat;
> -
> clear_bit(IMA_EMITTED_OPENWRITERS, &iint->atomic_flags);
>
> update = test_and_clear_bit(IMA_UPDATE_XATTR,
> &iint->atomic_flags);
> if ((iint->flags & IMA_NEW_FILE) ||
> - vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> - AT_STATX_SYNC_AS_STAT) ||
> - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> - stat.change_cookie != iint->real_inode.version) {
> + integrity_inode_attrs_changed(&iint->real_inode, file,
> + inode)) {
I'm working through my tests, and I don't think I can get away with this
change. The check for the inode->i_ino != attr->ino may result in a
re-evaluation because we're not updating the attr->ino while collecting
measurement on non-stacked file systems checks. Same for attr->dev not
updating.
I'll put this back in the next patch version, and still check ctime here
similar to the RFC version.
> iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> iint->measured_pcrs = 0;
> if (update)
> @@ -328,9 +324,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> real_inode = d_real_inode(file_dentry(file));
> if (real_inode != inode &&
> (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> - if (!IS_I_VERSION(real_inode) ||
> - integrity_inode_attrs_changed(&iint->real_inode,
> - real_inode)) {
> + if (integrity_inode_attrs_changed(&iint->real_inode,
> + file, real_inode)) {
> iint->flags &= ~IMA_DONE_MASK;
> iint->measured_pcrs = 0;
> }
>
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251212-xfs-ima-fixup-931780a62c2c
>
> Best regards,
> --
> Frederick Lawler <fred@cloudflare.com>
>
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Mimi Zohar @ 2026-01-18 18:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Coiby Xu, linux-integrity, Heiko Carstens, Roberto Sassu,
Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <CAMj1kXGx4ebaK87W7k0SNUNQnO9+=z1nmYxXC7retmp3OqRRFg@mail.gmail.com>
On Fri, 2026-01-16 at 18:27 +0100, Ard Biesheuvel wrote:
> On Fri, 16 Jan 2026 at 17:39, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Fri, 2026-01-16 at 14:18 +0100, Ard Biesheuvel wrote:
> > > On Fri, 16 Jan 2026 at 14:11, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > >
> > > > On Fri, 2026-01-16 at 10:41 +0100, Ard Biesheuvel wrote:
> > > > > On Thu, 15 Jan 2026 at 01:43, Coiby Xu <coxu@redhat.com> wrote:
> > > > > >
> > > > > > EVM and other LSMs need the ability to query the secure boot status of
> > > > > > the system, without directly calling the IMA arch_ima_get_secureboot
> > > > > > function. Refactor the secure boot status check into a general,
> > > > > > integrity-wide function named arch_integrity_get_secureboot.
> > > > > >
> > > > > > Define a new Kconfig option CONFIG_INTEGRITY_SECURE_BOOT, which is
> > > > > > automatically configured by the supported architectures. The existing
> > > > > > IMA_SECURE_AND_OR_TRUSTED_BOOT Kconfig loads the architecture specific
> > > > > > IMA policy based on the refactored secure boot status code.
> > > > > >
> > > > > > Reported-and-suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > > > Suggested-by: Roberto Sassu <roberto.sassu@huaweicloud.com>
> > > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > > > ---
> > > > > > arch/arm64/Kconfig | 1 +
> > > > > > arch/powerpc/Kconfig | 1 +
> > > > > > arch/powerpc/kernel/Makefile | 2 +-
> > > > > > arch/powerpc/kernel/ima_arch.c | 5 --
> > > > > > arch/powerpc/kernel/integrity_sb_arch.c | 13 +++++
> > > > > > arch/s390/Kconfig | 1 +
> > > > > > arch/s390/kernel/Makefile | 1 +
> > > > > > arch/s390/kernel/ima_arch.c | 6 --
> > > > > > arch/s390/kernel/integrity_sb_arch.c | 9 +++
> > > > > > arch/x86/Kconfig | 1 +
> > > > > > arch/x86/include/asm/efi.h | 4 +-
> > > > > > arch/x86/platform/efi/efi.c | 2 +-
> > > > > > include/linux/ima.h | 7 +--
> > > > > > include/linux/integrity.h | 8 +++
> > > > > > security/integrity/Kconfig | 6 ++
> > > > > > security/integrity/Makefile | 3 +
> > > > > > security/integrity/efi_secureboot.c | 56 +++++++++++++++++++
> > > > > > security/integrity/ima/ima_appraise.c | 2 +-
> > > > > > security/integrity/ima/ima_efi.c | 47 +---------------
> > > > > > security/integrity/ima/ima_main.c | 4 +-
> > > > > > security/integrity/platform_certs/load_uefi.c | 2 +-
> > > > > > 21 files changed, 111 insertions(+), 70 deletions(-)
> > > > > > create mode 100644 arch/powerpc/kernel/integrity_sb_arch.c
> > > > > > create mode 100644 arch/s390/kernel/integrity_sb_arch.c
> > > > > > create mode 100644 security/integrity/efi_secureboot.c
> > > > > >
> > > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > > > index 93173f0a09c7..4c265b7386bb 100644
> > > > > > --- a/arch/arm64/Kconfig
> > > > > > +++ b/arch/arm64/Kconfig
> > > > > > @@ -2427,6 +2427,7 @@ config EFI
> > > > > > select EFI_STUB
> > > > > > select EFI_GENERIC_STUB
> > > > > > imply IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > > > > + imply INTEGRITY_SECURE_BOOT
> > > > >
> > > > > This allows both to be en/disabled individually, which I don't think
> > > > > is what we want. It also results in more churn across the
> > > > > arch-specific Kconfigs than needed.
> > > > >
> > > > > Wouldn't it be better if IMA_SECURE_AND_OR_TRUSTED_BOOT 'select'ed
> > > > > INTEGRITY_SECURE_BOOT in its Kconfig definition?
> > > >
> > > > As much as possible, EVM (and other LSMs) shouldn't be dependent on another LSM,
> > > > in this case IMA, being configured.
> > >
> > > Sure, but that is not my point.
> > >
> > > This arrangement allows for IMA_SECURE_AND_OR_TRUSTED_BOOT to be
> > > enabled without INTEGRITY_SECURE_BOOT, resulting in the stub
> > > implementation of arch_integrity_get_secureboot() being used, which
> > > always returns false.
> >
> > I understand your concern, but instead of "select"ing INTEGRITY_SECURE_BOOT from
> > IMA_SECURE_AND_OR_TRUSTED_BOOT, how making IMA_SECURE_AND_OR_TRUSTED_BOOT
> > dependent on both IMA_ARCH_POLICY and INTEGRITY_SECURE_BOOT.
> >
>
> Given that INTEGRITY_SECURE_BOOT has no dependencies of its own,
> afaict, selecting it is the least disruptive option, as otherwise,
> existing configs will disable IMA_SECURE_AND_OR_TRUSTED_BOOT as the
> kernel is being upgraded. But conceptually, I agree that they are
> equivalent.
>
> > Including the "imply INTEGRITY_SECURE_BOOT" here in the arch Kconfig allows EVM
> > to query the secure boot state without relying on IMA_SECURE_AND_OR_TRUSTED_BOOT
> > being configured.
>
> Yes, I understand that this is the whole point of the exercise. But
> 'imply' should be used with care, and in this case, implying both from
> CONFIG_EFI really makes little sense. INTEGRITY_SECURE_BOOT should be
> selected by options that need the functionality, not 'implied' by
> options that might provide it.
As not all arch's implement arch_integrity_get_secureboot, the definition in
include/linux/integrity.h would need to be updated. Something like:
-#ifdef CONFIG_INTEGRITY_SECURE_BOOT
+#if (defined(CONFIG_INTEGRITY_SECURE_BOOT) && \
+ (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
+ || defined(CONFIG_PPC_SECURE_BOOT))
Then IMA_SECURE_AND_OR_TRUSTED_BOOT and EVM could select INTEGRITY_SECURE_BOOT,
as suggested.
Mimi
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Coiby Xu @ 2026-01-19 4:04 UTC (permalink / raw)
To: Ard Biesheuvel, Mimi Zohar
Cc: linux-integrity, Heiko Carstens, Roberto Sassu, Catalin Marinas,
Will Deacon, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <ac5e5e45c12e9b0bda19807e60b06057d74be0b3.camel@linux.ibm.com>
On Sun, Jan 18, 2026 at 01:25:52PM -0500, Mimi Zohar wrote:
>On Fri, 2026-01-16 at 18:27 +0100, Ard Biesheuvel wrote:
Hi Ard and Mimi,
Thanks for your discussion on improving the patch!
>> On Fri, 16 Jan 2026 at 17:39, Mimi Zohar <zohar@linux.ibm.com> wrote:
>> >
>> > On Fri, 2026-01-16 at 14:18 +0100, Ard Biesheuvel wrote:
>> > > On Fri, 16 Jan 2026 at 14:11, Mimi Zohar <zohar@linux.ibm.com> wrote:
>> > > >
>> > > > On Fri, 2026-01-16 at 10:41 +0100, Ard Biesheuvel wrote:
>> > > > > On Thu, 15 Jan 2026 at 01:43, Coiby Xu <coxu@redhat.com> wrote:
>> > > > > >
>> > > > > > EVM and other LSMs need the ability to query the secure boot status of
>> > > > > > the system, without directly calling the IMA arch_ima_get_secureboot
>> > > > > > function. Refactor the secure boot status check into a general,
>> > > > > > integrity-wide function named arch_integrity_get_secureboot.
>> > > > > >
>> > > > > > Define a new Kconfig option CONFIG_INTEGRITY_SECURE_BOOT, which is
>> > > > > > automatically configured by the supported architectures. The existing
>> > > > > > IMA_SECURE_AND_OR_TRUSTED_BOOT Kconfig loads the architecture specific
>> > > > > > IMA policy based on the refactored secure boot status code.
>> > > > > >
>> > > > > > Reported-and-suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> > > > > > Suggested-by: Roberto Sassu <roberto.sassu@huaweicloud.com>
>> > > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > > > > > ---
>> > > > > > arch/arm64/Kconfig | 1 +
>> > > > > > arch/powerpc/Kconfig | 1 +
>> > > > > > arch/powerpc/kernel/Makefile | 2 +-
>> > > > > > arch/powerpc/kernel/ima_arch.c | 5 --
>> > > > > > arch/powerpc/kernel/integrity_sb_arch.c | 13 +++++
>> > > > > > arch/s390/Kconfig | 1 +
>> > > > > > arch/s390/kernel/Makefile | 1 +
>> > > > > > arch/s390/kernel/ima_arch.c | 6 --
>> > > > > > arch/s390/kernel/integrity_sb_arch.c | 9 +++
>> > > > > > arch/x86/Kconfig | 1 +
>> > > > > > arch/x86/include/asm/efi.h | 4 +-
>> > > > > > arch/x86/platform/efi/efi.c | 2 +-
>> > > > > > include/linux/ima.h | 7 +--
>> > > > > > include/linux/integrity.h | 8 +++
>> > > > > > security/integrity/Kconfig | 6 ++
>> > > > > > security/integrity/Makefile | 3 +
>> > > > > > security/integrity/efi_secureboot.c | 56 +++++++++++++++++++
>> > > > > > security/integrity/ima/ima_appraise.c | 2 +-
>> > > > > > security/integrity/ima/ima_efi.c | 47 +---------------
>> > > > > > security/integrity/ima/ima_main.c | 4 +-
>> > > > > > security/integrity/platform_certs/load_uefi.c | 2 +-
>> > > > > > 21 files changed, 111 insertions(+), 70 deletions(-)
>> > > > > > create mode 100644 arch/powerpc/kernel/integrity_sb_arch.c
>> > > > > > create mode 100644 arch/s390/kernel/integrity_sb_arch.c
>> > > > > > create mode 100644 security/integrity/efi_secureboot.c
>> > > > > >
>> > > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> > > > > > index 93173f0a09c7..4c265b7386bb 100644
>> > > > > > --- a/arch/arm64/Kconfig
>> > > > > > +++ b/arch/arm64/Kconfig
>> > > > > > @@ -2427,6 +2427,7 @@ config EFI
>> > > > > > select EFI_STUB
>> > > > > > select EFI_GENERIC_STUB
>> > > > > > imply IMA_SECURE_AND_OR_TRUSTED_BOOT
>> > > > > > + imply INTEGRITY_SECURE_BOOT
>> > > > >
>> > > > > This allows both to be en/disabled individually, which I don't think
>> > > > > is what we want. It also results in more churn across the
>> > > > > arch-specific Kconfigs than needed.
>> > > > >
>> > > > > Wouldn't it be better if IMA_SECURE_AND_OR_TRUSTED_BOOT 'select'ed
>> > > > > INTEGRITY_SECURE_BOOT in its Kconfig definition?
>> > > >
>> > > > As much as possible, EVM (and other LSMs) shouldn't be dependent on another LSM,
>> > > > in this case IMA, being configured.
>> > >
>> > > Sure, but that is not my point.
>> > >
>> > > This arrangement allows for IMA_SECURE_AND_OR_TRUSTED_BOOT to be
>> > > enabled without INTEGRITY_SECURE_BOOT, resulting in the stub
>> > > implementation of arch_integrity_get_secureboot() being used, which
>> > > always returns false.
Since both INTEGRITY_SECURE_BOOT and IMA_SECURE_AND_OR_TRUSTED_BOOT
don't define a prompt, they are not user-configurable and will always be
enable/disabled together with arch-specific secure boot feature. So
despite the "imply" key word, the case where
IMA_SECURE_AND_OR_TRUSTED_BOOT is enabled whereas INTEGRITY_SECURE_BOOT
is disabled won't happen.
But I agree an arch may not care much about INTEGRITY_SECURE_BOOT so it
may be a churn. So limiting it to the scope of the integrity subsystem
can be a better idea.
>> >
>> > I understand your concern, but instead of "select"ing INTEGRITY_SECURE_BOOT from
>> > IMA_SECURE_AND_OR_TRUSTED_BOOT, how making IMA_SECURE_AND_OR_TRUSTED_BOOT
>> > dependent on both IMA_ARCH_POLICY and INTEGRITY_SECURE_BOOT.
>> >
>>
>> Given that INTEGRITY_SECURE_BOOT has no dependencies of its own,
>> afaict, selecting it is the least disruptive option, as otherwise,
>> existing configs will disable IMA_SECURE_AND_OR_TRUSTED_BOOT as the
>> kernel is being upgraded. But conceptually, I agree that they are
>> equivalent.
As already pointed out on by Mimi, INTEGRITY_SECURE_BOOT depend on
arch-specific secure boot feature. So we can't say INTEGRITY_SECURE_BOOT
has no dependencies.
>>
>> > Including the "imply INTEGRITY_SECURE_BOOT" here in the arch Kconfig allows EVM
>> > to query the secure boot state without relying on IMA_SECURE_AND_OR_TRUSTED_BOOT
>> > being configured.
>>
>> Yes, I understand that this is the whole point of the exercise. But
>> 'imply' should be used with care, and in this case, implying both from
>> CONFIG_EFI really makes little sense. INTEGRITY_SECURE_BOOT should be
>> selected by options that need the functionality, not 'implied' by
>> options that might provide it.
But again I agree INTEGRITY_SECURE_BOOT should "not 'implied' by options
that might provide it".
>
>As not all arch's implement arch_integrity_get_secureboot, the definition in
>include/linux/integrity.h would need to be updated. Something like:
>
>-#ifdef CONFIG_INTEGRITY_SECURE_BOOT
>+#if (defined(CONFIG_INTEGRITY_SECURE_BOOT) && \
>+ (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
>+ || defined(CONFIG_PPC_SECURE_BOOT))
>
>Then IMA_SECURE_AND_OR_TRUSTED_BOOT and EVM could select INTEGRITY_SECURE_BOOT,
>as suggested.
Since INTEGRITY_SECURE_BOOT has a dependency, select doesn't seem to be
a good choice. If EVM does select INTEGRITY_SECURE_BOOT,
INTEGRITY_SECURE_BOOT will be enabled even if arch-specific secure boot
feature is disabled and this can lead to a building failure. How about
always enabling INTEGRITY_SECURE_BOOT when secure boot feature is
enabled and also making IMA_SECURE_AND_OR_TRUSTED_BOOT depend on
INTEGRITY_SECURE_BOOT?
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 916d4f2bfc44..cd44b46d0325 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -97,6 +97,13 @@ config INTEGRITY_CA_MACHINE_KEYRING_MAX
will not be loaded. The remaining MOK keys are loaded into the
.platform keyring.
+config INTEGRITY_SECURE_BOOT
+ def_bool y
+ depends on EFI || PPC_SECURE_BOOT || S390
+ help
+ Provide secure boot related helper functions like querying the
+ secure boot status.
+
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 976e75f9b9ba..5dce572192d6 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -311,6 +311,7 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
config IMA_SECURE_AND_OR_TRUSTED_BOOT
bool
depends on IMA_ARCH_POLICY
+ depends on INTEGRITY_SECURE_BOOT
Another idea is make a tree-wide arch_get_secureboot i.e. to move
current arch_ima_get_secureboot code to arch-specific secure boot
implementation. By this way, there will no need for a new Kconfig option
INTEGRITY_SECURE_BOOT. But I'm not sure if there is any unforeseen
concern.
--
Best regards,
Coiby
^ permalink raw reply related
* Re: [PATCH 2/3] evm: Don't enable fix mode when secure boot is enabled
From: Coiby Xu @ 2026-01-19 4:10 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, linux-integrity, Heiko Carstens, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <f38b2512d51351f83c51b6e2b5dec11eb7e6959d.camel@huaweicloud.com>
On Fri, Jan 16, 2026 at 01:06:32PM +0100, Roberto Sassu wrote:
>On Thu, 2026-01-15 at 13:15 -0500, Mimi Zohar wrote:
>> On Thu, 2026-01-15 at 08:43 +0800, Coiby Xu wrote:
>> > Similar to IMA fix mode, forbid EVM fix mode when secure boot is
>> > enabled.
>> >
>> > Reported-and-suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> > Suggested-by: Roberto Sassu <roberto.sassu@huaweicloud.com>
>
>Ah, if possible, could you please change the email to
>roberto.sassu@huawei.com?
Thanks for the reminder! I'll use the above email.
--
Best regards,
Coiby
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Dave Hansen @ 2026-01-19 18:44 UTC (permalink / raw)
To: Mimi Zohar, Ard Biesheuvel
Cc: Coiby Xu, linux-integrity, Heiko Carstens, Roberto Sassu,
Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <ac5e5e45c12e9b0bda19807e60b06057d74be0b3.camel@linux.ibm.com>
On 1/18/26 10:25, Mimi Zohar wrote:
> As not all arch's implement arch_integrity_get_secureboot, the definition in
> include/linux/integrity.h would need to be updated. Something like:
>
> -#ifdef CONFIG_INTEGRITY_SECURE_BOOT
> +#if (defined(CONFIG_INTEGRITY_SECURE_BOOT) && \
> + (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
> + || defined(CONFIG_PPC_SECURE_BOOT))
>
> Then IMA_SECURE_AND_OR_TRUSTED_BOOT and EVM could select INTEGRITY_SECURE_BOOT,
> as suggested.
This seems to be going a wee bit sideways. :)
This kind of CONFIG complexity really should be left to Kconfig. C
macros really aren't a great place to do it.
The other idiom we use a lot is this in generic code:
#ifndef arch_foo
static inline void arch_foo(void) {}
#endif
Then all you have to do is make sure the arch header that #defines it is
included before the generic code. I'm not a super huge fan of these
because it can be hard to tell (for humans at least) _if_ the
architecture has done the #define.
But it sure beats that #ifdef maze.
^ permalink raw reply
* Re: [PATCH v4 5/6] keys/trusted_keys: establish PKWM as a trusted source
From: Jarkko Sakkinen @ 2026-01-19 23:14 UTC (permalink / raw)
To: Srish Srinivasan
Cc: linux-integrity, keyrings, linuxppc-dev, maddy, mpe, npiggin,
christophe.leroy, James.Bottomley, zohar, nayna, rnsastry,
linux-kernel, linux-security-module
In-Reply-To: <20260115100504.488665-6-ssrish@linux.ibm.com>
On Thu, Jan 15, 2026 at 03:35:03PM +0530, Srish Srinivasan wrote:
> The wrapping key does not exist by default and is generated by the
> hypervisor as a part of PKWM initialization. This key is then persisted by
> the hypervisor and is used to wrap trusted keys. These are variable length
> symmetric keys, which in the case of PowerVM Key Wrapping Module (PKWM) are
> generated using the kernel RNG. PKWM can be used as a trust source through
> the following example keyctl commands:
>
> keyctl add trusted my_trusted_key "new 32" @u
>
> Use the wrap_flags command option to set the secure boot requirement for
> the wrapping request through the following keyctl commands
>
> case1: no secure boot requirement. (default)
> keyctl usage: keyctl add trusted my_trusted_key "new 32" @u
> OR
> keyctl add trusted my_trusted_key "new 32 wrap_flags=0x00" @u
>
> case2: secure boot required to in either audit or enforce mode. set bit 0
> keyctl usage: keyctl add trusted my_trusted_key "new 32 wrap_flags=0x01" @u
>
> case3: secure boot required to be in enforce mode. set bit 1
> keyctl usage: keyctl add trusted my_trusted_key "new 32 wrap_flags=0x02" @u
>
> NOTE:
> -> Setting the secure boot requirement is NOT a must.
> -> Only either of the secure boot requirement options should be set. Not
> both.
> -> All the other bits are required to be not set.
> -> Set the kernel parameter trusted.source=pkwm to choose PKWM as the
> backend for trusted keys implementation.
> -> CONFIG_PSERIES_PLPKS must be enabled to build PKWM.
>
> Add PKWM, which is a combination of IBM PowerVM and Power LPAR Platform
> KeyStore, as a new trust source for trusted keys.
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> This version introduces a private pointer for backend specific fields and
> related changes specific to the PKWM backend, but defers migrating the TPM
> fields to this new framework. That will be done independently of this
> patch series.
> MAINTAINERS | 9 +
> include/keys/trusted-type.h | 7 +-
> include/keys/trusted_pkwm.h | 33 ++++
> security/keys/trusted-keys/Kconfig | 8 +
> security/keys/trusted-keys/Makefile | 2 +
> security/keys/trusted-keys/trusted_core.c | 6 +-
> security/keys/trusted-keys/trusted_pkwm.c | 190 ++++++++++++++++++++++
> 7 files changed, 253 insertions(+), 2 deletions(-)
> create mode 100644 include/keys/trusted_pkwm.h
> create mode 100644 security/keys/trusted-keys/trusted_pkwm.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cf755238c429..c98f1811f836 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14008,6 +14008,15 @@ S: Supported
> F: include/keys/trusted_dcp.h
> F: security/keys/trusted-keys/trusted_dcp.c
>
> +KEYS-TRUSTED-PLPKS
> +M: Srish Srinivasan <ssrish@linux.ibm.com>
> +M: Nayna Jain <nayna@linux.ibm.com>
> +L: linux-integrity@vger.kernel.org
> +L: keyrings@vger.kernel.org
> +S: Supported
> +F: include/keys/trusted_pkwm.h
> +F: security/keys/trusted-keys/trusted_pkwm.c
> +
> KEYS-TRUSTED-TEE
> M: Sumit Garg <sumit.garg@kernel.org>
> L: linux-integrity@vger.kernel.org
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 4eb64548a74f..03527162613f 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -19,7 +19,11 @@
>
> #define MIN_KEY_SIZE 32
> #define MAX_KEY_SIZE 128
> -#define MAX_BLOB_SIZE 512
> +#if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
> +#define MAX_BLOB_SIZE 1152
> +#else
> +#define MAX_BLOB_SIZE 512
> +#endif
> #define MAX_PCRINFO_SIZE 64
> #define MAX_DIGEST_SIZE 64
>
> @@ -46,6 +50,7 @@ struct trusted_key_options {
> uint32_t policydigest_len;
> unsigned char policydigest[MAX_DIGEST_SIZE];
> uint32_t policyhandle;
> + void *private;
> };
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> struct trusted_key_ops {
> diff --git a/include/keys/trusted_pkwm.h b/include/keys/trusted_pkwm.h
> new file mode 100644
> index 000000000000..4035b9776394
> --- /dev/null
> +++ b/include/keys/trusted_pkwm.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PKWM_TRUSTED_KEY_H
> +#define __PKWM_TRUSTED_KEY_H
> +
> +#include <keys/trusted-type.h>
> +#include <linux/bitops.h>
> +#include <linux/printk.h>
> +
> +extern struct trusted_key_ops pkwm_trusted_key_ops;
> +
> +struct trusted_pkwm_options {
> + u16 wrap_flags;
> +};
> +
> +static inline void dump_options(struct trusted_key_options *o)
> +{
> + const struct trusted_pkwm_options *pkwm;
> + bool sb_audit_or_enforce_bit;
> + bool sb_enforce_bit;
> +
> + pkwm = o->private;
> + sb_audit_or_enforce_bit = pkwm->wrap_flags & BIT(0);
> + sb_enforce_bit = pkwm->wrap_flags & BIT(1);
> +
> + if (sb_audit_or_enforce_bit)
> + pr_debug("secure boot mode required: audit or enforce");
> + else if (sb_enforce_bit)
> + pr_debug("secure boot mode required: enforce");
> + else
> + pr_debug("secure boot mode required: disabled");
> +}
> +
> +#endif
> diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
> index 204a68c1429d..9e00482d886a 100644
> --- a/security/keys/trusted-keys/Kconfig
> +++ b/security/keys/trusted-keys/Kconfig
> @@ -46,6 +46,14 @@ config TRUSTED_KEYS_DCP
> help
> Enable use of NXP's DCP (Data Co-Processor) as trusted key backend.
>
> +config TRUSTED_KEYS_PKWM
> + bool "PKWM-based trusted keys"
> + depends on PSERIES_PLPKS >= TRUSTED_KEYS
> + default y
> + select HAVE_TRUSTED_KEYS
> + help
> + Enable use of IBM PowerVM Key Wrapping Module (PKWM) as a trusted key backend.
> +
> if !HAVE_TRUSTED_KEYS
> comment "No trust source selected!"
> endif
> diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> index f0f3b27f688b..5fc053a21dad 100644
> --- a/security/keys/trusted-keys/Makefile
> +++ b/security/keys/trusted-keys/Makefile
> @@ -16,3 +16,5 @@ trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
> trusted-$(CONFIG_TRUSTED_KEYS_CAAM) += trusted_caam.o
>
> trusted-$(CONFIG_TRUSTED_KEYS_DCP) += trusted_dcp.o
> +
> +trusted-$(CONFIG_TRUSTED_KEYS_PKWM) += trusted_pkwm.o
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index b1680ee53f86..2d328de170e8 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -12,6 +12,7 @@
> #include <keys/trusted_caam.h>
> #include <keys/trusted_dcp.h>
> #include <keys/trusted_tpm.h>
> +#include <keys/trusted_pkwm.h>
> #include <linux/capability.h>
> #include <linux/err.h>
> #include <linux/init.h>
> @@ -31,7 +32,7 @@ MODULE_PARM_DESC(rng, "Select trusted key RNG");
>
> static char *trusted_key_source;
> module_param_named(source, trusted_key_source, charp, 0);
> -MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam or dcp)");
> +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam, dcp or pkwm)");
>
> static const struct trusted_key_source trusted_key_sources[] = {
> #if defined(CONFIG_TRUSTED_KEYS_TPM)
> @@ -46,6 +47,9 @@ static const struct trusted_key_source trusted_key_sources[] = {
> #if defined(CONFIG_TRUSTED_KEYS_DCP)
> { "dcp", &dcp_trusted_key_ops },
> #endif
> +#if defined(CONFIG_TRUSTED_KEYS_PKWM)
> + { "pkwm", &pkwm_trusted_key_ops },
> +#endif
> };
>
> DEFINE_STATIC_CALL_NULL(trusted_key_seal, *trusted_key_sources[0].ops->seal);
> diff --git a/security/keys/trusted-keys/trusted_pkwm.c b/security/keys/trusted-keys/trusted_pkwm.c
> new file mode 100644
> index 000000000000..4f391b77a907
> --- /dev/null
> +++ b/security/keys/trusted-keys/trusted_pkwm.c
> @@ -0,0 +1,190 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
> + */
> +
> +#include <keys/trusted_pkwm.h>
> +#include <keys/trusted-type.h>
> +#include <linux/build_bug.h>
> +#include <linux/key-type.h>
> +#include <linux/parser.h>
> +#include <asm/plpks.h>
> +
> +enum {
> + Opt_err,
> + Opt_wrap_flags,
> +};
> +
> +static const match_table_t key_tokens = {
> + {Opt_wrap_flags, "wrap_flags=%s"},
> + {Opt_err, NULL}
> +};
> +
> +static int getoptions(char *datablob, struct trusted_key_options *opt)
> +{
> + substring_t args[MAX_OPT_ARGS];
> + char *p = datablob;
> + int token;
> + int res;
> + u16 wrap_flags;
> + unsigned long token_mask = 0;
> + struct trusted_pkwm_options *pkwm;
> +
> + if (!datablob)
> + return 0;
> +
> + pkwm = opt->private;
> +
> + while ((p = strsep(&datablob, " \t"))) {
> + if (*p == '\0' || *p == ' ' || *p == '\t')
> + continue;
> +
> + token = match_token(p, key_tokens, args);
> + if (test_and_set_bit(token, &token_mask))
> + return -EINVAL;
> +
> + switch (token) {
> + case Opt_wrap_flags:
> + res = kstrtou16(args[0].from, 16, &wrap_flags);
> + if (res < 0 || wrap_flags > 2)
> + return -EINVAL;
> + pkwm->wrap_flags = wrap_flags;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> + return 0;
> +}
> +
> +static struct trusted_key_options *trusted_options_alloc(void)
> +{
> + struct trusted_key_options *options;
> + struct trusted_pkwm_options *pkwm;
> +
> + options = kzalloc(sizeof(*options), GFP_KERNEL);
> +
> + if (options) {
> + pkwm = kzalloc(sizeof(*pkwm), GFP_KERNEL);
> +
> + if (!pkwm) {
> + kfree_sensitive(options);
> + options = NULL;
> + } else {
> + options->private = pkwm;
> + }
> + }
> +
> + return options;
> +}
> +
> +static int trusted_pkwm_seal(struct trusted_key_payload *p, char *datablob)
> +{
> + struct trusted_key_options *options = NULL;
> + struct trusted_pkwm_options *pkwm = NULL;
> + u8 *input_buf, *output_buf;
> + u32 output_len, input_len;
> + int rc;
> +
> + options = trusted_options_alloc();
> +
> + if (!options)
> + return -ENOMEM;
> +
> + rc = getoptions(datablob, options);
> + if (rc < 0)
> + goto out;
> + dump_options(options);
> +
> + input_len = p->key_len;
> + input_buf = kmalloc(ALIGN(input_len, 4096), GFP_KERNEL);
> + if (!input_buf) {
> + pr_err("Input buffer allocation failed. Returning -ENOMEM.");
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + memcpy(input_buf, p->key, p->key_len);
> +
> + pkwm = options->private;
> +
> + rc = plpks_wrap_object(&input_buf, input_len, pkwm->wrap_flags,
> + &output_buf, &output_len);
> + if (!rc) {
> + memcpy(p->blob, output_buf, output_len);
> + p->blob_len = output_len;
> + dump_payload(p);
> + } else {
> + pr_err("Wrapping of payload key failed: %d\n", rc);
> + }
> +
> + kfree(input_buf);
> + kfree(output_buf);
> +
> +out:
> + kfree_sensitive(options->private);
> + kfree_sensitive(options);
> + return rc;
> +}
> +
> +static int trusted_pkwm_unseal(struct trusted_key_payload *p, char *datablob)
> +{
> + u8 *input_buf, *output_buf;
> + u32 input_len, output_len;
> + int rc;
> +
> + input_len = p->blob_len;
> + input_buf = kmalloc(ALIGN(input_len, 4096), GFP_KERNEL);
> + if (!input_buf) {
> + pr_err("Input buffer allocation failed. Returning -ENOMEM.");
> + return -ENOMEM;
> + }
> +
> + memcpy(input_buf, p->blob, p->blob_len);
> +
> + rc = plpks_unwrap_object(&input_buf, input_len, &output_buf,
> + &output_len);
> + if (!rc) {
> + memcpy(p->key, output_buf, output_len);
> + p->key_len = output_len;
> + dump_payload(p);
> + } else {
> + pr_err("Unwrapping of payload failed: %d\n", rc);
> + }
> +
> + kfree(input_buf);
> + kfree(output_buf);
> +
> + return rc;
> +}
> +
> +static int trusted_pkwm_init(void)
> +{
> + int ret;
> +
> + if (!plpks_wrapping_is_supported()) {
> + pr_err("H_PKS_WRAP_OBJECT interface not supported\n");
> + return -ENODEV;
> + }
> +
> + ret = plpks_gen_wrapping_key();
> + if (ret) {
> + pr_err("Failed to generate default wrapping key\n");
> + return -EINVAL;
> + }
> +
> + return register_key_type(&key_type_trusted);
> +}
> +
> +static void trusted_pkwm_exit(void)
> +{
> + unregister_key_type(&key_type_trusted);
> +}
> +
> +struct trusted_key_ops pkwm_trusted_key_ops = {
> + .migratable = 0, /* non-migratable */
> + .init = trusted_pkwm_init,
> + .seal = trusted_pkwm_seal,
> + .unseal = trusted_pkwm_unseal,
> + .exit = trusted_pkwm_exit,
> +};
> --
> 2.47.3
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v15 01/28] tpm: Initial step to reorganize TPM public headers
From: Jarkko Sakkinen @ 2026-01-19 23:40 UTC (permalink / raw)
To: Ross Philipson
Cc: linux-kernel, x86, linux-integrity, linux-doc, linux-crypto,
kexec, linux-efi, iommu, dpsmith, tglx, mingo, bp, hpa,
dave.hansen, ardb, mjg59, James.Bottomley, peterhuewe, jgg, luto,
nivedita, herbert, davem, corbet, ebiederm, dwmw2, baolu.lu,
kanth.ghatraju, andrew.cooper3, trenchboot-devel
In-Reply-To: <20251215233316.1076248-2-ross.philipson@oracle.com>
On Mon, Dec 15, 2025 at 03:32:49PM -0800, Ross Philipson wrote:
> Replace the existing public header tpm_command.h with the first two
> new public headers tpm1.h and tpm_common.h. In addition, related
> definitions in tpm1_cmd.c were moved to the new tpm1.h.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> ---
> drivers/char/tpm/tpm-buf.c | 3 +-
> drivers/char/tpm/tpm1-cmd.c | 13 +-----
> include/keys/trusted_tpm.h | 1 -
> include/linux/tpm.h | 3 ++
> include/linux/tpm1.h | 55 +++++++++++++++++++++++
> include/linux/tpm_command.h | 30 -------------
Removing tpm_command.h causes unnecessary noise.
It would be better to retain tpm_command.h, and simply supplement
it with TPM2 constants.
Also, what is the reason to not have both TPM1 and TPM2 in tpm.h?
To put the question in other words: is there something in tpm.h that
would be incompatible with early boot code?
I'd rather tweak that than have more files...
> include/linux/tpm_common.h | 22 +++++++++
> security/keys/trusted-keys/trusted_tpm1.c | 1 -
> security/keys/trusted-keys/trusted_tpm2.c | 1 -
> 9 files changed, 82 insertions(+), 47 deletions(-)
> create mode 100644 include/linux/tpm1.h
> delete mode 100644 include/linux/tpm_command.h
> create mode 100644 include/linux/tpm_common.h
>
> diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> index 1cb649938c01..dae23e6de269 100644
> --- a/drivers/char/tpm/tpm-buf.c
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -3,7 +3,6 @@
> * Handling of TPM command and other buffers.
> */
>
> -#include <linux/tpm_command.h>
> #include <linux/module.h>
> #include <linux/tpm.h>
>
> @@ -296,7 +295,7 @@ void tpm1_buf_append_extend(struct tpm_buf *buf, u32 pcr_idx, const u8 *hash)
> if (buf->flags & TPM_BUF_INVALID)
> return;
>
> - if (!tpm1_buf_is_command(buf, TPM_ORD_EXTEND)) {
> + if (!tpm1_buf_is_command(buf, TPM_ORD_PCR_EXTEND)) {
> WARN(1, "tpm_buf: invalid TPM_Extend command\n");
> buf->flags |= TPM_BUF_INVALID;
> return;
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index bc156d7d59f2..f29827b454d2 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -18,12 +18,9 @@
> #include <linux/mutex.h>
> #include <linux/spinlock.h>
> #include <linux/freezer.h>
> -#include <linux/tpm_command.h>
> #include <linux/tpm_eventlog.h>
> #include "tpm.h"
>
> -#define TPM_MAX_ORDINAL 243
> -
> /*
> * Array with one entry per ordinal defining the maximum amount
> * of time the chip could take to return the result. The ordinal
> @@ -308,9 +305,6 @@ unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
> return duration;
> }
>
> -#define TPM_ORD_STARTUP 153
> -#define TPM_ST_CLEAR 1
> -
> /**
> * tpm1_startup() - turn on the TPM
> * @chip: TPM chip to use
> @@ -478,7 +472,6 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
> return rc;
> }
>
> -#define TPM_ORD_GET_CAP 101
> ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> const char *desc, size_t min_cap_length)
> {
> @@ -574,7 +567,6 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> return rc;
> }
>
> -#define TPM_ORD_PCRREAD 21
> int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> {
> int rc;
> @@ -584,7 +576,7 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> return -ENOMEM;
>
> tpm_buf_init(buf, TPM_BUFSIZE);
> - tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD);
> + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_READ);
> tpm_buf_append_u32(buf, pcr_idx);
>
> rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE,
> @@ -599,7 +591,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> return rc;
> }
>
> -#define TPM_ORD_CONTINUE_SELFTEST 83
> /**
> * tpm1_continue_selftest() - run TPM's selftest
> * @chip: TPM chip to use
> @@ -716,8 +707,6 @@ int tpm1_auto_startup(struct tpm_chip *chip)
> return rc;
> }
>
> -#define TPM_ORD_SAVESTATE 152
> -
> /**
> * tpm1_pm_suspend() - pm suspend handler
> * @chip: TPM chip to use.
> diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> index 0fadc6a4f166..3a0fa3bc8454 100644
> --- a/include/keys/trusted_tpm.h
> +++ b/include/keys/trusted_tpm.h
> @@ -3,7 +3,6 @@
> #define __TRUSTED_TPM_H
>
> #include <keys/trusted-type.h>
> -#include <linux/tpm_command.h>
>
> extern struct trusted_key_ops trusted_key_tpm_ops;
>
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 8da49e8769d5..ef81e0b59657 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -25,6 +25,9 @@
> #include <crypto/hash_info.h>
> #include <crypto/aes.h>
>
> +#include "tpm_common.h"
> +#include "tpm1.h"
> +
> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> #define TPM_HEADER_SIZE 10
> #define TPM_BUFSIZE 4096
> diff --git a/include/linux/tpm1.h b/include/linux/tpm1.h
> new file mode 100644
> index 000000000000..54c6c211eb9e
> --- /dev/null
> +++ b/include/linux/tpm1.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2004,2007,2008 IBM Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + * Debora Velarde <dvelarde@us.ibm.com>
> + *
> + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org
> + */
> +#ifndef __LINUX_TPM1_H__
> +#define __LINUX_TPM1_H__
> +
> +/*
> + * TPM 1.2 Main Specification
> + * https://trustedcomputinggroup.org/resource/tpm-main-specification/
> + */
> +
> +/* Command TAGS */
> +enum tpm_command_tags {
> + TPM_TAG_RQU_COMMAND = 193,
> + TPM_TAG_RQU_AUTH1_COMMAND = 194,
> + TPM_TAG_RQU_AUTH2_COMMAND = 195,
> + TPM_TAG_RSP_COMMAND = 196,
> + TPM_TAG_RSP_AUTH1_COMMAND = 197,
> + TPM_TAG_RSP_AUTH2_COMMAND = 198,
> +};
> +
> +/* Command Ordinals */
> +enum tpm_command_ordinals {
> + TPM_ORD_CONTINUE_SELFTEST = 83,
> + TPM_ORD_GET_CAP = 101,
> + TPM_ORD_GET_RANDOM = 70,
> + TPM_ORD_PCR_EXTEND = 20,
> + TPM_ORD_PCR_READ = 21,
> + TPM_ORD_OSAP = 11,
> + TPM_ORD_OIAP = 10,
> + TPM_ORD_SAVESTATE = 152,
> + TPM_ORD_SEAL = 23,
> + TPM_ORD_STARTUP = 153,
> + TPM_ORD_UNSEAL = 24,
> +};
> +
> +/* Other constants */
> +#define SRKHANDLE 0x40000000
> +#define TPM_NONCE_SIZE 20
> +#define TPM_ST_CLEAR 1
> +
> +#endif
> diff --git a/include/linux/tpm_command.h b/include/linux/tpm_command.h
> deleted file mode 100644
> index 02038972a05f..000000000000
> --- a/include/linux/tpm_command.h
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __LINUX_TPM_COMMAND_H__
> -#define __LINUX_TPM_COMMAND_H__
> -
> -/*
> - * TPM Command constants from specifications at
> - * http://www.trustedcomputinggroup.org
> - */
> -
> -/* Command TAGS */
> -#define TPM_TAG_RQU_COMMAND 193
> -#define TPM_TAG_RQU_AUTH1_COMMAND 194
> -#define TPM_TAG_RQU_AUTH2_COMMAND 195
> -#define TPM_TAG_RSP_COMMAND 196
> -#define TPM_TAG_RSP_AUTH1_COMMAND 197
> -#define TPM_TAG_RSP_AUTH2_COMMAND 198
> -
> -/* Command Ordinals */
> -#define TPM_ORD_OIAP 10
> -#define TPM_ORD_OSAP 11
> -#define TPM_ORD_EXTEND 20
> -#define TPM_ORD_SEAL 23
> -#define TPM_ORD_UNSEAL 24
> -#define TPM_ORD_GET_RANDOM 70
> -
> -/* Other constants */
> -#define SRKHANDLE 0x40000000
> -#define TPM_NONCE_SIZE 20
> -
> -#endif
> diff --git a/include/linux/tpm_common.h b/include/linux/tpm_common.h
> new file mode 100644
> index 000000000000..b8be669913dd
> --- /dev/null
> +++ b/include/linux/tpm_common.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2004,2007,2008 IBM Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + * Debora Velarde <dvelarde@us.ibm.com>
> + *
> + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org
> + */
> +#ifndef __LINUX_TPM_COMMON_H__
> +#define __LINUX_TPM_COMMON_H__
> +
> +#define TPM_MAX_ORDINAL 243
> +
> +#endif
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 6e6a9fb48e63..3717a06a5212 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -17,7 +17,6 @@
> #include <keys/trusted-type.h>
> #include <linux/key-type.h>
> #include <linux/tpm.h>
> -#include <linux/tpm_command.h>
>
> #include <keys/trusted_tpm.h>
>
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 0a99bd051a25..e6000c71eeb6 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -9,7 +9,6 @@
> #include <linux/string.h>
> #include <linux/err.h>
> #include <linux/tpm.h>
> -#include <linux/tpm_command.h>
>
> #include <keys/trusted-type.h>
> #include <keys/trusted_tpm.h>
> --
> 2.43.7
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v15 01/28] tpm: Initial step to reorganize TPM public headers
From: Jarkko Sakkinen @ 2026-01-19 23:52 UTC (permalink / raw)
To: Ross Philipson
Cc: linux-kernel, x86, linux-integrity, linux-doc, linux-crypto,
kexec, linux-efi, iommu, dpsmith, tglx, mingo, bp, hpa,
dave.hansen, ardb, mjg59, James.Bottomley, peterhuewe, jgg, luto,
nivedita, herbert, davem, corbet, ebiederm, dwmw2, baolu.lu,
kanth.ghatraju, andrew.cooper3, trenchboot-devel
In-Reply-To: <aW7A-4xJSzln1HtH@kernel.org>
On Tue, Jan 20, 2026 at 01:40:49AM +0200, Jarkko Sakkinen wrote:
> On Mon, Dec 15, 2025 at 03:32:49PM -0800, Ross Philipson wrote:
> > Replace the existing public header tpm_command.h with the first two
> > new public headers tpm1.h and tpm_common.h. In addition, related
> > definitions in tpm1_cmd.c were moved to the new tpm1.h.
> >
> > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> > Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> > ---
> > drivers/char/tpm/tpm-buf.c | 3 +-
> > drivers/char/tpm/tpm1-cmd.c | 13 +-----
> > include/keys/trusted_tpm.h | 1 -
> > include/linux/tpm.h | 3 ++
> > include/linux/tpm1.h | 55 +++++++++++++++++++++++
> > include/linux/tpm_command.h | 30 -------------
>
> Removing tpm_command.h causes unnecessary noise.
>
> It would be better to retain tpm_command.h, and simply supplement
> it with TPM2 constants.
>
> Also, what is the reason to not have both TPM1 and TPM2 in tpm.h?
>
> To put the question in other words: is there something in tpm.h that
> would be incompatible with early boot code?
>
> I'd rather tweak that than have more files...
>
> > include/linux/tpm_common.h | 22 +++++++++
> > security/keys/trusted-keys/trusted_tpm1.c | 1 -
> > security/keys/trusted-keys/trusted_tpm2.c | 1 -
> > 9 files changed, 82 insertions(+), 47 deletions(-)
> > create mode 100644 include/linux/tpm1.h
> > delete mode 100644 include/linux/tpm_command.h
> > create mode 100644 include/linux/tpm_common.h
> >
> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> > index 1cb649938c01..dae23e6de269 100644
> > --- a/drivers/char/tpm/tpm-buf.c
> > +++ b/drivers/char/tpm/tpm-buf.c
> > @@ -3,7 +3,6 @@
> > * Handling of TPM command and other buffers.
> > */
> >
> > -#include <linux/tpm_command.h>
> > #include <linux/module.h>
> > #include <linux/tpm.h>
> >
> > @@ -296,7 +295,7 @@ void tpm1_buf_append_extend(struct tpm_buf *buf, u32 pcr_idx, const u8 *hash)
> > if (buf->flags & TPM_BUF_INVALID)
> > return;
> >
> > - if (!tpm1_buf_is_command(buf, TPM_ORD_EXTEND)) {
> > + if (!tpm1_buf_is_command(buf, TPM_ORD_PCR_EXTEND)) {
> > WARN(1, "tpm_buf: invalid TPM_Extend command\n");
> > buf->flags |= TPM_BUF_INVALID;
> > return;
> > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > index bc156d7d59f2..f29827b454d2 100644
> > --- a/drivers/char/tpm/tpm1-cmd.c
> > +++ b/drivers/char/tpm/tpm1-cmd.c
> > @@ -18,12 +18,9 @@
> > #include <linux/mutex.h>
> > #include <linux/spinlock.h>
> > #include <linux/freezer.h>
> > -#include <linux/tpm_command.h>
> > #include <linux/tpm_eventlog.h>
> > #include "tpm.h"
> >
> > -#define TPM_MAX_ORDINAL 243
> > -
> > /*
> > * Array with one entry per ordinal defining the maximum amount
> > * of time the chip could take to return the result. The ordinal
> > @@ -308,9 +305,6 @@ unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
> > return duration;
> > }
> >
> > -#define TPM_ORD_STARTUP 153
> > -#define TPM_ST_CLEAR 1
> > -
> > /**
> > * tpm1_startup() - turn on the TPM
> > * @chip: TPM chip to use
> > @@ -478,7 +472,6 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
> > return rc;
> > }
> >
> > -#define TPM_ORD_GET_CAP 101
> > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> > const char *desc, size_t min_cap_length)
> > {
> > @@ -574,7 +567,6 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> > return rc;
> > }
> >
> > -#define TPM_ORD_PCRREAD 21
> > int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> > {
> > int rc;
> > @@ -584,7 +576,7 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> > return -ENOMEM;
> >
> > tpm_buf_init(buf, TPM_BUFSIZE);
> > - tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD);
> > + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_READ);
> > tpm_buf_append_u32(buf, pcr_idx);
> >
> > rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE,
> > @@ -599,7 +591,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> > return rc;
> > }
> >
> > -#define TPM_ORD_CONTINUE_SELFTEST 83
> > /**
> > * tpm1_continue_selftest() - run TPM's selftest
> > * @chip: TPM chip to use
> > @@ -716,8 +707,6 @@ int tpm1_auto_startup(struct tpm_chip *chip)
> > return rc;
> > }
> >
> > -#define TPM_ORD_SAVESTATE 152
> > -
> > /**
> > * tpm1_pm_suspend() - pm suspend handler
> > * @chip: TPM chip to use.
> > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> > index 0fadc6a4f166..3a0fa3bc8454 100644
> > --- a/include/keys/trusted_tpm.h
> > +++ b/include/keys/trusted_tpm.h
> > @@ -3,7 +3,6 @@
> > #define __TRUSTED_TPM_H
> >
> > #include <keys/trusted-type.h>
> > -#include <linux/tpm_command.h>
> >
> > extern struct trusted_key_ops trusted_key_tpm_ops;
> >
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index 8da49e8769d5..ef81e0b59657 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -25,6 +25,9 @@
> > #include <crypto/hash_info.h>
> > #include <crypto/aes.h>
> >
> > +#include "tpm_common.h"
> > +#include "tpm1.h"
> > +
> > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> > #define TPM_HEADER_SIZE 10
> > #define TPM_BUFSIZE 4096
> > diff --git a/include/linux/tpm1.h b/include/linux/tpm1.h
> > new file mode 100644
> > index 000000000000..54c6c211eb9e
> > --- /dev/null
> > +++ b/include/linux/tpm1.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2004,2007,2008 IBM Corporation
> > + *
> > + * Authors:
> > + * Leendert van Doorn <leendert@watson.ibm.com>
> > + * Dave Safford <safford@watson.ibm.com>
> > + * Reiner Sailer <sailer@watson.ibm.com>
> > + * Kylene Hall <kjhall@us.ibm.com>
> > + * Debora Velarde <dvelarde@us.ibm.com>
> > + *
> > + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> > + *
> > + * Device driver for TCG/TCPA TPM (trusted platform module).
> > + * Specifications at www.trustedcomputinggroup.org
> > + */
> > +#ifndef __LINUX_TPM1_H__
> > +#define __LINUX_TPM1_H__
> > +
> > +/*
> > + * TPM 1.2 Main Specification
> > + * https://trustedcomputinggroup.org/resource/tpm-main-specification/
> > + */
> > +
> > +/* Command TAGS */
> > +enum tpm_command_tags {
> > + TPM_TAG_RQU_COMMAND = 193,
> > + TPM_TAG_RQU_AUTH1_COMMAND = 194,
> > + TPM_TAG_RQU_AUTH2_COMMAND = 195,
> > + TPM_TAG_RSP_COMMAND = 196,
> > + TPM_TAG_RSP_AUTH1_COMMAND = 197,
> > + TPM_TAG_RSP_AUTH2_COMMAND = 198,
> > +};
> > +
> > +/* Command Ordinals */
> > +enum tpm_command_ordinals {
> > + TPM_ORD_CONTINUE_SELFTEST = 83,
> > + TPM_ORD_GET_CAP = 101,
> > + TPM_ORD_GET_RANDOM = 70,
> > + TPM_ORD_PCR_EXTEND = 20,
> > + TPM_ORD_PCR_READ = 21,
> > + TPM_ORD_OSAP = 11,
> > + TPM_ORD_OIAP = 10,
> > + TPM_ORD_SAVESTATE = 152,
> > + TPM_ORD_SEAL = 23,
> > + TPM_ORD_STARTUP = 153,
> > + TPM_ORD_UNSEAL = 24,
> > +};
> > +
> > +/* Other constants */
> > +#define SRKHANDLE 0x40000000
> > +#define TPM_NONCE_SIZE 20
> > +#define TPM_ST_CLEAR 1
> > +
> > +#endif
> > diff --git a/include/linux/tpm_command.h b/include/linux/tpm_command.h
> > deleted file mode 100644
> > index 02038972a05f..000000000000
> > --- a/include/linux/tpm_command.h
> > +++ /dev/null
> > @@ -1,30 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0 */
> > -#ifndef __LINUX_TPM_COMMAND_H__
> > -#define __LINUX_TPM_COMMAND_H__
> > -
> > -/*
> > - * TPM Command constants from specifications at
> > - * http://www.trustedcomputinggroup.org
> > - */
> > -
> > -/* Command TAGS */
> > -#define TPM_TAG_RQU_COMMAND 193
> > -#define TPM_TAG_RQU_AUTH1_COMMAND 194
> > -#define TPM_TAG_RQU_AUTH2_COMMAND 195
> > -#define TPM_TAG_RSP_COMMAND 196
> > -#define TPM_TAG_RSP_AUTH1_COMMAND 197
> > -#define TPM_TAG_RSP_AUTH2_COMMAND 198
> > -
> > -/* Command Ordinals */
> > -#define TPM_ORD_OIAP 10
> > -#define TPM_ORD_OSAP 11
> > -#define TPM_ORD_EXTEND 20
> > -#define TPM_ORD_SEAL 23
> > -#define TPM_ORD_UNSEAL 24
> > -#define TPM_ORD_GET_RANDOM 70
> > -
> > -/* Other constants */
> > -#define SRKHANDLE 0x40000000
> > -#define TPM_NONCE_SIZE 20
> > -
> > -#endif
> > diff --git a/include/linux/tpm_common.h b/include/linux/tpm_common.h
> > new file mode 100644
> > index 000000000000..b8be669913dd
> > --- /dev/null
> > +++ b/include/linux/tpm_common.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2004,2007,2008 IBM Corporation
> > + *
> > + * Authors:
> > + * Leendert van Doorn <leendert@watson.ibm.com>
> > + * Dave Safford <safford@watson.ibm.com>
> > + * Reiner Sailer <sailer@watson.ibm.com>
> > + * Kylene Hall <kjhall@us.ibm.com>
> > + * Debora Velarde <dvelarde@us.ibm.com>
> > + *
> > + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> > + *
> > + * Device driver for TCG/TCPA TPM (trusted platform module).
> > + * Specifications at www.trustedcomputinggroup.org
> > + */
> > +#ifndef __LINUX_TPM_COMMON_H__
> > +#define __LINUX_TPM_COMMON_H__
> > +
> > +#define TPM_MAX_ORDINAL 243
> > +
> > +#endif
By retaining tpm_command.h, this file neither won't be necessary.
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index 6e6a9fb48e63..3717a06a5212 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -17,7 +17,6 @@
> > #include <keys/trusted-type.h>
> > #include <linux/key-type.h>
> > #include <linux/tpm.h>
> > -#include <linux/tpm_command.h>
> >
> > #include <keys/trusted_tpm.h>
> >
> > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> > index 0a99bd051a25..e6000c71eeb6 100644
> > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > @@ -9,7 +9,6 @@
> > #include <linux/string.h>
> > #include <linux/err.h>
> > #include <linux/tpm.h>
> > -#include <linux/tpm_command.h>
> >
> > #include <keys/trusted-type.h>
> > #include <keys/trusted_tpm.h>
> > --
> > 2.43.7
> >
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v15 02/28] tpm: Move TPM1 specific definitions and functions to new headers
From: Jarkko Sakkinen @ 2026-01-19 23:57 UTC (permalink / raw)
To: Ross Philipson
Cc: linux-kernel, x86, linux-integrity, linux-doc, linux-crypto,
kexec, linux-efi, iommu, dpsmith, tglx, mingo, bp, hpa,
dave.hansen, ardb, mjg59, James.Bottomley, peterhuewe, jgg, luto,
nivedita, herbert, davem, corbet, ebiederm, dwmw2, baolu.lu,
kanth.ghatraju, andrew.cooper3, trenchboot-devel
In-Reply-To: <20251215233316.1076248-3-ross.philipson@oracle.com>
On Mon, Dec 15, 2025 at 03:32:50PM -0800, Ross Philipson wrote:
> This gathers all the TPM1 definitions and structures into two separate
> header files (public tpm1.h and private tpm1_structs.h). The definitions
> moved to these files correspond to the TCG specification for TPM 1 family:
>
> TPM 1.2 Main Specification
> - https://trustedcomputinggroup.org/resource/tpm-main-specification/
>
> Note that the structures were pulled into tpm1_structs.h to allow their
> external reuse.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> ---
> drivers/char/tpm/tpm.h | 98 +--------------------------------
> drivers/char/tpm/tpm1-cmd.c | 5 --
> drivers/char/tpm/tpm1_structs.h | 97 ++++++++++++++++++++++++++++++++
I think you are overcomplicating the patch set and doing more
than you really need to do.
I.e. structs could go also just as well to tpm_command.h. We
will deal with if/when that file ever grows too large. It's
absolutely not a priority for this patch set.
> include/linux/tpm1.h | 34 +++++++++++-
> 4 files changed, 132 insertions(+), 102 deletions(-)
> create mode 100644 drivers/char/tpm/tpm1_structs.h
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index ca391b2a211c..1f9f8540eede 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -50,105 +50,9 @@ enum tpm_addr {
> TPM_ADDR = 0x4E,
> };
>
> -#define TPM_WARN_RETRY 0x800
> -#define TPM_WARN_DOING_SELFTEST 0x802
> -#define TPM_ERR_DEACTIVATED 0x6
> -#define TPM_ERR_DISABLED 0x7
> -#define TPM_ERR_FAILEDSELFTEST 0x1C
> -#define TPM_ERR_INVALID_POSTINIT 38
> -
> -#define TPM_TAG_RQU_COMMAND 193
> -
> /* TPM2 specific constants. */
> #define TPM2_SPACE_BUFFER_SIZE 16384 /* 16 kB */
>
> -struct stclear_flags_t {
> - __be16 tag;
> - u8 deactivated;
> - u8 disableForceClear;
> - u8 physicalPresence;
> - u8 physicalPresenceLock;
> - u8 bGlobalLock;
> -} __packed;
> -
> -struct tpm1_version {
> - u8 major;
> - u8 minor;
> - u8 rev_major;
> - u8 rev_minor;
> -} __packed;
> -
> -struct tpm1_version2 {
> - __be16 tag;
> - struct tpm1_version version;
> -} __packed;
> -
> -struct timeout_t {
> - __be32 a;
> - __be32 b;
> - __be32 c;
> - __be32 d;
> -} __packed;
> -
> -struct duration_t {
> - __be32 tpm_short;
> - __be32 tpm_medium;
> - __be32 tpm_long;
> -} __packed;
> -
> -struct permanent_flags_t {
> - __be16 tag;
> - u8 disable;
> - u8 ownership;
> - u8 deactivated;
> - u8 readPubek;
> - u8 disableOwnerClear;
> - u8 allowMaintenance;
> - u8 physicalPresenceLifetimeLock;
> - u8 physicalPresenceHWEnable;
> - u8 physicalPresenceCMDEnable;
> - u8 CEKPUsed;
> - u8 TPMpost;
> - u8 TPMpostLock;
> - u8 FIPS;
> - u8 operator;
> - u8 enableRevokeEK;
> - u8 nvLocked;
> - u8 readSRKPub;
> - u8 tpmEstablished;
> - u8 maintenanceDone;
> - u8 disableFullDALogicInfo;
> -} __packed;
> -
> -typedef union {
> - struct permanent_flags_t perm_flags;
> - struct stclear_flags_t stclear_flags;
> - __u8 owned;
> - __be32 num_pcrs;
> - struct tpm1_version version1;
> - struct tpm1_version2 version2;
> - __be32 manufacturer_id;
> - struct timeout_t timeout;
> - struct duration_t duration;
> -} cap_t;
> -
> -enum tpm_capabilities {
> - TPM_CAP_FLAG = 4,
> - TPM_CAP_PROP = 5,
> - TPM_CAP_VERSION_1_1 = 0x06,
> - TPM_CAP_VERSION_1_2 = 0x1A,
> -};
> -
> -enum tpm_sub_capabilities {
> - TPM_CAP_PROP_PCR = 0x101,
> - TPM_CAP_PROP_MANUFACTURER = 0x103,
> - TPM_CAP_FLAG_PERM = 0x108,
> - TPM_CAP_FLAG_VOL = 0x109,
> - TPM_CAP_PROP_OWNER = 0x111,
> - TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
> - TPM_CAP_PROP_TIS_DURATION = 0x120,
> -};
> -
> enum tpm2_pt_props {
> TPM2_PT_NONE = 0x00000000,
> TPM2_PT_GROUP = 0x00000100,
> @@ -229,6 +133,8 @@ enum tpm2_pt_props {
> * compiler warnings about stack frame size. */
> #define TPM_MAX_RNG_DATA 128
>
> +#include "tpm1_structs.h"
> +
> extern const struct class tpm_class;
> extern const struct class tpmrm_class;
> extern dev_t tpm_devt;
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index f29827b454d2..02f20a0aa37d 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -505,11 +505,6 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> }
> EXPORT_SYMBOL_GPL(tpm1_getcap);
>
> -struct tpm1_get_random_out {
> - __be32 rng_data_len;
> - u8 rng_data[TPM_MAX_RNG_DATA];
> -} __packed;
> -
> /**
> * tpm1_get_random() - get random bytes from the TPM's RNG
> * @chip: a &struct tpm_chip instance
> diff --git a/drivers/char/tpm/tpm1_structs.h b/drivers/char/tpm/tpm1_structs.h
> new file mode 100644
> index 000000000000..ad21376af5ab
> --- /dev/null
> +++ b/drivers/char/tpm/tpm1_structs.h
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + * Copyright (C) 2015 Intel Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + *
> + * Maintained by: <tpmdd-devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org
> + */
> +
> +#ifndef __TPM1_STRUCTS_H__
> +#define __TPM1_STRUCTS_H__
> +
> +struct stclear_flags_t {
> + __be16 tag;
> + u8 deactivated;
> + u8 disableForceClear;
> + u8 physicalPresence;
> + u8 physicalPresenceLock;
> + u8 bGlobalLock;
> +} __packed;
Don't retain alignment.
> +
> +struct tpm1_version {
> + u8 major;
> + u8 minor;
> + u8 rev_major;
> + u8 rev_minor;
> +} __packed;
> +
> +struct tpm1_version2 {
> + __be16 tag;
> + struct tpm1_version version;
> +} __packed;
> +
> +struct timeout_t {
> + __be32 a;
> + __be32 b;
> + __be32 c;
> + __be32 d;
> +} __packed;
> +
> +struct duration_t {
> + __be32 tpm_short;
> + __be32 tpm_medium;
> + __be32 tpm_long;
> +} __packed;
> +
> +struct permanent_flags_t {
> + __be16 tag;
> + u8 disable;
> + u8 ownership;
> + u8 deactivated;
> + u8 readPubek;
> + u8 disableOwnerClear;
> + u8 allowMaintenance;
> + u8 physicalPresenceLifetimeLock;
> + u8 physicalPresenceHWEnable;
> + u8 physicalPresenceCMDEnable;
> + u8 CEKPUsed;
> + u8 TPMpost;
> + u8 TPMpostLock;
> + u8 FIPS;
> + u8 operator;
> + u8 enableRevokeEK;
> + u8 nvLocked;
> + u8 readSRKPub;
> + u8 tpmEstablished;
> + u8 maintenanceDone;
> + u8 disableFullDALogicInfo;
> +} __packed;
> +
> +/* Gather all capabilities related information info one type */
> +typedef union {
> + struct permanent_flags_t perm_flags;
> + struct stclear_flags_t stclear_flags;
> + __u8 owned;
> + __be32 num_pcrs;
> + struct tpm1_version version1;
> + struct tpm1_version2 version2;
> + __be32 manufacturer_id;
> + struct timeout_t timeout;
> + struct duration_t duration;
> +} cap_t;
Don't retain alignment here.
> +
> +struct tpm1_get_random_out {
> + __be32 rng_data_len;
> + u8 rng_data[TPM_MAX_RNG_DATA];
> +} __packed;
> +
> +#endif
> diff --git a/include/linux/tpm1.h b/include/linux/tpm1.h
> index 54c6c211eb9e..5fad94ac8d15 100644
> --- a/include/linux/tpm1.h
> +++ b/include/linux/tpm1.h
> @@ -47,7 +47,39 @@ enum tpm_command_ordinals {
> TPM_ORD_UNSEAL = 24,
> };
>
> -/* Other constants */
> +enum tpm_capabilities {
> + TPM_CAP_FLAG = 4,
> + TPM_CAP_PROP = 5,
> + TPM_CAP_VERSION_1_1 = 0x06,
> + TPM_CAP_VERSION_1_2 = 0x1A,
> +};
> +
> +enum tpm_sub_capabilities {
> + TPM_CAP_PROP_PCR = 0x101,
> + TPM_CAP_PROP_MANUFACTURER = 0x103,
> + TPM_CAP_FLAG_PERM = 0x108,
> + TPM_CAP_FLAG_VOL = 0x109,
> + TPM_CAP_PROP_OWNER = 0x111,
> + TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
> + TPM_CAP_PROP_TIS_DURATION = 0x120,
> +};
> +
> +/* Return Codes */
> +enum tpm_return_codes {
> + TPM_BASE_MASK = 0,
> + TPM_NON_FATAL_MASK = 0x00000800,
> + TPM_SUCCESS = TPM_BASE_MASK + 0,
> + TPM_ERR_DEACTIVATED = TPM_BASE_MASK + 6,
> + TPM_ERR_DISABLED = TPM_BASE_MASK + 7,
> + TPM_ERR_FAIL = TPM_BASE_MASK + 9,
> + TPM_ERR_FAILEDSELFTEST = TPM_BASE_MASK + 28,
> + TPM_ERR_INVALID_POSTINIT = TPM_BASE_MASK + 38,
> + TPM_ERR_INVALID_FAMILY = TPM_BASE_MASK + 55,
> + TPM_WARN_RETRY = TPM_BASE_MASK + TPM_NON_FATAL_MASK + 0,
> + TPM_WARN_DOING_SELFTEST = TPM_BASE_MASK + TPM_NON_FATAL_MASK + 2,
> +};
> +
> +/* Misc. constants */
These constants should be relocated in a separate patch.
> #define SRKHANDLE 0x40000000
> #define TPM_NONCE_SIZE 20
> #define TPM_ST_CLEAR 1
> --
> 2.43.7
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v15 19/28] x86/tpm: Early TPM PCR extending driver
From: Jarkko Sakkinen @ 2026-01-20 0:10 UTC (permalink / raw)
To: Ross Philipson
Cc: linux-kernel, x86, linux-integrity, linux-doc, linux-crypto,
kexec, linux-efi, iommu, dpsmith, tglx, mingo, bp, hpa,
dave.hansen, ardb, mjg59, James.Bottomley, peterhuewe, jgg, luto,
nivedita, herbert, davem, corbet, ebiederm, dwmw2, baolu.lu,
kanth.ghatraju, andrew.cooper3, trenchboot-devel
In-Reply-To: <20251215233316.1076248-20-ross.philipson@oracle.com>
On Mon, Dec 15, 2025 at 03:33:07PM -0800, Ross Philipson wrote:
> Introduce an early driver that can interact minimally with the
> TPM. This allows the Secure Launch startup code to extend measurement
> values into the TPM's DRTM PCR banks early in the launch process.
>
> This driver implementation is very minimal, only supporting basic
> initialization and extend commands. An extend command can be sent to both
> a TPM 2.0 or 1.2 chip but only the TIS/FIFO interface is currently
> supported. The CRB interface is currently not supported. The TCG specs
> for these interface can be found here:
>
> https://trustedcomputinggroup.org/resource/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis/
> https://trustedcomputinggroup.org/resource/tpm-2-0-mobile-command-response-buffer-interface-specification/
>
> The driver could be extended for further operations if needed. This
> TPM dirver implementation relies as much as possible on existing mainline
> kernel TPM code.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
Wouldn't it be possible to simply link code from tpm-interface.c and
re-use PCR functions from tpm1-cmd.c and tpm2-cmd.c?
We can even split transmit code from rest of tpm-interface.c if that
is necessary for some reason.
I.e. link the code and define ops re-using code below.
If using tpm_transmit is impossible, less ideally you could also
implement your own tpm_transmit_cmd and link tpm1-cmd.o and tpm2-cmd.o,
which will work out as the symbol is there.
> ---
> arch/x86/boot/compressed/Makefile | 1 +
> arch/x86/boot/compressed/early_tpm_extend.c | 601 ++++++++++++++++++++
> arch/x86/boot/compressed/tpm.h | 42 ++
> 3 files changed, 644 insertions(+)
> create mode 100644 arch/x86/boot/compressed/early_tpm_extend.c
> create mode 100644 arch/x86/boot/compressed/tpm.h
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 0ea8a11ec271..b108e0edf367 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -114,6 +114,7 @@ endif
>
> slaunch-objs += $(obj)/sha1.o
> slaunch-objs += $(obj)/sha256.o
> +slaunch-objs += $(obj)/early_tpm_extend.o
>
> vmlinux-objs-$(CONFIG_SECURE_LAUNCH) += $(slaunch-objs)
>
> diff --git a/arch/x86/boot/compressed/early_tpm_extend.c b/arch/x86/boot/compressed/early_tpm_extend.c
> new file mode 100644
> index 000000000000..7cfdb7969878
> --- /dev/null
> +++ b/arch/x86/boot/compressed/early_tpm_extend.c
> @@ -0,0 +1,601 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2010-2012 United States Government, as represented by
> + * the Secretary of Defense. All rights reserved.
> + *
> + * based off of the original tools/vtpm_manager code base which is:
> + * Copyright (c) 2005, Intel Corp.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer in the documentation and/or other materials provided
> + * with the distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + * contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/string.h>
> +#include <crypto/sha2.h>
> +#include <asm/msr.h>
> +#include <asm/io.h>
> +
> +#include <linux/tpm_common.h>
> +#include <linux/tpm1.h>
> +#include <linux/tpm2.h>
> +#include <linux/tpm_ptp.h>
> +#include <linux/tpm_buf.h>
> +
> +#include "../../../../drivers/char/tpm/tpm1_structs.h"
> +#include "../../../../drivers/char/tpm/tpm2_structs.h"
> +
> +#include "tpm.h"
> +
> +static u8 tpm_buf_page[PAGE_SIZE];
> +
> +/*
> + * Single threaded environment only running on BSP. Use a single shared
> + * page for all TPM extend operations.
> + */
> +static inline struct tpm_buf *tpm_buf_alloc_page(void)
> +{
> + memset(tpm_buf_page, 0, PAGE_SIZE);
> + return (struct tpm_buf *)tpm_buf_page;
> +}
> +
> +static inline void tpm_buf_free_page(void)
> +{
> + memset(tpm_buf_page, 0, PAGE_SIZE);
> +}
> +
> +/* Pull in TPM buffer management support */
> +#undef WARN
> +#define WARN(c, f...)
> +#undef WARN_ON
> +#define WARN_ON(c) (0)
> +
> +#include "../../../../drivers/char/tpm/tpm-buf.c"
> +
> +static u32 tpm_get_alg_size(u16 alg_id)
> +{
> + switch (alg_id) {
> + case TPM_ALG_SHA1:
> + return TPM_DIGEST_SIZE;
> + case TPM_ALG_SHA256:
> + case TPM_ALG_SM3_256:
> + return SHA256_DIGEST_SIZE;
> + case TPM_ALG_SHA384:
> + return SHA384_DIGEST_SIZE;
> + case TPM_ALG_SHA512:
> + default:
> + return SHA512_DIGEST_SIZE;
> + };
> +}
> +
> +static inline u8 tpm_read8(struct tpm_chip *chip, u32 field)
> +{
> + void *mmio_addr = (void *)(uintptr_t)(chip->baseaddr | field);
> + return readb(mmio_addr);
> +}
> +
> +static inline void tpm_write8(struct tpm_chip *chip, u32 field, u8 val)
> +{
> + void *mmio_addr = (void *)(uintptr_t)(chip->baseaddr | field);
> + writeb(val, mmio_addr);
> +}
> +
> +static inline u32 tpm_read32(struct tpm_chip *chip, u32 field)
> +{
> + void *mmio_addr = (void *)(uintptr_t)(chip->baseaddr | field);
> + return readl(mmio_addr);
> +}
> +
> +static inline void tpm_write32(struct tpm_chip *chip, u32 field, u32 val)
> +{
> + void *mmio_addr = (void *)(uintptr_t)(chip->baseaddr | field);
> + writel(val, mmio_addr);
> +}
> +
> +static unsigned long ticks_per_ms = (5UL * 1000 * 1000 /* cpu_khz */ / 1000);
> +
> +static inline ktime_t tpm_now_ms(void)
> +{
> + return rdtsc()/ticks_per_ms;
> +}
> +
> +/*
> + * We're far too early to calibrate time. Assume a 5GHz processor (the upper
> + * end of the Fam19h range), which causes us to be wrong in the safe direction
> + * on slower systems.
> + */
> +static inline void tpm_mdelay(unsigned int msecs)
> +{
> + unsigned long ticks = msecs * ticks_per_ms;
> + unsigned long s, e;
> +
> + s = rdtsc();
> + do {
> + cpu_relax();
> + e = rdtsc();
> + } while ((e - s) < ticks);
> +}
> +
> +static inline u8 __tis_status(struct tpm_chip *chip)
> +{
> + return tpm_read8(chip, TPM_STS(chip->locality));
> +}
> +
> +static inline void __tis_cancel(struct tpm_chip *chip)
> +{
> + /* This causes the current command to be aborted */
> + tpm_write8(chip, TPM_STS(chip->locality), TPM_STS_COMMAND_READY);
> +}
> +
> +static int __tis_get_burstcount(struct tpm_chip *chip)
> +{
> + ktime_t stop;
> + int burstcnt;
> +
> + stop = tpm_now_ms() + chip->timeout_d;
> + do {
> + burstcnt = tpm_read8(chip, (TPM_STS(chip->locality) + 1));
> + burstcnt += tpm_read8(chip, TPM_STS(chip->locality) + 2) << 8;
> +
> + if (burstcnt)
> + return burstcnt;
> +
> + tpm_mdelay(TPM_TIMEOUT);
> + } while (tpm_now_ms() < stop);
> +
> + return -EBUSY;
> +}
> +
> +static int __tis_wait_for_stat(struct tpm_chip *chip, u8 mask, ktime_t timeout)
> +{
> + ktime_t stop;
> + u8 status;
> +
> + if ((__tis_status(chip) & mask) == mask)
> + return 0;
> +
> + stop = tpm_now_ms() + timeout;
> + do {
> + tpm_mdelay(TPM_TIMEOUT);
> +
> + status = __tis_status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + } while (tpm_now_ms() < stop);
> +
> + return -ETIME;
> +}
> +
> +static int __tis_recv_data(struct tpm_chip *chip, u8 *buf, int count)
> +{
> + int size = 0;
> + int burstcnt;
> +
> + while (size < count && __tis_wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID, chip->timeout_c) == 0) {
> + burstcnt = __tis_get_burstcount(chip);
> +
> + for ( ; burstcnt > 0 && size < count; --burstcnt)
> + buf[size++] = tpm_read8(chip, TPM_DATA_FIFO(chip->locality));
> + }
> +
> + return size;
> +}
> +
> +/**
> + * tpm_tis_check_locality - Check if the given locality is the active one
> + * @chip: The TPM chip instance
> + * @loc: The locality to check
> + *
> + * Return: true - locality active, false - not active
> + */
> +bool tpm_tis_check_locality(struct tpm_chip *chip, int loc)
> +{
> + if ((tpm_read8(chip, TPM_ACCESS(loc)) & (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) == (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) {
> + chip->locality = loc;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/**
> + * tpm_tis_release_locality - Release the active locality
> + * @chip: The TPM chip instance
> + */
> +void tpm_tis_release_locality(struct tpm_chip *chip)
> +{
> + if ((tpm_read8(chip, TPM_ACCESS(chip->locality)) & (TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID)) == (TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID))
> + tpm_write8(chip, TPM_ACCESS(chip->locality), TPM_ACCESS_RELINQUISH_LOCALITY);
> +
> + chip->locality = 0;
> +}
> +
> +/**
> + * tpm_tis_request_locality - Request to make the given locality the active one
> + * @chip: The TPM chip instance
> + * @loc: The locality to make active/set as current
> + *
> + * Return:
> + * >= 0 - Success, new active locality returned or locality already active
> + * < 0 - Error occurred
> + */
> +int tpm_tis_request_locality(struct tpm_chip *chip, int loc)
> +{
> + ktime_t stop;
> +
> + if (tpm_tis_check_locality(chip, loc))
> + return loc;
> +
> + /* Set the new locality */
> + tpm_write8(chip, TPM_ACCESS(loc), TPM_ACCESS_REQUEST_USE);
> +
> + stop = tpm_now_ms() + chip->timeout_b;
> + do {
> + if (tpm_tis_check_locality(chip, loc))
> + return loc;
> +
> + tpm_mdelay(TPM_TIMEOUT);
> + } while (tpm_now_ms() < stop);
> +
> + return -1;
> +}
> +
> +/**
> + * tpm_tis_disable_interrupts - Disable interrupts for the TPM, use polling mode only
> + * @chip: The TPM chip instance
> + */
> +void tpm_tis_disable_interrupts(struct tpm_chip *chip)
> +{
> + u32 intmask;
> +
> + intmask = tpm_read32(chip, TPM_INT_ENABLE(chip->locality));
> + /* Disable everything to make sure it is in a consistent state */
> + intmask &= ~(TPM_GLOBAL_INT_ENABLE | TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_STS_VALID_INT | TPM_INTF_DATA_AVAIL_INT);
> + tpm_write32(chip, TPM_INT_ENABLE(chip->locality), intmask);
> +}
> +
> +/**
> + * tpm_tis_recv - Receive response data from TPM via TIS FIFO
> + * @chip: The TPM chip instance
> + * @buf: The response buffer
> + * @count: Length of the response buffer
> + *
> + * Return:
> + * = 0 - Success, no response data
> + * > 0 - Success, value is the response data length
> + * < 0 - Error occurred
> + */
> +static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, int count)
> +{
> + int expected, status, size = 0, rc = -EIO;
> +
> + if (count < TPM_HEADER_SIZE)
> + goto out;
> +
> + /* Read first 10 bytes, including tag, paramsize, and result */
> + size = __tis_recv_data(chip, buf, TPM_HEADER_SIZE);
> + if (size < TPM_HEADER_SIZE)
> + goto out;
> +
> + expected = be32_to_cpu(*((u32 *)(buf + 2)));
> + if (expected > count)
> + goto out;
> +
> + size += __tis_recv_data(chip, &buf[TPM_HEADER_SIZE], expected - TPM_HEADER_SIZE);
> + if (size < expected) {
> + rc = -ETIME;
> + goto out;
> + }
> +
> + __tis_wait_for_stat(chip, TPM_STS_VALID, chip->timeout_c);
> +
> + status = __tis_status(chip);
> + if (status & TPM_STS_DATA_AVAIL) {
> + rc = -EIO;
> + goto out;
> + }
> +
> + return size;
> +out:
> + __tis_cancel(chip);
> + tpm_tis_release_locality(chip);
> + return rc;
> +}
> +
> +/**
> + * tpm_tis_send - Send command to TPM via TIS FIFO
> + * @chip: The TPM chip instance
> + * @buf: The command buffer
> + * @len: Length of the command buffer to send
> + *
> + * Return:
> + * = len - Success, all data sent
> + * < 0 - Error occurred
> + */
> +static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, int len)
> +{
> + int status, burstcnt = 0;
> + int count = 0;
> + int rc = 0;
> +
> + status = __tis_status(chip);
> + if ((status & TPM_STS_COMMAND_READY) == 0) {
> + __tis_cancel(chip);
> + if (__tis_wait_for_stat(chip, TPM_STS_COMMAND_READY, chip->timeout_b) < 0) {
> + rc = -ETIME;
> + goto out_err;
> + }
> + }
> +
> + while (count < len - 1) {
> + burstcnt = __tis_get_burstcount(chip);
> + for ( ; burstcnt > 0 && count < len - 1; --burstcnt)
> + tpm_write8(chip, TPM_DATA_FIFO(chip->locality), buf[count++]);
> +
> + __tis_wait_for_stat(chip, TPM_STS_VALID, chip->timeout_c);
> + status = __tis_status(chip);
> + if ((status & TPM_STS_DATA_EXPECT) == 0) {
> + rc = -EIO;
> + goto out_err;
> + }
> + }
> +
> + /* Write last byte */
> + tpm_write8(chip, TPM_DATA_FIFO(chip->locality), buf[count]);
> + __tis_wait_for_stat(chip, TPM_STS_VALID, chip->timeout_c);
> + status = __tis_status(chip);
> + if ((status & TPM_STS_DATA_EXPECT) != 0) {
> + rc = -EIO;
> + goto out_err;
> + }
> +
> + /* Go and do it */
> + tpm_write8(chip, TPM_STS(chip->locality), TPM_STS_GO);
> +
> + return len;
> +
> +out_err:
> + __tis_cancel(chip);
> + tpm_tis_release_locality(chip);
> + return rc;
> +}
> +
> +/**
> + * tpm_tis_transmit - Transmit a TPM FIFO command
> + * @chip: The TPM chip instance
> + * @buf: The request and response buffer object
> + * @bufsize: Entire size available in buffer
> + *
> + * Return:
> + * = 0 - Success, no returned data
> + * > 0 - Success, value is the return data length
> + * < 0 - Error occurred
> + */
> +static int tpm_tis_transmit(struct tpm_chip *chip, u8 *buf, u32 bufsize)
> +{
> + ktime_t stop;
> + u32 count;
> + u8 status;
> + int rc;
> +
> + count = be32_to_cpu(*((u32 *) (buf + 2)));
> + if (count == 0)
> + return -ENODATA;
> +
> + if (count > bufsize)
> + return -E2BIG;
> +
> + rc = tpm_tis_send(chip, buf, count);
> + if (rc < 0)
> + goto out;
> +
> + stop = tpm_now_ms() + TIS_DURATION;
> + do {
> + status = __tis_status(chip);
> + if ((status & (TPM_STS_DATA_AVAIL | TPM_STS_VALID)) == (TPM_STS_DATA_AVAIL | TPM_STS_VALID))
> + goto out_recv;
> +
> + if (status == TPM_STS_COMMAND_READY) {
> + rc = -ECANCELED;
> + goto out;
> + }
> +
> + tpm_mdelay(TPM_TIMEOUT);
> + rmb();
> + } while (tpm_now_ms() < stop);
> +
> + /* Cancel the command */
> + __tis_cancel(chip);
> + rc = -ETIME;
> + goto out;
> +
> +out_recv:
> + rc = tpm_tis_recv(chip, buf, bufsize);
> + if (rc >= 0) {
> + if (rc > 0 && rc < TPM_HEADER_SIZE)
> + return -EFAULT;
> + return rc;
> + }
> + /* Else return was an error, nothing to receive */
> +
> +out:
> + return rc;
> +}
> +
> +/**
> + * tpm_find_interface_and_family - interface FIFO/CRB, family 2.0 or 1.2
> + * @chip: The TPM chip instance
> + *
> + * Return: TPM family ID enum
> + */
> +static enum tpm_family tpm_find_interface_and_family(struct tpm_chip *chip)
> +{
> + struct tpm_intf_capability intf_cap;
> + struct tpm_interface_id intf_id;
> +
> + /* Sort out whether it is 1.x */
> + intf_cap.val = tpm_read32(chip, TPM_INTF_CAPS(0));
> + if ((intf_cap.interface_version == TPM_TIS_INTF_12) ||
> + (intf_cap.interface_version == TPM_TIS_INTF_13))
> + return TPM_FAMILY_12; /* Always TIS */
> +
> + /* Assume that it is 2.0 but check if the interface is CRB */
> + intf_id.val = tpm_read32(chip, TPM_INTF_ID(0));
> + if (intf_id.interface_type == TPM_CRB_INTF_ACTIVE)
> + return TPM_FAMILY_INVALID;
> +
> + /* Else TPM 2.0 with TIS interface */
> + return TPM_FAMILY_20;
> +}
> +
> +/**
> + * tpm1_pcr_extend - send a TPM1 extend command to the device
> + * @chip: a TPM chip to use
> + * @pcr_idx: the PCR index to extend for the current locality
> + * @hash: the SHA1 hash digest to extend
> + *
> + * Return:
> + * * 0 - OK
> + * * -errno - A system error
> + * * TPM_RC - A TPM error
> + */
> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> +{
> + int rc = 0;
> + struct tpm_buf *buf = tpm_buf_alloc_page();
> +
> + if (!buf)
> + return -ENOMEM;
> +
> + tpm_buf_init(buf, TPM_BUFSIZE);
> + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
> +
> + tpm_buf_append_u32(buf, pcr_idx);
> + tpm_buf_append(buf, hash, TPM_DIGEST_SIZE);
> +
> + rc = tpm_tis_transmit(chip, buf->data, PAGE_SIZE);
> +
> + /* Ignoring output */
> + if (rc > 0)
> + rc = 0;
> +
> + tpm_buf_free_page();
> +
> + return rc;
> +}
> +
> +/**
> + * tpm2_pcr_extend() - send a TPM2 extend command to the device
> + *
> + * @chip: TPM chip to use.
> + * @pcr_idx: index of the PCR.
> + * @digests: list of PCR banks and corresponding digest values to extend.
> + * @digest_count: count of digests to extend
> + *
> + * Return:
> + * * 0 - OK
> + * * -errno - A system error
> + * * TPM_RC - A TPM error
> + */
> +int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> + struct tpm_digest *digests, u32 digest_count)
> +{
> + struct tpm_buf *buf = tpm_buf_alloc_page();
> + int rc = 0, i;
> +
> + if (!buf)
> + return -ENOMEM;
> +
> + tpm_buf_init(buf, TPM_BUFSIZE);
> + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> +
> + tpm_buf_append_handle(buf, pcr_idx);
> +
> + /* Setup a NULL auth session for the command */
> + tpm_buf_append_u32(buf, 9);
> + /* auth handle */
> + tpm_buf_append_u32(buf, TPM2_RS_PW);
> + /* nonce */
> + tpm_buf_append_u16(buf, 0);
> + /* attributes */
> + tpm_buf_append_u8(buf, 0);
> + /* passphrase */
> + tpm_buf_append_u16(buf, 0);
> +
> + tpm_buf_append_u32(buf, digest_count);
> +
> + for (i = 0; i < digest_count; i++) {
> + tpm_buf_append_u16(buf, digests[i].alg_id);
> + tpm_buf_append(buf, (const unsigned char *)&digests[i].digest,
> + tpm_get_alg_size(digests[i].alg_id));
> + }
> +
> + rc = tpm_tis_transmit(chip, buf->data, PAGE_SIZE);
> +
> + /* Ignoring output */
> + if (rc > 0)
> + rc = 0;
> +
> + tpm_buf_free_page();
> +
> + return rc;
> +}
> +
> +int early_tpm_init(struct tpm_chip *chip, u64 baseaddr)
> +{
> + u32 didvid;
> +
> + memset(chip, 0, sizeof(*chip));
> + chip->baseaddr = baseaddr;
> +
> + chip->family = tpm_find_interface_and_family(chip);
> + if (chip->family == TPM_FAMILY_INVALID)
> + return TPM_ERR_INVALID_FAMILY;
> +
> + /* Set default timeouts */
> + chip->timeout_a = TIS_SHORT_TIMEOUT;
> + chip->timeout_b = TIS_LONG_TIMEOUT;
> + chip->timeout_c = TIS_SHORT_TIMEOUT;
> + chip->timeout_d = TIS_SHORT_TIMEOUT;
> +
> + /* Get the vendor and device ids */
> + didvid = tpm_read32(chip, TPM_DID_VID(0));
> + chip->did = didvid >> 16;
> + chip->vid = didvid & 0xFFFF;
> +
> + return TPM_SUCCESS;
> +}
> +
> +int early_tpm_fini(struct tpm_chip *chip)
> +{
> + tpm_tis_release_locality(chip);
> + memset(chip, 0, sizeof(*chip));
> +
> + return TPM_SUCCESS;
> +}
> diff --git a/arch/x86/boot/compressed/tpm.h b/arch/x86/boot/compressed/tpm.h
> new file mode 100644
> index 000000000000..6986593e0100
> --- /dev/null
> +++ b/arch/x86/boot/compressed/tpm.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef BOOT_COMPRESSED_TPM_H
> +#define BOOT_COMPRESSED_TPM_H
> +
> +enum early_tis_defaults {
> + TIS_MEM_X86_LPC_BASE = 0xFED40000,
> + TIS_MEM_X86_LEN = 0x5000,
> + TPM_TIMEOUT = 5, /* ms */
> + TIS_DURATION = 120000, /* 120 secs in ms */
> +};
> +
> +enum tpm_family {
> + TPM_FAMILY_INVALID = 0,
> + TPM_FAMILY_12 = 1,
> + TPM_FAMILY_20 = 2
> +};
> +
> +struct tpm_chip {
> + enum tpm_family family;
> + u64 baseaddr;
> + int locality;
> + int did;
> + int vid;
> +
> + /* in ms */
> + ktime_t timeout_a;
> + ktime_t timeout_b;
> + ktime_t timeout_c;
> + ktime_t timeout_d;
> +};
> +
> +bool tpm_tis_check_locality(struct tpm_chip *chip, int loc);
> +void tpm_tis_release_locality(struct tpm_chip *chip);
> +int tpm_tis_request_locality(struct tpm_chip *chip, int loc);
> +void tpm_tis_disable_interrupts(struct tpm_chip *chip);
> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash);
> +int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> + struct tpm_digest *digests, u32 digest_count);
> +int early_tpm_init(struct tpm_chip *chip, u64 baseaddr);
> +int early_tpm_fini(struct tpm_chip *chip);
> +
> +#endif /* BOOT_COMPRESSED_TPM_H */
> --
> 2.43.7
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 0/3] Make detecting the secure boot status integrity-wide
From: Alexander Egorenkov @ 2026-01-20 13:59 UTC (permalink / raw)
To: coxu; +Cc: hca, linux-integrity
In-Reply-To: <20260115004328.194142-1-coxu@redhat.com>
Tested the entire patch series with s390 (with _and_ without secure boot),
and everything seems to work as expected.
Tested-by: Alexander Egorenkov <egorenar@linux.ibm.com>
^ permalink raw reply
* [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Frederick Lawler @ 2026-01-20 20:20 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
introduced a means to track change detection for an inode
via ctime updates, opposed to setting kstat.change_cookie to
an i_version when calling into xfs_vn_getattr().
This introduced a regression for IMA such that an action
performed on a LOWER inode on a stacked file systems always
requires a re-evaluation if the LOWER file system does not
leverage kstat.change_cookie to track inode i_version or lacks
i_version support all together.
In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
will require re-evaluation. Stacking TPMFS on XFS for instance, once the
inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
leverages generic_fillattr() to update the change cookie.
This is because IMA caches kstat.change_cookie to compare against an
inode's i_version directly in integrity_inode_attrs_changed(), and thus
could be out of date depending on how file systems set
kstat.change_cookie.
To address this, require integrity_inode_attrs_changed() to query
vfs_getattr_nosec() to compare the cached version against
kstat.change_cookie directly. This ensures that when updates occur,
we're accessing the same changed inode version on changes, and fallback
to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
result mask.
Lastly, because EVM still relies on querying and caching a inode's
i_version directly, the integrity_inode_attrs_changed() falls back to the
original inode.i_version != cached comparison.
Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
We uncovered a case in kernels >= 6.13 where XFS is no longer updating
struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
using multigrain ctime (as well as other file systems) for
change detection in commit 1cf7e834a6fb ("xfs: switch to
multigrain timestamps").
Because file systems may implement i_version as they see fit, IMA
caching may be behind as well as file systems that don't support/export
i_version. Thus we're proposing to compare against the kstat.change_cookie
directly to the cached version, and fall back to a ctime guard when
that's not updated.
EVM is largely left alone since there's no trivial way to query a file
directly in the LSM call paths to obtain kstat.change_cookie &
kstat.ctime to cache. Thus retains accessing i_version directly.
Regression tests will be added to the Linux Test Project instead of
selftest to help catch future file system changes that may impact
future evaluation of IMA.
I'd like this to be backported to at least 6.18 if possible.
Below is a simplified test that demonstrates the issue:
_fragment.config_
CONFIG_XFS_FS=y
CONFIG_OVERLAY_FS=y
CONFIG_IMA=y
CONFIG_IMA_WRITE_POLICY=y
CONFIG_IMA_READ_POLICY=y
_./test.sh_
IMA_POLICY="/sys/kernel/security/ima/policy"
TEST_BIN="/bin/date"
MNT_BASE="/tmp/ima_test_root"
mkdir -p "$MNT_BASE"
mount -t tmpfs tmpfs "$MNT_BASE"
mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
mkfs.xfs -q "$MNT_BASE/xfs.img"
mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
mount -t overlay overlay -o \
"lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
"$MNT_BASE/ovl"
echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
target_prog="$MNT_BASE/ovl/test_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
if [[ "$audit_count" -eq 1 ]]; then
echo "PASS: Found exactly 1 audit event."
else
echo "FAIL: Expected 1 audit event, but found $audit_count."
exit 1
fi
---
Changes since RFC:
- Remove calls to I_IS_VERSION()
- Function documentation/comments
- Abide IMA/EVM change detection fallback invariants
- Combined ctime guard into version for attributes struct
- Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
---
Changes in v2:
- Updated commit description + message to clarify the problem.
- compare struct timespec64 to avoid collision possibility [Roberto].
- Don't check inode_attr_changed() in ima_check_last_writer()
- Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
---
include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
security/integrity/evm/evm_crypto.c | 4 +++-
security/integrity/evm/evm_main.c | 5 ++---
security/integrity/ima/ima_api.c | 20 +++++++++++++------
security/integrity/ima/ima_main.c | 18 ++++++++++-------
5 files changed, 65 insertions(+), 22 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -9,6 +9,8 @@
#include <linux/fs.h>
#include <linux/iversion.h>
+#include <linux/kernel.h>
+#include <linux/time64.h>
enum integrity_status {
INTEGRITY_PASS = 0,
@@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
/* An inode's attributes for detection of changes */
struct integrity_inode_attributes {
+ struct timespec64 ctime;
u64 version; /* track inode changes */
unsigned long ino;
dev_t dev;
@@ -42,8 +45,10 @@ struct integrity_inode_attributes {
*/
static inline void
integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
- u64 i_version, const struct inode *inode)
+ u64 i_version, struct timespec64 ctime,
+ const struct inode *inode)
{
+ attrs->ctime = ctime;
attrs->version = i_version;
attrs->dev = inode->i_sb->s_dev;
attrs->ino = inode->i_ino;
@@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
/*
* On stacked filesystems detect whether the inode or its content has changed.
+ *
+ * Must be called in process context.
*/
static inline bool
integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
- const struct inode *inode)
+ struct file *file, struct inode *inode)
{
- return (inode->i_sb->s_dev != attrs->dev ||
- inode->i_ino != attrs->ino ||
- !inode_eq_iversion(inode, attrs->version));
+ struct kstat stat;
+
+ might_sleep();
+
+ if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
+ return true;
+
+ /*
+ * EVM currently relies on backing inode i_version. While IS_I_VERSION
+ * is not a good indicator of i_version support, this still retains
+ * the logic such that a re-evaluation should still occur for EVM, and
+ * only for IMA if vfs_getattr_nosec() fails.
+ */
+ if (!file || vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT))
+ return !IS_I_VERSION(inode) ||
+ !inode_eq_iversion(inode, attrs->version);
+
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ return stat.change_cookie != attrs->version;
+
+ if (stat.result_mask & STATX_CTIME)
+ return !timespec64_equal(&stat.ctime, &attrs->ctime);
+
+ return true;
}
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
hmac_add_misc(desc, inode, type, data->digest);
if (inode != d_backing_inode(dentry) && iint) {
+ struct timespec64 ctime = {0};
+
if (IS_I_VERSION(inode))
i_version = inode_query_iversion(inode);
integrity_inode_attrs_store(&iint->metadata_inode, i_version,
- inode);
+ ctime, inode);
}
/* Portable EVM signatures must include an IMA hash */
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
bool ret = false;
if (iint) {
- ret = (!IS_I_VERSION(metadata_inode) ||
- integrity_inode_attrs_changed(&iint->metadata_inode,
- metadata_inode));
+ ret = integrity_inode_attrs_changed(&iint->metadata_inode,
+ NULL, metadata_inode);
if (ret)
iint->evm_status = INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
int length;
void *tmpbuf;
u64 i_version = 0;
+ struct timespec64 ctime = {0};
/*
* Always collect the modsig, because IMA might have already collected
@@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ if (stat.result_mask & STATX_CTIME)
+ ctime = stat.ctime;
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
@@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
iint->ima_hash = tmpbuf;
memcpy(iint->ima_hash, &hash, length);
- if (real_inode == inode)
+ if (real_inode == inode) {
iint->real_inode.version = i_version;
- else
+ iint->real_inode.ctime = ctime;
+ } else {
integrity_inode_attrs_store(&iint->real_inode, i_version,
- real_inode);
+ ctime, real_inode);
+ }
/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
if (!result)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -22,12 +22,14 @@
#include <linux/mount.h>
#include <linux/mman.h>
#include <linux/slab.h>
+#include <linux/stat.h>
#include <linux/xattr.h>
#include <linux/ima.h>
#include <linux/fs.h>
#include <linux/iversion.h>
#include <linux/evm.h>
#include <linux/crash_dump.h>
+#include <linux/time64.h>
#include "ima.h"
@@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
&iint->atomic_flags);
if ((iint->flags & IMA_NEW_FILE) ||
vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
- AT_STATX_SYNC_AS_STAT) ||
- !(stat.result_mask & STATX_CHANGE_COOKIE) ||
- stat.change_cookie != iint->real_inode.version) {
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT) ||
+ ((stat.result_mask & STATX_CHANGE_COOKIE) ?
+ stat.change_cookie != iint->real_inode.version :
+ (!(stat.result_mask & STATX_CTIME) ||
+ !timespec64_equal(&stat.ctime,
+ &iint->real_inode.ctime)))) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
@@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
real_inode = d_real_inode(file_dentry(file));
if (real_inode != inode &&
(action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
- if (!IS_I_VERSION(real_inode) ||
- integrity_inode_attrs_changed(&iint->real_inode,
- real_inode)) {
+ if (integrity_inode_attrs_changed(&iint->real_inode,
+ file, real_inode)) {
iint->flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@cloudflare.com>
^ permalink raw reply related
* Re: [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Jeff Layton @ 2026-01-20 21:22 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <20260120-xfs-ima-fixup-v2-1-f332ead8b043@cloudflare.com>
On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> introduced a means to track change detection for an inode
> via ctime updates, opposed to setting kstat.change_cookie to
> an i_version when calling into xfs_vn_getattr().
>
> This introduced a regression for IMA such that an action
> performed on a LOWER inode on a stacked file systems always
> requires a re-evaluation if the LOWER file system does not
> leverage kstat.change_cookie to track inode i_version or lacks
> i_version support all together.
>
> In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> leverages generic_fillattr() to update the change cookie.
>
> This is because IMA caches kstat.change_cookie to compare against an
> inode's i_version directly in integrity_inode_attrs_changed(), and thus
> could be out of date depending on how file systems set
> kstat.change_cookie.
>
> To address this, require integrity_inode_attrs_changed() to query
> vfs_getattr_nosec() to compare the cached version against
> kstat.change_cookie directly. This ensures that when updates occur,
> we're accessing the same changed inode version on changes, and fallback
> to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> result mask.
>
> Lastly, because EVM still relies on querying and caching a inode's
> i_version directly, the integrity_inode_attrs_changed() falls back to the
> original inode.i_version != cached comparison.
>
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> using multigrain ctime (as well as other file systems) for
> change detection in commit 1cf7e834a6fb ("xfs: switch to
> multigrain timestamps").
>
> Because file systems may implement i_version as they see fit, IMA
> caching may be behind as well as file systems that don't support/export
> i_version. Thus we're proposing to compare against the kstat.change_cookie
> directly to the cached version, and fall back to a ctime guard when
> that's not updated.
>
> EVM is largely left alone since there's no trivial way to query a file
> directly in the LSM call paths to obtain kstat.change_cookie &
> kstat.ctime to cache. Thus retains accessing i_version directly.
>
> Regression tests will be added to the Linux Test Project instead of
> selftest to help catch future file system changes that may impact
> future evaluation of IMA.
>
> I'd like this to be backported to at least 6.18 if possible.
>
> Below is a simplified test that demonstrates the issue:
>
> _fragment.config_
> CONFIG_XFS_FS=y
> CONFIG_OVERLAY_FS=y
> CONFIG_IMA=y
> CONFIG_IMA_WRITE_POLICY=y
> CONFIG_IMA_READ_POLICY=y
>
> _./test.sh_
>
> IMA_POLICY="/sys/kernel/security/ima/policy"
> TEST_BIN="/bin/date"
> MNT_BASE="/tmp/ima_test_root"
>
> mkdir -p "$MNT_BASE"
> mount -t tmpfs tmpfs "$MNT_BASE"
> mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
>
> dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> mkfs.xfs -q "$MNT_BASE/xfs.img"
> mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
>
> mount -t overlay overlay -o \
> "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> "$MNT_BASE/ovl"
>
> echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
>
> target_prog="$MNT_BASE/ovl/test_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
>
> audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
>
> if [[ "$audit_count" -eq 1 ]]; then
> echo "PASS: Found exactly 1 audit event."
> else
> echo "FAIL: Expected 1 audit event, but found $audit_count."
> exit 1
> fi
> ---
> Changes since RFC:
> - Remove calls to I_IS_VERSION()
> - Function documentation/comments
> - Abide IMA/EVM change detection fallback invariants
> - Combined ctime guard into version for attributes struct
> - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> ---
> Changes in v2:
> - Updated commit description + message to clarify the problem.
> - compare struct timespec64 to avoid collision possibility [Roberto].
> - Don't check inode_attr_changed() in ima_check_last_writer()
> - Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
> ---
> include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
> security/integrity/evm/evm_crypto.c | 4 +++-
> security/integrity/evm/evm_main.c | 5 ++---
> security/integrity/ima/ima_api.c | 20 +++++++++++++------
> security/integrity/ima/ima_main.c | 18 ++++++++++-------
> 5 files changed, 65 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -9,6 +9,8 @@
>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> +#include <linux/kernel.h>
> +#include <linux/time64.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
>
> /* An inode's attributes for detection of changes */
> struct integrity_inode_attributes {
> + struct timespec64 ctime;
> u64 version; /* track inode changes */
> unsigned long ino;
> dev_t dev;
> @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> */
> static inline void
> integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> - u64 i_version, const struct inode *inode)
> + u64 i_version, struct timespec64 ctime,
> + const struct inode *inode)
> {
> + attrs->ctime = ctime;
> attrs->version = i_version;
> attrs->dev = inode->i_sb->s_dev;
> attrs->ino = inode->i_ino;
> @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
>
> /*
> * On stacked filesystems detect whether the inode or its content has changed.
> + *
> + * Must be called in process context.
> */
> static inline bool
> integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> - const struct inode *inode)
> + struct file *file, struct inode *inode)
> {
> - return (inode->i_sb->s_dev != attrs->dev ||
> - inode->i_ino != attrs->ino ||
> - !inode_eq_iversion(inode, attrs->version));
> + struct kstat stat;
> +
> + might_sleep();
> +
> + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> + return true;
> +
> + /*
> + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> + * is not a good indicator of i_version support, this still retains
> + * the logic such that a re-evaluation should still occur for EVM, and
> + * only for IMA if vfs_getattr_nosec() fails.
> + */
> + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT))
> + return !IS_I_VERSION(inode) ||
> + !inode_eq_iversion(inode, attrs->version);
Seems like a reasonable way to handle getattr failure, given the
limitations on EVM.
> +
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + return stat.change_cookie != attrs->version;
> +
> + if (stat.result_mask & STATX_CTIME)
> + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> +
> + return true;
> }
>
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> hmac_add_misc(desc, inode, type, data->digest);
>
> if (inode != d_backing_inode(dentry) && iint) {
> + struct timespec64 ctime = {0};
> +
> if (IS_I_VERSION(inode))
> i_version = inode_query_iversion(inode);
It would be nice if you could change this codepath as well, but that
looks like a much bigger project. FWIW, I took a quick look:
The problematic codepath is get/setxattr, which can call into this but
currently only receives a dentry and not a struct path. That would
probably have to be changed. I think it's doable but it's a fair bit of
yak shaving.
> integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> - inode);
> + ctime, inode);
> }
>
> /* Portable EVM signatures must include an IMA hash */
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> bool ret = false;
>
> if (iint) {
> - ret = (!IS_I_VERSION(metadata_inode) ||
> - integrity_inode_attrs_changed(&iint->metadata_inode,
> - metadata_inode));
> + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> + NULL, metadata_inode);
> if (ret)
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> int length;
> void *tmpbuf;
> u64 i_version = 0;
> + struct timespec64 ctime = {0};
>
> /*
> * Always collect the modsig, because IMA might have already collected
> @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + if (stat.result_mask & STATX_CTIME)
> + ctime = stat.ctime;
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>
> iint->ima_hash = tmpbuf;
> memcpy(iint->ima_hash, &hash, length);
> - if (real_inode == inode)
> + if (real_inode == inode) {
> iint->real_inode.version = i_version;
> - else
> + iint->real_inode.ctime = ctime;
> + } else {
> integrity_inode_attrs_store(&iint->real_inode, i_version,
> - real_inode);
> + ctime, real_inode);
> + }
>
> /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> if (!result)
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -22,12 +22,14 @@
> #include <linux/mount.h>
> #include <linux/mman.h>
> #include <linux/slab.h>
> +#include <linux/stat.h>
> #include <linux/xattr.h>
> #include <linux/ima.h>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> #include <linux/evm.h>
> #include <linux/crash_dump.h>
> +#include <linux/time64.h>
>
> #include "ima.h"
>
> @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> &iint->atomic_flags);
> if ((iint->flags & IMA_NEW_FILE) ||
> vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> - AT_STATX_SYNC_AS_STAT) ||
> - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> - stat.change_cookie != iint->real_inode.version) {
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT) ||
> + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> + stat.change_cookie != iint->real_inode.version :
> + (!(stat.result_mask & STATX_CTIME) ||
> + !timespec64_equal(&stat.ctime,
> + &iint->real_inode.ctime)))) {
> iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> iint->measured_pcrs = 0;
> if (update)
> @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> real_inode = d_real_inode(file_dentry(file));
> if (real_inode != inode &&
> (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> - if (!IS_I_VERSION(real_inode) ||
> - integrity_inode_attrs_changed(&iint->real_inode,
> - real_inode)) {
> + if (integrity_inode_attrs_changed(&iint->real_inode,
> + file, real_inode)) {
> iint->flags &= ~IMA_DONE_MASK;
> iint->measured_pcrs = 0;
> }
>
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251212-xfs-ima-fixup-931780a62c2c
>
> Best regards,
Looks like a good change overall. Thanks for fixing this up. I don't
know IMA or EVM very well, so FWIW:
Acked-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Frederick Lawler @ 2026-01-20 21:49 UTC (permalink / raw)
To: Jeff Layton
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <1a7985c825cf4af3b2a87e89793ae0daf2bb9c49.camel@kernel.org>
On Tue, Jan 20, 2026 at 04:22:55PM -0500, Jeff Layton wrote:
> On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> > Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > introduced a means to track change detection for an inode
> > via ctime updates, opposed to setting kstat.change_cookie to
> > an i_version when calling into xfs_vn_getattr().
> >
> > This introduced a regression for IMA such that an action
> > performed on a LOWER inode on a stacked file systems always
> > requires a re-evaluation if the LOWER file system does not
> > leverage kstat.change_cookie to track inode i_version or lacks
> > i_version support all together.
> >
> > In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> > will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> > inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> > leverages generic_fillattr() to update the change cookie.
> >
> > This is because IMA caches kstat.change_cookie to compare against an
> > inode's i_version directly in integrity_inode_attrs_changed(), and thus
> > could be out of date depending on how file systems set
> > kstat.change_cookie.
> >
> > To address this, require integrity_inode_attrs_changed() to query
> > vfs_getattr_nosec() to compare the cached version against
> > kstat.change_cookie directly. This ensures that when updates occur,
> > we're accessing the same changed inode version on changes, and fallback
> > to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> > result mask.
> >
> > Lastly, because EVM still relies on querying and caching a inode's
> > i_version directly, the integrity_inode_attrs_changed() falls back to the
> > original inode.i_version != cached comparison.
> >
> > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> > struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> > using multigrain ctime (as well as other file systems) for
> > change detection in commit 1cf7e834a6fb ("xfs: switch to
> > multigrain timestamps").
> >
> > Because file systems may implement i_version as they see fit, IMA
> > caching may be behind as well as file systems that don't support/export
> > i_version. Thus we're proposing to compare against the kstat.change_cookie
> > directly to the cached version, and fall back to a ctime guard when
> > that's not updated.
> >
> > EVM is largely left alone since there's no trivial way to query a file
> > directly in the LSM call paths to obtain kstat.change_cookie &
> > kstat.ctime to cache. Thus retains accessing i_version directly.
> >
> > Regression tests will be added to the Linux Test Project instead of
> > selftest to help catch future file system changes that may impact
> > future evaluation of IMA.
> >
> > I'd like this to be backported to at least 6.18 if possible.
> >
> > Below is a simplified test that demonstrates the issue:
> >
> > _fragment.config_
> > CONFIG_XFS_FS=y
> > CONFIG_OVERLAY_FS=y
> > CONFIG_IMA=y
> > CONFIG_IMA_WRITE_POLICY=y
> > CONFIG_IMA_READ_POLICY=y
> >
> > _./test.sh_
> >
> > IMA_POLICY="/sys/kernel/security/ima/policy"
> > TEST_BIN="/bin/date"
> > MNT_BASE="/tmp/ima_test_root"
> >
> > mkdir -p "$MNT_BASE"
> > mount -t tmpfs tmpfs "$MNT_BASE"
> > mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
> >
> > dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> > mkfs.xfs -q "$MNT_BASE/xfs.img"
> > mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> > cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
> >
> > mount -t overlay overlay -o \
> > "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> > "$MNT_BASE/ovl"
> >
> > echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
> >
> > target_prog="$MNT_BASE/ovl/test_prog"
> > setpriv --reuid nobody "$target_prog"
> > setpriv --reuid nobody "$target_prog"
> > setpriv --reuid nobody "$target_prog"
> >
> > audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
> >
> > if [[ "$audit_count" -eq 1 ]]; then
> > echo "PASS: Found exactly 1 audit event."
> > else
> > echo "FAIL: Expected 1 audit event, but found $audit_count."
> > exit 1
> > fi
> > ---
> > Changes since RFC:
> > - Remove calls to I_IS_VERSION()
> > - Function documentation/comments
> > - Abide IMA/EVM change detection fallback invariants
> > - Combined ctime guard into version for attributes struct
> > - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> > ---
> > Changes in v2:
> > - Updated commit description + message to clarify the problem.
> > - compare struct timespec64 to avoid collision possibility [Roberto].
> > - Don't check inode_attr_changed() in ima_check_last_writer()
> > - Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
> > ---
> > include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
> > security/integrity/evm/evm_crypto.c | 4 +++-
> > security/integrity/evm/evm_main.c | 5 ++---
> > security/integrity/ima/ima_api.c | 20 +++++++++++++------
> > security/integrity/ima/ima_main.c | 18 ++++++++++-------
> > 5 files changed, 65 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -9,6 +9,8 @@
> >
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > +#include <linux/kernel.h>
> > +#include <linux/time64.h>
> >
> > enum integrity_status {
> > INTEGRITY_PASS = 0,
> > @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
> >
> > /* An inode's attributes for detection of changes */
> > struct integrity_inode_attributes {
> > + struct timespec64 ctime;
> > u64 version; /* track inode changes */
> > unsigned long ino;
> > dev_t dev;
> > @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> > */
> > static inline void
> > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > - u64 i_version, const struct inode *inode)
> > + u64 i_version, struct timespec64 ctime,
> > + const struct inode *inode)
> > {
> > + attrs->ctime = ctime;
> > attrs->version = i_version;
> > attrs->dev = inode->i_sb->s_dev;
> > attrs->ino = inode->i_ino;
> > @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> >
> > /*
> > * On stacked filesystems detect whether the inode or its content has changed.
> > + *
> > + * Must be called in process context.
> > */
> > static inline bool
> > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > - const struct inode *inode)
> > + struct file *file, struct inode *inode)
> > {
> > - return (inode->i_sb->s_dev != attrs->dev ||
> > - inode->i_ino != attrs->ino ||
> > - !inode_eq_iversion(inode, attrs->version));
> > + struct kstat stat;
> > +
> > + might_sleep();
> > +
> > + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> > + return true;
> > +
> > + /*
> > + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> > + * is not a good indicator of i_version support, this still retains
> > + * the logic such that a re-evaluation should still occur for EVM, and
> > + * only for IMA if vfs_getattr_nosec() fails.
> > + */
> > + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > + AT_STATX_SYNC_AS_STAT))
> > + return !IS_I_VERSION(inode) ||
> > + !inode_eq_iversion(inode, attrs->version);
>
> Seems like a reasonable way to handle getattr failure, given the
> limitations on EVM.
>
> > +
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + return stat.change_cookie != attrs->version;
> > +
> > + if (stat.result_mask & STATX_CTIME)
> > + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> > +
> > + return true;
> > }
> >
> >
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > hmac_add_misc(desc, inode, type, data->digest);
> >
> > if (inode != d_backing_inode(dentry) && iint) {
> > + struct timespec64 ctime = {0};
> > +
> > if (IS_I_VERSION(inode))
> > i_version = inode_query_iversion(inode);
>
> It would be nice if you could change this codepath as well, but that
> looks like a much bigger project. FWIW, I took a quick look:
>
> The problematic codepath is get/setxattr, which can call into this but
> currently only receives a dentry and not a struct path. That would
> probably have to be changed. I think it's doable but it's a fair bit of
> yak shaving.
Agreed. I was looking at that a couple weeks ago to include a patch 2
for EVM, and came to the same conclusion. I did find struct iattr on the
setxattr which has struct file *, but that seemed dubious to at best.
Alternatively, create security_get/setxattr() equivalent hooks, or
have an internal mapping of sorts.
>
> > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > - inode);
> > + ctime, inode);
> > }
> >
> > /* Portable EVM signatures must include an IMA hash */
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > bool ret = false;
> >
> > if (iint) {
> > - ret = (!IS_I_VERSION(metadata_inode) ||
> > - integrity_inode_attrs_changed(&iint->metadata_inode,
> > - metadata_inode));
> > + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> > + NULL, metadata_inode);
> > if (ret)
> > iint->evm_status = INTEGRITY_UNKNOWN;
> > }
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > int length;
> > void *tmpbuf;
> > u64 i_version = 0;
> > + struct timespec64 ctime = {0};
> >
> > /*
> > * Always collect the modsig, because IMA might have already collected
> > @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > + if (stat.result_mask & STATX_CTIME)
> > + ctime = stat.ctime;
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
> > @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> >
> > iint->ima_hash = tmpbuf;
> > memcpy(iint->ima_hash, &hash, length);
> > - if (real_inode == inode)
> > + if (real_inode == inode) {
> > iint->real_inode.version = i_version;
> > - else
> > + iint->real_inode.ctime = ctime;
> > + } else {
> > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > - real_inode);
> > + ctime, real_inode);
> > + }
> >
> > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > if (!result)
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -22,12 +22,14 @@
> > #include <linux/mount.h>
> > #include <linux/mman.h>
> > #include <linux/slab.h>
> > +#include <linux/stat.h>
> > #include <linux/xattr.h>
> > #include <linux/ima.h>
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > #include <linux/evm.h>
> > #include <linux/crash_dump.h>
> > +#include <linux/time64.h>
> >
> > #include "ima.h"
> >
> > @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > &iint->atomic_flags);
> > if ((iint->flags & IMA_NEW_FILE) ||
> > vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > - AT_STATX_SYNC_AS_STAT) ||
> > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > - stat.change_cookie != iint->real_inode.version) {
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > + AT_STATX_SYNC_AS_STAT) ||
> > + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> > + stat.change_cookie != iint->real_inode.version :
> > + (!(stat.result_mask & STATX_CTIME) ||
> > + !timespec64_equal(&stat.ctime,
> > + &iint->real_inode.ctime)))) {
> > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > iint->measured_pcrs = 0;
> > if (update)
> > @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > real_inode = d_real_inode(file_dentry(file));
> > if (real_inode != inode &&
> > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > - if (!IS_I_VERSION(real_inode) ||
> > - integrity_inode_attrs_changed(&iint->real_inode,
> > - real_inode)) {
> > + if (integrity_inode_attrs_changed(&iint->real_inode,
> > + file, real_inode)) {
> > iint->flags &= ~IMA_DONE_MASK;
> > iint->measured_pcrs = 0;
> > }
> >
> > ---
> > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> >
> > Best regards,
>
> Looks like a good change overall. Thanks for fixing this up. I don't
> know IMA or EVM very well, so FWIW:
Thanks! Fun investigation & fix.
>
> Acked-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Jeff Layton @ 2026-01-20 22:24 UTC (permalink / raw)
To: Frederick Lawler
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <aW_4V93kczhzFjQ3@CMGLRV3>
On Tue, 2026-01-20 at 15:49 -0600, Frederick Lawler wrote:
> On Tue, Jan 20, 2026 at 04:22:55PM -0500, Jeff Layton wrote:
> > On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> > > Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > introduced a means to track change detection for an inode
> > > via ctime updates, opposed to setting kstat.change_cookie to
> > > an i_version when calling into xfs_vn_getattr().
> > >
> > > This introduced a regression for IMA such that an action
> > > performed on a LOWER inode on a stacked file systems always
> > > requires a re-evaluation if the LOWER file system does not
> > > leverage kstat.change_cookie to track inode i_version or lacks
> > > i_version support all together.
> > >
> > > In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> > > will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> > > inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> > > leverages generic_fillattr() to update the change cookie.
> > >
> > > This is because IMA caches kstat.change_cookie to compare against an
> > > inode's i_version directly in integrity_inode_attrs_changed(), and thus
> > > could be out of date depending on how file systems set
> > > kstat.change_cookie.
> > >
> > > To address this, require integrity_inode_attrs_changed() to query
> > > vfs_getattr_nosec() to compare the cached version against
> > > kstat.change_cookie directly. This ensures that when updates occur,
> > > we're accessing the same changed inode version on changes, and fallback
> > > to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> > > result mask.
> > >
> > > Lastly, because EVM still relies on querying and caching a inode's
> > > i_version directly, the integrity_inode_attrs_changed() falls back to the
> > > original inode.i_version != cached comparison.
> > >
> > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > ---
> > > We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> > > struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> > > using multigrain ctime (as well as other file systems) for
> > > change detection in commit 1cf7e834a6fb ("xfs: switch to
> > > multigrain timestamps").
> > >
> > > Because file systems may implement i_version as they see fit, IMA
> > > caching may be behind as well as file systems that don't support/export
> > > i_version. Thus we're proposing to compare against the kstat.change_cookie
> > > directly to the cached version, and fall back to a ctime guard when
> > > that's not updated.
> > >
> > > EVM is largely left alone since there's no trivial way to query a file
> > > directly in the LSM call paths to obtain kstat.change_cookie &
> > > kstat.ctime to cache. Thus retains accessing i_version directly.
> > >
> > > Regression tests will be added to the Linux Test Project instead of
> > > selftest to help catch future file system changes that may impact
> > > future evaluation of IMA.
> > >
> > > I'd like this to be backported to at least 6.18 if possible.
> > >
> > > Below is a simplified test that demonstrates the issue:
> > >
> > > _fragment.config_
> > > CONFIG_XFS_FS=y
> > > CONFIG_OVERLAY_FS=y
> > > CONFIG_IMA=y
> > > CONFIG_IMA_WRITE_POLICY=y
> > > CONFIG_IMA_READ_POLICY=y
> > >
> > > _./test.sh_
> > >
> > > IMA_POLICY="/sys/kernel/security/ima/policy"
> > > TEST_BIN="/bin/date"
> > > MNT_BASE="/tmp/ima_test_root"
> > >
> > > mkdir -p "$MNT_BASE"
> > > mount -t tmpfs tmpfs "$MNT_BASE"
> > > mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
> > >
> > > dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> > > mkfs.xfs -q "$MNT_BASE/xfs.img"
> > > mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> > > cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
> > >
> > > mount -t overlay overlay -o \
> > > "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> > > "$MNT_BASE/ovl"
> > >
> > > echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
> > >
> > > target_prog="$MNT_BASE/ovl/test_prog"
> > > setpriv --reuid nobody "$target_prog"
> > > setpriv --reuid nobody "$target_prog"
> > > setpriv --reuid nobody "$target_prog"
> > >
> > > audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
> > >
> > > if [[ "$audit_count" -eq 1 ]]; then
> > > echo "PASS: Found exactly 1 audit event."
> > > else
> > > echo "FAIL: Expected 1 audit event, but found $audit_count."
> > > exit 1
> > > fi
> > > ---
> > > Changes since RFC:
> > > - Remove calls to I_IS_VERSION()
> > > - Function documentation/comments
> > > - Abide IMA/EVM change detection fallback invariants
> > > - Combined ctime guard into version for attributes struct
> > > - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> > > ---
> > > Changes in v2:
> > > - Updated commit description + message to clarify the problem.
> > > - compare struct timespec64 to avoid collision possibility [Roberto].
> > > - Don't check inode_attr_changed() in ima_check_last_writer()
> > > - Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
> > > ---
> > > include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
> > > security/integrity/evm/evm_crypto.c | 4 +++-
> > > security/integrity/evm/evm_main.c | 5 ++---
> > > security/integrity/ima/ima_api.c | 20 +++++++++++++------
> > > security/integrity/ima/ima_main.c | 18 ++++++++++-------
> > > 5 files changed, 65 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
> > > --- a/include/linux/integrity.h
> > > +++ b/include/linux/integrity.h
> > > @@ -9,6 +9,8 @@
> > >
> > > #include <linux/fs.h>
> > > #include <linux/iversion.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/time64.h>
> > >
> > > enum integrity_status {
> > > INTEGRITY_PASS = 0,
> > > @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
> > >
> > > /* An inode's attributes for detection of changes */
> > > struct integrity_inode_attributes {
> > > + struct timespec64 ctime;
> > > u64 version; /* track inode changes */
> > > unsigned long ino;
> > > dev_t dev;
> > > @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> > > */
> > > static inline void
> > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > - u64 i_version, const struct inode *inode)
> > > + u64 i_version, struct timespec64 ctime,
> > > + const struct inode *inode)
> > > {
> > > + attrs->ctime = ctime;
> > > attrs->version = i_version;
> > > attrs->dev = inode->i_sb->s_dev;
> > > attrs->ino = inode->i_ino;
> > > @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > >
> > > /*
> > > * On stacked filesystems detect whether the inode or its content has changed.
> > > + *
> > > + * Must be called in process context.
> > > */
> > > static inline bool
> > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > - const struct inode *inode)
> > > + struct file *file, struct inode *inode)
> > > {
> > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > - inode->i_ino != attrs->ino ||
> > > - !inode_eq_iversion(inode, attrs->version));
> > > + struct kstat stat;
> > > +
> > > + might_sleep();
> > > +
> > > + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> > > + return true;
> > > +
> > > + /*
> > > + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> > > + * is not a good indicator of i_version support, this still retains
> > > + * the logic such that a re-evaluation should still occur for EVM, and
> > > + * only for IMA if vfs_getattr_nosec() fails.
> > > + */
> > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT))
> > > + return !IS_I_VERSION(inode) ||
> > > + !inode_eq_iversion(inode, attrs->version);
> >
> > Seems like a reasonable way to handle getattr failure, given the
> > limitations on EVM.
> >
> > > +
> > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > + return stat.change_cookie != attrs->version;
> > > +
> > > + if (stat.result_mask & STATX_CTIME)
> > > + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> > > +
> > > + return true;
> > > }
> > >
> > >
> > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
> > > --- a/security/integrity/evm/evm_crypto.c
> > > +++ b/security/integrity/evm/evm_crypto.c
> > > @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > hmac_add_misc(desc, inode, type, data->digest);
> > >
> > > if (inode != d_backing_inode(dentry) && iint) {
> > > + struct timespec64 ctime = {0};
> > > +
> > > if (IS_I_VERSION(inode))
> > > i_version = inode_query_iversion(inode);
> >
> > It would be nice if you could change this codepath as well, but that
> > looks like a much bigger project. FWIW, I took a quick look:
> >
> > The problematic codepath is get/setxattr, which can call into this but
> > currently only receives a dentry and not a struct path. That would
> > probably have to be changed. I think it's doable but it's a fair bit of
> > yak shaving.
>
> Agreed. I was looking at that a couple weeks ago to include a patch 2
> for EVM, and came to the same conclusion. I did find struct iattr on the
> setxattr which has struct file *, but that seemed dubious to at best.
>
Yeah, it's not set in all cases. setxattr is a pathname-based
operation, so an open file sometimes just doesn't exist.
> Alternatively, create security_get/setxattr() equivalent hooks, or
> have an internal mapping of sorts.
>
>
Those are possibilities too.
A dentry without a mount is somewhat ambiguous though. IMO, it would be
best to just change all of the xattr_handler functions to take struct
path instead of a dentry.
Maybe a good topic for LSF/MM...
> > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > - inode);
> > > + ctime, inode);
> > > }
> > >
> > > /* Portable EVM signatures must include an IMA hash */
> > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> > > --- a/security/integrity/evm/evm_main.c
> > > +++ b/security/integrity/evm/evm_main.c
> > > @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > bool ret = false;
> > >
> > > if (iint) {
> > > - ret = (!IS_I_VERSION(metadata_inode) ||
> > > - integrity_inode_attrs_changed(&iint->metadata_inode,
> > > - metadata_inode));
> > > + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> > > + NULL, metadata_inode);
> > > if (ret)
> > > iint->evm_status = INTEGRITY_UNKNOWN;
> > > }
> > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
> > > --- a/security/integrity/ima/ima_api.c
> > > +++ b/security/integrity/ima/ima_api.c
> > > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > int length;
> > > void *tmpbuf;
> > > u64 i_version = 0;
> > > + struct timespec64 ctime = {0};
> > >
> > > /*
> > > * Always collect the modsig, because IMA might have already collected
> > > @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > * to an initial measurement/appraisal/audit, but was modified to
> > > * assume the file changed.
> > > */
> > > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > AT_STATX_SYNC_AS_STAT);
> > > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > > - i_version = stat.change_cookie;
> > > + if (!result) {
> > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > + i_version = stat.change_cookie;
> > > + if (stat.result_mask & STATX_CTIME)
> > > + ctime = stat.ctime;
> > > + }
> > > hash.hdr.algo = algo;
> > > hash.hdr.length = hash_digest_size[algo];
> > >
> > > @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > >
> > > iint->ima_hash = tmpbuf;
> > > memcpy(iint->ima_hash, &hash, length);
> > > - if (real_inode == inode)
> > > + if (real_inode == inode) {
> > > iint->real_inode.version = i_version;
> > > - else
> > > + iint->real_inode.ctime = ctime;
> > > + } else {
> > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > - real_inode);
> > > + ctime, real_inode);
> > > + }
> > >
> > > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > > if (!result)
> > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -22,12 +22,14 @@
> > > #include <linux/mount.h>
> > > #include <linux/mman.h>
> > > #include <linux/slab.h>
> > > +#include <linux/stat.h>
> > > #include <linux/xattr.h>
> > > #include <linux/ima.h>
> > > #include <linux/fs.h>
> > > #include <linux/iversion.h>
> > > #include <linux/evm.h>
> > > #include <linux/crash_dump.h>
> > > +#include <linux/time64.h>
> > >
> > > #include "ima.h"
> > >
> > > @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > &iint->atomic_flags);
> > > if ((iint->flags & IMA_NEW_FILE) ||
> > > vfs_getattr_nosec(&file->f_path, &stat,
> > > - STATX_CHANGE_COOKIE,
> > > - AT_STATX_SYNC_AS_STAT) ||
> > > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > > - stat.change_cookie != iint->real_inode.version) {
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT) ||
> > > + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> > > + stat.change_cookie != iint->real_inode.version :
> > > + (!(stat.result_mask & STATX_CTIME) ||
> > > + !timespec64_equal(&stat.ctime,
> > > + &iint->real_inode.ctime)))) {
> > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > iint->measured_pcrs = 0;
> > > if (update)
> > > @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > real_inode = d_real_inode(file_dentry(file));
> > > if (real_inode != inode &&
> > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > - if (!IS_I_VERSION(real_inode) ||
> > > - integrity_inode_attrs_changed(&iint->real_inode,
> > > - real_inode)) {
> > > + if (integrity_inode_attrs_changed(&iint->real_inode,
> > > + file, real_inode)) {
> > > iint->flags &= ~IMA_DONE_MASK;
> > > iint->measured_pcrs = 0;
> > > }
> > >
> > > ---
> > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> > >
> > > Best regards,
> >
> > Looks like a good change overall. Thanks for fixing this up. I don't
> > know IMA or EVM very well, so FWIW:
>
> Thanks! Fun investigation & fix.
>
> >
> > Acked-by: Jeff Layton <jlayton@kernel.org>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* [PATCH] efi: Mark UEFI skip cert quirks list as __initconst
From: Thomas Weißschuh @ 2026-01-21 7:43 UTC (permalink / raw)
To: Jarkko Sakkinen, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
Thomas Weißschuh
The only user is marked as __init, so there is no reason to keep this
data around after bootup.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
security/integrity/platform_certs/load_uefi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index d1fdd113450a..26523a2b29c7 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -19,7 +19,7 @@
* a crash disabling EFI runtime services. The following quirk skips reading
* these variables.
*/
-static const struct dmi_system_id uefi_skip_cert[] = {
+static const struct dmi_system_id uefi_skip_cert[] __initconst = {
{ UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,1") },
{ UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,2") },
{ UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,3") },
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20260121-uefi-certs-initconst-d5dadc5f01dd
Best regards,
--
Thomas Weißschuh <thomas.weissschuh@linutronix.de>
^ permalink raw reply related
* [PATCH] ima_setup.sh: Fix check of signed policy requirement
From: Petr Vorel @ 2026-01-21 8:33 UTC (permalink / raw)
To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity
Kernel code in arch_get_ima_policy() depends also on
CONFIG_IMA_ARCH_POLICY added in v5.0:
d958083a8f640 ("x86/ima: define arch_get_ima_policy() for x86")
Fixes: c38b528783 ("ima_{conditionals, policy}: Handle policy required to be signed")
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi Mimi, all,
FYI I'd like to merge it this week to get it into LTP release.
Kind regards,
Petr
testcases/kernel/security/integrity/ima/tests/ima_setup.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index 1bce78d425..df0b8d1532 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -466,10 +466,11 @@ require_evmctl()
}
# 56dc986a6b20b ("ima: require signed IMA policy when UEFI secure boot is enabled") # v6.5-rc4
+# d958083a8f640 ("x86/ima: define arch_get_ima_policy() for x86") # v5.0
check_need_signed_policy()
{
tst_secureboot_enabled && tst_kvcmp -ge '6.5' && tst_require_kconfigs \
- 'CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY'
+ 'CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY,CONFIG_IMA_ARCH_POLICY'
}
# loop device is needed to use only for tmpfs
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Mimi Zohar @ 2026-01-21 15:29 UTC (permalink / raw)
To: Dave Hansen, Ard Biesheuvel
Cc: Coiby Xu, linux-integrity, Heiko Carstens, Roberto Sassu,
Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <79185163-bf8f-4490-9396-3fd73b7a0c73@intel.com>
Hi Dave!
On Mon, 2026-01-19 at 10:44 -0800, Dave Hansen wrote:
> On 1/18/26 10:25, Mimi Zohar wrote:
> > As not all arch's implement arch_integrity_get_secureboot, the definition in
> > include/linux/integrity.h would need to be updated. Something like:
> >
> > -#ifdef CONFIG_INTEGRITY_SECURE_BOOT
> > +#if (defined(CONFIG_INTEGRITY_SECURE_BOOT) && \
> > + (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
> > + || defined(CONFIG_PPC_SECURE_BOOT))
> >
> > Then IMA_SECURE_AND_OR_TRUSTED_BOOT and EVM could select INTEGRITY_SECURE_BOOT,
> > as suggested.
>
> This seems to be going a wee bit sideways. :)
Agreed, that was my point. :) "imply" was cleaner, but Ard objected to two
imply's.
>
> This kind of CONFIG complexity really should be left to Kconfig. C
> macros really aren't a great place to do it.
>
> The other idiom we use a lot is this in generic code:
>
> #ifndef arch_foo
> static inline void arch_foo(void) {}
> #endif
>
> Then all you have to do is make sure the arch header that #defines it is
> included before the generic code. I'm not a super huge fan of these
> because it can be hard to tell (for humans at least) _if_ the
> architecture has done the #define.
>
> But it sure beats that #ifdef maze.
Sure.
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Mimi Zohar @ 2026-01-21 15:40 UTC (permalink / raw)
To: Coiby Xu, Ard Biesheuvel, Dave Hansen
Cc: linux-integrity, Heiko Carstens, Roberto Sassu, Catalin Marinas,
Will Deacon, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <aW2i3yacr5TvWU-m@Rk>
On Mon, 2026-01-19 at 12:04 +0800, Coiby Xu wrote:
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 976e75f9b9ba..5dce572192d6 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -311,6 +311,7 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> config IMA_SECURE_AND_OR_TRUSTED_BOOT
> bool
> depends on IMA_ARCH_POLICY
> + depends on INTEGRITY_SECURE_BOOT
>
>
> Another idea is make a tree-wide arch_get_secureboot i.e. to move
> current arch_ima_get_secureboot code to arch-specific secure boot
> implementation. By this way, there will no need for a new Kconfig option
> INTEGRITY_SECURE_BOOT. But I'm not sure if there is any unforeseen
> concern.
Originally basing IMA policy on the secure boot mode was an exception. As long
as making it public isn't an issue any longer, this sounds to me. Ard, Dave, do
you have any issues with replacing arch_ima_get_secureboot() with
arch_get_secureboot()?
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Ard Biesheuvel @ 2026-01-21 16:25 UTC (permalink / raw)
To: Mimi Zohar
Cc: Coiby Xu, Dave Hansen, linux-integrity, Heiko Carstens,
Roberto Sassu, Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <1a0b6e5601a673a81f8823de0815f92b7afbeb60.camel@linux.ibm.com>
On Wed, 21 Jan 2026 at 16:41, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Mon, 2026-01-19 at 12:04 +0800, Coiby Xu wrote:
>
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 976e75f9b9ba..5dce572192d6 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -311,6 +311,7 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> > config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > bool
> > depends on IMA_ARCH_POLICY
> > + depends on INTEGRITY_SECURE_BOOT
> >
> >
> > Another idea is make a tree-wide arch_get_secureboot i.e. to move
> > current arch_ima_get_secureboot code to arch-specific secure boot
> > implementation. By this way, there will no need for a new Kconfig option
> > INTEGRITY_SECURE_BOOT. But I'm not sure if there is any unforeseen
> > concern.
>
> Originally basing IMA policy on the secure boot mode was an exception. As long
> as making it public isn't an issue any longer, this sounds to me. Ard, Dave, do
> you have any issues with replacing arch_ima_get_secureboot() with
> arch_get_secureboot()?
I don't see an issue with that. If there is a legitimate need to
determine this even if IMA is not enabled, then this makes sense.
^ permalink raw reply
* [PATCH] evm: Use ordered xattrs list to calculate HMAC in evm_init_hmac()
From: Roberto Sassu @ 2026-01-22 8:07 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, eric.snowberg, paul, jmorris, serge
Cc: linux-integrity, linux-security-module, linux-kernel,
Roberto Sassu
From: Roberto Sassu <roberto.sassu@huawei.com>
Commit 8e5d9f916a96 ("smack: deduplicate xattr setting in
smack_inode_init_security()") introduced xattr_dupval() to simplify setting
the xattrs to be provided by the SMACK LSM on inode creation, in the
smack_inode_init_security().
Unfortunately, moving lsm_get_xattr_slot() caused the SMACK64TRANSMUTE
xattr be added in the array of new xattrs before SMACK64. This causes the
HMAC of xattrs calculated by evm_init_hmac() for new files to diverge from
the one calculated by both evm_calc_hmac_or_hash() and evmctl.
evm_init_hmac() calculates the HMAC of the xattrs of new files based on the
order LSMs provide them, while evm_calc_hmac_or_hash() and evmctl calculate
the HMAC based on an ordered xattrs list.
Fix the issue by making evm_init_hmac() calculate the HMAC of new files
based on the ordered xattrs list too.
Fixes: 8e5d9f916a96 ("smack: deduplicate xattr setting in smack_inode_init_security()")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/evm/evm_crypto.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57..5a8cef45bacf 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -401,6 +401,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
{
struct shash_desc *desc;
const struct xattr *xattr;
+ struct xattr_list *xattr_entry;
desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
if (IS_ERR(desc)) {
@@ -408,11 +409,16 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
return PTR_ERR(desc);
}
- for (xattr = xattrs; xattr->name; xattr++) {
- if (!evm_protected_xattr(xattr->name))
- continue;
+ list_for_each_entry_lockless(xattr_entry, &evm_config_xattrnames,
+ list) {
+ for (xattr = xattrs; xattr->name; xattr++) {
+ if (strcmp(xattr_entry->name +
+ XATTR_SECURITY_PREFIX_LEN, xattr->name) != 0)
+ continue;
- crypto_shash_update(desc, xattr->value, xattr->value_len);
+ crypto_shash_update(desc, xattr->value,
+ xattr->value_len);
+ }
}
hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Roberto Sassu @ 2026-01-22 9:52 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <20260120-xfs-ima-fixup-v2-1-f332ead8b043@cloudflare.com>
On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> introduced a means to track change detection for an inode
> via ctime updates, opposed to setting kstat.change_cookie to
> an i_version when calling into xfs_vn_getattr().
>
> This introduced a regression for IMA such that an action
> performed on a LOWER inode on a stacked file systems always
> requires a re-evaluation if the LOWER file system does not
> leverage kstat.change_cookie to track inode i_version or lacks
> i_version support all together.
>
> In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> leverages generic_fillattr() to update the change cookie.
>
> This is because IMA caches kstat.change_cookie to compare against an
> inode's i_version directly in integrity_inode_attrs_changed(), and thus
> could be out of date depending on how file systems set
> kstat.change_cookie.
>
> To address this, require integrity_inode_attrs_changed() to query
> vfs_getattr_nosec() to compare the cached version against
> kstat.change_cookie directly. This ensures that when updates occur,
> we're accessing the same changed inode version on changes, and fallback
> to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> result mask.
>
> Lastly, because EVM still relies on querying and caching a inode's
> i_version directly, the integrity_inode_attrs_changed() falls back to the
> original inode.i_version != cached comparison.
>
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> using multigrain ctime (as well as other file systems) for
> change detection in commit 1cf7e834a6fb ("xfs: switch to
> multigrain timestamps").
>
> Because file systems may implement i_version as they see fit, IMA
> caching may be behind as well as file systems that don't support/export
> i_version. Thus we're proposing to compare against the kstat.change_cookie
> directly to the cached version, and fall back to a ctime guard when
> that's not updated.
>
> EVM is largely left alone since there's no trivial way to query a file
> directly in the LSM call paths to obtain kstat.change_cookie &
> kstat.ctime to cache. Thus retains accessing i_version directly.
>
> Regression tests will be added to the Linux Test Project instead of
> selftest to help catch future file system changes that may impact
> future evaluation of IMA.
>
> I'd like this to be backported to at least 6.18 if possible.
>
> Below is a simplified test that demonstrates the issue:
>
> _fragment.config_
> CONFIG_XFS_FS=y
> CONFIG_OVERLAY_FS=y
> CONFIG_IMA=y
> CONFIG_IMA_WRITE_POLICY=y
> CONFIG_IMA_READ_POLICY=y
>
> _./test.sh_
>
> IMA_POLICY="/sys/kernel/security/ima/policy"
> TEST_BIN="/bin/date"
> MNT_BASE="/tmp/ima_test_root"
>
> mkdir -p "$MNT_BASE"
> mount -t tmpfs tmpfs "$MNT_BASE"
> mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
>
> dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> mkfs.xfs -q "$MNT_BASE/xfs.img"
> mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
>
> mount -t overlay overlay -o \
> "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> "$MNT_BASE/ovl"
>
> echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
>
> target_prog="$MNT_BASE/ovl/test_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
>
> audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
>
> if [[ "$audit_count" -eq 1 ]]; then
> echo "PASS: Found exactly 1 audit event."
> else
> echo "FAIL: Expected 1 audit event, but found $audit_count."
> exit 1
> fi
> ---
> Changes since RFC:
> - Remove calls to I_IS_VERSION()
> - Function documentation/comments
> - Abide IMA/EVM change detection fallback invariants
> - Combined ctime guard into version for attributes struct
> - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> ---
> Changes in v2:
> - Updated commit description + message to clarify the problem.
> - compare struct timespec64 to avoid collision possibility [Roberto].
> - Don't check inode_attr_changed() in ima_check_last_writer()
> - Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
> ---
> include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
> security/integrity/evm/evm_crypto.c | 4 +++-
> security/integrity/evm/evm_main.c | 5 ++---
> security/integrity/ima/ima_api.c | 20 +++++++++++++------
> security/integrity/ima/ima_main.c | 18 ++++++++++-------
> 5 files changed, 65 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -9,6 +9,8 @@
>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> +#include <linux/kernel.h>
> +#include <linux/time64.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
>
> /* An inode's attributes for detection of changes */
> struct integrity_inode_attributes {
> + struct timespec64 ctime;
I found the helper timespec64_to_ns(), I think it would be better for
memory occupation perspective to fit in the version field.
Thanks
Roberto
> u64 version; /* track inode changes */
> unsigned long ino;
> dev_t dev;
> @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> */
> static inline void
> integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> - u64 i_version, const struct inode *inode)
> + u64 i_version, struct timespec64 ctime,
> + const struct inode *inode)
> {
> + attrs->ctime = ctime;
> attrs->version = i_version;
> attrs->dev = inode->i_sb->s_dev;
> attrs->ino = inode->i_ino;
> @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
>
> /*
> * On stacked filesystems detect whether the inode or its content has changed.
> + *
> + * Must be called in process context.
> */
> static inline bool
> integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> - const struct inode *inode)
> + struct file *file, struct inode *inode)
> {
> - return (inode->i_sb->s_dev != attrs->dev ||
> - inode->i_ino != attrs->ino ||
> - !inode_eq_iversion(inode, attrs->version));
> + struct kstat stat;
> +
> + might_sleep();
> +
> + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> + return true;
> +
> + /*
> + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> + * is not a good indicator of i_version support, this still retains
> + * the logic such that a re-evaluation should still occur for EVM, and
> + * only for IMA if vfs_getattr_nosec() fails.
> + */
> + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT))
> + return !IS_I_VERSION(inode) ||
> + !inode_eq_iversion(inode, attrs->version);
> +
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + return stat.change_cookie != attrs->version;
> +
> + if (stat.result_mask & STATX_CTIME)
> + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> +
> + return true;
> }
>
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> hmac_add_misc(desc, inode, type, data->digest);
>
> if (inode != d_backing_inode(dentry) && iint) {
> + struct timespec64 ctime = {0};
> +
> if (IS_I_VERSION(inode))
> i_version = inode_query_iversion(inode);
> integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> - inode);
> + ctime, inode);
> }
>
> /* Portable EVM signatures must include an IMA hash */
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> bool ret = false;
>
> if (iint) {
> - ret = (!IS_I_VERSION(metadata_inode) ||
> - integrity_inode_attrs_changed(&iint->metadata_inode,
> - metadata_inode));
> + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> + NULL, metadata_inode);
> if (ret)
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> int length;
> void *tmpbuf;
> u64 i_version = 0;
> + struct timespec64 ctime = {0};
>
> /*
> * Always collect the modsig, because IMA might have already collected
> @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + if (stat.result_mask & STATX_CTIME)
> + ctime = stat.ctime;
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>
> iint->ima_hash = tmpbuf;
> memcpy(iint->ima_hash, &hash, length);
> - if (real_inode == inode)
> + if (real_inode == inode) {
> iint->real_inode.version = i_version;
> - else
> + iint->real_inode.ctime = ctime;
> + } else {
> integrity_inode_attrs_store(&iint->real_inode, i_version,
> - real_inode);
> + ctime, real_inode);
> + }
>
> /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> if (!result)
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -22,12 +22,14 @@
> #include <linux/mount.h>
> #include <linux/mman.h>
> #include <linux/slab.h>
> +#include <linux/stat.h>
> #include <linux/xattr.h>
> #include <linux/ima.h>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> #include <linux/evm.h>
> #include <linux/crash_dump.h>
> +#include <linux/time64.h>
>
> #include "ima.h"
>
> @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> &iint->atomic_flags);
> if ((iint->flags & IMA_NEW_FILE) ||
> vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> - AT_STATX_SYNC_AS_STAT) ||
> - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> - stat.change_cookie != iint->real_inode.version) {
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT) ||
> + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> + stat.change_cookie != iint->real_inode.version :
> + (!(stat.result_mask & STATX_CTIME) ||
> + !timespec64_equal(&stat.ctime,
> + &iint->real_inode.ctime)))) {
> iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> iint->measured_pcrs = 0;
> if (update)
> @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> real_inode = d_real_inode(file_dentry(file));
> if (real_inode != inode &&
> (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> - if (!IS_I_VERSION(real_inode) ||
> - integrity_inode_attrs_changed(&iint->real_inode,
> - real_inode)) {
> + if (integrity_inode_attrs_changed(&iint->real_inode,
> + file, real_inode)) {
> iint->flags &= ~IMA_DONE_MASK;
> iint->measured_pcrs = 0;
> }
>
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251212-xfs-ima-fixup-931780a62c2c
>
> Best regards,
^ permalink raw reply
* [PATCH v3] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Frederick Lawler @ 2026-01-22 16:49 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
introduced a means to track change detection for an inode
via ctime updates, opposed to setting kstat.change_cookie to
an i_version when calling into xfs_vn_getattr().
This introduced a regression for IMA such that an action
performed on a LOWER inode on a stacked file systems always
requires a re-evaluation if the LOWER file system does not
leverage kstat.change_cookie to track inode i_version or lacks
i_version support all together.
In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
will require re-evaluation. Stacking TMPFS on XFS for instance, once the
inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
leverages generic_fillattr() to update the change cookie.
This is because IMA caches kstat.change_cookie to compare against an
inode's i_version directly in integrity_inode_attrs_changed(), and thus
could be out of date depending on how file systems set
kstat.change_cookie.
To address this, require integrity_inode_attrs_changed() to query
vfs_getattr_nosec() to compare the cached version against
kstat.change_cookie directly. This ensures that when updates occur,
we're accessing the same changed inode version on changes, and fallback
to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
result mask.
Lastly, because EVM still relies on querying and caching a inode's
i_version directly, the integrity_inode_attrs_changed() falls back to the
original inode.i_version != cached comparison.
Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
We uncovered a case in kernels >= 6.13 where XFS is no longer updating
struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
using multigrain ctime (as well as other file systems) for
change detection in commit 1cf7e834a6fb ("xfs: switch to
multigrain timestamps").
Because file systems may implement i_version as they see fit, IMA
caching may be behind as well as file systems that don't support/export
i_version. Thus we're proposing to compare against the kstat.change_cookie
directly to the cached version, and fall back to a ctime guard when
that's not updated.
EVM is largely left alone since there's no trivial way to query a file
directly in the LSM call paths to obtain kstat.change_cookie &
kstat.ctime to cache. Thus retains accessing i_version directly.
Regression tests will be added to the Linux Test Project instead of
selftest to help catch future file system changes that may impact
future evaluation of IMA.
I'd like this to be backported to at least 6.18 if possible.
Below is a simplified test that demonstrates the issue:
_fragment.config_
CONFIG_XFS_FS=y
CONFIG_OVERLAY_FS=y
CONFIG_IMA=y
CONFIG_IMA_WRITE_POLICY=y
CONFIG_IMA_READ_POLICY=y
_./test.sh_
IMA_POLICY="/sys/kernel/security/ima/policy"
TEST_BIN="/bin/date"
MNT_BASE="/tmp/ima_test_root"
mkdir -p "$MNT_BASE"
mount -t tmpfs tmpfs "$MNT_BASE"
mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
mkfs.xfs -q "$MNT_BASE/xfs.img"
mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
mount -t overlay overlay -o \
"lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
"$MNT_BASE/ovl"
echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
target_prog="$MNT_BASE/ovl/test_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
if [[ "$audit_count" -eq 1 ]]; then
echo "PASS: Found exactly 1 audit event."
else
echo "FAIL: Expected 1 audit event, but found $audit_count."
exit 1
fi
---
Changes in v3:
- Prefer timespec64_to_ns() to leverage attr.version. [Roberto]
- s/TPMFS/TMPFS/ in description.
- Link to v2: https://lore.kernel.org/r/20260120-xfs-ima-fixup-v2-1-f332ead8b043@cloudflare.com
Changes in v2:
- Updated commit description + message to clarify the problem.
- compare struct timespec64 to avoid collision possibility [Roberto].
- Don't check inode_attr_changed() in ima_check_last_writer()
- Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
Changes since RFC:
- Remove calls to I_IS_VERSION()
- Function documentation/comments
- Abide IMA/EVM change detection fallback invariants
- Combined ctime guard into version for attributes struct
- Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
---
include/linux/integrity.h | 35 +++++++++++++++++++++++++++++++----
security/integrity/evm/evm_main.c | 5 ++---
security/integrity/ima/ima_api.c | 11 ++++++++---
security/integrity/ima/ima_main.c | 17 ++++++++++-------
4 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..034f0a1ed48ca8c19c764e302bbfc555dad92cde 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -9,6 +9,8 @@
#include <linux/fs.h>
#include <linux/iversion.h>
+#include <linux/kernel.h>
+#include <linux/time64.h>
enum integrity_status {
INTEGRITY_PASS = 0,
@@ -51,14 +53,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
/*
* On stacked filesystems detect whether the inode or its content has changed.
+ *
+ * Must be called in process context.
*/
static inline bool
integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
- const struct inode *inode)
+ struct file *file, struct inode *inode)
{
- return (inode->i_sb->s_dev != attrs->dev ||
- inode->i_ino != attrs->ino ||
- !inode_eq_iversion(inode, attrs->version));
+ struct kstat stat;
+
+ might_sleep();
+
+ if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
+ return true;
+
+ /*
+ * EVM currently relies on backing inode i_version. While IS_I_VERSION
+ * is not a good indicator of i_version support, this still retains
+ * the logic such that a re-evaluation should still occur for EVM, and
+ * only for IMA if vfs_getattr_nosec() fails.
+ */
+ if (!file || vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT))
+ return !IS_I_VERSION(inode) ||
+ !inode_eq_iversion(inode, attrs->version);
+
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ return stat.change_cookie != attrs->version;
+
+ if (stat.result_mask & STATX_CTIME)
+ return timespec64_to_ns(&stat.ctime) != (s64)attrs->version;
+
+ return true;
}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
bool ret = false;
if (iint) {
- ret = (!IS_I_VERSION(metadata_inode) ||
- integrity_inode_attrs_changed(&iint->metadata_inode,
- metadata_inode));
+ ret = integrity_inode_attrs_changed(&iint->metadata_inode,
+ NULL, metadata_inode);
if (ret)
iint->evm_status = INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ else if (stat.result_mask & STATX_CTIME)
+ i_version = timespec64_to_ns(&stat.ctime);
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..8ac42b03740eb93bf23b15cb9039af6cd32aa999 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -28,6 +28,7 @@
#include <linux/iversion.h>
#include <linux/evm.h>
#include <linux/crash_dump.h>
+#include <linux/time64.h>
#include "ima.h"
@@ -199,10 +200,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
&iint->atomic_flags);
if ((iint->flags & IMA_NEW_FILE) ||
vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
- AT_STATX_SYNC_AS_STAT) ||
- !(stat.result_mask & STATX_CHANGE_COOKIE) ||
- stat.change_cookie != iint->real_inode.version) {
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT) ||
+ ((stat.result_mask & STATX_CHANGE_COOKIE) ?
+ stat.change_cookie != iint->real_inode.version :
+ (!(stat.result_mask & STATX_CTIME) ||
+ timespec64_to_ns(&stat.ctime) !=
+ (s64)iint->real_inode.version))) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
@@ -328,9 +332,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
real_inode = d_real_inode(file_dentry(file));
if (real_inode != inode &&
(action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
- if (!IS_I_VERSION(real_inode) ||
- integrity_inode_attrs_changed(&iint->real_inode,
- real_inode)) {
+ if (integrity_inode_attrs_changed(&iint->real_inode,
+ file, real_inode)) {
iint->flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@cloudflare.com>
^ permalink raw reply related
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