* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Kuniyuki Iwashima @ 2026-01-08 10:17 UTC (permalink / raw)
To: Günther Noack
Cc: Justin Suess, Paul Moore, James Morris, Serge E . Hallyn,
Simon Horman, Mickaël Salaün, linux-security-module,
Tingmao Wang, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <aV5WTGvQB0XI8Q_N@google.com>
On Wed, Jan 7, 2026 at 4:49 AM Günther Noack <gnoack@google.com> wrote:
>
> On Tue, Jan 06, 2026 at 11:33:32PM -0800, Kuniyuki Iwashima wrote:
> > +VFS maintainers
> >
> > On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
> > >
> > > Hello!
> > >
> > > On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> > > > On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > > > Motivation
> > > > > ---
> > > > >
> > > > > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > > > > identifying object from a policy perspective is the path passed to
> > > > > connect(2). However, this operation currently restricts LSMs that rely
> > > > > on VFS-based mediation, because the pathname resolved during connect()
> > > > > is not preserved in a form visible to existing hooks before connection
> > > > > establishment.
> > > >
> > > > Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> > > > and security_unix_may_send() ?
> > >
> > > Thanks for bringing it up!
> > >
> > > That path is set by the process that acts as the listening side for
> > > the socket. The listening and the connecting process might not live
> > > in the same mount namespace, and in that case, it would not match the
> > > path which is passed by the client in the struct sockaddr_un.
> >
> > Thanks for the explanation !
> >
> > So basically what you need is resolving unix_sk(sk)->addr.name
> > by kern_path() and comparing its d_backing_inode(path.dentry)
> > with d_backing_inode (unix_sk(sk)->path.dendtry).
> >
> > If the new hook is only used by Landlock, I'd prefer doing that on
> > the existing connect() hooks.
>
> I've talked about that in the "Alternative: Use existing LSM hooks" section in
> https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
>
> If we resolve unix_sk(sk)->addr.name ourselves in the Landlock hook
> again, we would resolve the path twice: Once in unix_find_bsd() in
> net/unix/af_unix.c (the Time-Of-Use), and once in the Landlock
> security hook for the connect() operation (the Time-Of-Check).
>
> If I understand you correctly, you are suggesting that we check that
> the inode resolved by af_unix
> (d_backing_inode(unix_sk(sk)->path.dentry)) is the same as the one
> that we resolve in Landlock ourselves, and therefore we can detect the
> TOCTOU race and pretend that this is equivalent to the case where
> af_unix resolved to the same inode with the path that Landlock
> observed?
>
> If the walked file system hierarchy changes in between these two
> accesses, Landlock enforces the policy based on path elements that
> have changed in between.
>
> * We start with a Landlock policy where Unix connect() is restricted
> by default, but is permitted on "foo/bar2" and everything underneath
> it. The hierarchy is:
>
> foo/
> bar/
> baz.sock
> bar2/ <--- Landlock rule: socket connect() allowed here and below
>
> * We connect() to the path "foo/bar/baz.sock"
> * af_unix.c path lookup resolves "foo/bar/baz.sock" (TOU)
> This works because Landlock is not checked at this point yet.
> * In between the two lookups:
> * the directory foo/bar gets renamed to foo/bar.old
> * foo/bar2 gets moved to foo/bar
> * baz.sock gets moved into the (new) foo/bar directory
> * Landlock check: path lookup of "foo/bar/baz.sock" (TOC)
> and subsequent policy check using the resolved path.
>
> This succeeds because connect() is permitted on foo/bar2 and
> beneath. We also check that the resolved inode is the same as the
> one resolved by af_unix.c.
>
> And now the reasoning is basically that this is fine because the
> (inode) result of the two lookups was the same and we pretend that the
> Landlock path lookup was the one where the actual permission check was
> done?
Right. IIUC, even in your patch, the file could be renamed
while LSM is checking the path, no ? I think holding the
path ref does not lock concurrent rename operations.
To me, it's not a small race and basically it's the same with
the ops below,
sk1.bind('test')
sk1.listen()
os.rename('test', 'test2')
sk2.connect('test2')
sk1.bind('test')
sk1.listen()
sk2.connect('test1')
os.rename('test', 'test2')
and the important part is whether the path _was_ the
allowed one when LSM checked the path.
>
> Some pieces of this which I am still unsure about:
>
> * What we are supposed to do when the two resolved inodes are not the
> same, because we detected the race? We can not allow the connection
> in that case, but it would be wrong to deny it as well. I'm not
> sure whether returning one of the -ERESTART* variants is feasible in
> this place and bubbles up correctly to the system call / io_uring
> layer.
Imagine that the rename ops was done a bit earlier, which is
before the first lookup in unix_find_bsd(). Then, the socket
will not be found, and -ECONNREFUSED is returned.
LSM pcan pretend as such.
>
> * What if other kinds of permission checks happen on a different
> lookup code path? (If another stacked LSM had a similar
> implementation with yet another path lookup based on a different
> kind of policy, and if a race happened in between, it could at least
> be possible that for one variant of the path, it would be OK for
> Landlock but not the other LSM, and for the other variant of the
> path it would be OK for the other LSM but not Landlock, and then the
> connection could get accepted even if that would not have been
> allowed on one of the two paths alone.) I find this a somewhat
> brittle implementation approach.
Do you mean that the evaluation of the stacked LSMs could
return 0 if one of them allows it even though other LSMs deny ?
>
> * Would have to double check the unix_dgram_connect code paths in
> af_unix to see whether this is feasible for DGRAM sockets:
>
> There is a way to connect() a connectionless DGRAM socket, and in
> that case, the path lookup in af_unix happens normally only during
> connect(),
Note that connected DGRAM socket can send() data to other sockets
by specifying the peer name in each send(), and even they can
disconnect by connect(AF_UNSPEC).
> very far apart from the initial security_unix_may_send()
> LSM hook which is used for DGRAM sockets - It would be weird if we
> were able to connect() a DGRAM socket, thinking that now all path
> lookups are done, but then when you try to send a message through
> it, Landlock surprisingly does the path lookup again based on a very
> old and possibly outdated path. If Landlock's path lookup fails
> (e.g. because the path has disappeared, or because the inode now
> differs), retries won't be able to recover this any more. Normally,
> the path does not need to get resolved any more once the DGRAM
> socket is connected.
>
> Noteworthy: When Unix servers restart, they commonly unlink the old
> socket inode in the same place and create a new one with bind(). So
> as the time window for the race increases, it is actually a common
> scenario that a different inode with appear under the same path.
>
> I have to digest this idea a bit. I find it less intuitive than using
> the exact same struct path with a newly introduced hook, but it does
> admittedly mitigate the problem somewhat. I'm just not feeling very
> comfortable with security policy code that requires difficult
> reasoning. 🤔 Or maybe I interpreted too much into your suggestion. :)
> I'd be interested to hear what you think.
>
> —Günther
^ permalink raw reply
* [PATCH v6] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Ryan Foster @ 2026-01-07 21:51 UTC (permalink / raw)
To: serge; +Cc: foster.ryan.r, linux-kernel, linux-security-module, paul, selinux
In-Reply-To: <aV15rKEt3rvW3hBK@mail.hallyn.com>
Here's v6 with both fixes combined. The Dec 29 version you have in caps-next
is correct for the namespace config - v6 keeps that and adds the KUNIT=y
dependency to fix the Intel CI build error.
Changes in v6:
- Namespace config: all three namespaces are independent children of
init_user_ns (same as Dec 29 you reviewed)
- Build fix: depends on KUNIT=y prevents link errors when KUNIT=m
The Dec 30 patch accidentally reverted the namespace fix when I was adding the
KUNIT=y part. This v6 has both fixes working together.
Thanks, Ryan
Add comprehensive KUnit tests for the namespace-related capability
functions that Serge Hallyn refactored in commit 9891d2f79a9f
("Clarify the rootid_owns_currentns").
The tests verify:
- Basic functionality: UID 0 in init namespace, invalid vfsuid,
non-zero UIDs
- Actual namespace traversal: Creating user namespaces with different
UID mappings where uid 0 maps to different kuids (e.g., 1000, 2000,
3000)
- Hierarchy traversal: Testing multiple nested namespaces to verify
correct namespace hierarchy traversal
This addresses the feedback to "test the actual functionality" by
creating real user namespaces with different values for the
namespace's uid 0, rather than just basic input validation.
The test file is included at the end of commoncap.c when
CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, following the
standard kernel pattern (e.g., scsi_lib.c, ext4/mballoc.c). This
allows tests to access static functions in the same compilation unit
without modifying production code based on test configuration.
The tests require CONFIG_USER_NS to be enabled since they rely on user
namespace mapping functionality. The Kconfig dependency ensures the
tests only build when this requirement is met.
All 7 tests pass:
- test_vfsuid_root_in_currentns_init_ns
- test_vfsuid_root_in_currentns_invalid
- test_vfsuid_root_in_currentns_nonzero
- test_kuid_root_in_ns_init_ns_uid0
- test_kuid_root_in_ns_init_ns_nonzero
- test_kuid_root_in_ns_with_mapping
- test_kuid_root_in_ns_with_different_mappings
Updated MAINTAINER capabilities to include commoncap test
Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
---
MAINTAINERS | 1 +
security/Kconfig | 17 +++
security/commoncap.c | 4 +
security/commoncap_test.c | 288 ++++++++++++++++++++++++++++++++++++++
4 files changed, 310 insertions(+)
create mode 100644 security/commoncap_test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c0030e126fc8..6f162c736dfb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5682,6 +5682,7 @@ F: include/trace/events/capability.h
F: include/uapi/linux/capability.h
F: kernel/capability.c
F: security/commoncap.c
+F: security/commoncap_test.c
CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER
M: Kevin Tsai <ktsai@capellamicro.com>
diff --git a/security/Kconfig b/security/Kconfig
index 285f284dfcac..6a4393fce9a1 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -284,6 +284,23 @@ config LSM
If unsure, leave this as the default.
+config SECURITY_COMMONCAP_KUNIT_TEST
+ bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y && USER_NS
+ default KUNIT_ALL_TESTS
+ help
+ This builds the commoncap KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
source "security/Kconfig.hardening"
endmenu
diff --git a/security/commoncap.c b/security/commoncap.c
index 8a23dfab7fac..3399535808fe 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1521,3 +1521,7 @@ DEFINE_LSM(capability) = {
};
#endif /* CONFIG_SECURITY */
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+#include "commoncap_test.c"
+#endif
diff --git a/security/commoncap_test.c b/security/commoncap_test.c
new file mode 100644
index 000000000000..e9b278be37f1
--- /dev/null
+++ b/security/commoncap_test.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for commoncap.c security functions
+ *
+ * Tests for security-critical functions in the capability subsystem,
+ * particularly namespace-related capability checks.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+#include <linux/cred.h>
+#include <linux/mnt_idmapping.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/refcount.h>
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+
+/* Functions are static in commoncap.c, but we can call them since we're
+ * included in the same compilation unit when tests are enabled.
+ */
+
+/**
+ * test_vfsuid_root_in_currentns_init_ns - Test vfsuid_root_in_currentns with init ns
+ *
+ * Verifies that UID 0 in the init namespace correctly owns the current
+ * namespace when running in init_user_ns.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_init_ns(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create UID 0 in init namespace */
+ kuid = KUIDT_INIT(0);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* In init namespace, UID 0 should own current namespace */
+ KUNIT_EXPECT_TRUE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_invalid - Test vfsuid_root_in_currentns with invalid vfsuid
+ *
+ * Verifies that an invalid vfsuid correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_invalid(struct kunit *test)
+{
+ vfsuid_t invalid_vfsuid;
+
+ /* Use the predefined invalid vfsuid */
+ invalid_vfsuid = INVALID_VFSUID;
+
+ /* Invalid vfsuid should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(invalid_vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_nonzero - Test vfsuid_root_in_currentns with non-zero UID
+ *
+ * Verifies that a non-zero UID correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_nonzero(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create a non-zero UID */
+ kuid = KUIDT_INIT(1000);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* Non-zero UID should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_uid0 - Test kuid_root_in_ns with init namespace and UID 0
+ *
+ * Verifies that kuid_root_in_ns correctly identifies UID 0 in init namespace.
+ * This tests the core namespace traversal logic. In init namespace, UID 0
+ * maps to itself, so it should own the namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_uid0(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(0);
+ init_ns = &init_user_ns;
+
+ /* UID 0 should own init namespace */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_nonzero - Test kuid_root_in_ns with init namespace and non-zero UID
+ *
+ * Verifies that kuid_root_in_ns correctly rejects non-zero UIDs in init namespace.
+ * Only UID 0 should own a namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_nonzero(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(1000);
+ init_ns = &init_user_ns;
+
+ /* Non-zero UID should not own namespace */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * create_test_user_ns_with_mapping - Create a mock user namespace with UID mapping
+ *
+ * Creates a minimal user namespace structure for testing where uid 0 in the
+ * namespace maps to a specific kuid in the parent namespace.
+ *
+ * @test: KUnit test context
+ * @parent_ns: Parent namespace (typically init_user_ns)
+ * @mapped_kuid: The kuid that uid 0 in this namespace maps to in parent
+ *
+ * Returns: Pointer to allocated namespace, or NULL on failure
+ */
+static struct user_namespace *create_test_user_ns_with_mapping(struct kunit *test,
+ struct user_namespace *parent_ns,
+ kuid_t mapped_kuid)
+{
+ struct user_namespace *ns;
+ struct uid_gid_extent extent;
+
+ /* Allocate a test namespace - use kzalloc to zero all fields */
+ ns = kunit_kzalloc(test, sizeof(*ns), GFP_KERNEL);
+ if (!ns)
+ return NULL;
+
+ /* Initialize basic namespace structure fields */
+ ns->parent = parent_ns;
+ ns->level = parent_ns ? parent_ns->level + 1 : 0;
+ ns->owner = mapped_kuid;
+ ns->group = KGIDT_INIT(0);
+
+ /* Initialize ns_common structure */
+ refcount_set(&ns->ns.__ns_ref, 1);
+ ns->ns.inum = 0; /* Mock inum */
+
+ /* Set up uid mapping: uid 0 in this namespace maps to mapped_kuid in parent
+ * Format: first (uid in ns) : lower_first (kuid in parent) : count
+ * So: uid 0 in ns -> kuid mapped_kuid in parent
+ * This means from_kuid(ns, mapped_kuid) returns 0
+ */
+ extent.first = 0; /* uid 0 in this namespace */
+ extent.lower_first = __kuid_val(mapped_kuid); /* maps to this kuid in parent */
+ extent.count = 1;
+
+ ns->uid_map.extent[0] = extent;
+ ns->uid_map.nr_extents = 1;
+
+ /* Set up gid mapping: gid 0 maps to gid 0 in parent (simplified) */
+ extent.first = 0;
+ extent.lower_first = 0;
+ extent.count = 1;
+
+ ns->gid_map.extent[0] = extent;
+ ns->gid_map.nr_extents = 1;
+
+ return ns;
+}
+
+/**
+ * test_kuid_root_in_ns_with_mapping - Test kuid_root_in_ns with namespace where uid 0
+ * maps to different kuid
+ *
+ * Creates a user namespace where uid 0 maps to kuid 1000 in the parent namespace.
+ * Verifies that kuid_root_in_ns correctly identifies kuid 1000 as owning the namespace.
+ *
+ * Note: kuid_root_in_ns walks up the namespace hierarchy, so it checks the current
+ * namespace first, then parent, then parent's parent, etc. So:
+ * - kuid 1000 owns test_ns because from_kuid(test_ns, 1000) == 0
+ * - kuid 0 also owns test_ns because from_kuid(init_user_ns, 0) == 0
+ * (checked in parent)
+ *
+ * This tests the actual functionality as requested: creating namespaces with
+ * different values for the namespace's uid 0.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_mapping(struct kunit *test)
+{
+ struct user_namespace *test_ns;
+ struct user_namespace *parent_ns;
+ kuid_t mapped_kuid, other_kuid;
+
+ parent_ns = &init_user_ns;
+ mapped_kuid = KUIDT_INIT(1000);
+ other_kuid = KUIDT_INIT(2000);
+
+ test_ns = create_test_user_ns_with_mapping(test, parent_ns, mapped_kuid);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_ns);
+
+ /* kuid 1000 should own test_ns because it maps to uid 0 in test_ns */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(mapped_kuid, test_ns));
+
+ /* kuid 0 should also own test_ns (checked via parent init_user_ns) */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), test_ns));
+
+ /* Other kuids should not own test_ns */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(other_kuid, test_ns));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(500), test_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_with_different_mappings - Test with multiple namespaces
+ *
+ * Creates multiple user namespaces with different UID mappings to verify
+ * that kuid_root_in_ns correctly distinguishes between namespaces.
+ *
+ * Each namespace maps uid 0 to a different kuid, and we verify that each
+ * kuid only owns its corresponding namespace (plus kuid 0 owns all via
+ * init_user_ns parent).
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_different_mappings(struct kunit *test)
+{
+ struct user_namespace *ns1, *ns2, *ns3;
+
+ /* Create three independent namespaces, each mapping uid 0 to different kuids */
+ ns1 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(1000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns1);
+
+ ns2 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(2000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns2);
+
+ ns3 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(3000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns3);
+
+ /* Test ns1: kuid 1000 owns it, kuid 0 owns it (via parent), others do not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns1));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns1));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns1));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns1));
+
+ /* Test ns2: kuid 2000 owns it, kuid 0 owns it (via parent), others do not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns2));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns2));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns2));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns2));
+
+ /* Test ns3: kuid 3000 owns it, kuid 0 owns it (via parent), others do not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns3));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns3));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns3));
+}
+
+static struct kunit_case commoncap_test_cases[] = {
+ KUNIT_CASE(test_vfsuid_root_in_currentns_init_ns),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_invalid),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_uid0),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_with_mapping),
+ KUNIT_CASE(test_kuid_root_in_ns_with_different_mappings),
+ {}
+};
+
+static struct kunit_suite commoncap_test_suite = {
+ .name = "commoncap",
+ .test_cases = commoncap_test_cases,
+};
+
+kunit_test_suite(commoncap_test_suite);
+
+MODULE_LICENSE("GPL");
+
+#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
--
2.52.0
^ permalink raw reply related
* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Paul Moore @ 2026-01-07 21:54 UTC (permalink / raw)
To: Justin Suess
Cc: James Morris, Serge E . Hallyn, Kuniyuki Iwashima, Simon Horman,
Mickaël Salaün, Günther Noack,
linux-security-module, Tingmao Wang, netdev
In-Reply-To: <20251231213314.2979118-1-utilityemal77@gmail.com>
On Wed, Dec 31, 2025 at 4:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
>
> Hi,
>
> This patch introduces a new LSM hook unix_path_connect.
>
> The idea for this patch and the hook came from Günther Noack, who
> is cc'd. Much credit to him for the idea and discussion.
>
> This patch is based on the lsm next branch.
>
> Motivation
> ---
>
> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> identifying object from a policy perspective is the path passed to
> connect(2). However, this operation currently restricts LSMs that rely
> on VFS-based mediation, because the pathname resolved during connect()
> is not preserved in a form visible to existing hooks before connection
> establishment. As a result, LSMs such as Landlock cannot currently
> restrict connections to named UNIX domain sockets by their VFS path.
>
> This gap has been discussed previously (e.g. in the context of Landlock's
> path-based access controls). [1] [2]
>
> I've cc'd the netdev folks as well on this, as the placement of this hook is
> important and in a core unix socket function.
>
> Design Choices
> ---
>
> The hook is called in net/unix/af_unix.c in the function unix_find_bsd().
>
> The hook takes a single parameter, a const struct path* to the named unix
> socket to which the connection is being established.
>
> The hook takes place after normal permissions checks, and after the
> inode is determined to be a socket. It however, takes place before
> the socket is actually connected to.
>
> If the hook returns non-zero it will do a put on the path, and return.
>
> References
> ---
>
> [1]: https://github.com/landlock-lsm/linux/issues/36#issue-2354007438
> [2]: https://lore.kernel.org/linux-security-module/cover.1767115163.git.m@maowtm.org/
>
> Kind Regards,
> Justin Suess
>
> Justin Suess (1):
> lsm: Add hook unix_path_connect
>
> include/linux/lsm_hook_defs.h | 1 +
> include/linux/security.h | 6 ++++++
> net/unix/af_unix.c | 8 ++++++++
> security/security.c | 16 ++++++++++++++++
> 4 files changed, 31 insertions(+)
A couple of things related to the documentation aspects of this patch.
First, since this is just a single patch, and will need to be part of
a larger patchset to gain acceptance[1], please skip the cover letter
and ensure that the patch's description contains all the important
information. Similarly, while it is fine to include references to
other sources of discussion in the patch's description, the links
should not replace a proper explanation of the patch. Whenever you
are writing a patch description, imagine yourself ten years in the
future, on a plane with no/terrible network access, trying to debug an
issue and all you have for historical information is the git log. I
promise you, it's not as outlandish as it might seem ;)
[1] See my other reply regarding new LSM hook guidance; this patch
will need to be part of a larger patchset that actually makes use of
this hook.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Paul Moore @ 2026-01-07 21:43 UTC (permalink / raw)
To: Justin Suess
Cc: gnoack3000, gnoack, horms, jmorris, kuniyu, linux-security-module,
m, mic, netdev, serge
In-Reply-To: <20260101194551.4017198-1-utilityemal77@gmail.com>
On Thu, Jan 1, 2026 at 2:45 PM Justin Suess <utilityemal77@gmail.com> wrote:
> On 1/1/26 07:13, Günther Noack wrote:
> > On Wed, Dec 31, 2025 at 04:33:14PM -0500, Justin Suess wrote:
> >> Adds an LSM hook unix_path_connect.
> >>
> >> This hook is called to check the path of a named unix socket before a
> >> connection is initiated.
> >>
> >> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> >> Cc: Günther Noack <gnoack3000@gmail.com>
> >> ---
> >> include/linux/lsm_hook_defs.h | 1 +
> >> include/linux/security.h | 6 ++++++
> >> net/unix/af_unix.c | 8 ++++++++
> >> security/security.c | 16 ++++++++++++++++
> >> 4 files changed, 31 insertions(+)
...
> >> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> >> index 55cdebfa0da0..af1a6083a69b 100644
> >> --- a/net/unix/af_unix.c
> >> +++ b/net/unix/af_unix.c
> >> @@ -1226,6 +1226,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> >> if (!S_ISSOCK(inode->i_mode))
> >> goto path_put;
> >>
> >> + /*
> >> + * We call the hook because we know that the inode is a socket
> >> + * and we hold a valid reference to it via the path.
> >> + */
> >> + err = security_unix_path_connect(&path);
> >> + if (err)
> >> + goto path_put;
> >
> > In this place, the hook call is done also for the coredump socket.
> >
> > The coredump socket is a system-wide setting, and it feels weird to me
> > that unprivileged processes should be able to inhibit that connection?
>
> No I don't think they should be able to. Does this look better?
Expect more comments on this patch, but this is important enough that
I wanted to reply separately.
As a reminder, we do have guidance regarding the addition of new LSM
hooks, there is a pointer to the document in MAINTAINERS, but here is
a direct link to the relevant section:
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks
The guidance has three bullet points, the first, and perhaps most
important, states:
"Hooks should be designed to be LSM agnostic. While it is possible
that only one LSM might implement the hook at the time of submission,
the hook's behavior should be generic enough that other LSMs could
provide a meaningful implementation."
This is one of the reasons why we generally don't make the LSM hook
calls conditional on kernel state outside of the LSM, e.g.
SOCK_COREDUMP. While Landlock may not want to implement any access
controls on a SOCK_COREDUMP socket, it's entirely possible that
another LSM which doesn't have untrusted processes defining security
policy may want to use this as a point of access control or
visibility/auditing. Further, I think it would be a good idea to also
pass the @type and @flags parameter to the hook; at the very least
Landlock would need the flags parameter to check for SOCK_COREDUMP.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Jeff Layton @ 2026-01-07 21:10 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: <aV66am9A5MmdNPbY@CMGLRV3>
On Wed, 2026-01-07 at 13:56 -0600, Frederick Lawler wrote:
> On Tue, Jan 06, 2026 at 02:50:31PM -0500, Jeff Layton wrote:
> > On Tue, 2026-01-06 at 13:33 -0600, Frederick Lawler wrote:
> > > On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> > > > Hi Jeff,
> > > >
> > > > On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > > > > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > > > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > > > > is no longer able to correctly track inode.i_version due to the struct
> > > > > > kstat.change_cookie no longer containing an updated i_version.
> > > > > >
> > > > > > Introduce a fallback mechanism for IMA that instead tracks a
> > > > > > integrity_ctime_guard() in absence of or outdated i_version
> > > > > > for stacked file systems.
> > > > > >
> > > > > > EVM is left alone since it mostly cares about the backing inode.
> > > > > >
> > > > > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > > > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > > > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > > > ---
> > > > > > The motivation behind this was that file systems that use the
> > > > > > cookie to set the i_version for stacked file systems may still do so.
> > > > > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > > > > The assumption is that the ctime will be different if the i_version is
> > > > > > different anyway for non-stacked file systems.
> > > > > >
> > > > > > I'm not too pleased with passing in struct file* to
> > > > > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > > > > that for now, but I couldn't come up with another idea to get the
> > > > > > stat without coming up with a new stat function to accommodate just
> > > > > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > > > > file system support to EVM (which doesn't make much sense to me
> > > > > > at the moment).
> > > > > >
> > > > > > I plan on adding in self test infrastructure for the v1, but I would
> > > > > > like to get some early feedback on the approach first.
> > > > > > ---
> > > > > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > > > > security/integrity/evm/evm_crypto.c | 2 +-
> > > > > > security/integrity/evm/evm_main.c | 2 +-
> > > > > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > > > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > > > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > > > > >
> > > > > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > > > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > > > > --- a/include/linux/integrity.h
> > > > > > +++ b/include/linux/integrity.h
> > > > > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > > > > >
> > > > > > /* An inode's attributes for detection of changes */
> > > > > > struct integrity_inode_attributes {
> > > > > > + u64 ctime_guard;
> > > > > > u64 version; /* track inode changes */
> > > > > > unsigned long ino;
> > > > > > dev_t dev;
> > > > > > };
> > > > > >
> > > > > > +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.
> > > > > > */
> > > > > > static inline void
> > > > > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > > - u64 i_version, const struct inode *inode)
> > > > > > + u64 i_version, u64 ctime_guard,
> > > > > > + const struct inode *inode)
> > > > > > {
> > > > > > + attrs->ctime_guard = ctime_guard;
> > > > > > attrs->version = i_version;
> > > > > > attrs->dev = inode->i_sb->s_dev;
> > > > > > attrs->ino = inode->i_ino;
> > > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > > */
> > > > > > 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;
> > > > > > +
> > > > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > > > + inode->i_ino != attrs->ino)
> > > > > > + return true;
> > > > > > +
> > > > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > > > + return false;
> > > > > > +
> > > > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > > > + AT_STATX_SYNC_AS_STAT))
> > > > > > + return true;
> > > > > > +
> > > > >
> > > > > This is rather odd. You're sampling the i_version field directly, but
> > > > > if it's not equal then you go through ->getattr() to get the ctime.
> > > > >
> > > > > It's particularly odd since you don't know whether the i_version field
> > > > > is even implemented on the fs. On filesystems where it isn't, the
> > > > > i_version field generally stays at 0, so won't this never fall through
> > > > > to do the vfs_getattr_nosec() call on those filesystems?
> > > > >
> > > >
> > > > You're totally right. I didn't consider FS's caching the value at zero.
> > >
> > > Actually, I'm going to amend this. I think I did consider FSs without an
> > > implementation. Where this is called at, it is often guarded by a
> > > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> > > understanding this correctly, the check call doesn't occur unless the inode
> > > has i_version support.
> > >
> >
> >
> > It depends on what you mean by i_version support:
> >
> > That flag just tells the VFS that it needs to bump the i_version field
> > when updating timestamps. It's not a reliable indicator of whether the
> > i_version field is suitable for the purpose you want here.
> >
>
> So, it would make sense then to also remove those guards at the callsite
> then if the intention is to compare against the cookie & ctime regardless?
>
Yes, I'd say so. IS_I_VERSION() is not a reliable indicator of whether
the filesystem can provide a proper change attribute. For instance,
cephfs and NFS don't set that flag, but they can (sometimes) provide
STATX_CHANGE_COOKIE. Going through getattr() is a much more reliable
method.
> > The problem here and the one that we ultimately fixed with multigrain
> > timestamps is that XFS in particular will bump i_version on any change
> > to the log. That includes atime updates due to reads.
> >
> > XFS still tracks the i_version the way it always has, but we've stopped
> > getattr() from reporting it because it's not suitable for the purpose
> > that nfsd (and IMA) need it for.
> >
> > > It seems to me the suggestion then is to remove the IS_I_VERSION()
> > > checks guarding the call sites, grab both ctime and cookie from stat,
> > > and if IS_I_VERSION() use that, otherwise cookie, and compare
> > > against the cached i_version with one of those values, and then fall
> > > back to ctime?
> > >
> >
> > Not exactly.
> >
> > You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
> > then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
> > use that. If it's not then use the ctime.
>
> Ok, I think I understand. To reiterate my understanding, ignore calling
> into inode_eq_iversion() all together.
>
> return other_checks || ((mask & cookie) ? cache->i_version == cookie_val :
> compare_ctime())
>
Yes, that psuedocode looks about right. IS_I_VERSION(),
inode_eq_iversion(), etc. are really filesystem-internal APIs and not
something IMA should be relying on as a fstype-agnostic LSM.
> >
> > The part I'm not sure about is whether it's actually safe to do this.
> > vfs_getattr_nosec() can block in some situations. Is it ok to do this
> > in any context where integrity_inode_attrs_changed() may be called?
> >
> > ISTR that this was an issue at one point, but maybe isn't now that IMA
> > is an LSM?
>
> Poking around, callers to integrity_inode_attrs_changed() are currently behind
> mutex_lock(&iinit->mutex) (similar to vfs_getattr_nosec() calls), and currently
> only called from process_measurement().
>
> While I can't say for certain this will always be the case for future use
> use cases, would it be helpful to include a may_sleep() in
> integrity_inode_attrs_changed() to drive this point and make a comment?
>
Those kind of annotations do tend to be useful.
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Frederick Lawler @ 2026-01-07 19:56 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: <25b6d1b42ea07b058be4e4f48bb5a7c6b879b3ed.camel@kernel.org>
On Tue, Jan 06, 2026 at 02:50:31PM -0500, Jeff Layton wrote:
> On Tue, 2026-01-06 at 13:33 -0600, Frederick Lawler wrote:
> > On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> > > Hi Jeff,
> > >
> > > On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > > > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > > > is no longer able to correctly track inode.i_version due to the struct
> > > > > kstat.change_cookie no longer containing an updated i_version.
> > > > >
> > > > > Introduce a fallback mechanism for IMA that instead tracks a
> > > > > integrity_ctime_guard() in absence of or outdated i_version
> > > > > for stacked file systems.
> > > > >
> > > > > EVM is left alone since it mostly cares about the backing inode.
> > > > >
> > > > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > > ---
> > > > > The motivation behind this was that file systems that use the
> > > > > cookie to set the i_version for stacked file systems may still do so.
> > > > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > > > The assumption is that the ctime will be different if the i_version is
> > > > > different anyway for non-stacked file systems.
> > > > >
> > > > > I'm not too pleased with passing in struct file* to
> > > > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > > > that for now, but I couldn't come up with another idea to get the
> > > > > stat without coming up with a new stat function to accommodate just
> > > > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > > > file system support to EVM (which doesn't make much sense to me
> > > > > at the moment).
> > > > >
> > > > > I plan on adding in self test infrastructure for the v1, but I would
> > > > > like to get some early feedback on the approach first.
> > > > > ---
> > > > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > > > security/integrity/evm/evm_crypto.c | 2 +-
> > > > > security/integrity/evm/evm_main.c | 2 +-
> > > > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > > > >
> > > > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > > > --- a/include/linux/integrity.h
> > > > > +++ b/include/linux/integrity.h
> > > > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > > > >
> > > > > /* An inode's attributes for detection of changes */
> > > > > struct integrity_inode_attributes {
> > > > > + u64 ctime_guard;
> > > > > u64 version; /* track inode changes */
> > > > > unsigned long ino;
> > > > > dev_t dev;
> > > > > };
> > > > >
> > > > > +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.
> > > > > */
> > > > > static inline void
> > > > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > - u64 i_version, const struct inode *inode)
> > > > > + u64 i_version, u64 ctime_guard,
> > > > > + const struct inode *inode)
> > > > > {
> > > > > + attrs->ctime_guard = ctime_guard;
> > > > > attrs->version = i_version;
> > > > > attrs->dev = inode->i_sb->s_dev;
> > > > > attrs->ino = inode->i_ino;
> > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > */
> > > > > 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;
> > > > > +
> > > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > > + inode->i_ino != attrs->ino)
> > > > > + return true;
> > > > > +
> > > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > > + return false;
> > > > > +
> > > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > > + AT_STATX_SYNC_AS_STAT))
> > > > > + return true;
> > > > > +
> > > >
> > > > This is rather odd. You're sampling the i_version field directly, but
> > > > if it's not equal then you go through ->getattr() to get the ctime.
> > > >
> > > > It's particularly odd since you don't know whether the i_version field
> > > > is even implemented on the fs. On filesystems where it isn't, the
> > > > i_version field generally stays at 0, so won't this never fall through
> > > > to do the vfs_getattr_nosec() call on those filesystems?
> > > >
> > >
> > > You're totally right. I didn't consider FS's caching the value at zero.
> >
> > Actually, I'm going to amend this. I think I did consider FSs without an
> > implementation. Where this is called at, it is often guarded by a
> > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> > understanding this correctly, the check call doesn't occur unless the inode
> > has i_version support.
> >
>
>
> It depends on what you mean by i_version support:
>
> That flag just tells the VFS that it needs to bump the i_version field
> when updating timestamps. It's not a reliable indicator of whether the
> i_version field is suitable for the purpose you want here.
>
So, it would make sense then to also remove those guards at the callsite
then if the intention is to compare against the cookie & ctime regardless?
> The problem here and the one that we ultimately fixed with multigrain
> timestamps is that XFS in particular will bump i_version on any change
> to the log. That includes atime updates due to reads.
>
> XFS still tracks the i_version the way it always has, but we've stopped
> getattr() from reporting it because it's not suitable for the purpose
> that nfsd (and IMA) need it for.
>
> > It seems to me the suggestion then is to remove the IS_I_VERSION()
> > checks guarding the call sites, grab both ctime and cookie from stat,
> > and if IS_I_VERSION() use that, otherwise cookie, and compare
> > against the cached i_version with one of those values, and then fall
> > back to ctime?
> >
>
> Not exactly.
>
> You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
> then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
> use that. If it's not then use the ctime.
Ok, I think I understand. To reiterate my understanding, ignore calling
into inode_eq_iversion() all together.
return other_checks || ((mask & cookie) ? cache->i_version == cookie_val :
compare_ctime())
>
> The part I'm not sure about is whether it's actually safe to do this.
> vfs_getattr_nosec() can block in some situations. Is it ok to do this
> in any context where integrity_inode_attrs_changed() may be called?
>
> ISTR that this was an issue at one point, but maybe isn't now that IMA
> is an LSM?
Poking around, callers to integrity_inode_attrs_changed() are currently behind
mutex_lock(&iinit->mutex) (similar to vfs_getattr_nosec() calls), and currently
only called from process_measurement().
While I can't say for certain this will always be the case for future use
use cases, would it be helpful to include a may_sleep() in
integrity_inode_attrs_changed() to drive this point and make a comment?
>
> > >
> > > > Ideally, you should just call vfs_getattr_nosec() early on with
> > > > STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
> > > > STATX_CHANGE_COOKIE if it's set in the returned mask.
> > > >
> > >
> > > Yes, that makes sense.
> > >
> > > I'll spin that in v1, thanks!
> > >
> > > > > + return attrs->ctime_guard != integrity_ctime_guard(stat);
> > > > > }
> > > > >
> > > > >
> > > > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> > > > > --- a/security/integrity/evm/evm_crypto.c
> > > > > +++ b/security/integrity/evm/evm_crypto.c
> > > > > @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > > > if (IS_I_VERSION(inode))
> > > > > i_version = inode_query_iversion(inode);
> > > > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > > > - inode);
> > > > > + 0, 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..0712802628fd6533383f9855687e19bef7b771c7 100644
> > > > > --- a/security/integrity/evm/evm_main.c
> > > > > +++ b/security/integrity/evm/evm_main.c
> > > > > @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > > > if (iint) {
> > > > > ret = (!IS_I_VERSION(metadata_inode) ||
> > > > > integrity_inode_attrs_changed(&iint->metadata_inode,
> > > > > - 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..72bba8daa588a0f4e45e4249276edb54ca3d77ef 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;
> > > > > + u64 ctime_guard = 0;
> > > > >
> > > > > /*
> > > > > * Always collect the modsig, because IMA might have already collected
> > > > > @@ -272,10 +273,16 @@ 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_guard = integrity_ctime_guard(stat);
> > > > > + }
> > > > > hash.hdr.algo = algo;
> > > > > hash.hdr.length = hash_digest_size[algo];
> > > > >
> > > > > @@ -305,11 +312,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_guard = ctime_guard;
> > > > > + } else {
> > > > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > > > - real_inode);
> > > > > + ctime_guard, 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..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 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>
> > > > > @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > > > {
> > > > > fmode_t mode = file->f_mode;
> > > > > bool update;
> > > > > + int ret;
> > > > >
> > > > > if (!(mode & FMODE_WRITE))
> > > > > return;
> > > > > @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > > >
> > > > > 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) {
> > > > > + ret = vfs_getattr_nosec(&file->f_path, &stat,
> > > > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > > > + AT_STATX_SYNC_AS_STAT);
> > > > > + if ((iint->flags & IMA_NEW_FILE) || ret ||
> > > > > + (!ret && stat.change_cookie != iint->real_inode.version) ||
> > > > > + (!ret && integrity_ctime_guard(stat) !=
> > > > > + iint->real_inode.ctime_guard)) {
> > > > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > > > iint->measured_pcrs = 0;
> > > > > if (update)
> > > > > @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > > > if (!IS_I_VERSION(real_inode) ||
> > > > > integrity_inode_attrs_changed(&iint->real_inode,
> > > > > - 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,
> > > >
> > > > --
> > > > Jeff Layton <jlayton@kernel.org>
>
> --
> Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Project Financing Opportunity
From: SAUDI INVESTORS GROUP @ 2026-01-07 19:23 UTC (permalink / raw)
To: linux-security-module
Assalamu Alaikum,
My name is Rasheed Faisal. I represent a group of loan funding investors based in Saudi Arabia.
We would like to know if you are currently seeking financing for a project. If so, please share the required funding amount and your preferred loan tenure. I will review the details with our investor group and get back to you as soon as possible.
Thank you for your time, and I look forward to hearing from you.
Kind regards,
Rasheed Faisal
Email. office.rasheedfaisal@gmail.com
^ permalink raw reply
* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Günther Noack @ 2026-01-07 16:57 UTC (permalink / raw)
To: Justin Suess
Cc: Kuniyuki Iwashima, Paul Moore, James Morris, Serge E . Hallyn,
Simon Horman, Mickaël Salaün, linux-security-module,
Tingmao Wang, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <2da3f1ae-1fe1-40c4-8748-9fb371e696f0@gmail.com>
On Wed, Jan 07, 2026 at 07:19:02AM -0500, Justin Suess wrote:
> On 1/7/26 02:33, Kuniyuki Iwashima wrote:
> > +VFS maintainers
> >
> > On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
> >> Hello!
> >>
> >> On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> >>> On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> >>>> Motivation
> >>>> ---
> >>>>
> >>>> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> >>>> identifying object from a policy perspective is the path passed to
> >>>> connect(2). However, this operation currently restricts LSMs that rely
> >>>> on VFS-based mediation, because the pathname resolved during connect()
> >>>> is not preserved in a form visible to existing hooks before connection
> >>>> establishment.
> >>> Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> >>> and security_unix_may_send() ?
> >> Thanks for bringing it up!
> >>
> >> That path is set by the process that acts as the listening side for
> >> the socket. The listening and the connecting process might not live
> >> in the same mount namespace, and in that case, it would not match the
> >> path which is passed by the client in the struct sockaddr_un.
> > Thanks for the explanation !
> >
> > So basically what you need is resolving unix_sk(sk)->addr.name
> > by kern_path() and comparing its d_backing_inode(path.dentry)
> > with d_backing_inode (unix_sk(sk)->path.dendtry).
> >
> > If the new hook is only used by Landlock, I'd prefer doing that on
> > the existing connect() hooks.
> I see. Did you have a particular hook in mind to extend?
>
> One complication I see is whatever hook this gets added to
> would also need CONFIG_SECURITY_PATH, since logically this restriction
> would fall under it:
>
> From security/Kconfig:
>
> config SECURITY_PATH
> bool "Security hooks for pathname based access control"
> depends on SECURITY
> help
> This enables the security hooks for pathname based access control.
> If enabled, a security module can use these hooks to
> implement pathname based access controls.
> If you are unsure how to answer this question, answer N.
>
> config SECURITY_NETWORK
> bool "Socket and Networking Security Hooks"
> depends on SECURITY
> help
> This enables the socket and networking security hooks.
> If enabled, a security module can use these hooks to
> implement socket and networking access controls.
> If you are unsure how to answer this question, answer N.
>
> Logically, this type of access control falls under both categories, so must be
> gated by both features. No existing LSM hooks are gated by both afaik, so
> there is not really an existing logical place to extend an existing hook without
> changing what features are required to be enabled for existing users.
>
> I do see more uses for this hook that just landlock, bpf lsm hooks
> or other non-labeling LSMs like apparmor or TOMOYO could take advantage
> of this as well.
Apologies, I overlooked your reply earlier today.
The existing hooks that are called from af_unix.c are:
- security_unix_stream_connect() for SOCK_STREAM unix(7) sockets
- security_unix_may_send() for SOCK_DGRAM unix(7) sockets
Apart from that, at a higher level, there are also
- security_socket_connect()
- security_socket_sendmsg() and security_socket_recvmsg()
These are used from net/socket.c.
For the connectionless dgram Unix sockets, we would need to tell apart the cases
where sendmsg()/recvmsg() are used with and without a sockaddr. (Dgram sockets
can be either connected with connect() and then have a fixed sockaddr, or they
can be passed a remote sockaddr with each message send and receive operation.)
This can told apart in security_socket_sendmsg() from the msg argument, but it
doesn't look like we could tell it apart from security_unix_may_send().
Landlock already depends on CONFIG_SECURITY_NETWORK and CONFIG_SECURITY_PATH,
so we would not need to have further #ifdefs to use one of these hooks.
There are other difficulties I found which worry me and which I listed in the
other mail at https://lore.kernel.org/all/aV5WTGvQB0XI8Q_N@google.com/.
—Günther
^ permalink raw reply
* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Günther Noack @ 2026-01-07 12:49 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Justin Suess, Paul Moore, James Morris, Serge E . Hallyn,
Simon Horman, Mickaël Salaün, linux-security-module,
Tingmao Wang, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <CAAVpQUB6gnfovRZAg_BfVKPuS868dFj7HxthbxRL-nZvcsOzCg@mail.gmail.com>
On Tue, Jan 06, 2026 at 11:33:32PM -0800, Kuniyuki Iwashima wrote:
> +VFS maintainers
>
> On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
> >
> > Hello!
> >
> > On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> > > On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > > Motivation
> > > > ---
> > > >
> > > > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > > > identifying object from a policy perspective is the path passed to
> > > > connect(2). However, this operation currently restricts LSMs that rely
> > > > on VFS-based mediation, because the pathname resolved during connect()
> > > > is not preserved in a form visible to existing hooks before connection
> > > > establishment.
> > >
> > > Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> > > and security_unix_may_send() ?
> >
> > Thanks for bringing it up!
> >
> > That path is set by the process that acts as the listening side for
> > the socket. The listening and the connecting process might not live
> > in the same mount namespace, and in that case, it would not match the
> > path which is passed by the client in the struct sockaddr_un.
>
> Thanks for the explanation !
>
> So basically what you need is resolving unix_sk(sk)->addr.name
> by kern_path() and comparing its d_backing_inode(path.dentry)
> with d_backing_inode (unix_sk(sk)->path.dendtry).
>
> If the new hook is only used by Landlock, I'd prefer doing that on
> the existing connect() hooks.
I've talked about that in the "Alternative: Use existing LSM hooks" section in
https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
If we resolve unix_sk(sk)->addr.name ourselves in the Landlock hook
again, we would resolve the path twice: Once in unix_find_bsd() in
net/unix/af_unix.c (the Time-Of-Use), and once in the Landlock
security hook for the connect() operation (the Time-Of-Check).
If I understand you correctly, you are suggesting that we check that
the inode resolved by af_unix
(d_backing_inode(unix_sk(sk)->path.dentry)) is the same as the one
that we resolve in Landlock ourselves, and therefore we can detect the
TOCTOU race and pretend that this is equivalent to the case where
af_unix resolved to the same inode with the path that Landlock
observed?
If the walked file system hierarchy changes in between these two
accesses, Landlock enforces the policy based on path elements that
have changed in between.
* We start with a Landlock policy where Unix connect() is restricted
by default, but is permitted on "foo/bar2" and everything underneath
it. The hierarchy is:
foo/
bar/
baz.sock
bar2/ <--- Landlock rule: socket connect() allowed here and below
* We connect() to the path "foo/bar/baz.sock"
* af_unix.c path lookup resolves "foo/bar/baz.sock" (TOU)
This works because Landlock is not checked at this point yet.
* In between the two lookups:
* the directory foo/bar gets renamed to foo/bar.old
* foo/bar2 gets moved to foo/bar
* baz.sock gets moved into the (new) foo/bar directory
* Landlock check: path lookup of "foo/bar/baz.sock" (TOC)
and subsequent policy check using the resolved path.
This succeeds because connect() is permitted on foo/bar2 and
beneath. We also check that the resolved inode is the same as the
one resolved by af_unix.c.
And now the reasoning is basically that this is fine because the
(inode) result of the two lookups was the same and we pretend that the
Landlock path lookup was the one where the actual permission check was
done?
Some pieces of this which I am still unsure about:
* What we are supposed to do when the two resolved inodes are not the
same, because we detected the race? We can not allow the connection
in that case, but it would be wrong to deny it as well. I'm not
sure whether returning one of the -ERESTART* variants is feasible in
this place and bubbles up correctly to the system call / io_uring
layer.
* What if other kinds of permission checks happen on a different
lookup code path? (If another stacked LSM had a similar
implementation with yet another path lookup based on a different
kind of policy, and if a race happened in between, it could at least
be possible that for one variant of the path, it would be OK for
Landlock but not the other LSM, and for the other variant of the
path it would be OK for the other LSM but not Landlock, and then the
connection could get accepted even if that would not have been
allowed on one of the two paths alone.) I find this a somewhat
brittle implementation approach.
* Would have to double check the unix_dgram_connect code paths in
af_unix to see whether this is feasible for DGRAM sockets:
There is a way to connect() a connectionless DGRAM socket, and in
that case, the path lookup in af_unix happens normally only during
connect(), very far apart from the initial security_unix_may_send()
LSM hook which is used for DGRAM sockets - It would be weird if we
were able to connect() a DGRAM socket, thinking that now all path
lookups are done, but then when you try to send a message through
it, Landlock surprisingly does the path lookup again based on a very
old and possibly outdated path. If Landlock's path lookup fails
(e.g. because the path has disappeared, or because the inode now
differs), retries won't be able to recover this any more. Normally,
the path does not need to get resolved any more once the DGRAM
socket is connected.
Noteworthy: When Unix servers restart, they commonly unlink the old
socket inode in the same place and create a new one with bind(). So
as the time window for the race increases, it is actually a common
scenario that a different inode with appear under the same path.
I have to digest this idea a bit. I find it less intuitive than using
the exact same struct path with a newly introduced hook, but it does
admittedly mitigate the problem somewhat. I'm just not feeling very
comfortable with security policy code that requires difficult
reasoning. 🤔 Or maybe I interpreted too much into your suggestion. :)
I'd be interested to hear what you think.
—Günther
^ permalink raw reply
* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Justin Suess @ 2026-01-07 12:19 UTC (permalink / raw)
To: Kuniyuki Iwashima, Günther Noack
Cc: Paul Moore, James Morris, Serge E . Hallyn, Simon Horman,
Mickaël Salaün, linux-security-module, Tingmao Wang,
netdev, Alexander Viro, Christian Brauner
In-Reply-To: <CAAVpQUB6gnfovRZAg_BfVKPuS868dFj7HxthbxRL-nZvcsOzCg@mail.gmail.com>
On 1/7/26 02:33, Kuniyuki Iwashima wrote:
> +VFS maintainers
>
> On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
>> Hello!
>>
>> On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
>>> On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
>>>> Motivation
>>>> ---
>>>>
>>>> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
>>>> identifying object from a policy perspective is the path passed to
>>>> connect(2). However, this operation currently restricts LSMs that rely
>>>> on VFS-based mediation, because the pathname resolved during connect()
>>>> is not preserved in a form visible to existing hooks before connection
>>>> establishment.
>>> Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
>>> and security_unix_may_send() ?
>> Thanks for bringing it up!
>>
>> That path is set by the process that acts as the listening side for
>> the socket. The listening and the connecting process might not live
>> in the same mount namespace, and in that case, it would not match the
>> path which is passed by the client in the struct sockaddr_un.
> Thanks for the explanation !
>
> So basically what you need is resolving unix_sk(sk)->addr.name
> by kern_path() and comparing its d_backing_inode(path.dentry)
> with d_backing_inode (unix_sk(sk)->path.dendtry).
>
> If the new hook is only used by Landlock, I'd prefer doing that on
> the existing connect() hooks.
I see. Did you have a particular hook in mind to extend?
One complication I see is whatever hook this gets added to
would also need CONFIG_SECURITY_PATH, since logically this restriction
would fall under it:
From security/Kconfig:
config SECURITY_PATH
bool "Security hooks for pathname based access control"
depends on SECURITY
help
This enables the security hooks for pathname based access control.
If enabled, a security module can use these hooks to
implement pathname based access controls.
If you are unsure how to answer this question, answer N.
config SECURITY_NETWORK
bool "Socket and Networking Security Hooks"
depends on SECURITY
help
This enables the socket and networking security hooks.
If enabled, a security module can use these hooks to
implement socket and networking access controls.
If you are unsure how to answer this question, answer N.
Logically, this type of access control falls under both categories, so must be
gated by both features. No existing LSM hooks are gated by both afaik, so
there is not really an existing logical place to extend an existing hook without
changing what features are required to be enabled for existing users.
I do see more uses for this hook that just landlock, bpf lsm hooks
or other non-labeling LSMs like apparmor or TOMOYO could take advantage
of this as well.
Günther did you have anything to add?
>> For more details, see
>> https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
>> and
>> https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
>>
>> Justin: Maybe we could add that reasoning to the cover letter in the
>> next version of the patch?
>>
>> –Günther
^ permalink raw reply
* Re: [PATCH v3 2/3] ima: trim N IMA event log records
From: Roberto Sassu @ 2026-01-07 10:06 UTC (permalink / raw)
To: steven chen, linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260106020713.3994-3-chenste@linux.microsoft.com>
On Mon, 2026-01-05 at 18:07 -0800, steven chen wrote:
> Trim N entries of the IMA event logs. Clean the hash table if
> ima_flush_htable is set.
>
> Provide a userspace interface ima_trim_log that can be used to input
> number N to let kernel to trim N entries of IMA event logs. When read
> this interface, it returns number of entries trimmed last time.
>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
> .../admin-guide/kernel-parameters.txt | 4 +
> security/integrity/ima/ima.h | 2 +
> security/integrity/ima/ima_fs.c | 164 +++++++++++++++++-
> security/integrity/ima/ima_queue.c | 85 +++++++++
> 4 files changed, 251 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e92c0056e4e0..cd1a1d0bf0e2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2197,6 +2197,10 @@
> Use the canonical format for the binary runtime
> measurements, instead of host native format.
>
> + ima_flush_htable [IMA]
> + Flush the measurement list hash table when trim all
> + or a part of it for deletion.
> +
> ima_hash= [IMA]
> Format: { md5 | sha1 | rmd160 | sha256 | sha384
> | sha512 | ... }
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index e3d71d8d56e3..2102c523dca0 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
>
> #ifdef CONFIG_IMA_KEXEC
> void ima_measure_kexec_event(const char *event_name);
> +long ima_delete_event_log(long req_val);
> #else
> static inline void ima_measure_kexec_event(const char *event_name) {}
> +static inline long ima_delete_event_log(long req_val) { return 0; }
> #endif
>
> /*
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 87045b09f120..67ff0cfc3d3f 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -21,6 +21,9 @@
> #include <linux/rcupdate.h>
> #include <linux/parser.h>
> #include <linux/vmalloc.h>
> +#include <linux/ktime.h>
> +#include <linux/timekeeping.h>
> +#include <linux/ima.h>
>
> #include "ima.h"
>
> @@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>
> static int valid_policy = 1;
>
> +#define IMA_LOG_TRIM_REQ_LENGTH 11
> +#define IMA_LOG_TRIM_EVENT_LEN 256
Shouldn't this belong to the next patch?
> +
> +static long trimcount;
> +/* mutex protects atomicity of trimming measurement list
> + * and also protects atomicity the measurement list read
> + * write operation.
> + */
> +static DEFINE_MUTEX(ima_measure_lock);
> +static long ima_measure_users;
> +
> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> loff_t *ppos, atomic_long_t *val)
> {
> @@ -202,16 +216,77 @@ static const struct seq_operations ima_measurments_seqops = {
> .show = ima_measurements_show
> };
>
> +/*
> + * _ima_measurements_open - open the IMA measurements file
> + * @inode: inode of the file being opened
> + * @file: file being opened
> + * @seq_ops: sequence operations for the file
> + *
> + * Returns 0 on success, or negative error code.
> + * Implements mutual exclusion between readers and writer
> + * of the measurements file. Multiple readers are allowed,
> + * but writer get exclusive access only no other readers/writers.
> + * Readers is not allowed when there is a writer.
> + */
> +static int _ima_measurements_open(struct inode *inode, struct file *file,
> + const struct seq_operations *seq_ops)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + if (write && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + mutex_lock(&ima_measure_lock);
> + if ((write && ima_measure_users != 0) ||
> + (!write && ima_measure_users < 0)) {
> + mutex_unlock(&ima_measure_lock);
> + return -EBUSY;
> + }
> +
> + ret = seq_open(file, seq_ops);
> + if (ret < 0) {
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> + }
> +
> + if (write)
> + ima_measure_users--;
> + else
> + ima_measure_users++;
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> +}
> +
> static int ima_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_measurments_seqops);
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static int ima_measurements_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + mutex_lock(&ima_measure_lock);
> + ret = seq_release(inode, file);
> + if (!ret) {
> + if (write)
> + ima_measure_users++;
> + else
> + ima_measure_users--;
> + }
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> }
>
> static const struct file_operations ima_measurements_ops = {
> .open = ima_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> };
>
> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
> @@ -279,14 +354,83 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>
> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_ascii_measurements_seqops);
> + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
> }
>
> static const struct file_operations ima_ascii_measurements_ops = {
> .open = ima_ascii_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> +};
> +
> +static int ima_log_trim_open(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> +
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
> +{
> + char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
> + ssize_t len;
> +
> + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
> + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
> +}
> +
> +static ssize_t ima_log_trim_write(struct file *file,
> + const char __user *buf, size_t datalen, loff_t *ppos)
> +{
> + long count, n, ret;
> +
> + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + n = (int)datalen;
> +
> + ret = kstrtol_from_user(buf, n, 10, &count);
> + if (ret < 0)
> + goto out;
> +
> + ret = ima_delete_event_log(count);
> +
> + if (ret < 0)
> + goto out;
> +
> + trimcount = ret;
> +
> + ret = datalen;
> +out:
> + return ret;
> +}
> +
> +static int ima_log_trim_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> +
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return ima_measurements_release(inode, file);
> +}
> +
> +static const struct file_operations ima_log_trim_ops = {
> + .open = ima_log_trim_open,
> + .read = ima_log_trim_read,
> + .write = ima_log_trim_write,
> + .llseek = generic_file_llseek,
> + .release = ima_log_trim_release
> };
>
> static ssize_t ima_read_policy(char *path)
> @@ -528,6 +672,18 @@ int __init ima_fs_init(void)
> goto out;
> }
>
> + if (IS_ENABLED(CONFIG_IMA_LOG_TRIMMING)) {
> + dentry = securityfs_create_file("ima_trim_log",
> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
> + ima_dir, NULL, &ima_log_trim_ops);
> + if (IS_ERR(dentry)) {
> + ret = PTR_ERR(dentry);
> + goto out;
> + }
> + }
> +
> + trimcount = 0;
> +
> dentry = securityfs_create_file("runtime_measurements_count",
> S_IRUSR | S_IRGRP, ima_dir, NULL,
> &ima_measurements_count_ops);
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 590637e81ad1..33bb5414b8cc 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -22,6 +22,14 @@
>
> #define AUDIT_CAUSE_LEN_MAX 32
>
> +bool ima_flush_htable;
> +static int __init ima_flush_htable_setup(char *str)
> +{
> + ima_flush_htable = true;
> + return 1;
> +}
> +__setup("ima_flush_htable", ima_flush_htable_setup);
> +
> /* pre-allocated array of tpm_digest structures to extend a PCR */
> static struct tpm_digest *digests;
>
> @@ -220,6 +228,83 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> return result;
> }
>
> +/**
> + * ima_delete_event_log - delete IMA event entry
> + * @num_records: number of records to delete
> + *
> + * delete num_records entries off the measurement list.
> + * Returns the number of entries deleted, or negative error code.
This is not according to the format stated in the documentation.
> + */
> +long ima_delete_event_log(long num_records)
> +{
> + long len, cur = num_records, tmp_len = 0;
> + struct ima_queue_entry *qe, *qe_tmp;
> + LIST_HEAD(ima_measurements_staged);
> + struct list_head *list_ptr;
> +
> + if (num_records <= 0)
> + return num_records;
> +
> + if (!IS_ENABLED(CONFIG_IMA_LOG_TRIMMING))
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&ima_extend_list_mutex);
> + len = atomic_long_read(&ima_htable.len);
> +
> + if (num_records > len) {
> + mutex_unlock(&ima_extend_list_mutex);
> + return -ENOENT;
> + }
> +
> + list_ptr = &ima_measurements;
> +
> + if (cur == len) {
> + list_replace(&ima_measurements, &ima_measurements_staged);
> + INIT_LIST_HEAD(&ima_measurements);
> + atomic_long_set(&ima_htable.len, 0);
> + list_ptr = &ima_measurements_staged;
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size = 0;
Like in my patch, we should have kept the original value of
binary_runtime_size, to avoid breaking the kexec critical data records.
> + }
> +
> + list_for_each_entry(qe, list_ptr, later) {
> + if (num_records > 0) {
> + if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable)
> + hlist_del_rcu(&qe->hnext);
> +
> + --num_records;
> + if (num_records == 0)
> + qe_tmp = qe;
> + continue;
> + }
> + if (len != cur && IS_ENABLED(CONFIG_IMA_KEXEC))
> + tmp_len += get_binary_runtime_size(qe->entry);
> + else
> + break;
> + }
> +
> + if (len != cur) {
> + __list_cut_position(&ima_measurements_staged, &ima_measurements,
> + &qe_tmp->later);
> + atomic_long_sub(cur, &ima_htable.len);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size = tmp_len;
> + }
> +
> + mutex_unlock(&ima_extend_list_mutex);
> +
> + if (ima_flush_htable)
> + synchronize_rcu();
> +
> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
> + ima_free_template_entry(qe->entry);
> + list_del(&qe->later);
> + kfree(qe);
If you don't flush the hash table, you cannot delete the entry.
Roberto
> + }
> +
> + return cur;
> +}
> +
> int ima_restore_measurement_entry(struct ima_template_entry *entry)
> {
> int result = 0;
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jens Wiklander @ 2026-01-07 9:38 UTC (permalink / raw)
To: Sudeep Holla
Cc: Alexandre Belloni, Uwe Kleine-König, Jonathan Corbet,
Sumit Garg, Olivia Mackall, Herbert Xu, Clément Léger,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <aV0Qx5BOso5co3tm@bogus>
On Tue, Jan 6, 2026 at 2:40 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Mon, Jan 05, 2026 at 10:16:09AM +0100, Jens Wiklander wrote:
> > Hi,
> >
> > On Thu, Dec 18, 2025 at 5:29 PM Jens Wiklander
> > <jens.wiklander@linaro.org> wrote:
> > >
> > > On Thu, Dec 18, 2025 at 2:53 PM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > > >
> > > > On 18/12/2025 08:21:27+0100, Jens Wiklander wrote:
> > > > > Hi,
> > > > >
> > > > > On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
> > > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > the objective of this series is to make tee driver stop using callbacks
> > > > > > in struct device_driver. These were superseded by bus methods in 2006
> > > > > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > > > > methods.")) but nobody cared to convert all subsystems accordingly.
> > > > > >
> > > > > > Here the tee drivers are converted. The first commit is somewhat
> > > > > > unrelated, but simplifies the conversion (and the drivers). It
> > > > > > introduces driver registration helpers that care about setting the bus
> > > > > > and owner. (The latter is missing in all drivers, so by using these
> > > > > > helpers the drivers become more correct.)
> > > > > >
> > > > > > v1 of this series is available at
> > > > > > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> > > > > >
> > > > > > Changes since v1:
> > > > > >
> > > > > > - rebase to v6.19-rc1 (no conflicts)
> > > > > > - add tags received so far
> > > > > > - fix whitespace issues pointed out by Sumit Garg
> > > > > > - fix shutdown callback to shutdown and not remove
> > > > > >
> > > > > > As already noted in v1's cover letter, this series should go in during a
> > > > > > single merge window as there are runtime warnings when the series is
> > > > > > only applied partially. Sumit Garg suggested to apply the whole series
> > > > > > via Jens Wiklander's tree.
> > > > > > If this is done the dependencies in this series are honored, in case the
> > > > > > plan changes: Patches #4 - #17 depend on the first two.
> > > > > >
> > > > > > Note this series is only build tested.
> > > > > >
> > > > > > Uwe Kleine-König (17):
> > > > > > tee: Add some helpers to reduce boilerplate for tee client drivers
> > > > > > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > > > > > tee: Adapt documentation to cover recent additions
> > > > > > hwrng: optee - Make use of module_tee_client_driver()
> > > > > > hwrng: optee - Make use of tee bus methods
> > > > > > rtc: optee: Migrate to use tee specific driver registration function
> > > > > > rtc: optee: Make use of tee bus methods
> > > > > > efi: stmm: Make use of module_tee_client_driver()
> > > > > > efi: stmm: Make use of tee bus methods
> > > > > > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > > > > > firmware: arm_scmi: Make use of tee bus methods
> > > > > > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > > > > > firmware: tee_bnxt: Make use of tee bus methods
> > > > > > KEYS: trusted: Migrate to use tee specific driver registration
> > > > > > function
> > > > > > KEYS: trusted: Make use of tee bus methods
> > > > > > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > > > > > tpm/tpm_ftpm_tee: Make use of tee bus methods
> > > > > >
> > > > > > Documentation/driver-api/tee.rst | 18 +----
> > > > > > drivers/char/hw_random/optee-rng.c | 26 ++----
> > > > > > drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> > > > > > drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> > > > > > drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> > > > > > drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> > > > > > drivers/rtc/rtc-optee.c | 27 ++-----
> > > > > > drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> > > > > > include/linux/tee_drv.h | 12 +++
> > > > > > security/keys/trusted-keys/trusted_tee.c | 17 ++--
> > > > > > 10 files changed, 164 insertions(+), 138 deletions(-)
> > > > > >
> > > > > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > > > > --
> > > > > > 2.47.3
> > > > > >
> > > > >
> > > > > Thank you for the nice cleanup, Uwe.
> > > > >
> > > > > I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
> > > > > tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
> > > > >
> > > > > The branch is based on v6.19-rc1, and I'll try to keep it stable for
> > > > > others to depend on, if needed. Let's see if we can agree on taking
> > > > > the remaining patches via that branch.
> > > >
> > > > 6 and 7 can go through your branch.
> > >
> > > Good, I've added them to my branch now.
> >
> > This entire patch set should go in during a single merge window. I
> > will not send any pull request until I'm sure all patches will be
> > merged.
> >
> > So far (if I'm not mistaken), only the patches I've already added to
> > next have appeared next. I can take the rest of the patches, too, but
> > I need OK for the following:
> >
>
> [...]
>
> >
> > Sudeep, you seem happy with the following patches
> > - firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > - firmware: arm_scmi: Make use of tee bus methods
> > OK if I take them via my tree, or would you rather take them yourself?
> >
>
> I am happy if you want to take all of them in one go. I think I have
> already acked it. Please shout if you need anything else from me, happy to
> help in anyway to make it easier to handle this change set.
Thanks, I've applied all the patches in the series now, since it
otherwise causes warnings during boot.
/Jens
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jens Wiklander @ 2026-01-07 9:36 UTC (permalink / raw)
To: Jon Hunter
Cc: Uwe Kleine-König, Jonathan Corbet, Sumit Garg,
Olivia Mackall, Herbert Xu, Clément Léger,
Alexandre Belloni, Ard Biesheuvel, Maxime Coquelin,
Alexandre Torgue, Sumit Garg, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Rafał Miłecki,
Michael Chan, Pavan Chebbi, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-doc,
linux-crypto, linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, linux-mips, netdev, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe,
linux-tegra@vger.kernel.org
In-Reply-To: <d14a9c41-9df7-438f-bb58-097644d5d93f@nvidia.com>
Hi Jon,
On Tue, Jan 6, 2026 at 10:40 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
> Hi Uwe,
>
> On 15/12/2025 14:16, Uwe Kleine-König wrote:
> > Hello,
> >
> > the objective of this series is to make tee driver stop using callbacks
> > in struct device_driver. These were superseded by bus methods in 2006
> > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > methods.")) but nobody cared to convert all subsystems accordingly.
> >
> > Here the tee drivers are converted. The first commit is somewhat
> > unrelated, but simplifies the conversion (and the drivers). It
> > introduces driver registration helpers that care about setting the bus
> > and owner. (The latter is missing in all drivers, so by using these
> > helpers the drivers become more correct.)
> >
> > v1 of this series is available at
> > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> >
> > Changes since v1:
> >
> > - rebase to v6.19-rc1 (no conflicts)
> > - add tags received so far
> > - fix whitespace issues pointed out by Sumit Garg
> > - fix shutdown callback to shutdown and not remove
> >
> > As already noted in v1's cover letter, this series should go in during a
> > single merge window as there are runtime warnings when the series is
> > only applied partially. Sumit Garg suggested to apply the whole series
> > via Jens Wiklander's tree.
> > If this is done the dependencies in this series are honored, in case the
> > plan changes: Patches #4 - #17 depend on the first two.
> >
> > Note this series is only build tested.
> >
> > Uwe Kleine-König (17):
> > tee: Add some helpers to reduce boilerplate for tee client drivers
> > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > tee: Adapt documentation to cover recent additions
> > hwrng: optee - Make use of module_tee_client_driver()
> > hwrng: optee - Make use of tee bus methods
> > rtc: optee: Migrate to use tee specific driver registration function
> > rtc: optee: Make use of tee bus methods
> > efi: stmm: Make use of module_tee_client_driver()
> > efi: stmm: Make use of tee bus methods
> > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > firmware: arm_scmi: Make use of tee bus methods
> > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > firmware: tee_bnxt: Make use of tee bus methods
> > KEYS: trusted: Migrate to use tee specific driver registration
> > function
> > KEYS: trusted: Make use of tee bus methods
> > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > tpm/tpm_ftpm_tee: Make use of tee bus methods
>
>
> On the next-20260105 I am seeing the following warnings ...
>
> WARNING KERN Driver 'optee-rng' needs updating - please use bus_type methods
> WARNING KERN Driver 'scmi-optee' needs updating - please use bus_type methods
> WARNING KERN Driver 'tee_bnxt_fw' needs updating - please use bus_type methods
>
> I bisected the first warning and this point to the following
> commit ...
>
> # first bad commit: [a707eda330b932bcf698be9460e54e2f389e24b7] tee: Add some helpers to reduce boilerplate for tee client drivers
>
> I have not bisected the others, but guess they are related
> to this series. Do you observe the same?
Yes, I see the same.
I'm sorry, I didn't realize that someone might bisect this when I took
only a few of the patches into next. I've applied all the patches in
this series now.
Thanks,
Jens
^ permalink raw reply
* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Kuniyuki Iwashima @ 2026-01-07 7:33 UTC (permalink / raw)
To: Günther Noack
Cc: Justin Suess, Paul Moore, James Morris, Serge E . Hallyn,
Simon Horman, Mickaël Salaün, linux-security-module,
Tingmao Wang, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <aVuaqij9nXhLfAvN@google.com>
+VFS maintainers
On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
>
> Hello!
>
> On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> > On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > Motivation
> > > ---
> > >
> > > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > > identifying object from a policy perspective is the path passed to
> > > connect(2). However, this operation currently restricts LSMs that rely
> > > on VFS-based mediation, because the pathname resolved during connect()
> > > is not preserved in a form visible to existing hooks before connection
> > > establishment.
> >
> > Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> > and security_unix_may_send() ?
>
> Thanks for bringing it up!
>
> That path is set by the process that acts as the listening side for
> the socket. The listening and the connecting process might not live
> in the same mount namespace, and in that case, it would not match the
> path which is passed by the client in the struct sockaddr_un.
Thanks for the explanation !
So basically what you need is resolving unix_sk(sk)->addr.name
by kern_path() and comparing its d_backing_inode(path.dentry)
with d_backing_inode (unix_sk(sk)->path.dendtry).
If the new hook is only used by Landlock, I'd prefer doing that on
the existing connect() hooks.
>
> For more details, see
> https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
> and
> https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
>
> Justin: Maybe we could add that reasoning to the cover letter in the
> next version of the patch?
>
> –Günther
^ permalink raw reply
* Re: [PATCH v2 07/27] rust: cred: add __rust_helper to helpers
From: Alice Ryhl @ 2026-01-07 6:53 UTC (permalink / raw)
To: Paul Moore
Cc: rust-for-linux, linux-kernel, Boqun Feng, Gary Guo, Serge Hallyn,
linux-security-module
In-Reply-To: <CAHC9VhTqptD8_QWsUehss27AEhQq8oi6=rtA+dQTCAO1VoEYbQ@mail.gmail.com>
On Wed, Jan 7, 2026 at 2:29 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Jan 6, 2026 at 8:25 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Mon, Jan 5, 2026 at 7:42 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > >
> > > This is needed to inline these helpers into Rust code.
> > >
> > > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> > > Reviewed-by: Gary Guo <gary@garyguo.net>
> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > ---
> > > Cc: Paul Moore <paul@paul-moore.com>
> > > Cc: Serge Hallyn <sergeh@kernel.org>
> > > Cc: linux-security-module@vger.kernel.org
> > > ---
> > > rust/helpers/cred.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Acked-by: Paul Moore <paul@paul-moore.com>
>
> Ooops, sorry, I just saw that these were okay to merge into the
> various subsystem trees. Merged into lsm/dev, thanks.
Thanks!
^ permalink raw reply
* Re: [PATCH] lsm: make keys for static branch static
From: Paul Moore @ 2026-01-07 1:57 UTC (permalink / raw)
To: Ben Dooks, linux-kernel, linux-security-module
Cc: serge, jmorris, paul, Ben Dooks
In-Reply-To: <20260106171332.69558-1-ben.dooks@codethink.co.uk>
On Jan 6, 2026 Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> The key use for static-branches are not refrenced by name outside
> of the security/security.c file, so make them static. This stops
> the sparse warnings about "Should it be static?" such as:
>
> security/security.c: note: in included file:
> ./include/linux/lsm_hook_defs.h:29:1: warning: symbol 'security_hook_active_binder_set_context_mgr_0' was not declared. Should it be static?
> ./include/linux/lsm_hook_defs.h:29:1: warning: symbol 'security_hook_active_binder_set_context_mgr_1' was not declared. Should it be static?
> ./include/linux/lsm_hook_defs.h:29:1: warning: symbol 'security_hook_active_binder_set_context_mgr_2' was not declared. Should it be static?
> ./include/linux/lsm_hook_defs.h:30:1: warning: symbol 'security_hook_active_binder_transaction_0' was not declared. Should it be static?
> ...
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> security/security.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Merged into lsm/dev, thanks.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] Cred: Remove unused set_security_override_from_ctx()
From: Paul Moore @ 2026-01-07 1:52 UTC (permalink / raw)
To: Casey Schaufler, David Howells, Linux kernel mailing list,
Serge Hallyn
Cc: max.kellermann, LSM List, Casey Schaufler
In-Reply-To: <6c29c8ad-6aa8-4f50-98c8-81b363666ae8@schaufler-ca.com>
On Dec 22, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> The function set_security_override_from_ctx() has no in-tree callers
> since 6.14. Remove it.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> kernel/cred.c | 23 -----------------------
> 1 file changed, 23 deletions(-)
Merged into lsm/dev, thanks.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 18/27] rust: security: add __rust_helper to helpers
From: Paul Moore @ 2026-01-07 1:29 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, linux-kernel, Boqun Feng, Gary Guo,
Greg Kroah-Hartman, linux-security-module
In-Reply-To: <20260105-define-rust-helper-v2-18-51da5f454a67@google.com>
On Mon, Jan 5, 2026 at 7:43 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> This is needed to inline these helpers into Rust code.
>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-security-module@vger.kernel.org
> ---
> rust/helpers/security.c | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
Merged into lsm/dev, thanks.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 07/27] rust: cred: add __rust_helper to helpers
From: Paul Moore @ 2026-01-07 1:29 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, linux-kernel, Boqun Feng, Gary Guo, Serge Hallyn,
linux-security-module
In-Reply-To: <CAHC9VhQthnDRi3yXxnD8W_vAsxKOJPh8Zd1YxpF_fU5YGkj3SQ@mail.gmail.com>
On Tue, Jan 6, 2026 at 8:25 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Jan 5, 2026 at 7:42 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > This is needed to inline these helpers into Rust code.
> >
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> > Reviewed-by: Gary Guo <gary@garyguo.net>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Serge Hallyn <sergeh@kernel.org>
> > Cc: linux-security-module@vger.kernel.org
> > ---
> > rust/helpers/cred.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Acked-by: Paul Moore <paul@paul-moore.com>
Ooops, sorry, I just saw that these were okay to merge into the
various subsystem trees. Merged into lsm/dev, thanks.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 07/27] rust: cred: add __rust_helper to helpers
From: Paul Moore @ 2026-01-07 1:25 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, linux-kernel, Boqun Feng, Gary Guo, Serge Hallyn,
linux-security-module
In-Reply-To: <20260105-define-rust-helper-v2-7-51da5f454a67@google.com>
On Mon, Jan 5, 2026 at 7:42 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> This is needed to inline these helpers into Rust code.
>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Serge Hallyn <sergeh@kernel.org>
> Cc: linux-security-module@vger.kernel.org
> ---
> rust/helpers/cred.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] nfs: unify security_inode_listsecurity() calls
From: Paul Moore @ 2026-01-06 22:13 UTC (permalink / raw)
To: Stephen Smalley, trondmy, anna
Cc: okorniev, linux-nfs, linux-security-module, selinux,
Stephen Smalley
In-Reply-To: <20251203195728.8592-1-stephen.smalley.work@gmail.com>
On Dec 3, 2025 Stephen Smalley <stephen.smalley.work@gmail.com> wrote:
>
> commit 243fea134633 ("NFSv4.2: fix listxattr to return selinux
> security label") introduced a direct call to
> security_inode_listsecurity() in nfs4_listxattr(). However,
> nfs4_listxattr() already indirectly called
> security_inode_listsecurity() via nfs4_listxattr_nfs4_label() if
> CONFIG_NFS_V4_SECURITY_LABEL is enabled and the server has the
> NFS_CAP_SECURITY_LABEL capability enabled. This duplication was fixed
> by commit 9acb237deff7 ("NFSv4.2: another fix for listxattr") by
> making the second call conditional on NFS_CAP_SECURITY_LABEL not being
> set by the server. However, the combination of the two changes
> effectively makes one call to security_inode_listsecurity() in every
> case - which is the desired behavior since getxattr() always returns a
> security xattr even if it has to synthesize one. Further, the two
> different calls produce different xattr name ordering between
> security.* and user.* xattr names. Unify the two separate calls into a
> single call and get rid of nfs4_listxattr_nfs4_label() altogether.
>
> Link: https://lore.kernel.org/selinux/CAEjxPJ6e8z__=MP5NfdUxkOMQ=EnUFSjWFofP4YPwHqK=Ki5nw@mail.gmail.com/
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> fs/nfs/nfs4proc.c | 38 +++-----------------------------------
> 1 file changed, 3 insertions(+), 35 deletions(-)
It's been over a month without any comments, positive or negative, so
I'm going to go ahead and merge this into lsm/dev; if anyone has any
objections, ACKS, etc. please speak up soon.
Thanks Stephen.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v5] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Serge E. Hallyn @ 2026-01-06 21:07 UTC (permalink / raw)
To: Ryan Foster; +Cc: linux-kernel, linux-security-module, paul, selinux, serge
In-Reply-To: <20251230151450.196371-1-foster.ryan.r@gmail.com>
On Tue, Dec 30, 2025 at 07:13:09AM -0800, Ryan Foster wrote:
> Hi all,
> Sorry for the spam, this aims to fix both issues. Attempted to reproduce
> CI config build locally.
> Thanks, Ryan
> ---
>
> Add comprehensive KUnit tests for the namespace-related capability
> functions that Serge Hallyn refactored in commit 9891d2f79a9f
> ("Clarify the rootid_owns_currentns").
>
> The tests verify:
> - Basic functionality: UID 0 in init namespace, invalid vfsuid,
> non-zero UIDs
> - Actual namespace traversal: Creating user namespaces with different
> UID mappings where uid 0 maps to different kuids (e.g., 1000, 2000,
> 3000)
> - Hierarchy traversal: Testing multiple nested namespaces to verify
> correct namespace hierarchy traversal
>
> This addresses the feedback to "test the actual functionality" by
> creating real user namespaces with different values for the
> namespace's uid 0, rather than just basic input validation.
>
> The test file is included at the end of commoncap.c when
> CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, following the
> standard kernel pattern (e.g., scsi_lib.c, ext4/mballoc.c). This
> allows tests to access static functions in the same compilation unit
> without modifying production code based on test configuration.
>
> The tests require CONFIG_USER_NS to be enabled since they rely on user
> namespace mapping functionality. The Kconfig dependency ensures the
> tests only build when this requirement is met.
>
> All 7 tests pass:
> - test_vfsuid_root_in_currentns_init_ns
> - test_vfsuid_root_in_currentns_invalid
> - test_vfsuid_root_in_currentns_nonzero
> - test_kuid_root_in_ns_init_ns_uid0
> - test_kuid_root_in_ns_init_ns_nonzero
> - test_kuid_root_in_ns_with_mapping
> - test_kuid_root_in_ns_with_different_mappings
>
> Updated MAINTAINER capabilities to include commoncap test
>
> Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
I've applied this one to #caps-next. I note that there is another v5
which was sent the next day, but this patch seemed more correct.
Although I did prefer the commit message in the other one. Please
let me know if I should switch them, although if so, then I think
you need a v6, because the ns3 test seemed broken in the other one.
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Thanks,
-serge
> ---
> MAINTAINERS | 1 +
> security/Kconfig | 17 +++
> security/commoncap.c | 4 +
> security/commoncap_test.c | 290 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 312 insertions(+)
> create mode 100644 security/commoncap_test.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c0030e126fc8..6f162c736dfb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5682,6 +5682,7 @@ F: include/trace/events/capability.h
> F: include/uapi/linux/capability.h
> F: kernel/capability.c
> F: security/commoncap.c
> +F: security/commoncap_test.c
>
> CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER
> M: Kevin Tsai <ktsai@capellamicro.com>
> diff --git a/security/Kconfig b/security/Kconfig
> index 285f284dfcac..6a4393fce9a1 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -284,6 +284,23 @@ config LSM
>
> If unsure, leave this as the default.
>
> +config SECURITY_COMMONCAP_KUNIT_TEST
> + bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
> + depends on KUNIT=y && USER_NS
> + default KUNIT_ALL_TESTS
> + help
> + This builds the commoncap KUnit tests.
> +
> + KUnit tests run during boot and output the results to the debug log
> + in TAP format (https://testanything.org/). Only useful for kernel devs
> + running KUnit test harness and are not for inclusion into a
> + production build.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> source "security/Kconfig.hardening"
>
> endmenu
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 8a23dfab7fac..3399535808fe 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1521,3 +1521,7 @@ DEFINE_LSM(capability) = {
> };
>
> #endif /* CONFIG_SECURITY */
> +
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +#include "commoncap_test.c"
> +#endif
> diff --git a/security/commoncap_test.c b/security/commoncap_test.c
> new file mode 100644
> index 000000000000..1088364a54e6
> --- /dev/null
> +++ b/security/commoncap_test.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * KUnit tests for commoncap.c security functions
> + *
> + * Tests for security-critical functions in the capability subsystem,
> + * particularly namespace-related capability checks.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/user_namespace.h>
> +#include <linux/uidgid.h>
> +#include <linux/cred.h>
> +#include <linux/mnt_idmapping.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/refcount.h>
> +
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +
> +/* Functions are static in commoncap.c, but we can call them since we're
> + * included in the same compilation unit when tests are enabled.
> + */
> +
> +/**
> + * test_vfsuid_root_in_currentns_init_ns - Test vfsuid_root_in_currentns with init ns
> + *
> + * Verifies that UID 0 in the init namespace correctly owns the current
> + * namespace when running in init_user_ns.
> + *
> + * @test: KUnit test context
> + */
> +static void test_vfsuid_root_in_currentns_init_ns(struct kunit *test)
> +{
> + vfsuid_t vfsuid;
> + kuid_t kuid;
> +
> + /* Create UID 0 in init namespace */
> + kuid = KUIDT_INIT(0);
> + vfsuid = VFSUIDT_INIT(kuid);
> +
> + /* In init namespace, UID 0 should own current namespace */
> + KUNIT_EXPECT_TRUE(test, vfsuid_root_in_currentns(vfsuid));
> +}
> +
> +/**
> + * test_vfsuid_root_in_currentns_invalid - Test vfsuid_root_in_currentns with invalid vfsuid
> + *
> + * Verifies that an invalid vfsuid correctly returns false.
> + *
> + * @test: KUnit test context
> + */
> +static void test_vfsuid_root_in_currentns_invalid(struct kunit *test)
> +{
> + vfsuid_t invalid_vfsuid;
> +
> + /* Use the predefined invalid vfsuid */
> + invalid_vfsuid = INVALID_VFSUID;
> +
> + /* Invalid vfsuid should return false */
> + KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(invalid_vfsuid));
> +}
> +
> +/**
> + * test_vfsuid_root_in_currentns_nonzero - Test vfsuid_root_in_currentns with non-zero UID
> + *
> + * Verifies that a non-zero UID correctly returns false.
> + *
> + * @test: KUnit test context
> + */
> +static void test_vfsuid_root_in_currentns_nonzero(struct kunit *test)
> +{
> + vfsuid_t vfsuid;
> + kuid_t kuid;
> +
> + /* Create a non-zero UID */
> + kuid = KUIDT_INIT(1000);
> + vfsuid = VFSUIDT_INIT(kuid);
> +
> + /* Non-zero UID should return false */
> + KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(vfsuid));
> +}
> +
> +/**
> + * test_kuid_root_in_ns_init_ns_uid0 - Test kuid_root_in_ns with init namespace and UID 0
> + *
> + * Verifies that kuid_root_in_ns correctly identifies UID 0 in init namespace.
> + * This tests the core namespace traversal logic. In init namespace, UID 0
> + * maps to itself, so it should own the namespace.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_init_ns_uid0(struct kunit *test)
> +{
> + kuid_t kuid;
> + struct user_namespace *init_ns;
> +
> + kuid = KUIDT_INIT(0);
> + init_ns = &init_user_ns;
> +
> + /* UID 0 should own init namespace */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(kuid, init_ns));
> +}
> +
> +/**
> + * test_kuid_root_in_ns_init_ns_nonzero - Test kuid_root_in_ns with init namespace and non-zero UID
> + *
> + * Verifies that kuid_root_in_ns correctly rejects non-zero UIDs in init namespace.
> + * Only UID 0 should own a namespace.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_init_ns_nonzero(struct kunit *test)
> +{
> + kuid_t kuid;
> + struct user_namespace *init_ns;
> +
> + kuid = KUIDT_INIT(1000);
> + init_ns = &init_user_ns;
> +
> + /* Non-zero UID should not own namespace */
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(kuid, init_ns));
> +}
> +
> +/**
> + * create_test_user_ns_with_mapping - Create a mock user namespace with UID mapping
> + *
> + * Creates a minimal user namespace structure for testing where uid 0 in the
> + * namespace maps to a specific kuid in the parent namespace.
> + *
> + * @test: KUnit test context
> + * @parent_ns: Parent namespace (typically init_user_ns)
> + * @mapped_kuid: The kuid that uid 0 in this namespace maps to in parent
> + *
> + * Returns: Pointer to allocated namespace, or NULL on failure
> + */
> +static struct user_namespace *create_test_user_ns_with_mapping(struct kunit *test,
> + struct user_namespace *parent_ns,
> + kuid_t mapped_kuid)
> +{
> + struct user_namespace *ns;
> + struct uid_gid_extent extent;
> +
> + /* Allocate a test namespace - use kzalloc to zero all fields */
> + ns = kunit_kzalloc(test, sizeof(*ns), GFP_KERNEL);
> + if (!ns)
> + return NULL;
> +
> + /* Initialize basic namespace structure fields */
> + ns->parent = parent_ns;
> + ns->level = parent_ns ? parent_ns->level + 1 : 0;
> + ns->owner = mapped_kuid;
> + ns->group = KGIDT_INIT(0);
> +
> + /* Initialize ns_common structure */
> + refcount_set(&ns->ns.__ns_ref, 1);
> + ns->ns.inum = 0; /* Mock inum */
> +
> + /* Set up uid mapping: uid 0 in this namespace maps to mapped_kuid in parent
> + * Format: first (uid in ns) : lower_first (kuid in parent) : count
> + * So: uid 0 in ns -> kuid mapped_kuid in parent
> + * This means from_kuid(ns, mapped_kuid) returns 0
> + */
> + extent.first = 0; /* uid 0 in this namespace */
> + extent.lower_first = __kuid_val(mapped_kuid); /* maps to this kuid in parent */
> + extent.count = 1;
> +
> + ns->uid_map.extent[0] = extent;
> + ns->uid_map.nr_extents = 1;
> +
> + /* Set up gid mapping: gid 0 maps to gid 0 in parent (simplified) */
> + extent.first = 0;
> + extent.lower_first = 0;
> + extent.count = 1;
> +
> + ns->gid_map.extent[0] = extent;
> + ns->gid_map.nr_extents = 1;
> +
> + return ns;
> +}
> +
> +/**
> + * test_kuid_root_in_ns_with_mapping - Test kuid_root_in_ns with namespace where uid 0
> + * maps to different kuid
> + *
> + * Creates a user namespace where uid 0 maps to kuid 1000 in the parent namespace.
> + * Verifies that kuid_root_in_ns correctly identifies kuid 1000 as owning the namespace.
> + *
> + * Note: kuid_root_in_ns walks up the namespace hierarchy, so it checks the current
> + * namespace first, then parent, then parent's parent, etc. So:
> + * - kuid 1000 owns test_ns because from_kuid(test_ns, 1000) == 0
> + * - kuid 0 also owns test_ns because from_kuid(init_user_ns, 0) == 0
> + * (checked in parent)
> + *
> + * This tests the actual functionality as requested: creating namespaces with
> + * different values for the namespace's uid 0.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_with_mapping(struct kunit *test)
> +{
> + struct user_namespace *test_ns;
> + struct user_namespace *parent_ns;
> + kuid_t mapped_kuid, other_kuid;
> +
> + parent_ns = &init_user_ns;
> + mapped_kuid = KUIDT_INIT(1000);
> + other_kuid = KUIDT_INIT(2000);
> +
> + test_ns = create_test_user_ns_with_mapping(test, parent_ns, mapped_kuid);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_ns);
> +
> + /* kuid 1000 should own test_ns because it maps to uid 0 in test_ns */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(mapped_kuid, test_ns));
> +
> + /* kuid 0 should also own test_ns (checked via parent init_user_ns) */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), test_ns));
> +
> + /* Other kuids should not own test_ns */
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(other_kuid, test_ns));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(500), test_ns));
> +}
> +
> +/**
> + * test_kuid_root_in_ns_with_different_mappings - Test with multiple namespaces
> + *
> + * Creates multiple user namespaces with different UID mappings to verify
> + * that kuid_root_in_ns correctly handles different namespace hierarchies.
> + *
> + * Since kuid_root_in_ns walks up the hierarchy, kuids that map to 0 in init_user_ns
> + * will own all namespaces, while kuids that only map to 0 in specific namespaces
> + * will only own those namespaces and their children.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_with_different_mappings(struct kunit *test)
> +{
> + struct user_namespace *ns1, *ns2, *ns3;
> +
> + /* Create ns1 where uid 0 maps to kuid 1000 */
> + ns1 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(1000));
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns1);
> +
> + /* Create ns2 where uid 0 maps to kuid 2000 */
> + ns2 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(2000));
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns2);
> +
> + /* Create ns3 as a child of ns1 where uid 0 maps to kuid 3000 */
> + ns3 = create_test_user_ns_with_mapping(test, ns1, KUIDT_INIT(3000));
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns3);
> +
> + /* Test ns1: kuid 1000 owns it, kuid 0 owns it (via parent), kuid 2000 does not */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns1));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns1));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns1));
> +
> + /* Test ns2: kuid 2000 owns it, kuid 0 owns it (via parent), kuid 1000 does not */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns2));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns2));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns2));
> +
> + /* Test ns3: kuid 3000 owns it, kuid 1000 owns it (via parent ns1),
> + * kuid 0 owns it (via init_user_ns), kuid 2000 does not
> + */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns3));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns3));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns3));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns3));
> +}
> +
> +static struct kunit_case commoncap_test_cases[] = {
> + KUNIT_CASE(test_vfsuid_root_in_currentns_init_ns),
> + KUNIT_CASE(test_vfsuid_root_in_currentns_invalid),
> + KUNIT_CASE(test_vfsuid_root_in_currentns_nonzero),
> + KUNIT_CASE(test_kuid_root_in_ns_init_ns_uid0),
> + KUNIT_CASE(test_kuid_root_in_ns_init_ns_nonzero),
> + KUNIT_CASE(test_kuid_root_in_ns_with_mapping),
> + KUNIT_CASE(test_kuid_root_in_ns_with_different_mappings),
> + {}
> +};
> +
> +static struct kunit_suite commoncap_test_suite = {
> + .name = "commoncap",
> + .test_cases = commoncap_test_cases,
> +};
> +
> +kunit_test_suite(commoncap_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +
> +#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Jeff Layton @ 2026-01-06 19:50 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: <aV1jhIS24tE-dL9A@CMGLRV3>
On Tue, 2026-01-06 at 13:33 -0600, Frederick Lawler wrote:
> On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> > Hi Jeff,
> >
> > On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > > is no longer able to correctly track inode.i_version due to the struct
> > > > kstat.change_cookie no longer containing an updated i_version.
> > > >
> > > > Introduce a fallback mechanism for IMA that instead tracks a
> > > > integrity_ctime_guard() in absence of or outdated i_version
> > > > for stacked file systems.
> > > >
> > > > EVM is left alone since it mostly cares about the backing inode.
> > > >
> > > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > ---
> > > > The motivation behind this was that file systems that use the
> > > > cookie to set the i_version for stacked file systems may still do so.
> > > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > > The assumption is that the ctime will be different if the i_version is
> > > > different anyway for non-stacked file systems.
> > > >
> > > > I'm not too pleased with passing in struct file* to
> > > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > > that for now, but I couldn't come up with another idea to get the
> > > > stat without coming up with a new stat function to accommodate just
> > > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > > file system support to EVM (which doesn't make much sense to me
> > > > at the moment).
> > > >
> > > > I plan on adding in self test infrastructure for the v1, but I would
> > > > like to get some early feedback on the approach first.
> > > > ---
> > > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > > security/integrity/evm/evm_crypto.c | 2 +-
> > > > security/integrity/evm/evm_main.c | 2 +-
> > > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > > --- a/include/linux/integrity.h
> > > > +++ b/include/linux/integrity.h
> > > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > > >
> > > > /* An inode's attributes for detection of changes */
> > > > struct integrity_inode_attributes {
> > > > + u64 ctime_guard;
> > > > u64 version; /* track inode changes */
> > > > unsigned long ino;
> > > > dev_t dev;
> > > > };
> > > >
> > > > +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.
> > > > */
> > > > static inline void
> > > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > - u64 i_version, const struct inode *inode)
> > > > + u64 i_version, u64 ctime_guard,
> > > > + const struct inode *inode)
> > > > {
> > > > + attrs->ctime_guard = ctime_guard;
> > > > attrs->version = i_version;
> > > > attrs->dev = inode->i_sb->s_dev;
> > > > attrs->ino = inode->i_ino;
> > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > */
> > > > 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;
> > > > +
> > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > + inode->i_ino != attrs->ino)
> > > > + return true;
> > > > +
> > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > + return false;
> > > > +
> > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > + AT_STATX_SYNC_AS_STAT))
> > > > + return true;
> > > > +
> > >
> > > This is rather odd. You're sampling the i_version field directly, but
> > > if it's not equal then you go through ->getattr() to get the ctime.
> > >
> > > It's particularly odd since you don't know whether the i_version field
> > > is even implemented on the fs. On filesystems where it isn't, the
> > > i_version field generally stays at 0, so won't this never fall through
> > > to do the vfs_getattr_nosec() call on those filesystems?
> > >
> >
> > You're totally right. I didn't consider FS's caching the value at zero.
>
> Actually, I'm going to amend this. I think I did consider FSs without an
> implementation. Where this is called at, it is often guarded by a
> !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> understanding this correctly, the check call doesn't occur unless the inode
> has i_version support.
>
It depends on what you mean by i_version support:
That flag just tells the VFS that it needs to bump the i_version field
when updating timestamps. It's not a reliable indicator of whether the
i_version field is suitable for the purpose you want here.
The problem here and the one that we ultimately fixed with multigrain
timestamps is that XFS in particular will bump i_version on any change
to the log. That includes atime updates due to reads.
XFS still tracks the i_version the way it always has, but we've stopped
getattr() from reporting it because it's not suitable for the purpose
that nfsd (and IMA) need it for.
> It seems to me the suggestion then is to remove the IS_I_VERSION()
> checks guarding the call sites, grab both ctime and cookie from stat,
> and if IS_I_VERSION() use that, otherwise cookie, and compare
> against the cached i_version with one of those values, and then fall
> back to ctime?
>
Not exactly.
You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
use that. If it's not then use the ctime.
The part I'm not sure about is whether it's actually safe to do this.
vfs_getattr_nosec() can block in some situations. Is it ok to do this
in any context where integrity_inode_attrs_changed() may be called?
ISTR that this was an issue at one point, but maybe isn't now that IMA
is an LSM?
> >
> > > Ideally, you should just call vfs_getattr_nosec() early on with
> > > STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
> > > STATX_CHANGE_COOKIE if it's set in the returned mask.
> > >
> >
> > Yes, that makes sense.
> >
> > I'll spin that in v1, thanks!
> >
> > > > + return attrs->ctime_guard != integrity_ctime_guard(stat);
> > > > }
> > > >
> > > >
> > > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> > > > --- a/security/integrity/evm/evm_crypto.c
> > > > +++ b/security/integrity/evm/evm_crypto.c
> > > > @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > > if (IS_I_VERSION(inode))
> > > > i_version = inode_query_iversion(inode);
> > > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > > - inode);
> > > > + 0, 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..0712802628fd6533383f9855687e19bef7b771c7 100644
> > > > --- a/security/integrity/evm/evm_main.c
> > > > +++ b/security/integrity/evm/evm_main.c
> > > > @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > > if (iint) {
> > > > ret = (!IS_I_VERSION(metadata_inode) ||
> > > > integrity_inode_attrs_changed(&iint->metadata_inode,
> > > > - 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..72bba8daa588a0f4e45e4249276edb54ca3d77ef 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;
> > > > + u64 ctime_guard = 0;
> > > >
> > > > /*
> > > > * Always collect the modsig, because IMA might have already collected
> > > > @@ -272,10 +273,16 @@ 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_guard = integrity_ctime_guard(stat);
> > > > + }
> > > > hash.hdr.algo = algo;
> > > > hash.hdr.length = hash_digest_size[algo];
> > > >
> > > > @@ -305,11 +312,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_guard = ctime_guard;
> > > > + } else {
> > > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > > - real_inode);
> > > > + ctime_guard, 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..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 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>
> > > > @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > > {
> > > > fmode_t mode = file->f_mode;
> > > > bool update;
> > > > + int ret;
> > > >
> > > > if (!(mode & FMODE_WRITE))
> > > > return;
> > > > @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > >
> > > > 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) {
> > > > + ret = vfs_getattr_nosec(&file->f_path, &stat,
> > > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > > + AT_STATX_SYNC_AS_STAT);
> > > > + if ((iint->flags & IMA_NEW_FILE) || ret ||
> > > > + (!ret && stat.change_cookie != iint->real_inode.version) ||
> > > > + (!ret && integrity_ctime_guard(stat) !=
> > > > + iint->real_inode.ctime_guard)) {
> > > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > > iint->measured_pcrs = 0;
> > > > if (update)
> > > > @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > > if (!IS_I_VERSION(real_inode) ||
> > > > integrity_inode_attrs_changed(&iint->real_inode,
> > > > - 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,
> > >
> > > --
> > > Jeff Layton <jlayton@kernel.org>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Frederick Lawler @ 2026-01-06 19:33 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: <aV07lY6NOkNvUk3Z@CMGLRV3>
On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> Hi Jeff,
>
> On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > is no longer able to correctly track inode.i_version due to the struct
> > > kstat.change_cookie no longer containing an updated i_version.
> > >
> > > Introduce a fallback mechanism for IMA that instead tracks a
> > > integrity_ctime_guard() in absence of or outdated i_version
> > > for stacked file systems.
> > >
> > > EVM is left alone since it mostly cares about the backing inode.
> > >
> > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > ---
> > > The motivation behind this was that file systems that use the
> > > cookie to set the i_version for stacked file systems may still do so.
> > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > The assumption is that the ctime will be different if the i_version is
> > > different anyway for non-stacked file systems.
> > >
> > > I'm not too pleased with passing in struct file* to
> > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > that for now, but I couldn't come up with another idea to get the
> > > stat without coming up with a new stat function to accommodate just
> > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > file system support to EVM (which doesn't make much sense to me
> > > at the moment).
> > >
> > > I plan on adding in self test infrastructure for the v1, but I would
> > > like to get some early feedback on the approach first.
> > > ---
> > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > security/integrity/evm/evm_crypto.c | 2 +-
> > > security/integrity/evm/evm_main.c | 2 +-
> > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > --- a/include/linux/integrity.h
> > > +++ b/include/linux/integrity.h
> > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > >
> > > /* An inode's attributes for detection of changes */
> > > struct integrity_inode_attributes {
> > > + u64 ctime_guard;
> > > u64 version; /* track inode changes */
> > > unsigned long ino;
> > > dev_t dev;
> > > };
> > >
> > > +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.
> > > */
> > > static inline void
> > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > - u64 i_version, const struct inode *inode)
> > > + u64 i_version, u64 ctime_guard,
> > > + const struct inode *inode)
> > > {
> > > + attrs->ctime_guard = ctime_guard;
> > > attrs->version = i_version;
> > > attrs->dev = inode->i_sb->s_dev;
> > > attrs->ino = inode->i_ino;
> > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > */
> > > 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;
> > > +
> > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > + inode->i_ino != attrs->ino)
> > > + return true;
> > > +
> > > + if (inode_eq_iversion(inode, attrs->version))
> > > + return false;
> > > +
> > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT))
> > > + return true;
> > > +
> >
> > This is rather odd. You're sampling the i_version field directly, but
> > if it's not equal then you go through ->getattr() to get the ctime.
> >
> > It's particularly odd since you don't know whether the i_version field
> > is even implemented on the fs. On filesystems where it isn't, the
> > i_version field generally stays at 0, so won't this never fall through
> > to do the vfs_getattr_nosec() call on those filesystems?
> >
>
> You're totally right. I didn't consider FS's caching the value at zero.
Actually, I'm going to amend this. I think I did consider FSs without an
implementation. Where this is called at, it is often guarded by a
!IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
understanding this correctly, the check call doesn't occur unless the inode
has i_version support.
It seems to me the suggestion then is to remove the IS_I_VERSION()
checks guarding the call sites, grab both ctime and cookie from stat,
and if IS_I_VERSION() use that, otherwise cookie, and compare
against the cached i_version with one of those values, and then fall
back to ctime?
>
> > Ideally, you should just call vfs_getattr_nosec() early on with
> > STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
> > STATX_CHANGE_COOKIE if it's set in the returned mask.
> >
>
> Yes, that makes sense.
>
> I'll spin that in v1, thanks!
>
> > > + return attrs->ctime_guard != integrity_ctime_guard(stat);
> > > }
> > >
> > >
> > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> > > --- a/security/integrity/evm/evm_crypto.c
> > > +++ b/security/integrity/evm/evm_crypto.c
> > > @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > if (IS_I_VERSION(inode))
> > > i_version = inode_query_iversion(inode);
> > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > - inode);
> > > + 0, 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..0712802628fd6533383f9855687e19bef7b771c7 100644
> > > --- a/security/integrity/evm/evm_main.c
> > > +++ b/security/integrity/evm/evm_main.c
> > > @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > if (iint) {
> > > ret = (!IS_I_VERSION(metadata_inode) ||
> > > integrity_inode_attrs_changed(&iint->metadata_inode,
> > > - 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..72bba8daa588a0f4e45e4249276edb54ca3d77ef 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;
> > > + u64 ctime_guard = 0;
> > >
> > > /*
> > > * Always collect the modsig, because IMA might have already collected
> > > @@ -272,10 +273,16 @@ 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_guard = integrity_ctime_guard(stat);
> > > + }
> > > hash.hdr.algo = algo;
> > > hash.hdr.length = hash_digest_size[algo];
> > >
> > > @@ -305,11 +312,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_guard = ctime_guard;
> > > + } else {
> > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > - real_inode);
> > > + ctime_guard, 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..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 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>
> > > @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > {
> > > fmode_t mode = file->f_mode;
> > > bool update;
> > > + int ret;
> > >
> > > if (!(mode & FMODE_WRITE))
> > > return;
> > > @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > >
> > > 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) {
> > > + ret = vfs_getattr_nosec(&file->f_path, &stat,
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT);
> > > + if ((iint->flags & IMA_NEW_FILE) || ret ||
> > > + (!ret && stat.change_cookie != iint->real_inode.version) ||
> > > + (!ret && integrity_ctime_guard(stat) !=
> > > + iint->real_inode.ctime_guard)) {
> > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > iint->measured_pcrs = 0;
> > > if (update)
> > > @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > if (!IS_I_VERSION(real_inode) ||
> > > integrity_inode_attrs_changed(&iint->real_inode,
> > > - 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,
> >
> > --
> > Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 06/36] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2026-01-06 17:34 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Thomas Gleixner, Thomas Graf, Uladzislau Rezki,
Waiman Long, kasan-dev, linux-crypto, linux-doc, linux-kbuild,
linux-kernel, linux-mm, linux-security-module, linux-sparse,
linux-wireless, llvm, rcu
In-Reply-To: <993d381a-c24e-41d2-a0be-c1b0b5d8cbe9@I-love.SAKURA.ne.jp>
On Tue, Jan 06, 2026 at 10:21PM +0900, Tetsuo Handa wrote:
> On 2025/12/20 0:39, Marco Elver wrote:
> > Introduce basic compatibility with cleanup.h infrastructure.
>
> Can Compiler-Based Context- and Locking-Analysis work with conditional guards
> (unlock only if lock succeeded) ?
>
> I consider that replacing mutex_lock() with mutex_lock_killable() helps reducing
> frequency of hung tasks under heavy load where many processes are preempted waiting
> for the same mutex to become available (e.g.
> https://syzkaller.appspot.com/bug?extid=8f41dccfb6c03cc36fd6 ).
>
> But e.g. commit f49573f2f53e ("tty: use lock guard()s in tty_io") already replaced
> plain mutex_lock()/mutex_unlock() with plain guard(mutex). If I propose a patch for
> replacing mutex_lock() with mutex_lock_killable(), can I use conditional guards?
> (Would be yes if Compiler-Based Context- and Locking-Analysis can work, would be no
> if Compiler-Based Context- and Locking-Analysis cannot work) ?
It works for cond guards, so yes. But, only if support for
mutex_lock_killable() is added. At the moment mutex.h only has:
...
DEFINE_LOCK_GUARD_1(mutex, struct mutex, mutex_lock(_T->lock), mutex_unlock(_T->lock))
DEFINE_LOCK_GUARD_1_COND(mutex, _try, mutex_trylock(_T->lock))
DEFINE_LOCK_GUARD_1_COND(mutex, _intr, mutex_lock_interruptible(_T->lock), _RET == 0)
DECLARE_LOCK_GUARD_1_ATTRS(mutex, __acquires(_T), __releases(*(struct mutex **)_T))
#define class_mutex_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex, _T)
DECLARE_LOCK_GUARD_1_ATTRS(mutex_try, __acquires(_T), __releases(*(struct mutex **)_T))
#define class_mutex_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex_try, _T)
DECLARE_LOCK_GUARD_1_ATTRS(mutex_intr, __acquires(_T), __releases(*(struct mutex **)_T))
#define class_mutex_intr_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex_intr, _T)
...
And we also have a test in lib/test_context-analysis.c checking it
actually works:
...
scoped_cond_guard(mutex_try, return, &d->mtx) {
d->counter++;
}
scoped_cond_guard(mutex_intr, return, &d->mtx) {
d->counter++;
}
...
What's missing is a variant for mutex_lock_killable(), but that should
be similar to the mutex_lock_interruptible() variant.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox