Message-Id: <20260130-xfs-ima-fixup-v5-0-57e84ea91712@cloudflare.com>
Date: Fri, 30 Jan 2026 16:39:53 -0600
From: Frederick Lawler <fred@...udflare.com>
To: Mimi Zohar <zohar@...ux.ibm.com>, 
 Roberto Sassu <roberto.sassu@...wei.com>, 
 Dmitry Kasatkin <dmitry.kasatkin@...il.com>, 
 Eric Snowberg <eric.snowberg@...cle.com>, Paul Moore <paul@...l-moore.com>, 
 James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>, 
 "Darrick J. Wong" <djwong@...nel.org>, 
 Christian Brauner <brauner@...nel.org>, Josef Bacik <josef@...icpanda.com>, 
 Jeff Layton <jlayton@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-integrity@...r.kernel.org, 
 linux-security-module@...r.kernel.org, kernel-team@...udflare.com, 
 Frederick Lawler <fred@...udflare.com>
Subject: [PATCH v5 0/3] ima: Detect changes to files via kstat changes
 rather than i_version

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
unnecessarily measures files in stacked file systems. This is due
to the LOWER or UPPER FS not updating kstat.change_cookie to the
recent i_version on request. Thus, for XFS, zero is being compared
against the inode's i_version directly, and is always behind.

We're proposing to compare against the kstat.change_cookie
directly to the cached version, and fall back to a ctime comparison,
if STATX_CHANGE_COOKIE is not supplied in the result mask.

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.

Patches 1 & 2 are preparation patches. Ideally patch 2 is squashed into
3, though not strictly necessary.

Below is a simplified test that demonstrates the issue such that
there are multiple unnecessary measurements occurring for actions on
a file in a stacked TMPFS on XFS, prior to the file moved over to TMPFS:

_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

Signed-off-by: Frederick Lawler <fred@...udflare.com>
---
Changes in v5:
- Split into patch series. [Mimi]
- Link to v4: https://lore.kernel.org/r/20260129-xfs-ima-fixup-v4-1-6bb89df7b6a3@cloudflare.com

Changes in v4:
- No functional changes.
- Add Reviewed-by & Fixes tags.
- Link to v3: https://lore.kernel.org/r/20260122-xfs-ima-fixup-v3-1-20335a8aa836@cloudflare.com

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

---
Frederick Lawler (3):
      ima: Unify vfs_getattr_nosec() stat comparisons under helper function
      ima: Make integrity_inode_attrs_changed() call into vfs
      ima: Use kstat.ctime as a fallback change detection for stacked fs

 include/linux/integrity.h         | 43 +++++++++++++++++++++++++++++++++++----
 security/integrity/evm/evm_main.c |  5 ++---
 security/integrity/ima/ima_api.c  | 11 +++++++---
 security/integrity/ima/ima_main.c | 11 +++++-----
 4 files changed, 54 insertions(+), 16 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c

Best regards,
-- 
Frederick Lawler <fred@...udflare.com>

Powered by blists - more mailing lists