* [PATCH] ipe: document AT_EXECVE_CHECK TOCTOU issue on OverlayFS
From: wufan @ 2026-01-30 0:14 UTC (permalink / raw)
To: linux-security-module
Cc: linux-doc, linux-kernel, corbet, mic, miklos, amir73il,
linux-unionfs, Fan Wu
From: Fan Wu <wufan@kernel.org>
Document a known TOCTOU (time-of-check to time-of-use) issue when using
AT_EXECVE_CHECK with read() on OverlayFS. The deny_write_access()
protection is only held during the syscall, allowing a copy-up operation
to be triggered afterward, causing subsequent read() calls to return
content from the unprotected upper layer.
This is generally not a concern for typical IPE deployments since
dm-verity and fs-verity protected files are effectively read-only.
However, OverlayFS with a writable upper layer presents a special case.
Document mitigation strategies including mounting overlay as read-only
and using mmap() instead of read(). Note that the mmap() mitigation
relies on current OverlayFS implementation details and should not be
considered a security guarantee.
Signed-off-by: Fan Wu <wufan@kernel.org>
---
Documentation/admin-guide/LSM/ipe.rst | 32 +++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
index a756d8158531..b621a98fe5e2 100644
--- a/Documentation/admin-guide/LSM/ipe.rst
+++ b/Documentation/admin-guide/LSM/ipe.rst
@@ -110,6 +110,34 @@ intercepts during the execution process, this mechanism needs the interpreter
to take the initiative, and existing interpreters won't be automatically
supported unless the signal call is added.
+.. WARNING::
+
+ There is a known TOCTOU (time-of-check to time-of-use) issue with
+ ``AT_EXECVE_CHECK`` when interpreters use ``read()`` to obtain script
+ contents after the check [#atacexecvecheck_toctou]_. The ``AT_EXECVE_CHECK``
+ protection (via ``deny_write_access()``) is only held during the syscall.
+ After it returns, the file can be modified before the interpreter reads it.
+
+ In typical IPE deployments, this is not a concern because files protected
+ by dm-verity or fs-verity are effectively read-only and cannot be modified.
+ However, OverlayFS presents a special case: when the lower layer is
+ dm-verity protected (read-only) but the upper layer is writable, an
+ attacker with write access can trigger a copy-up operation after the
+ ``AT_EXECVE_CHECK`` returns, causing subsequent ``read()`` calls to return
+ content from the unprotected upper layer instead of the verified lower layer.
+
+ To mitigate this issue on OverlayFS:
+
+ - Mount the overlay as read-only, or restrict write access to the upper
+ layer.
+ - Interpreters may use ``mmap()`` instead of ``read()`` to obtain script
+ contents. Currently, OverlayFS fixes the underlying real file reference
+ at ``open()`` time for mmap operations, so mmap will continue to access
+ the original lower layer file even after a copy-up. However, this
+ behavior is an implementation detail of OverlayFS and is not guaranteed
+ to remain stable across kernel versions. Do not rely on this as a
+ security guarantee.
+
Threat Model
------------
@@ -833,3 +861,7 @@ A:
kernel's fsverity support; IPE does not impose any
restrictions on the digest algorithm itself;
thus, this list may be out of date.
+
+.. [#atacexecvecheck_toctou] See the O_DENY_WRITE RFC discussion for details on
+ this TOCTOU issue:
+ https://lore.kernel.org/all/20250822170800.2116980-1-mic@digikod.net/
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Paul Moore @ 2026-01-29 23:06 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Vlastimil Babka, linux-security-module, linux-integrity, selinux,
john.johansen, zohar, roberto.sassu, wufan, mic, gnoack, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng,
linux-mm, David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <5392220f-c29e-4cbd-8dae-59fbea4c6491@lucifer.local>
On Thu, Jan 29, 2026 at 1:59 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
> On Thu, Jan 29, 2026 at 01:31:05PM -0500, Paul Moore wrote:
> > On Thu, Jan 29, 2026 at 12:11 PM Lorenzo Stoakes
> > <lorenzo.stoakes@oracle.com> wrote:
> > > On Thu, Jan 29, 2026 at 06:02:00PM +0100, Vlastimil Babka wrote:
...
> > > But that's future work, for an -rc8 hotfix ...
> >
> > Expect a patch later today.
>
> Perfect thank you very much!
A link to the patch is below. Assuming I don't hear any negative
comments, I'll plan to merge this into lsm/stable-6.19 tomorrow and
send it up to Linus early next week; this should give us a day or two
in linux-next and then most of the week in Linus' tree before the
v6.19 release.
https://lore.kernel.org/linux-security-module/20260129225132.420484-2-paul@paul-moore.com
--
paul-moore.com
^ permalink raw reply
* [PATCH] lsm: preserve /proc/sys/vm/mmap_min_addr when !CONFIG_SECURITY
From: Paul Moore @ 2026-01-29 22:51 UTC (permalink / raw)
To: linux-security-module; +Cc: linux-mm, lorenzo.stoakes
While reworking the LSM initialization code the
/proc/sys/vm/mmap_min_addr handler was inadvertently caught up in the
change and the procfs entry wasn't setup when CONFIG_SECURITY was not
selected at kernel build time. This patch restores the previous behavior
and ensures that the procfs entry is setup regardless of the
CONFIG_SECURITY state.
Future work will improve upon this, likely by moving the procfs handler
into the mm subsystem, but this patch should resolve the immediate
regression.
Fixes: 4ab5efcc2829 ("lsm: consolidate all of the LSM framework initcalls")
Reported-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/lsm.h | 9 ---------
security/lsm_init.c | 7 +------
security/min_addr.c | 5 ++---
3 files changed, 3 insertions(+), 18 deletions(-)
diff --git a/security/lsm.h b/security/lsm.h
index 81aadbc61685..db77cc83e158 100644
--- a/security/lsm.h
+++ b/security/lsm.h
@@ -37,15 +37,6 @@ int lsm_task_alloc(struct task_struct *task);
/* LSM framework initializers */
-#ifdef CONFIG_MMU
-int min_addr_init(void);
-#else
-static inline int min_addr_init(void)
-{
- return 0;
-}
-#endif /* CONFIG_MMU */
-
#ifdef CONFIG_SECURITYFS
int securityfs_init(void);
#else
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 05bd52e6b1f2..573e2a7250c4 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -489,12 +489,7 @@ int __init security_init(void)
*/
static int __init security_initcall_pure(void)
{
- int rc_adr, rc_lsm;
-
- rc_adr = min_addr_init();
- rc_lsm = lsm_initcall(pure);
-
- return (rc_adr ? rc_adr : rc_lsm);
+ return lsm_initcall(pure);
}
pure_initcall(security_initcall_pure);
diff --git a/security/min_addr.c b/security/min_addr.c
index 0fde5ec9abc8..56e4f9d25929 100644
--- a/security/min_addr.c
+++ b/security/min_addr.c
@@ -5,8 +5,6 @@
#include <linux/sysctl.h>
#include <linux/minmax.h>
-#include "lsm.h"
-
/* amount of vm to protect from userspace access by both DAC and the LSM*/
unsigned long mmap_min_addr;
/* amount of vm to protect from userspace using CAP_SYS_RAWIO (DAC) */
@@ -54,10 +52,11 @@ static const struct ctl_table min_addr_sysctl_table[] = {
},
};
-int __init min_addr_init(void)
+static int __init mmap_min_addr_init(void)
{
register_sysctl_init("vm", min_addr_sysctl_table);
update_mmap_min_addr();
return 0;
}
+pure_initcall(mmap_min_addr_init);
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v2 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Mickaël Salaün @ 2026-01-29 21:27 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <a6d6479888d9d216a3f2e7bb133523f856f92461.1767115163.git.m@maowtm.org>
On Tue, Dec 30, 2025 at 05:20:20PM +0000, Tingmao Wang wrote:
> Extend the existing abstract UNIX socket scoping to pathname sockets as
> well. Basically all of the logic is reused between the two types, just
> that pathname sockets scoping are controlled by another bit, and has its
> own audit request type (since the current one is named
> "abstract_unix_socket").
>
> Closes: https://github.com/landlock-lsm/linux/issues/51
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes in v2:
> - Factor out common code in hook_unix_stream_connect and
> hook_unix_may_send into check_socket_access(), and inline
> is_abstract_socket().
>
> security/landlock/audit.c | 4 ++
> security/landlock/audit.h | 1 +
> security/landlock/task.c | 109 ++++++++++++++++++++++----------------
> 3 files changed, 67 insertions(+), 47 deletions(-)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index e899995f1fd5..0626cc553ab0 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -75,6 +75,10 @@ get_blocker(const enum landlock_request_type type,
> WARN_ON_ONCE(access_bit != -1);
> return "scope.abstract_unix_socket";
>
> + case LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET:
> + WARN_ON_ONCE(access_bit != -1);
> + return "scope.pathname_unix_socket";
> +
> case LANDLOCK_REQUEST_SCOPE_SIGNAL:
> WARN_ON_ONCE(access_bit != -1);
> return "scope.signal";
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 92428b7fc4d8..1c9ce8588102 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -21,6 +21,7 @@ enum landlock_request_type {
> LANDLOCK_REQUEST_NET_ACCESS,
> LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> LANDLOCK_REQUEST_SCOPE_SIGNAL,
> + LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET,
> };
>
> /*
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 833bc0cfe5c9..10dc356baf6f 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -232,35 +232,81 @@ static bool domain_is_scoped(const struct landlock_ruleset *const client,
> return false;
> }
>
> +/**
> + * sock_is_scoped - Check if socket connect or send should be restricted
> + * based on scope controls.
> + *
> + * @other: The server socket.
> + * @domain: The client domain.
> + * @scope: The relevant scope bit to check (i.e. pathname or abstract).
> + *
> + * Returns: True if connect should be restricted, false otherwise.
> + */
> static bool sock_is_scoped(struct sock *const other,
> - const struct landlock_ruleset *const domain)
> + const struct landlock_ruleset *const domain,
> + access_mask_t scope)
> {
> const struct landlock_ruleset *dom_other;
>
> /* The credentials will not change. */
> lockdep_assert_held(&unix_sk(other)->lock);
> dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> - return domain_is_scoped(domain, dom_other,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + return domain_is_scoped(domain, dom_other, scope);
> }
>
> -static bool is_abstract_socket(struct sock *const sock)
> +/* Allow us to quickly test if the current domain scopes any form of socket */
Missing final dot.
> +static const struct access_masks unix_scope = {
> + .scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
> +};
> +
> +/*
> + * UNIX sockets can have three types of addresses: pathname (a filesystem path),
> + * unnamed (not bound to an address), and abstract (sun_path[0] is '\0').
> + * Unnamed sockets include those created with socketpair() and unbound sockets.
> + * We do not restrict unnamed sockets since they have no address to identify.
Not because they have no address but because they cannot be used to
reach a new peer, right?
> + */
> +static int
> +check_socket_access(struct sock *const other,
> + const struct landlock_cred_security *const subject,
> + const size_t handle_layer)
> {
> - struct unix_address *addr = unix_sk(sock)->addr;
> + const struct unix_address *addr = unix_sk(other)->addr;
> + access_mask_t scope;
> + enum landlock_request_type request_type;
>
> + /* Unnamed sockets are not restricted. */
> if (!addr)
> - return false;
> + return 0;
>
> + /*
> + * Abstract and pathname Unix sockets have separate scope and audit
UNIX
> + * request type.
> + */
> if (addr->len >= offsetof(struct sockaddr_un, sun_path) + 1 &&
> - addr->name->sun_path[0] == '\0')
> - return true;
> + addr->name->sun_path[0] == '\0') {
> + scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
> + request_type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET;
> + } else {
> + scope = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> + request_type = LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET;
> + }
>
> - return false;
> -}
> + if (!sock_is_scoped(other, subject->domain, scope))
> + return 0;
>
> -static const struct access_masks unix_scope = {
> - .scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET,
> -};
> + landlock_log_denial(subject, &(struct landlock_request) {
> + .type = request_type,
> + .audit = {
> + .type = LSM_AUDIT_DATA_NET,
> + .u.net = &(struct lsm_network_audit) {
> + .sk = other,
> + },
> + },
> + .layer_plus_one = handle_layer + 1,
> + });
> + return -EPERM;
> +}
>
> static int hook_unix_stream_connect(struct sock *const sock,
> struct sock *const other,
> @@ -275,23 +321,7 @@ static int hook_unix_stream_connect(struct sock *const sock,
> if (!subject)
> return 0;
>
> - if (!is_abstract_socket(other))
> - return 0;
> -
> - if (!sock_is_scoped(other, subject->domain))
> - return 0;
> -
> - landlock_log_denial(subject, &(struct landlock_request) {
> - .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> - .audit = {
> - .type = LSM_AUDIT_DATA_NET,
> - .u.net = &(struct lsm_network_audit) {
> - .sk = other,
> - },
> - },
> - .layer_plus_one = handle_layer + 1,
> - });
> - return -EPERM;
> + return check_socket_access(other, subject, handle_layer);
> }
>
> static int hook_unix_may_send(struct socket *const sock,
> @@ -302,6 +332,7 @@ static int hook_unix_may_send(struct socket *const sock,
> landlock_get_applicable_subject(current_cred(), unix_scope,
> &handle_layer);
>
> + /* Quick return for non-landlocked tasks. */
> if (!subject)
> return 0;
>
> @@ -312,23 +343,7 @@ static int hook_unix_may_send(struct socket *const sock,
> if (unix_peer(sock->sk) == other->sk)
> return 0;
>
> - if (!is_abstract_socket(other->sk))
> - return 0;
> -
> - if (!sock_is_scoped(other->sk, subject->domain))
> - return 0;
> -
> - landlock_log_denial(subject, &(struct landlock_request) {
> - .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> - .audit = {
> - .type = LSM_AUDIT_DATA_NET,
> - .u.net = &(struct lsm_network_audit) {
> - .sk = other->sk,
> - },
> - },
> - .layer_plus_one = handle_layer + 1,
> - });
> - return -EPERM;
> + return check_socket_access(other->sk, subject, handle_layer);
> }
>
> static const struct access_masks signal_scope = {
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v2 6/6] selftests/landlock: Add pathname socket variants for more tests
From: Mickaël Salaün @ 2026-01-29 21:28 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <88de5bed60b06ba97088d87803f7bb3dbcc9a808.1767115163.git.m@maowtm.org>
The whole series looks good, thanks!
On Tue, Dec 30, 2025 at 05:20:24PM +0000, Tingmao Wang wrote:
> While this produces a lot of change, it does allow us to "simultaneously"
> test both abstract and pathname UNIX sockets with reletively little code
> duplication, since they are really similar.
>
> Tests touched: scoped_vs_unscoped, outside_socket,
> various_address_sockets, datagram_sockets, self_connect.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> .../selftests/landlock/scoped_unix_test.c | 599 ++++++++++++------
> 1 file changed, 395 insertions(+), 204 deletions(-)
>
> diff --git a/tools/testing/selftests/landlock/scoped_unix_test.c b/tools/testing/selftests/landlock/scoped_unix_test.c
> index 669418c97509..6d1541f77dbe 100644
> --- a/tools/testing/selftests/landlock/scoped_unix_test.c
> +++ b/tools/testing/selftests/landlock/scoped_unix_test.c
> @@ -536,8 +536,12 @@ TEST_F(scoped_audit, connect_to_child)
>
> FIXTURE(scoped_vs_unscoped)
> {
> - struct service_fixture parent_stream_address, parent_dgram_address,
> - child_stream_address, child_dgram_address;
> + struct service_fixture parent_stream_address_abstract,
> + parent_dgram_address_abstract, child_stream_address_abstract,
> + child_dgram_address_abstract;
> + struct service_fixture parent_stream_address_pathname,
> + parent_dgram_address_pathname, child_stream_address_pathname,
> + child_dgram_address_pathname;
> };
>
> #include "scoped_multiple_domain_variants.h"
> @@ -546,35 +550,75 @@ FIXTURE_SETUP(scoped_vs_unscoped)
> {
> drop_caps(_metadata);
>
> - memset(&self->parent_stream_address, 0,
> - sizeof(self->parent_stream_address));
> - set_unix_address(&self->parent_stream_address, 0, true);
> - memset(&self->parent_dgram_address, 0,
> - sizeof(self->parent_dgram_address));
> - set_unix_address(&self->parent_dgram_address, 1, true);
> - memset(&self->child_stream_address, 0,
> - sizeof(self->child_stream_address));
> - set_unix_address(&self->child_stream_address, 2, true);
> - memset(&self->child_dgram_address, 0,
> - sizeof(self->child_dgram_address));
> - set_unix_address(&self->child_dgram_address, 3, true);
> + ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
> +
> + /* Abstract addresses. */
> + memset(&self->parent_stream_address_abstract, 0,
> + sizeof(self->parent_stream_address_abstract));
> + set_unix_address(&self->parent_stream_address_abstract, 0, true);
> + memset(&self->parent_dgram_address_abstract, 0,
> + sizeof(self->parent_dgram_address_abstract));
> + set_unix_address(&self->parent_dgram_address_abstract, 1, true);
> + memset(&self->child_stream_address_abstract, 0,
> + sizeof(self->child_stream_address_abstract));
> + set_unix_address(&self->child_stream_address_abstract, 2, true);
> + memset(&self->child_dgram_address_abstract, 0,
> + sizeof(self->child_dgram_address_abstract));
> + set_unix_address(&self->child_dgram_address_abstract, 3, true);
> +
> + /* Pathname addresses. */
> + memset(&self->parent_stream_address_pathname, 0,
> + sizeof(self->parent_stream_address_pathname));
> + set_unix_address(&self->parent_stream_address_pathname, 4, false);
> + memset(&self->parent_dgram_address_pathname, 0,
> + sizeof(self->parent_dgram_address_pathname));
> + set_unix_address(&self->parent_dgram_address_pathname, 5, false);
> + memset(&self->child_stream_address_pathname, 0,
> + sizeof(self->child_stream_address_pathname));
> + set_unix_address(&self->child_stream_address_pathname, 6, false);
> + memset(&self->child_dgram_address_pathname, 0,
> + sizeof(self->child_dgram_address_pathname));
> + set_unix_address(&self->child_dgram_address_pathname, 7, false);
> }
>
> FIXTURE_TEARDOWN(scoped_vs_unscoped)
> {
> + EXPECT_EQ(0, remove_path(self->parent_stream_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, remove_path(self->parent_dgram_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, remove_path(self->child_stream_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, remove_path(self->child_dgram_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
> }
>
> /*
> * Test unix_stream_connect and unix_may_send for parent, child and
> * grand child processes when they can have scoped or non-scoped domains.
> */
> -TEST_F(scoped_vs_unscoped, unix_scoping)
> +static void test_scoped_vs_unscoped(
> + struct __test_metadata *const _metadata,
> + FIXTURE_DATA(scoped_vs_unscoped) * self,
> + const FIXTURE_VARIANT(scoped_vs_unscoped) * variant,
> + const bool abstract)
> {
> pid_t child;
> int status;
> bool can_connect_to_parent, can_connect_to_child;
> int pipe_parent[2];
> int stream_server_parent, dgram_server_parent;
> + const __u16 scope = abstract ? LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> + const struct service_fixture *parent_stream_address =
> + abstract ? &self->parent_stream_address_abstract :
> + &self->parent_stream_address_pathname;
> + const struct service_fixture *parent_dgram_address =
> + abstract ? &self->parent_dgram_address_abstract :
> + &self->parent_dgram_address_pathname;
> + const struct service_fixture *child_stream_address =
> + abstract ? &self->child_stream_address_abstract :
> + &self->child_stream_address_pathname;
> + const struct service_fixture *child_dgram_address =
> + abstract ? &self->child_dgram_address_abstract :
> + &self->child_dgram_address_pathname;
>
> can_connect_to_child = (variant->domain_grand_child != SCOPE_SANDBOX);
> can_connect_to_parent = (can_connect_to_child &&
> @@ -585,8 +629,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> if (variant->domain_all == OTHER_SANDBOX)
> create_fs_domain(_metadata);
> else if (variant->domain_all == SCOPE_SANDBOX)
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> child = fork();
> ASSERT_LE(0, child);
> @@ -600,8 +643,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> if (variant->domain_children == OTHER_SANDBOX)
> create_fs_domain(_metadata);
> else if (variant->domain_children == SCOPE_SANDBOX)
> - create_scoped_domain(
> - _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> grand_child = fork();
> ASSERT_LE(0, grand_child);
> @@ -616,9 +658,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> if (variant->domain_grand_child == OTHER_SANDBOX)
> create_fs_domain(_metadata);
> else if (variant->domain_grand_child == SCOPE_SANDBOX)
> - create_scoped_domain(
> - _metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> stream_client = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_client);
> @@ -626,15 +666,13 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> ASSERT_LE(0, dgram_client);
>
> ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
> - stream_err = connect(
> - stream_client,
> - &self->child_stream_address.unix_addr,
> - self->child_stream_address.unix_addr_len);
> + stream_err = connect(stream_client,
> + &child_stream_address->unix_addr,
> + child_stream_address->unix_addr_len);
> stream_errno = errno;
> - dgram_err = connect(
> - dgram_client,
> - &self->child_dgram_address.unix_addr,
> - self->child_dgram_address.unix_addr_len);
> + dgram_err = connect(dgram_client,
> + &child_dgram_address->unix_addr,
> + child_dgram_address->unix_addr_len);
> dgram_errno = errno;
> if (can_connect_to_child) {
> EXPECT_EQ(0, stream_err);
> @@ -653,14 +691,12 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
>
> ASSERT_EQ(1, read(pipe_parent[0], &buf, 1));
> stream_err = connect(
> - stream_client,
> - &self->parent_stream_address.unix_addr,
> - self->parent_stream_address.unix_addr_len);
> + stream_client, &parent_stream_address->unix_addr,
> + parent_stream_address->unix_addr_len);
> stream_errno = errno;
> dgram_err = connect(
> - dgram_client,
> - &self->parent_dgram_address.unix_addr,
> - self->parent_dgram_address.unix_addr_len);
> + dgram_client, &parent_dgram_address->unix_addr,
> + parent_dgram_address->unix_addr_len);
> dgram_errno = errno;
> if (can_connect_to_parent) {
> EXPECT_EQ(0, stream_err);
> @@ -681,8 +717,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> if (variant->domain_child == OTHER_SANDBOX)
> create_fs_domain(_metadata);
> else if (variant->domain_child == SCOPE_SANDBOX)
> - create_scoped_domain(
> - _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> stream_server_child = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_server_child);
> @@ -690,11 +725,11 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> ASSERT_LE(0, dgram_server_child);
>
> ASSERT_EQ(0, bind(stream_server_child,
> - &self->child_stream_address.unix_addr,
> - self->child_stream_address.unix_addr_len));
> - ASSERT_EQ(0, bind(dgram_server_child,
> - &self->child_dgram_address.unix_addr,
> - self->child_dgram_address.unix_addr_len));
> + &child_stream_address->unix_addr,
> + child_stream_address->unix_addr_len));
> + ASSERT_EQ(0,
> + bind(dgram_server_child, &child_dgram_address->unix_addr,
> + child_dgram_address->unix_addr_len));
> ASSERT_EQ(0, listen(stream_server_child, backlog));
>
> ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> @@ -708,19 +743,16 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> if (variant->domain_parent == OTHER_SANDBOX)
> create_fs_domain(_metadata);
> else if (variant->domain_parent == SCOPE_SANDBOX)
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> stream_server_parent = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_server_parent);
> dgram_server_parent = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_server_parent);
> - ASSERT_EQ(0, bind(stream_server_parent,
> - &self->parent_stream_address.unix_addr,
> - self->parent_stream_address.unix_addr_len));
> - ASSERT_EQ(0, bind(dgram_server_parent,
> - &self->parent_dgram_address.unix_addr,
> - self->parent_dgram_address.unix_addr_len));
> + ASSERT_EQ(0, bind(stream_server_parent, &parent_stream_address->unix_addr,
> + parent_stream_address->unix_addr_len));
> + ASSERT_EQ(0, bind(dgram_server_parent, &parent_dgram_address->unix_addr,
> + parent_dgram_address->unix_addr_len));
>
> ASSERT_EQ(0, listen(stream_server_parent, backlog));
>
> @@ -734,57 +766,119 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
> _metadata->exit_code = KSFT_FAIL;
> }
>
> +TEST_F(scoped_vs_unscoped, unix_scoping_abstract)
> +{
> + test_scoped_vs_unscoped(_metadata, self, variant, true);
> +}
> +
> +TEST_F(scoped_vs_unscoped, unix_scoping_pathname)
> +{
> + test_scoped_vs_unscoped(_metadata, self, variant, false);
> +}
> +
> FIXTURE(outside_socket)
> {
> - struct service_fixture address, transit_address;
> + struct service_fixture address_abstract, transit_address_abstract;
> + struct service_fixture address_pathname, transit_address_pathname;
> };
>
> FIXTURE_VARIANT(outside_socket)
> {
> const bool child_socket;
> const int type;
> + const bool abstract;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(outside_socket, abstract_allow_dgram_child) {
> + /* clang-format on */
> + .child_socket = true,
> + .type = SOCK_DGRAM,
> + .abstract = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(outside_socket, abstract_deny_dgram_server) {
> + /* clang-format on */
> + .child_socket = false,
> + .type = SOCK_DGRAM,
> + .abstract = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(outside_socket, abstract_allow_stream_child) {
> + /* clang-format on */
> + .child_socket = true,
> + .type = SOCK_STREAM,
> + .abstract = true,
> };
>
> /* clang-format off */
> -FIXTURE_VARIANT_ADD(outside_socket, allow_dgram_child) {
> +FIXTURE_VARIANT_ADD(outside_socket, abstract_deny_stream_server) {
> + /* clang-format on */
> + .child_socket = false,
> + .type = SOCK_STREAM,
> + .abstract = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(outside_socket, pathname_allow_dgram_child) {
> /* clang-format on */
> .child_socket = true,
> .type = SOCK_DGRAM,
> + .abstract = false,
> };
>
> /* clang-format off */
> -FIXTURE_VARIANT_ADD(outside_socket, deny_dgram_server) {
> +FIXTURE_VARIANT_ADD(outside_socket, pathname_deny_dgram_server) {
> /* clang-format on */
> .child_socket = false,
> .type = SOCK_DGRAM,
> + .abstract = false,
> };
>
> /* clang-format off */
> -FIXTURE_VARIANT_ADD(outside_socket, allow_stream_child) {
> +FIXTURE_VARIANT_ADD(outside_socket, pathname_allow_stream_child) {
> /* clang-format on */
> .child_socket = true,
> .type = SOCK_STREAM,
> + .abstract = false,
> };
>
> /* clang-format off */
> -FIXTURE_VARIANT_ADD(outside_socket, deny_stream_server) {
> +FIXTURE_VARIANT_ADD(outside_socket, pathname_deny_stream_server) {
> /* clang-format on */
> .child_socket = false,
> .type = SOCK_STREAM,
> + .abstract = false,
> };
>
> FIXTURE_SETUP(outside_socket)
> {
> drop_caps(_metadata);
>
> - memset(&self->transit_address, 0, sizeof(self->transit_address));
> - set_unix_address(&self->transit_address, 0, true);
> - memset(&self->address, 0, sizeof(self->address));
> - set_unix_address(&self->address, 1, true);
> + ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
> +
> + /* Abstract addresses. */
> + memset(&self->transit_address_abstract, 0,
> + sizeof(self->transit_address_abstract));
> + set_unix_address(&self->transit_address_abstract, 0, true);
> + memset(&self->address_abstract, 0, sizeof(self->address_abstract));
> + set_unix_address(&self->address_abstract, 1, true);
> +
> + /* Pathname addresses. */
> + memset(&self->transit_address_pathname, 0,
> + sizeof(self->transit_address_pathname));
> + set_unix_address(&self->transit_address_pathname, 2, false);
> + memset(&self->address_pathname, 0, sizeof(self->address_pathname));
> + set_unix_address(&self->address_pathname, 3, false);
> }
>
> FIXTURE_TEARDOWN(outside_socket)
> {
> + EXPECT_EQ(0, remove_path(self->transit_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, remove_path(self->address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
> }
>
> /*
> @@ -798,6 +892,15 @@ TEST_F(outside_socket, socket_with_different_domain)
> int pipe_child[2], pipe_parent[2];
> char buf_parent;
> int server_socket;
> + const __u16 scope = variant->abstract ?
> + LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> + const struct service_fixture *transit_address =
> + variant->abstract ? &self->transit_address_abstract :
> + &self->transit_address_pathname;
> + const struct service_fixture *address =
> + variant->abstract ? &self->address_abstract :
> + &self->address_pathname;
>
> ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> @@ -812,8 +915,7 @@ TEST_F(outside_socket, socket_with_different_domain)
> EXPECT_EQ(0, close(pipe_child[0]));
>
> /* Client always has a domain. */
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> if (variant->child_socket) {
> int data_socket, passed_socket, stream_server;
> @@ -823,8 +925,8 @@ TEST_F(outside_socket, socket_with_different_domain)
> stream_server = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_server);
> ASSERT_EQ(0, bind(stream_server,
> - &self->transit_address.unix_addr,
> - self->transit_address.unix_addr_len));
> + &transit_address->unix_addr,
> + transit_address->unix_addr_len));
> ASSERT_EQ(0, listen(stream_server, backlog));
> ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> data_socket = accept(stream_server, NULL, NULL);
> @@ -839,8 +941,8 @@ TEST_F(outside_socket, socket_with_different_domain)
>
> /* Waits for parent signal for connection. */
> ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
> - err = connect(client_socket, &self->address.unix_addr,
> - self->address.unix_addr_len);
> + err = connect(client_socket, &address->unix_addr,
> + address->unix_addr_len);
> if (variant->child_socket) {
> EXPECT_EQ(0, err);
> } else {
> @@ -859,9 +961,8 @@ TEST_F(outside_socket, socket_with_different_domain)
>
> ASSERT_LE(0, client_child);
> ASSERT_EQ(1, read(pipe_child[0], &buf_parent, 1));
> - ASSERT_EQ(0, connect(client_child,
> - &self->transit_address.unix_addr,
> - self->transit_address.unix_addr_len));
> + ASSERT_EQ(0, connect(client_child, &transit_address->unix_addr,
> + transit_address->unix_addr_len));
> server_socket = recv_fd(client_child);
> EXPECT_EQ(0, close(client_child));
> } else {
> @@ -870,10 +971,10 @@ TEST_F(outside_socket, socket_with_different_domain)
> ASSERT_LE(0, server_socket);
>
> /* Server always has a domain. */
> - create_scoped_domain(_metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> - ASSERT_EQ(0, bind(server_socket, &self->address.unix_addr,
> - self->address.unix_addr_len));
> + ASSERT_EQ(0,
> + bind(server_socket, &address->unix_addr, address->unix_addr_len));
> if (variant->type == SOCK_STREAM)
> ASSERT_EQ(0, listen(server_socket, backlog));
>
> @@ -888,52 +989,85 @@ TEST_F(outside_socket, socket_with_different_domain)
> _metadata->exit_code = KSFT_FAIL;
> }
>
> -static const char stream_path[] = TMP_DIR "/stream.sock";
> -static const char dgram_path[] = TMP_DIR "/dgram.sock";
> -
> /* clang-format off */
> -FIXTURE(various_address_sockets) {};
> +FIXTURE(various_address_sockets) {
> + struct service_fixture stream_pathname_addr, dgram_pathname_addr;
> + struct service_fixture stream_abstract_addr, dgram_abstract_addr;
> +};
> /* clang-format on */
>
> -FIXTURE_VARIANT(various_address_sockets)
> -{
> - const int domain;
> +/*
> + * Test all 4 combinations of abstract and pathname socket scope bits,
> + * plus a case with no Landlock domain at all.
> + */
> +/* clang-format off */
> +FIXTURE_VARIANT(various_address_sockets) {
> + /* clang-format on */
> + const __u16 scope_bits;
> + const bool no_sandbox;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(various_address_sockets, scope_abstract) {
> + /* clang-format on */
> + .scope_bits = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(various_address_sockets, scope_pathname) {
> + /* clang-format on */
> + .scope_bits = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
> };
>
> /* clang-format off */
> -FIXTURE_VARIANT_ADD(various_address_sockets, pathname_socket_scoped_domain) {
> +FIXTURE_VARIANT_ADD(various_address_sockets, scope_both) {
> /* clang-format on */
> - .domain = SCOPE_SANDBOX,
> + .scope_bits = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
> };
>
> /* clang-format off */
> -FIXTURE_VARIANT_ADD(various_address_sockets, pathname_socket_other_domain) {
> +FIXTURE_VARIANT_ADD(various_address_sockets, scope_none) {
> /* clang-format on */
> - .domain = OTHER_SANDBOX,
> + .scope_bits = 0,
> };
>
> /* clang-format off */
> -FIXTURE_VARIANT_ADD(various_address_sockets, pathname_socket_no_domain) {
> +FIXTURE_VARIANT_ADD(various_address_sockets, no_domain) {
> /* clang-format on */
> - .domain = NO_SANDBOX,
> + .no_sandbox = true,
> };
>
> FIXTURE_SETUP(various_address_sockets)
> {
> drop_caps(_metadata);
>
> - umask(0077);
> - ASSERT_EQ(0, mkdir(TMP_DIR, 0700));
> + ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
> +
> + memset(&self->stream_pathname_addr, 0, sizeof(self->stream_pathname_addr));
> + set_unix_address(&self->stream_pathname_addr, 0, false);
> + memset(&self->dgram_pathname_addr, 0, sizeof(self->dgram_pathname_addr));
> + set_unix_address(&self->dgram_pathname_addr, 1, false);
> +
> + memset(&self->stream_abstract_addr, 0, sizeof(self->stream_abstract_addr));
> + set_unix_address(&self->stream_abstract_addr, 2, true);
> + memset(&self->dgram_abstract_addr, 0, sizeof(self->dgram_abstract_addr));
> + set_unix_address(&self->dgram_abstract_addr, 3, true);
> }
>
> FIXTURE_TEARDOWN(various_address_sockets)
> {
> - EXPECT_EQ(0, unlink(stream_path));
> - EXPECT_EQ(0, unlink(dgram_path));
> - EXPECT_EQ(0, rmdir(TMP_DIR));
> + EXPECT_EQ(0, remove_path(self->stream_pathname_addr.unix_addr.sun_path));
> + EXPECT_EQ(0, remove_path(self->dgram_pathname_addr.unix_addr.sun_path));
> + EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
> }
>
> -TEST_F(various_address_sockets, scoped_pathname_sockets)
> +/*
> + * Test interaction of various scope flags (controlled by variant->domain)
> + * with pathname and abstract sockets when connecting from a sandboxed
> + * child.
> + */
> +TEST_F(various_address_sockets, scoped_sockets)
> {
> pid_t child;
> int status;
> @@ -942,25 +1076,10 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
> int unnamed_sockets[2];
> int stream_pathname_socket, dgram_pathname_socket,
> stream_abstract_socket, dgram_abstract_socket, data_socket;
> - struct service_fixture stream_abstract_addr, dgram_abstract_addr;
> - struct sockaddr_un stream_pathname_addr = {
> - .sun_family = AF_UNIX,
> - };
> - struct sockaddr_un dgram_pathname_addr = {
> - .sun_family = AF_UNIX,
> - };
> -
> - /* Pathname address. */
> - snprintf(stream_pathname_addr.sun_path,
> - sizeof(stream_pathname_addr.sun_path), "%s", stream_path);
> - snprintf(dgram_pathname_addr.sun_path,
> - sizeof(dgram_pathname_addr.sun_path), "%s", dgram_path);
> -
> - /* Abstract address. */
> - memset(&stream_abstract_addr, 0, sizeof(stream_abstract_addr));
> - set_unix_address(&stream_abstract_addr, 0, true);
> - memset(&dgram_abstract_addr, 0, sizeof(dgram_abstract_addr));
> - set_unix_address(&dgram_abstract_addr, 1, true);
> + bool pathname_restricted =
> + (variant->scope_bits & LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET);
> + bool abstract_restricted =
> + (variant->scope_bits & LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
>
> /* Unnamed address for datagram socket. */
> ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_DGRAM, 0, unnamed_sockets));
> @@ -975,82 +1094,103 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
> EXPECT_EQ(0, close(pipe_parent[1]));
> EXPECT_EQ(0, close(unnamed_sockets[1]));
>
> - if (variant->domain == SCOPE_SANDBOX)
> - create_scoped_domain(
> - _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> - else if (variant->domain == OTHER_SANDBOX)
> + /* Create domain based on variant. */
> + if (variant->scope_bits)
> + create_scoped_domain(_metadata, variant->scope_bits);
> + else if (!variant->no_sandbox)
> create_fs_domain(_metadata);
>
> /* Waits for parent to listen. */
> ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
> EXPECT_EQ(0, close(pipe_parent[0]));
>
> - /* Checks that we can send data through a datagram socket. */
> + /* Checks that we can send data through a unnamed socket. */
an unnamed
> ASSERT_EQ(1, write(unnamed_sockets[0], "a", 1));
> EXPECT_EQ(0, close(unnamed_sockets[0]));
>
> /* Connects with pathname sockets. */
> stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_pathname_socket);
> - ASSERT_EQ(0,
> - connect(stream_pathname_socket, &stream_pathname_addr,
> - sizeof(stream_pathname_addr)));
> - ASSERT_EQ(1, write(stream_pathname_socket, "b", 1));
> + err = connect(stream_pathname_socket,
> + &self->stream_pathname_addr.unix_addr,
> + self->stream_pathname_addr.unix_addr_len);
> + if (pathname_restricted) {
> + EXPECT_EQ(-1, err);
> + EXPECT_EQ(EPERM, errno);
> + } else {
> + EXPECT_EQ(0, err);
> + ASSERT_EQ(1, write(stream_pathname_socket, "b", 1));
> + }
> EXPECT_EQ(0, close(stream_pathname_socket));
>
> - /* Sends without connection. */
> + /* Sends without connection (pathname). */
> dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_pathname_socket);
> err = sendto(dgram_pathname_socket, "c", 1, 0,
> - &dgram_pathname_addr, sizeof(dgram_pathname_addr));
> - EXPECT_EQ(1, err);
> + &self->dgram_pathname_addr.unix_addr,
> + self->dgram_pathname_addr.unix_addr_len);
> + if (pathname_restricted) {
> + EXPECT_EQ(-1, err);
> + EXPECT_EQ(EPERM, errno);
> + } else {
> + EXPECT_EQ(1, err);
> + }
> +
> + /* Sends with connection (pathname). */
> + err = connect(dgram_pathname_socket,
> + &self->dgram_pathname_addr.unix_addr,
> + self->dgram_pathname_addr.unix_addr_len);
> + if (pathname_restricted) {
> + EXPECT_EQ(-1, err);
> + EXPECT_EQ(EPERM, errno);
> + } else {
> + EXPECT_EQ(0, err);
> + ASSERT_EQ(1, write(dgram_pathname_socket, "d", 1));
> + }
>
> - /* Sends with connection. */
> - ASSERT_EQ(0,
> - connect(dgram_pathname_socket, &dgram_pathname_addr,
> - sizeof(dgram_pathname_addr)));
> - ASSERT_EQ(1, write(dgram_pathname_socket, "d", 1));
> EXPECT_EQ(0, close(dgram_pathname_socket));
>
> /* Connects with abstract sockets. */
> stream_abstract_socket = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_abstract_socket);
> err = connect(stream_abstract_socket,
> - &stream_abstract_addr.unix_addr,
> - stream_abstract_addr.unix_addr_len);
> - if (variant->domain == SCOPE_SANDBOX) {
> + &self->stream_abstract_addr.unix_addr,
> + self->stream_abstract_addr.unix_addr_len);
> + if (abstract_restricted) {
> EXPECT_EQ(-1, err);
> EXPECT_EQ(EPERM, errno);
> } else {
> EXPECT_EQ(0, err);
> ASSERT_EQ(1, write(stream_abstract_socket, "e", 1));
> }
> +
> EXPECT_EQ(0, close(stream_abstract_socket));
>
> - /* Sends without connection. */
> + /* Sends without connection (abstract). */
> dgram_abstract_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_abstract_socket);
> err = sendto(dgram_abstract_socket, "f", 1, 0,
> - &dgram_abstract_addr.unix_addr,
> - dgram_abstract_addr.unix_addr_len);
> - if (variant->domain == SCOPE_SANDBOX) {
> + &self->dgram_abstract_addr.unix_addr,
> + self->dgram_abstract_addr.unix_addr_len);
> + if (abstract_restricted) {
> EXPECT_EQ(-1, err);
> EXPECT_EQ(EPERM, errno);
> } else {
> EXPECT_EQ(1, err);
> }
>
> - /* Sends with connection. */
> + /* Sends with connection (abstract). */
> err = connect(dgram_abstract_socket,
> - &dgram_abstract_addr.unix_addr,
> - dgram_abstract_addr.unix_addr_len);
> - if (variant->domain == SCOPE_SANDBOX) {
> + &self->dgram_abstract_addr.unix_addr,
> + self->dgram_abstract_addr.unix_addr_len);
> + if (abstract_restricted) {
> EXPECT_EQ(-1, err);
> EXPECT_EQ(EPERM, errno);
> } else {
> EXPECT_EQ(0, err);
> ASSERT_EQ(1, write(dgram_abstract_socket, "g", 1));
> }
> +
> EXPECT_EQ(0, close(dgram_abstract_socket));
>
> _exit(_metadata->exit_code);
> @@ -1062,27 +1202,30 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
> /* Sets up pathname servers. */
> stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_pathname_socket);
> - ASSERT_EQ(0, bind(stream_pathname_socket, &stream_pathname_addr,
> - sizeof(stream_pathname_addr)));
> + ASSERT_EQ(0, bind(stream_pathname_socket,
> + &self->stream_pathname_addr.unix_addr,
> + self->stream_pathname_addr.unix_addr_len));
> ASSERT_EQ(0, listen(stream_pathname_socket, backlog));
>
> dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_pathname_socket);
> - ASSERT_EQ(0, bind(dgram_pathname_socket, &dgram_pathname_addr,
> - sizeof(dgram_pathname_addr)));
> + ASSERT_EQ(0, bind(dgram_pathname_socket,
> + &self->dgram_pathname_addr.unix_addr,
> + self->dgram_pathname_addr.unix_addr_len));
>
> /* Sets up abstract servers. */
> stream_abstract_socket = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_abstract_socket);
> - ASSERT_EQ(0,
> - bind(stream_abstract_socket, &stream_abstract_addr.unix_addr,
> - stream_abstract_addr.unix_addr_len));
> + ASSERT_EQ(0, bind(stream_abstract_socket,
> + &self->stream_abstract_addr.unix_addr,
> + self->stream_abstract_addr.unix_addr_len));
> + ASSERT_EQ(0, listen(stream_abstract_socket, backlog));
>
> dgram_abstract_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_abstract_socket);
> - ASSERT_EQ(0, bind(dgram_abstract_socket, &dgram_abstract_addr.unix_addr,
> - dgram_abstract_addr.unix_addr_len));
> - ASSERT_EQ(0, listen(stream_abstract_socket, backlog));
> + ASSERT_EQ(0, bind(dgram_abstract_socket,
> + &self->dgram_abstract_addr.unix_addr,
> + self->dgram_abstract_addr.unix_addr_len));
>
> ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> EXPECT_EQ(0, close(pipe_parent[1]));
> @@ -1092,24 +1235,31 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
> ASSERT_EQ('a', buf_parent);
> EXPECT_LE(0, close(unnamed_sockets[1]));
>
> - /* Reads from pathname sockets. */
> - data_socket = accept(stream_pathname_socket, NULL, NULL);
> - ASSERT_LE(0, data_socket);
> - ASSERT_EQ(1, read(data_socket, &buf_parent, sizeof(buf_parent)));
> - ASSERT_EQ('b', buf_parent);
> - EXPECT_EQ(0, close(data_socket));
> - EXPECT_EQ(0, close(stream_pathname_socket));
> + if (!pathname_restricted) {
> + /*
> + * Reads from pathname sockets if we expect child to be able to
> + * send.
> + */
> + data_socket = accept(stream_pathname_socket, NULL, NULL);
> + ASSERT_LE(0, data_socket);
> + ASSERT_EQ(1,
> + read(data_socket, &buf_parent, sizeof(buf_parent)));
> + ASSERT_EQ('b', buf_parent);
> + EXPECT_EQ(0, close(data_socket));
>
> - ASSERT_EQ(1,
> - read(dgram_pathname_socket, &buf_parent, sizeof(buf_parent)));
> - ASSERT_EQ('c', buf_parent);
> - ASSERT_EQ(1,
> - read(dgram_pathname_socket, &buf_parent, sizeof(buf_parent)));
> - ASSERT_EQ('d', buf_parent);
> - EXPECT_EQ(0, close(dgram_pathname_socket));
> + ASSERT_EQ(1, read(dgram_pathname_socket, &buf_parent,
> + sizeof(buf_parent)));
> + ASSERT_EQ('c', buf_parent);
> + ASSERT_EQ(1, read(dgram_pathname_socket, &buf_parent,
> + sizeof(buf_parent)));
> + ASSERT_EQ('d', buf_parent);
> + }
>
> - if (variant->domain != SCOPE_SANDBOX) {
> - /* Reads from abstract sockets if allowed to send. */
> + if (!abstract_restricted) {
> + /*
> + * Reads from abstract sockets if we expect child to be able to
> + * send.
> + */
> data_socket = accept(stream_abstract_socket, NULL, NULL);
> ASSERT_LE(0, data_socket);
> ASSERT_EQ(1,
> @@ -1125,30 +1275,73 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
> ASSERT_EQ('g', buf_parent);
> }
>
> - /* Waits for all abstract socket tests. */
> + /* Waits for child to complete, and only close the socket afterwards. */
> ASSERT_EQ(child, waitpid(child, &status, 0));
> EXPECT_EQ(0, close(stream_abstract_socket));
> EXPECT_EQ(0, close(dgram_abstract_socket));
> + EXPECT_EQ(0, close(stream_pathname_socket));
> + EXPECT_EQ(0, close(dgram_pathname_socket));
>
> if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> WEXITSTATUS(status) != EXIT_SUCCESS)
> _metadata->exit_code = KSFT_FAIL;
> }
>
> -TEST(datagram_sockets)
> +/* Fixture for datagram_sockets and self_connect tests */
> +FIXTURE(socket_type_test)
> {
> struct service_fixture connected_addr, non_connected_addr;
> +};
> +
> +FIXTURE_VARIANT(socket_type_test)
> +{
> + const bool abstract;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(socket_type_test, abstract) {
> + /* clang-format on */
> + .abstract = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(socket_type_test, pathname) {
> + /* clang-format on */
> + .abstract = false,
> +};
> +
> +FIXTURE_SETUP(socket_type_test)
> +{
> + drop_caps(_metadata);
> +
> + if (!variant->abstract)
> + ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
> +
> + memset(&self->connected_addr, 0, sizeof(self->connected_addr));
> + set_unix_address(&self->connected_addr, 0, variant->abstract);
> + memset(&self->non_connected_addr, 0, sizeof(self->non_connected_addr));
> + set_unix_address(&self->non_connected_addr, 1, variant->abstract);
> +}
> +
> +FIXTURE_TEARDOWN(socket_type_test)
> +{
> + if (!variant->abstract) {
> + EXPECT_EQ(0, remove_path(self->connected_addr.unix_addr.sun_path));
> + EXPECT_EQ(0, remove_path(self->non_connected_addr.unix_addr.sun_path));
> + EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
> + }
> +}
> +
> +TEST_F(socket_type_test, datagram_sockets)
> +{
> int server_conn_socket, server_unconn_socket;
> int pipe_parent[2], pipe_child[2];
> int status;
> char buf;
> pid_t child;
> -
> - drop_caps(_metadata);
> - memset(&connected_addr, 0, sizeof(connected_addr));
> - set_unix_address(&connected_addr, 0, true);
> - memset(&non_connected_addr, 0, sizeof(non_connected_addr));
> - set_unix_address(&non_connected_addr, 1, true);
> + const __u16 scope = variant->abstract ?
> + LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
>
> ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> @@ -1169,8 +1362,9 @@ TEST(datagram_sockets)
> /* Waits for parent to listen. */
> ASSERT_EQ(1, read(pipe_parent[0], &buf, 1));
> ASSERT_EQ(0,
> - connect(client_conn_socket, &connected_addr.unix_addr,
> - connected_addr.unix_addr_len));
> + connect(client_conn_socket,
> + &self->connected_addr.unix_addr,
> + self->connected_addr.unix_addr_len));
>
> /*
> * Both connected and non-connected sockets can send data when
> @@ -1178,13 +1372,12 @@ TEST(datagram_sockets)
> */
> ASSERT_EQ(1, send(client_conn_socket, ".", 1, 0));
> ASSERT_EQ(1, sendto(client_unconn_socket, ".", 1, 0,
> - &non_connected_addr.unix_addr,
> - non_connected_addr.unix_addr_len));
> + &self->non_connected_addr.unix_addr,
> + self->non_connected_addr.unix_addr_len));
> ASSERT_EQ(1, write(pipe_child[1], ".", 1));
>
> /* Scopes the domain. */
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> /*
> * Connected socket sends data to the receiver, but the
> @@ -1192,8 +1385,8 @@ TEST(datagram_sockets)
> */
> ASSERT_EQ(1, send(client_conn_socket, ".", 1, 0));
> ASSERT_EQ(-1, sendto(client_unconn_socket, ".", 1, 0,
> - &non_connected_addr.unix_addr,
> - non_connected_addr.unix_addr_len));
> + &self->non_connected_addr.unix_addr,
> + self->non_connected_addr.unix_addr_len));
> ASSERT_EQ(EPERM, errno);
> ASSERT_EQ(1, write(pipe_child[1], ".", 1));
>
> @@ -1210,10 +1403,11 @@ TEST(datagram_sockets)
> ASSERT_LE(0, server_conn_socket);
> ASSERT_LE(0, server_unconn_socket);
>
> - ASSERT_EQ(0, bind(server_conn_socket, &connected_addr.unix_addr,
> - connected_addr.unix_addr_len));
> - ASSERT_EQ(0, bind(server_unconn_socket, &non_connected_addr.unix_addr,
> - non_connected_addr.unix_addr_len));
> + ASSERT_EQ(0, bind(server_conn_socket, &self->connected_addr.unix_addr,
> + self->connected_addr.unix_addr_len));
> + ASSERT_EQ(0, bind(server_unconn_socket,
> + &self->non_connected_addr.unix_addr,
> + self->non_connected_addr.unix_addr_len));
> ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
>
> /* Waits for child to test. */
> @@ -1238,52 +1432,49 @@ TEST(datagram_sockets)
> _metadata->exit_code = KSFT_FAIL;
> }
>
> -TEST(self_connect)
> +TEST_F(socket_type_test, self_connect)
> {
> - struct service_fixture connected_addr, non_connected_addr;
> int connected_socket, non_connected_socket, status;
> pid_t child;
> -
> - drop_caps(_metadata);
> - memset(&connected_addr, 0, sizeof(connected_addr));
> - set_unix_address(&connected_addr, 0, true);
> - memset(&non_connected_addr, 0, sizeof(non_connected_addr));
> - set_unix_address(&non_connected_addr, 1, true);
> + const __u16 scope = variant->abstract ?
> + LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
>
> connected_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
> non_connected_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, connected_socket);
> ASSERT_LE(0, non_connected_socket);
>
> - ASSERT_EQ(0, bind(connected_socket, &connected_addr.unix_addr,
> - connected_addr.unix_addr_len));
> - ASSERT_EQ(0, bind(non_connected_socket, &non_connected_addr.unix_addr,
> - non_connected_addr.unix_addr_len));
> + ASSERT_EQ(0, bind(connected_socket, &self->connected_addr.unix_addr,
> + self->connected_addr.unix_addr_len));
> + ASSERT_EQ(0, bind(non_connected_socket,
> + &self->non_connected_addr.unix_addr,
> + self->non_connected_addr.unix_addr_len));
>
> child = fork();
> ASSERT_LE(0, child);
> if (child == 0) {
> /* Child's domain is scoped. */
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> /*
> * The child inherits the sockets, and cannot connect or
> * send data to them.
> */
> ASSERT_EQ(-1,
> - connect(connected_socket, &connected_addr.unix_addr,
> - connected_addr.unix_addr_len));
> + connect(connected_socket,
> + &self->connected_addr.unix_addr,
> + self->connected_addr.unix_addr_len));
> ASSERT_EQ(EPERM, errno);
>
> ASSERT_EQ(-1, sendto(connected_socket, ".", 1, 0,
> - &connected_addr.unix_addr,
> - connected_addr.unix_addr_len));
> + &self->connected_addr.unix_addr,
> + self->connected_addr.unix_addr_len));
> ASSERT_EQ(EPERM, errno);
>
> ASSERT_EQ(-1, sendto(non_connected_socket, ".", 1, 0,
> - &non_connected_addr.unix_addr,
> - non_connected_addr.unix_addr_len));
> + &self->non_connected_addr.unix_addr,
> + self->non_connected_addr.unix_addr_len));
> ASSERT_EQ(EPERM, errno);
>
> EXPECT_EQ(0, close(connected_socket));
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v2 5/6] selftests/landlock: Repurpose scoped_abstract_unix_test.c for pathname sockets too.
From: Mickaël Salaün @ 2026-01-29 21:28 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <53b9883648225d5a08e82d2636ab0b4fda003bc9.1767115163.git.m@maowtm.org>
Commit messages should fit in 72 columns. The subject can be a bit more
but we should avoid that, and it should not end with a dot.
On Tue, Dec 30, 2025 at 05:20:23PM +0000, Tingmao Wang wrote:
> Since there is very little difference between abstract and pathname
> sockets in terms of testing of the scoped access checks (the only
> difference is in which scope bit control which form of socket), it makes
> sense to reuse the existing test for both type of sockets. Therefore, we
> rename scoped_abstract_unix_test.c to scoped_unix_test.c and extend the
> scoped_domains test to test pathname (i.e. non-abstract) sockets too.
>
> Since we can't change the variant data of scoped_domains (as it is defined
> in the shared .h file), we do this by extracting the actual test code into
> a function, and call it from different test cases.
>
> Also extend scoped_audit (this time we can use variants) to test both
> abstract and pathname sockets. For pathname sockets, audit_log_lsm_data
> will produce path="..." (or hex if path contains control characters) with
> absolute paths from the dentry, so we need to construct the escaped regex
> for the real path like in fs_test.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> ...bstract_unix_test.c => scoped_unix_test.c} | 256 ++++++++++++++----
> 1 file changed, 206 insertions(+), 50 deletions(-)
> rename tools/testing/selftests/landlock/{scoped_abstract_unix_test.c => scoped_unix_test.c} (81%)
>
> diff --git a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c b/tools/testing/selftests/landlock/scoped_unix_test.c
> similarity index 81%
> rename from tools/testing/selftests/landlock/scoped_abstract_unix_test.c
> rename to tools/testing/selftests/landlock/scoped_unix_test.c
> index 4a790e2d387d..669418c97509 100644
> --- a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
> +++ b/tools/testing/selftests/landlock/scoped_unix_test.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Landlock tests - Abstract UNIX socket
> + * Landlock tests - Scoped access checks for UNIX socket (abstract and
> + * pathname)
> *
> * Copyright © 2024 Tahera Fahimi <fahimitahera@gmail.com>
> */
> @@ -19,6 +20,7 @@
> #include <sys/un.h>
> #include <sys/wait.h>
> #include <unistd.h>
> +#include <stdlib.h>
>
> #include "audit.h"
> #include "common.h"
> @@ -47,7 +49,8 @@ static void create_fs_domain(struct __test_metadata *const _metadata)
>
> FIXTURE(scoped_domains)
> {
> - struct service_fixture stream_address, dgram_address;
> + struct service_fixture stream_address_abstract, dgram_address_abstract,
> + stream_address_pathname, dgram_address_pathname;
> };
>
> #include "scoped_base_variants.h"
> @@ -56,27 +59,62 @@ FIXTURE_SETUP(scoped_domains)
> {
> drop_caps(_metadata);
>
> - memset(&self->stream_address, 0, sizeof(self->stream_address));
> - memset(&self->dgram_address, 0, sizeof(self->dgram_address));
> - set_unix_address(&self->stream_address, 0, true);
> - set_unix_address(&self->dgram_address, 1, true);
> + ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
> +
> + memset(&self->stream_address_abstract, 0,
> + sizeof(self->stream_address_abstract));
> + memset(&self->dgram_address_abstract, 0,
> + sizeof(self->dgram_address_abstract));
> + memset(&self->stream_address_pathname, 0,
> + sizeof(self->stream_address_pathname));
> + memset(&self->dgram_address_pathname, 0,
> + sizeof(self->dgram_address_pathname));
> + set_unix_address(&self->stream_address_abstract, 0, true);
> + set_unix_address(&self->dgram_address_abstract, 1, true);
> + set_unix_address(&self->stream_address_pathname, 0, false);
> + set_unix_address(&self->dgram_address_pathname, 1, false);
> +}
> +
> +/* Remove @path if it exists */
> +int remove_path(const char *path)
> +{
> + if (unlink(path) == -1) {
> + if (errno != ENOENT)
> + return -errno;
> + }
> + return 0;
> }
>
> FIXTURE_TEARDOWN(scoped_domains)
> {
> + EXPECT_EQ(0, remove_path(self->stream_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, remove_path(self->dgram_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
> }
>
> /*
> * Test unix_stream_connect() and unix_may_send() for a child connecting to its
> * parent, when they have scoped domain or no domain.
> */
> -TEST_F(scoped_domains, connect_to_parent)
> +static void test_connect_to_parent(struct __test_metadata *const _metadata,
> + FIXTURE_DATA(scoped_domains) * self,
> + const FIXTURE_VARIANT(scoped_domains) *
> + variant,
> + const bool abstract)
> {
> pid_t child;
> bool can_connect_to_parent;
> int status;
> int pipe_parent[2];
> int stream_server, dgram_server;
> + const __u16 scope = abstract ? LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> + const struct service_fixture *stream_address =
> + abstract ? &self->stream_address_abstract :
> + &self->stream_address_pathname;
> + const struct service_fixture *dgram_address =
> + abstract ? &self->dgram_address_abstract :
> + &self->dgram_address_pathname;
>
> /*
> * can_connect_to_parent is true if a child process can connect to its
> @@ -87,8 +125,7 @@ TEST_F(scoped_domains, connect_to_parent)
>
> ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> if (variant->domain_both) {
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
> if (!__test_passed(_metadata))
> return;
> }
> @@ -102,8 +139,7 @@ TEST_F(scoped_domains, connect_to_parent)
>
> EXPECT_EQ(0, close(pipe_parent[1]));
> if (variant->domain_child)
> - create_scoped_domain(
> - _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> stream_client = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_client);
> @@ -113,8 +149,8 @@ TEST_F(scoped_domains, connect_to_parent)
> /* Waits for the server. */
> ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
>
> - err = connect(stream_client, &self->stream_address.unix_addr,
> - self->stream_address.unix_addr_len);
> + err = connect(stream_client, &stream_address->unix_addr,
> + stream_address->unix_addr_len);
> if (can_connect_to_parent) {
> EXPECT_EQ(0, err);
> } else {
> @@ -123,8 +159,8 @@ TEST_F(scoped_domains, connect_to_parent)
> }
> EXPECT_EQ(0, close(stream_client));
>
> - err = connect(dgram_client, &self->dgram_address.unix_addr,
> - self->dgram_address.unix_addr_len);
> + err = connect(dgram_client, &dgram_address->unix_addr,
> + dgram_address->unix_addr_len);
> if (can_connect_to_parent) {
> EXPECT_EQ(0, err);
> } else {
> @@ -137,17 +173,16 @@ TEST_F(scoped_domains, connect_to_parent)
> }
> EXPECT_EQ(0, close(pipe_parent[0]));
> if (variant->domain_parent)
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> stream_server = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, stream_server);
> dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_server);
> - ASSERT_EQ(0, bind(stream_server, &self->stream_address.unix_addr,
> - self->stream_address.unix_addr_len));
> - ASSERT_EQ(0, bind(dgram_server, &self->dgram_address.unix_addr,
> - self->dgram_address.unix_addr_len));
> + ASSERT_EQ(0, bind(stream_server, &stream_address->unix_addr,
> + stream_address->unix_addr_len));
> + ASSERT_EQ(0, bind(dgram_server, &dgram_address->unix_addr,
> + dgram_address->unix_addr_len));
> ASSERT_EQ(0, listen(stream_server, backlog));
>
> /* Signals to child that the parent is listening. */
> @@ -166,7 +201,11 @@ TEST_F(scoped_domains, connect_to_parent)
> * Test unix_stream_connect() and unix_may_send() for a parent connecting to
> * its child, when they have scoped domain or no domain.
> */
> -TEST_F(scoped_domains, connect_to_child)
> +static void test_connect_to_child(struct __test_metadata *const _metadata,
> + FIXTURE_DATA(scoped_domains) * self,
> + const FIXTURE_VARIANT(scoped_domains) *
> + variant,
> + const bool abstract)
> {
> pid_t child;
> bool can_connect_to_child;
> @@ -174,6 +213,14 @@ TEST_F(scoped_domains, connect_to_child)
> int pipe_child[2], pipe_parent[2];
> char buf;
> int stream_client, dgram_client;
> + const __u16 scope = abstract ? LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> + const struct service_fixture *stream_address =
> + abstract ? &self->stream_address_abstract :
> + &self->stream_address_pathname;
> + const struct service_fixture *dgram_address =
> + abstract ? &self->dgram_address_abstract :
> + &self->dgram_address_pathname;
>
> /*
> * can_connect_to_child is true if a parent process can connect to its
> @@ -185,8 +232,7 @@ TEST_F(scoped_domains, connect_to_child)
> ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> if (variant->domain_both) {
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
> if (!__test_passed(_metadata))
> return;
> }
> @@ -199,8 +245,7 @@ TEST_F(scoped_domains, connect_to_child)
> EXPECT_EQ(0, close(pipe_parent[1]));
> EXPECT_EQ(0, close(pipe_child[0]));
> if (variant->domain_child)
> - create_scoped_domain(
> - _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> /* Waits for the parent to be in a domain, if any. */
> ASSERT_EQ(1, read(pipe_parent[0], &buf, 1));
> @@ -209,11 +254,10 @@ TEST_F(scoped_domains, connect_to_child)
> ASSERT_LE(0, stream_server);
> dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_server);
> - ASSERT_EQ(0,
> - bind(stream_server, &self->stream_address.unix_addr,
> - self->stream_address.unix_addr_len));
> - ASSERT_EQ(0, bind(dgram_server, &self->dgram_address.unix_addr,
> - self->dgram_address.unix_addr_len));
> + ASSERT_EQ(0, bind(stream_server, &stream_address->unix_addr,
> + stream_address->unix_addr_len));
> + ASSERT_EQ(0, bind(dgram_server, &dgram_address->unix_addr,
> + dgram_address->unix_addr_len));
> ASSERT_EQ(0, listen(stream_server, backlog));
>
> /* Signals to the parent that child is listening. */
> @@ -230,8 +274,7 @@ TEST_F(scoped_domains, connect_to_child)
> EXPECT_EQ(0, close(pipe_parent[0]));
>
> if (variant->domain_parent)
> - create_scoped_domain(_metadata,
> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata, scope);
>
> /* Signals that the parent is in a domain, if any. */
> ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> @@ -243,11 +286,11 @@ TEST_F(scoped_domains, connect_to_child)
>
> /* Waits for the child to listen */
> ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
> - err_stream = connect(stream_client, &self->stream_address.unix_addr,
> - self->stream_address.unix_addr_len);
> + err_stream = connect(stream_client, &stream_address->unix_addr,
> + stream_address->unix_addr_len);
> errno_stream = errno;
> - err_dgram = connect(dgram_client, &self->dgram_address.unix_addr,
> - self->dgram_address.unix_addr_len);
> + err_dgram = connect(dgram_client, &dgram_address->unix_addr,
> + dgram_address->unix_addr_len);
> errno_dgram = errno;
> if (can_connect_to_child) {
> EXPECT_EQ(0, err_stream);
> @@ -268,19 +311,79 @@ TEST_F(scoped_domains, connect_to_child)
> _metadata->exit_code = KSFT_FAIL;
> }
>
> +/*
> + * Test unix_stream_connect() and unix_may_send() for a child connecting to its
> + * parent, when they have scoped domain or no domain.
> + */
> +TEST_F(scoped_domains, abstract_connect_to_parent)
> +{
> + test_connect_to_parent(_metadata, self, variant, true);
> +}
> +
> +/*
> + * Test unix_stream_connect() and unix_may_send() for a parent connecting to
> + * its child, when they have scoped domain or no domain.
> + */
> +TEST_F(scoped_domains, abstract_connect_to_child)
> +{
> + test_connect_to_child(_metadata, self, variant, true);
> +}
> +
> +/*
> + * Test unix_stream_connect() and unix_may_send() for a child connecting to its
> + * parent with pathname sockets.
> + */
> +TEST_F(scoped_domains, pathname_connect_to_parent)
> +{
> + test_connect_to_parent(_metadata, self, variant, false);
> +}
> +
> +/*
> + * Test unix_stream_connect() and unix_may_send() for a parent connecting to
> + * its child with pathname sockets.
> + */
> +TEST_F(scoped_domains, pathname_connect_to_child)
> +{
> + test_connect_to_child(_metadata, self, variant, false);
> +}
> +
> FIXTURE(scoped_audit)
> {
> - struct service_fixture dgram_address;
> + struct service_fixture dgram_address_abstract, dgram_address_pathname;
> struct audit_filter audit_filter;
> int audit_fd;
> };
>
> +FIXTURE_VARIANT(scoped_audit)
> +{
> + const bool abstract_socket;
> +};
> +
> +// clang-format off
We always use /* */ comments. Ditto for all clang-format markups.
> +FIXTURE_VARIANT_ADD(scoped_audit, abstract_socket)
> +{
> + // clang-format on
> + .abstract_socket = true,
> +};
> +
> +// clang-format off
> +FIXTURE_VARIANT_ADD(scoped_audit, pathname_socket)
> +{
> + // clang-format on
> + .abstract_socket = false,
> +};
> +
> FIXTURE_SETUP(scoped_audit)
> {
> disable_caps(_metadata);
>
> - memset(&self->dgram_address, 0, sizeof(self->dgram_address));
> - set_unix_address(&self->dgram_address, 1, true);
> + ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
> + memset(&self->dgram_address_abstract, 0,
> + sizeof(self->dgram_address_abstract));
> + memset(&self->dgram_address_pathname, 0,
> + sizeof(self->dgram_address_pathname));
> + set_unix_address(&self->dgram_address_abstract, 1, true);
> + set_unix_address(&self->dgram_address_pathname, 1, false);
>
> set_cap(_metadata, CAP_AUDIT_CONTROL);
> self->audit_fd = audit_init_with_exe_filter(&self->audit_filter);
> @@ -291,6 +394,8 @@ FIXTURE_SETUP(scoped_audit)
> FIXTURE_TEARDOWN_PARENT(scoped_audit)
> {
> EXPECT_EQ(0, audit_cleanup(-1, NULL));
> + EXPECT_EQ(0, remove_path(self->dgram_address_pathname.unix_addr.sun_path));
> + EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
> }
>
> /* python -c 'print(b"\0selftests-landlock-abstract-unix-".hex().upper())' */
> @@ -308,6 +413,12 @@ TEST_F(scoped_audit, connect_to_child)
> char buf;
> int dgram_client;
> struct audit_records records;
> + struct service_fixture *const dgram_address =
> + variant->abstract_socket ? &self->dgram_address_abstract :
> + &self->dgram_address_pathname;
> + size_t log_match_remaining = 500;
const
Why this number? Could you please follow the same logic as in
matches_log_fs_extra()?
> + char log_match[log_match_remaining];
> + char *log_match_cursor = log_match;
>
> /* Makes sure there is no superfluous logged records. */
> EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> @@ -330,8 +441,8 @@ TEST_F(scoped_audit, connect_to_child)
>
> dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
> ASSERT_LE(0, dgram_server);
> - ASSERT_EQ(0, bind(dgram_server, &self->dgram_address.unix_addr,
> - self->dgram_address.unix_addr_len));
> + ASSERT_EQ(0, bind(dgram_server, &dgram_address->unix_addr,
> + dgram_address->unix_addr_len));
>
> /* Signals to the parent that child is listening. */
> ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> @@ -345,7 +456,9 @@ TEST_F(scoped_audit, connect_to_child)
> EXPECT_EQ(0, close(pipe_child[1]));
> EXPECT_EQ(0, close(pipe_parent[0]));
>
> - create_scoped_domain(_metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + create_scoped_domain(_metadata,
> + LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET);
>
> /* Signals that the parent is in a domain, if any. */
> ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> @@ -355,19 +468,62 @@ TEST_F(scoped_audit, connect_to_child)
>
> /* Waits for the child to listen */
> ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
> - err_dgram = connect(dgram_client, &self->dgram_address.unix_addr,
> - self->dgram_address.unix_addr_len);
> + err_dgram = connect(dgram_client, &dgram_address->unix_addr,
> + dgram_address->unix_addr_len);
> EXPECT_EQ(-1, err_dgram);
> EXPECT_EQ(EPERM, errno);
>
> - EXPECT_EQ(
> - 0,
> - audit_match_record(
> - self->audit_fd, AUDIT_LANDLOCK_ACCESS,
> + if (variant->abstract_socket) {
> + log_match_cursor = stpncpy(
> + log_match,
> REGEX_LANDLOCK_PREFIX
> " blockers=scope\\.abstract_unix_socket path=" ABSTRACT_SOCKET_PATH_PREFIX
> "[0-9A-F]\\+$",
> - NULL));
> + log_match_remaining);
> + log_match_remaining =
> + sizeof(log_match) - (log_match_cursor - log_match);
> + ASSERT_NE(0, log_match_remaining);
> + } else {
> + /*
> + * It is assumed that absolute_path does not contain control
> + * characters nor spaces, see audit_string_contains_control().
> + */
> + char *absolute_path =
const char *absolute_path
> + realpath(dgram_address->unix_addr.sun_path, NULL);
> +
> + EXPECT_NE(NULL, absolute_path)
> + {
> + TH_LOG("realpath() failed: %s", strerror(errno));
> + return;
> + }
> +
> + log_match_cursor =
> + stpncpy(log_match,
> + REGEX_LANDLOCK_PREFIX
> + " blockers=scope\\.pathname_unix_socket path=\"",
> + log_match_remaining);
> + log_match_remaining =
> + sizeof(log_match) - (log_match_cursor - log_match);
> + ASSERT_NE(0, log_match_remaining);
> + log_match_cursor = regex_escape(absolute_path, log_match_cursor,
> + log_match_remaining);
> + free(absolute_path);
> + if (log_match_cursor < 0) {
> + TH_LOG("regex_escape() failed (buffer too small)");
> + return;
> + }
> + log_match_remaining =
> + sizeof(log_match) - (log_match_cursor - log_match);
> + ASSERT_NE(0, log_match_remaining);
> + log_match_cursor =
> + stpncpy(log_match_cursor, "\"$", log_match_remaining);
> + log_match_remaining =
> + sizeof(log_match) - (log_match_cursor - log_match);
> + ASSERT_NE(0, log_match_remaining);
> + }
> +
> + EXPECT_EQ(0, audit_match_record(self->audit_fd, AUDIT_LANDLOCK_ACCESS,
> + log_match, NULL));
>
> ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> EXPECT_EQ(0, close(dgram_client));
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v2 3/6] samples/landlock: Support LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Mickaël Salaün @ 2026-01-29 21:27 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <5e9ac4104e5f70cdf250a3dea9a553a65a36fff9.1767115163.git.m@maowtm.org>
We should have a (potentially small) description of what this patch
does, even if it's a bit redundant with the subject.
On Tue, Dec 30, 2025 at 05:20:21PM +0000, Tingmao Wang wrote:
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> I've decided to use "u" as the character to control this scope bit since
> it stands for (normal) Unix sockets. Imo using "p" or "n" would make it less
> clear / memorable. Open to suggestions.
Looks good to me.
>
> Also, open to suggestion whether socket scoping (pathname and abstract)
> should be enabled by default, if LL_SCOPED is not set. This would break
> backward compatibility, but maybe we shouldn't guarentee backward
> compatibility of this sandboxer in the first place, and almost all cases
> of Landlock usage would want socket scoping.
I agree that this example could have better defaults, but this should be
done with a standalone patch series. An important point to keep in mind
is that this example is used by developers (e.g. potential copy/paste),
so we need to be careful to not encourage them to create code which is
backward incompatible. I think the best way to do it is to request a
default behavior for a specific Landlock ABI version (e.g. with a new
parameter).
I'd also like this example to still be simple to understand, update, and
maintain.
>
> samples/landlock/sandboxer.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index e7af02f98208..2de14e1c787d 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -234,14 +234,16 @@ static bool check_ruleset_scope(const char *const env_var,
> bool error = false;
> bool abstract_scoping = false;
> bool signal_scoping = false;
> + bool named_scoping = false;
>
> /* Scoping is not supported by Landlock ABI */
> if (!(ruleset_attr->scoped &
> - (LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET | LANDLOCK_SCOPE_SIGNAL)))
> + (LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET | LANDLOCK_SCOPE_SIGNAL |
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET)))
> goto out_unset;
>
> env_type_scope = getenv(env_var);
> - /* Scoping is not supported by the user */
> + /* Scoping is not requested by the user */
> if (!env_type_scope || strcmp("", env_type_scope) == 0)
> goto out_unset;
>
> @@ -254,6 +256,9 @@ static bool check_ruleset_scope(const char *const env_var,
> } else if (strcmp("s", ipc_scoping_name) == 0 &&
> !signal_scoping) {
> signal_scoping = true;
> + } else if (strcmp("u", ipc_scoping_name) == 0 &&
> + !named_scoping) {
> + named_scoping = true;
> } else {
> fprintf(stderr, "Unknown or duplicate scope \"%s\"\n",
> ipc_scoping_name);
> @@ -270,6 +275,8 @@ static bool check_ruleset_scope(const char *const env_var,
> ruleset_attr->scoped &= ~LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
> if (!signal_scoping)
> ruleset_attr->scoped &= ~LANDLOCK_SCOPE_SIGNAL;
> + if (!named_scoping)
> + ruleset_attr->scoped &= ~LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
>
> unsetenv(env_var);
> return error;
> @@ -299,7 +306,7 @@ static bool check_ruleset_scope(const char *const env_var,
>
> /* clang-format on */
>
> -#define LANDLOCK_ABI_LAST 7
> +#define LANDLOCK_ABI_LAST 8
>
> #define XSTR(s) #s
> #define STR(s) XSTR(s)
> @@ -325,6 +332,7 @@ static const char help[] =
> "* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
> " - \"a\" to restrict opening abstract unix sockets\n"
> " - \"s\" to restrict sending signals\n"
> + " - \"u\" to restrict opening pathname (non-abstract) unix sockets\n"
> "\n"
> "A sandboxer should not log denied access requests to avoid spamming logs, "
> "but to test audit we can set " ENV_FORCE_LOG_NAME "=1\n"
> @@ -334,7 +342,7 @@ static const char help[] =
> ENV_FS_RW_NAME "=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
> ENV_TCP_BIND_NAME "=\"9418\" "
> ENV_TCP_CONNECT_NAME "=\"80:443\" "
> - ENV_SCOPED_NAME "=\"a:s\" "
> + ENV_SCOPED_NAME "=\"a:s:u\" "
> "%1$s bash -i\n"
> "\n"
> "This sandboxer can use Landlock features up to ABI version "
> @@ -356,7 +364,8 @@ int main(const int argc, char *const argv[], char *const *const envp)
> .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
> LANDLOCK_ACCESS_NET_CONNECT_TCP,
> .scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> - LANDLOCK_SCOPE_SIGNAL,
> + LANDLOCK_SCOPE_SIGNAL |
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
> };
> int supported_restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
> int set_restrict_flags = 0;
> @@ -436,6 +445,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
> /* Removes LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON for ABI < 7 */
> supported_restrict_flags &=
> ~LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
> + __attribute__((fallthrough));
> + case 7:
> + /* Removes LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET for ABI < 8 */
> + ruleset_attr.scoped &= ~LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
>
> /* Must be printed for any ABI < LANDLOCK_ABI_LAST. */
> fprintf(stderr,
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v2 1/6] landlock: Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET scope bit to uAPI
From: Mickaël Salaün @ 2026-01-29 21:27 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <88f914eb2024fa1fffe219e34c33a835b31730b8.1767115163.git.m@maowtm.org>
On Tue, Dec 30, 2025 at 05:20:19PM +0000, Tingmao Wang wrote:
> Add the new scope bit to the uAPI header, add documentation, and bump ABI
> version to 8.
This patch and the next one should be fold together. If a new UAPI is
added, it should come with the kernel implementation.
>
> This documentation edit specifically calls out the security implications of
> not restricting sockets.
>
> Fix some minor cosmetic issue in landlock.h around the changed lines as
> well.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes in v2:
> - Fix grammar
>
> Note that in the code block in "Defining and enforcing a security policy"
> the switch case currently jumps from 5 to 7. This should be fixed by
> https://lore.kernel.org/all/20251216210248.4150777-1-samasth.norway.ananda@oracle.com/
>
> Documentation/userspace-api/landlock.rst | 37 ++++++++++++++++---
> include/uapi/linux/landlock.h | 8 +++-
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 2 +-
> tools/testing/selftests/landlock/base_test.c | 2 +-
> .../testing/selftests/landlock/scoped_test.c | 2 +-
> 6 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 1d0c2c15c22e..5620a2be1091 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -83,7 +83,8 @@ to be explicit about the denied-by-default access rights.
> LANDLOCK_ACCESS_NET_CONNECT_TCP,
> .scoped =
> LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> - LANDLOCK_SCOPE_SIGNAL,
> + LANDLOCK_SCOPE_SIGNAL |
> + LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
> };
>
> Because we may not know which kernel version an application will be executed
> @@ -127,6 +128,10 @@ version, and only use the available subset of access rights:
> /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
> ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> LANDLOCK_SCOPE_SIGNAL);
> + __attribute__((fallthrough));
> + case 7:
> + /* Removes LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET for ABI < 8 */
> + ruleset_attr.scoped &= ~LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> }
>
> This enables the creation of an inclusive ruleset that will contain our rules.
> @@ -328,10 +333,15 @@ The operations which can be scoped are:
> This limits the sending of signals to target processes which run within the
> same or a nested Landlock domain.
>
> -``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET``
> - This limits the set of abstract :manpage:`unix(7)` sockets to which we can
> - :manpage:`connect(2)` to socket addresses which were created by a process in
> - the same or a nested Landlock domain.
> +``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET`` and ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET``
> + This limits the set of :manpage:`unix(7)` sockets to which we can
> + :manpage:`connect(2)` to socket addresses which were created by a
> + process in the same or a nested Landlock domain.
> + ``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET`` applies to abstract sockets,
> + and ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` applies to pathname
> + sockets.
The following part is not needed:
> Even though pathname sockets are represented in the
> + filesystem, Landlock filesystem rules do not currently control access
> + to them.
>
> A :manpage:`sendto(2)` on a non-connected datagram socket is treated as if
> it were doing an implicit :manpage:`connect(2)` and will be blocked if the
> @@ -604,6 +614,23 @@ Landlock audit events with the ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``,
> sys_landlock_restrict_self(). See Documentation/admin-guide/LSM/landlock.rst
> for more details on audit.
>
> +Pathname UNIX socket (ABI < 8)
> +------------------------------
> +
> +Starting with the Landlock ABI version 8, it is possible to restrict
> +connections to a pathname (non-abstract) :manpage:`unix(7)` socket by
> +setting ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` to the ``scoped`` ruleset
> +attribute. This works the same way as the abstract socket scoping.
> +
> +This allows sandboxing applications using only Landlock to protect against
> +bypasses relying on connecting to Unix sockets of other services running
> +under the same user. These services typically assume that any process
> +capable of connecting to a local Unix socket, or connecting with the
> +expected user credentials, is trusted. Without this protection, sandbox
> +escapes may be possible, especially when running in a standard desktop
> +environment, such as by using systemd-run, or sockets exposed by other
> +common applications.
> +
> .. _kernel_support:
>
> Kernel support
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f030adc462ee..590c6d4171a0 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -364,10 +364,14 @@ struct landlock_net_port_attr {
> * related Landlock domain (e.g., a parent domain or a non-sandboxed process).
> * - %LANDLOCK_SCOPE_SIGNAL: Restrict a sandboxed process from sending a signal
> * to another process outside the domain.
> + * - %LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET: Restrict a sandboxed process from
> + * connecting to a pathname UNIX socket created by a process outside the
> + * related Landlock domain.
> */
> /* clang-format off */
> #define LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET (1ULL << 0)
> -#define LANDLOCK_SCOPE_SIGNAL (1ULL << 1)
> -/* clang-format on*/
> +#define LANDLOCK_SCOPE_SIGNAL (1ULL << 1)
> +#define LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET (1ULL << 2)
> +/* clang-format on */
>
> #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 65b5ff051674..d653e14dba10 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -27,7 +27,7 @@
> #define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
> #define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET)
>
> -#define LANDLOCK_LAST_SCOPE LANDLOCK_SCOPE_SIGNAL
> +#define LANDLOCK_LAST_SCOPE LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
> #define LANDLOCK_MASK_SCOPE ((LANDLOCK_LAST_SCOPE << 1) - 1)
> #define LANDLOCK_NUM_SCOPE __const_hweight64(LANDLOCK_MASK_SCOPE)
>
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0116e9f93ffe..66fd196be85a 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -161,7 +161,7 @@ static const struct file_operations ruleset_fops = {
> * Documentation/userspace-api/landlock.rst should be updated to reflect the
> * UAPI change.
> */
> -const int landlock_abi_version = 7;
> +const int landlock_abi_version = 8;
>
> /**
> * sys_landlock_create_ruleset - Create a new ruleset
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 7b69002239d7..f4b1a275d8d9 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -76,7 +76,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(7, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/scoped_test.c b/tools/testing/selftests/landlock/scoped_test.c
> index b90f76ed0d9c..7f83512a328d 100644
> --- a/tools/testing/selftests/landlock/scoped_test.c
> +++ b/tools/testing/selftests/landlock/scoped_test.c
> @@ -12,7 +12,7 @@
>
> #include "common.h"
>
> -#define ACCESS_LAST LANDLOCK_SCOPE_SIGNAL
> +#define ACCESS_LAST LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
>
> TEST(ruleset_with_unknown_scope)
> {
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-29 20:28 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260125195853.109967-4-gnoack3000@gmail.com>
On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> The layer masks data structure tracks the requested but unfulfilled
> access rights during an operation's security check. It stores one bit
> for each combination of access right and layer index. If the bit is
> set, that access right is not granted (yet) in the given layer and we
> have to traverse the path further upwards to grant it.
>
> Previously, the layer masks were stored as arrays mapping from access
> right indices to layer_mask_t. The layer_mask_t value then indicates
> all layers in which the given access right is still (tentatively)
> denied.
>
> This patch introduces struct layer_access_masks instead: This struct
> contains an array with the access_mask_t of each (tentatively) denied
> access right in that layer.
>
> The hypothesis of this patch is that this simplifies the code enough
> so that the resulting code will run faster:
>
> * We can use bitwise operations in multiple places where we previously
> looped over bits individually with macros. (Should require less
> branch speculation and lends itself to better loop unrolling.)
>
> * Code is ~75 lines smaller.
>
> Other noteworthy changes:
>
> * In no_more_access(), call a new helper function may_refer(), which
> only solves the asymmetric case. Previously, the code interleaved
> the checks for the two symmetric cases in RENAME_EXCHANGE. It feels
> that the code is clearer when renames without RENAME_EXCHANGE are
> more obviously the normal case.
>
> Tradeoffs:
>
> This change improves performance, at a slight size increase to the
> layer masks data structure.
>
> At the moment, for the filesystem access rights, the data structure
> has the same size as before, but once we introduce the 17th filesystem
> access right, it will double in size (from 32 to 64 bytes), as
> access_mask_t grows from 16 to 32 bit. See the link below for
> measurements.
>
> Link: https://lore.kernel.org/all/20260120.haeCh4li9Vae@digikod.net/
When adding extra links, please add a [1] reference at the end and use
this reference in the commit message.
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> security/landlock/access.h | 10 +-
> security/landlock/audit.c | 84 +++------
> security/landlock/audit.h | 3 +-
> security/landlock/domain.c | 45 +++--
> security/landlock/domain.h | 4 +-
> security/landlock/fs.c | 352 ++++++++++++++++--------------------
> security/landlock/net.c | 11 +-
> security/landlock/ruleset.c | 88 ++++-----
> security/landlock/ruleset.h | 21 ++-
> 9 files changed, 274 insertions(+), 344 deletions(-)
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Lorenzo Stoakes @ 2026-01-29 18:58 UTC (permalink / raw)
To: Paul Moore
Cc: Vlastimil Babka, linux-security-module, linux-integrity, selinux,
john.johansen, zohar, roberto.sassu, wufan, mic, gnoack, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng,
linux-mm, David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <CAHC9VhS_y2gy2nAK7C-OpchME0pzS34-QN2bY9t5SRBXArz8Xw@mail.gmail.com>
On Thu, Jan 29, 2026 at 01:31:05PM -0500, Paul Moore wrote:
> On Thu, Jan 29, 2026 at 12:11 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Thu, Jan 29, 2026 at 06:02:00PM +0100, Vlastimil Babka wrote:
> > > Agreed, the mmap_min_addr should stay visible and applied unconditionally.
> > > AFAICS the only relation to SECURITY/LSM is whether CONFIG_LSM_MMAP_MIN_ADDR
> > > is used as an additional lower limit to both CONFIG_DEFAULT_MMAP_MIN_ADDR
> > > and the sysctl-written value?
> >
> > Thanks, yeah we should probably actually move the non-LSM-relevant stuff
> > out to mm to be honest.
>
> Yes, definitely. Send the LSM and VM lists some patches after the
> upcoming merge window closes and I'll make sure they are merged once
> fully ACK'd.
Great thank you! Will add to todo. I think that's a sensible thing we can
do to help you keep this code sane.
>
> > But that's future work, for an -rc8 hotfix ...
>
> Expect a patch later today.
Perfect thank you very much!
>
> --
> paul-moore.com
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Paul Moore @ 2026-01-29 18:31 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Vlastimil Babka, linux-security-module, linux-integrity, selinux,
john.johansen, zohar, roberto.sassu, wufan, mic, gnoack, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng,
linux-mm, David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <01cb28cb-56b7-4862-bf27-07e4bf17115e@lucifer.local>
On Thu, Jan 29, 2026 at 12:11 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Thu, Jan 29, 2026 at 06:02:00PM +0100, Vlastimil Babka wrote:
> > Agreed, the mmap_min_addr should stay visible and applied unconditionally.
> > AFAICS the only relation to SECURITY/LSM is whether CONFIG_LSM_MMAP_MIN_ADDR
> > is used as an additional lower limit to both CONFIG_DEFAULT_MMAP_MIN_ADDR
> > and the sysctl-written value?
>
> Thanks, yeah we should probably actually move the non-LSM-relevant stuff
> out to mm to be honest.
Yes, definitely. Send the LSM and VM lists some patches after the
upcoming merge window closes and I'll make sure they are merged once
fully ACK'd.
> But that's future work, for an -rc8 hotfix ...
Expect a patch later today.
--
paul-moore.com
^ permalink raw reply
* [PATCH v4] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Frederick Lawler @ 2026-01-29 18:07 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
introduced a means to track change detection for an inode
via ctime updates, opposed to setting kstat.change_cookie to
an i_version when calling into xfs_vn_getattr().
This introduced a regression for IMA such that an action
performed on a LOWER inode on a stacked file systems always
requires a re-evaluation if the LOWER file system does not
leverage kstat.change_cookie to track inode i_version or lacks
i_version support all together.
In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
will require re-evaluation. Stacking TMPFS on XFS for instance, once the
inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
leverages generic_fillattr() to update the change cookie.
This is because IMA caches kstat.change_cookie to compare against an
inode's i_version directly in integrity_inode_attrs_changed(), and thus
could be out of date depending on how file systems set
kstat.change_cookie.
To address this, require integrity_inode_attrs_changed() to query
vfs_getattr_nosec() to compare the cached version against
kstat.change_cookie directly. This ensures that when updates occur,
we're accessing the same changed inode version on changes, and fallback
to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
result mask.
Lastly, because EVM still relies on querying and caching a inode's
i_version directly, the integrity_inode_attrs_changed() falls back to the
original inode.i_version != cached comparison.
Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
Suggested-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
We uncovered a case in kernels >= 6.13 where XFS is no longer updating
struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
using multigrain ctime (as well as other file systems) for
change detection in commit 1cf7e834a6fb ("xfs: switch to
multigrain timestamps").
Because file systems may implement i_version as they see fit, IMA
caching may be behind as well as file systems that don't support/export
i_version. Thus we're proposing to compare against the kstat.change_cookie
directly to the cached version, and fall back to a ctime guard when
that's not updated.
EVM is largely left alone since there's no trivial way to query a file
directly in the LSM call paths to obtain kstat.change_cookie &
kstat.ctime to cache. Thus retains accessing i_version directly.
Regression tests will be added to the Linux Test Project instead of
selftest to help catch future file system changes that may impact
future evaluation of IMA.
I'd like this to be backported to at least 6.18 if possible.
Below is a simplified test that demonstrates the issue:
_fragment.config_
CONFIG_XFS_FS=y
CONFIG_OVERLAY_FS=y
CONFIG_IMA=y
CONFIG_IMA_WRITE_POLICY=y
CONFIG_IMA_READ_POLICY=y
_./test.sh_
IMA_POLICY="/sys/kernel/security/ima/policy"
TEST_BIN="/bin/date"
MNT_BASE="/tmp/ima_test_root"
mkdir -p "$MNT_BASE"
mount -t tmpfs tmpfs "$MNT_BASE"
mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
mkfs.xfs -q "$MNT_BASE/xfs.img"
mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
mount -t overlay overlay -o \
"lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
"$MNT_BASE/ovl"
echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
target_prog="$MNT_BASE/ovl/test_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
if [[ "$audit_count" -eq 1 ]]; then
echo "PASS: Found exactly 1 audit event."
else
echo "FAIL: Expected 1 audit event, but found $audit_count."
exit 1
fi
---
Changes in v4:
- No functional changes.
- Add Reviewed-by & Fixes tags.
- Link to v3: https://lore.kernel.org/r/20260122-xfs-ima-fixup-v3-1-20335a8aa836@cloudflare.com
Changes in v3:
- Prefer timespec64_to_ns() to leverage attr.version. [Roberto]
- s/TPMFS/TMPFS/ in description.
- Link to v2: https://lore.kernel.org/r/20260120-xfs-ima-fixup-v2-1-f332ead8b043@cloudflare.com
Changes in v2:
- Updated commit description + message to clarify the problem.
- compare struct timespec64 to avoid collision possibility [Roberto].
- Don't check inode_attr_changed() in ima_check_last_writer()
- Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
Changes since RFC:
- Remove calls to I_IS_VERSION()
- Function documentation/comments
- Abide IMA/EVM change detection fallback invariants
- Combined ctime guard into version for attributes struct
- Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
---
include/linux/integrity.h | 35 +++++++++++++++++++++++++++++++----
security/integrity/evm/evm_main.c | 5 ++---
security/integrity/ima/ima_api.c | 11 ++++++++---
security/integrity/ima/ima_main.c | 17 ++++++++++-------
4 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..034f0a1ed48ca8c19c764e302bbfc555dad92cde 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -9,6 +9,8 @@
#include <linux/fs.h>
#include <linux/iversion.h>
+#include <linux/kernel.h>
+#include <linux/time64.h>
enum integrity_status {
INTEGRITY_PASS = 0,
@@ -51,14 +53,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
/*
* On stacked filesystems detect whether the inode or its content has changed.
+ *
+ * Must be called in process context.
*/
static inline bool
integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
- const struct inode *inode)
+ struct file *file, struct inode *inode)
{
- return (inode->i_sb->s_dev != attrs->dev ||
- inode->i_ino != attrs->ino ||
- !inode_eq_iversion(inode, attrs->version));
+ struct kstat stat;
+
+ might_sleep();
+
+ if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
+ return true;
+
+ /*
+ * EVM currently relies on backing inode i_version. While IS_I_VERSION
+ * is not a good indicator of i_version support, this still retains
+ * the logic such that a re-evaluation should still occur for EVM, and
+ * only for IMA if vfs_getattr_nosec() fails.
+ */
+ if (!file || vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT))
+ return !IS_I_VERSION(inode) ||
+ !inode_eq_iversion(inode, attrs->version);
+
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ return stat.change_cookie != attrs->version;
+
+ if (stat.result_mask & STATX_CTIME)
+ return timespec64_to_ns(&stat.ctime) != (s64)attrs->version;
+
+ return true;
}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
bool ret = false;
if (iint) {
- ret = (!IS_I_VERSION(metadata_inode) ||
- integrity_inode_attrs_changed(&iint->metadata_inode,
- metadata_inode));
+ ret = integrity_inode_attrs_changed(&iint->metadata_inode,
+ NULL, metadata_inode);
if (ret)
iint->evm_status = INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ else if (stat.result_mask & STATX_CTIME)
+ i_version = timespec64_to_ns(&stat.ctime);
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..8ac42b03740eb93bf23b15cb9039af6cd32aa999 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -28,6 +28,7 @@
#include <linux/iversion.h>
#include <linux/evm.h>
#include <linux/crash_dump.h>
+#include <linux/time64.h>
#include "ima.h"
@@ -199,10 +200,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
&iint->atomic_flags);
if ((iint->flags & IMA_NEW_FILE) ||
vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
- AT_STATX_SYNC_AS_STAT) ||
- !(stat.result_mask & STATX_CHANGE_COOKIE) ||
- stat.change_cookie != iint->real_inode.version) {
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT) ||
+ ((stat.result_mask & STATX_CHANGE_COOKIE) ?
+ stat.change_cookie != iint->real_inode.version :
+ (!(stat.result_mask & STATX_CTIME) ||
+ timespec64_to_ns(&stat.ctime) !=
+ (s64)iint->real_inode.version))) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
@@ -328,9 +332,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
real_inode = d_real_inode(file_dentry(file));
if (real_inode != inode &&
(action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
- if (!IS_I_VERSION(real_inode) ||
- integrity_inode_attrs_changed(&iint->real_inode,
- real_inode)) {
+ if (integrity_inode_attrs_changed(&iint->real_inode,
+ file, real_inode)) {
iint->flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@cloudflare.com>
^ permalink raw reply related
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Lorenzo Stoakes @ 2026-01-29 17:09 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Paul Moore, linux-security-module, linux-integrity, selinux,
john.johansen, zohar, roberto.sassu, wufan, mic, gnoack, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng,
linux-mm, David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <74286aca-a565-489f-ad2c-886c650ea2bc@suse.cz>
On Thu, Jan 29, 2026 at 06:02:00PM +0100, Vlastimil Babka wrote:
> Agreed, the mmap_min_addr should stay visible and applied unconditionally.
> AFAICS the only relation to SECURITY/LSM is whether CONFIG_LSM_MMAP_MIN_ADDR
> is used as an additional lower limit to both CONFIG_DEFAULT_MMAP_MIN_ADDR
> and the sysctl-written value?
Thanks, yeah we should probably actually move the non-LSM-relevant stuff
out to mm to be honest.
But that's future work, for an -rc8 hotfix we need to make the init of this
particular module not dependent on normal LSM initialisation, as horrid as
that is...
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Vlastimil Babka @ 2026-01-29 17:02 UTC (permalink / raw)
To: Lorenzo Stoakes, Paul Moore
Cc: linux-security-module, linux-integrity, selinux, john.johansen,
zohar, roberto.sassu, wufan, mic, gnoack, kees, mortonm, casey,
penguin-kernel, nicolas.bouchinet, xiujianfeng, linux-mm,
David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <14638978-b133-457a-ae9c-31ba54e3964c@lucifer.local>
On 1/29/26 17:48, Lorenzo Stoakes wrote:
> On Thu, Jan 29, 2026 at 04:31:16PM +0000, Lorenzo Stoakes wrote:
>
> Sorry to clarify here I meant to say - if I set CONFIG_SECURITY but _not_
> CONFIG_SECURITY_SELINUX the tunable does in fact still appear (and afaict
> still work...)
>
> So LSM_MMAP_MIN_ADDR is really weird to require SECURITY_SELINUX, perhaps a
> historic artifact where we wanted a different default or something like
> this?
>
> I know that we use that in preference to CONFIG_DEFAULT_MMAP_MIN_ADDR if
> specified.
>
> The description really probably needs updating.
>
> The key config here we should be looking at is DEFAULT_MMAP_MIN_ADDR which
> emphatically does _not_ require CONFIG_SECURITY and also in its description
> explicitly mentions the tunable:
>
> This value can be changed after boot using the
> /proc/sys/vm/mmap_min_addr tunable.
>
> The mmap_min_addr global value exposed in min_addr.c is referenced in
> several places in mm and other parts of the kernel - fs/exec.c,
> fs/userlandfd.c, kernel/sys.c, mm/mmap.c, mm/vma.c.
>
> So this now silently going to zero everywhere and ignoring
> CONFIG_DEFAULT_MMAP_MIN_ADDR is surely a userspace-breaking regression and
> needs fixing in rc8?
>
> Which means that... people can now mmap() at NULL everywhere despite setting
> CONFIG_DEFAULT_MMAP_MIN_ADDR > 0? :)
>
> That seems like a _really bad idea_ (TM).
>
> So this is emphatically not a report of a trivial self test break, but
> rather of something more serious AFAICT.
>
> So yeah I think this has to be reverted/fixed.
Agreed, the mmap_min_addr should stay visible and applied unconditionally.
AFAICS the only relation to SECURITY/LSM is whether CONFIG_LSM_MMAP_MIN_ADDR
is used as an additional lower limit to both CONFIG_DEFAULT_MMAP_MIN_ADDR
and the sysctl-written value?
^ permalink raw reply
* Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-29 16:54 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260129.691d9b85a887@gnoack.org>
On Thu, Jan 29, 2026 at 08:56:37AM +0100, Günther Noack wrote:
> On Wed, Jan 28, 2026 at 10:34:02PM +0100, Mickaël Salaün wrote:
> > On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> > > Tradeoffs:
> > >
> > > This change improves performance, at a slight size increase to the
> > > layer masks data structure.
> > >
> > > At the moment, for the filesystem access rights, the data structure
> > > has the same size as before, but once we introduce the 17th filesystem
> > > access right, it will double in size (from 32 to 64 bytes), as
> >
> > ...for all access rights (e.g. even if there is no new network one)
>
> Added.
>
> > > --- a/security/landlock/audit.c
> > > +++ b/security/landlock/audit.c
> > > @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
> > >
> > > #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> > >
> > > +/* get_denied_layer - get the youngest layer that denied the access_request */
> >
> > /* Get the youngest layer that denied the access_request. */
>
> OK, done. I also changed to non-docstring style for the
> access_mask_subset() helper.
>
> >
> > > static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > > access_mask_t *const access_request,
> > > - const layer_mask_t (*const layer_masks)[],
> > > - const size_t layer_masks_size)
> > > + const struct layer_access_masks *masks)
> > > {
> > > - const unsigned long access_req = *access_request;
> > > - unsigned long access_bit;
> > > - access_mask_t missing = 0;
> > > - long youngest_layer = -1;
> > > -
> > > - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > > - long layer;
> > > -
> > > - if (!mask)
> > > - continue;
> > > -
> > > - /* __fls(1) == 0 */
> > > - layer = __fls(mask);
> > > - if (layer > youngest_layer) {
> > > - youngest_layer = layer;
> > > - missing = BIT(access_bit);
> > > - } else if (layer == youngest_layer) {
> > > - missing |= BIT(access_bit);
> > > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> >
> > size_t i
>
> This is one of the two places where this didn't work.
>
> The loop goes from top to bottom here, and the "i >= 0" check would
> always be true for a size_t.
>
> If there is a more idiomatic way to write that loop, I can switch to
> it, but would otherwise lean towards keeping it as it is?
Indeed. We can use ssize_t as in get_hierarchy().
>
>
> > > + if (masks->access[i] & *access_request) {
> > > + *access_request &= masks->access[i];
> > > + return i;
> > > }
> > > }
> > >
> > > - for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> > > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> >
> > size_t i
>
> Ditto, the loop goes from top to bottom here.
>
>
> > > + const access_mask_t denied = masks->access[i] & optional_access;
> > > + const unsigned long newly_denied = denied & ~all_denied;
> > >
>
>
> > > -static bool
> > > -scope_to_request(const access_mask_t access_request,
> > > - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> > > +static bool scope_to_request(const access_mask_t access_request,
> > > + struct layer_access_masks *masks)
> > > {
> > > - const unsigned long access_req = access_request;
> > > - unsigned long access_bit;
> > > + bool saw_unfulfilled_access = false;
> > >
> > > - if (WARN_ON_ONCE(!layer_masks))
> > > + if (WARN_ON_ONCE(!masks))
> > > return true;
> > >
> > > - for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> > > - (*layer_masks)[access_bit] = 0;
> > > -
> > > - return is_layer_masks_allowed(layer_masks);
> > > + for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> > > + masks->access[i] &= access_request;
> > > + if (masks->access[i])
> >
> > {
> >
> > > + saw_unfulfilled_access = true;
> >
> > break;
> > }
>
> Two lines above, this loop mutates masks->access[...]:
>
> masks->access[i] &= access_request
>
> If we break the loop early, we would not actually scope it down to the
> request entirely? Is this safe?
You're right, don't add this break. BTW, would a test catch it?
>
> > > + }
> > > + return !saw_unfulfilled_access;
> > > }
>
> –Günther
>
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Lorenzo Stoakes @ 2026-01-29 16:48 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, john.johansen,
zohar, roberto.sassu, wufan, mic, gnoack, kees, mortonm, casey,
penguin-kernel, nicolas.bouchinet, xiujianfeng, linux-mm,
David Hildenbrand, Vlastimil Babka, Liam R. Howlett,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <0146e385-935b-4f66-9e6d-51bb47ae4bdc@lucifer.local>
On Thu, Jan 29, 2026 at 04:31:16PM +0000, Lorenzo Stoakes wrote:
> +cc linux-mm, maintainers/reviewers of mm/Kconfig
>
> On Fri, Oct 17, 2025 at 04:48:24PM -0400, Paul Moore wrote:
> > The LSM framework itself registers a small number of initcalls, this
> > patch converts these initcalls into the new initcall mechanism.
> >
> > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> > Reviewed-by: John Johansen <john.johhansen@canonical.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> Hi,
>
> This commit message doesn't mention at all that you've removed
> /proc/sys/vm/mmap_min_addr altogether if CONFIG_SECURITY is not set.
>
> Did you intend this change? If you did you should probably mention that
> you're doing this :)
>
> I mean it's a bit late now as this is upstream (but not _too_ late as we
> have rc8 ;), but this has broken something for me locally (mremap mm
> selftest) and I bisected to this commit.
>
> Note that CONFIG_SECURITY states:
>
> This allows you to choose different security modules to be
> configured into your kernel.
>
> If this option is not selected, the default Linux security
> model will be used.
>
> So is the 'default' Linux security model not to provide this tunable at
> all?
>
> Though I see LSM_MMAP_MIN_ADDR depends on SECURITY && SECURITY_SELINUX, the
> Makefile in security/ has:
>
> obj-$(CONFIG_MMU) += min_addr.o
>
> Which suggests that min_addr depends on MMU only, and not on
> LSM_MMAP_MIN_ADDR at all...
>
> And I don't have CONFIG_SECURITY_SELINUX set yet have
> /proc/sys/vm/mmap_min_addr?
Sorry to clarify here I meant to say - if I set CONFIG_SECURITY but _not_
CONFIG_SECURITY_SELINUX the tunable does in fact still appear (and afaict
still work...)
So LSM_MMAP_MIN_ADDR is really weird to require SECURITY_SELINUX, perhaps a
historic artifact where we wanted a different default or something like
this?
I know that we use that in preference to CONFIG_DEFAULT_MMAP_MIN_ADDR if
specified.
The description really probably needs updating.
The key config here we should be looking at is DEFAULT_MMAP_MIN_ADDR which
emphatically does _not_ require CONFIG_SECURITY and also in its description
explicitly mentions the tunable:
This value can be changed after boot using the
/proc/sys/vm/mmap_min_addr tunable.
The mmap_min_addr global value exposed in min_addr.c is referenced in
several places in mm and other parts of the kernel - fs/exec.c,
fs/userlandfd.c, kernel/sys.c, mm/mmap.c, mm/vma.c.
So this now silently going to zero everywhere and ignoring
CONFIG_DEFAULT_MMAP_MIN_ADDR is surely a userspace-breaking regression and
needs fixing in rc8?
Which means that... people can now mmap() at NULL everywhere despite setting
CONFIG_DEFAULT_MMAP_MIN_ADDR > 0? :)
That seems like a _really bad idea_ (TM).
So this is emphatically not a report of a trivial self test break, but
rather of something more serious AFAICT.
So yeah I think this has to be reverted/fixed.
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Lorenzo Stoakes @ 2026-01-29 16:31 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, john.johansen,
zohar, roberto.sassu, wufan, mic, gnoack, kees, mortonm, casey,
penguin-kernel, nicolas.bouchinet, xiujianfeng, linux-mm,
David Hildenbrand, Vlastimil Babka, Liam R. Howlett,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <20251017204815.505363-21-paul@paul-moore.com>
+cc linux-mm, maintainers/reviewers of mm/Kconfig
On Fri, Oct 17, 2025 at 04:48:24PM -0400, Paul Moore wrote:
> The LSM framework itself registers a small number of initcalls, this
> patch converts these initcalls into the new initcall mechanism.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johhansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Hi,
This commit message doesn't mention at all that you've removed
/proc/sys/vm/mmap_min_addr altogether if CONFIG_SECURITY is not set.
Did you intend this change? If you did you should probably mention that
you're doing this :)
I mean it's a bit late now as this is upstream (but not _too_ late as we
have rc8 ;), but this has broken something for me locally (mremap mm
selftest) and I bisected to this commit.
Note that CONFIG_SECURITY states:
This allows you to choose different security modules to be
configured into your kernel.
If this option is not selected, the default Linux security
model will be used.
So is the 'default' Linux security model not to provide this tunable at
all?
Though I see LSM_MMAP_MIN_ADDR depends on SECURITY && SECURITY_SELINUX, the
Makefile in security/ has:
obj-$(CONFIG_MMU) += min_addr.o
Which suggests that min_addr depends on MMU only, and not on
LSM_MMAP_MIN_ADDR at all...
And I don't have CONFIG_SECURITY_SELINUX set yet have
/proc/sys/vm/mmap_min_addr?
So yeah, this is all very very confusing.
So I think maybe we need a revert/hotfix here if this was unintended?
I think we might be breaking userspace here... For one the mremap mm
selftest breaks immediately :)
Note that prior to this change the default of 64k seems to be set which
seems to contradict the docs in Documentation/admin-guide/sysctl/vm.rst:
By default this value is set to 0 and no protections will be enforced by
the security module. Setting this value to something like 64k will allow
the vast majority of applications to work correctly and provide defense in
depth against future potential kernel bugs.
Also to add to the fun, we have CONFIG_DEFAULT_MMAP_MIN_ADDR as defined in
mm/Kconfig:
config DEFAULT_MMAP_MIN_ADDR
int "Low address space to protect from user allocation"
depends on MMU
default 4096
Which is _only_ referenced in security/min_addr.c which of course, is now
not being used at all.
So we have a config option that people _think_ they are setting to
something to enforce a minimum address but are in fact not if
!CONFIG_SECURITY?
Thanks, Lorenzo
> ---
> security/inode.c | 3 +--
> security/lsm.h | 20 ++++++++++++++++++++
> security/lsm_init.c | 14 ++++++++++++--
> security/min_addr.c | 5 +++--
> 4 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/security/inode.c b/security/inode.c
> index 6620c3e42af2..ab8d6a2acadb 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -368,7 +368,7 @@ static const struct file_operations lsm_ops = {
> };
> #endif
>
> -static int __init securityfs_init(void)
> +int __init securityfs_init(void)
> {
> int retval;
>
> @@ -387,4 +387,3 @@ static int __init securityfs_init(void)
> #endif
> return 0;
> }
> -core_initcall(securityfs_init);
> diff --git a/security/lsm.h b/security/lsm.h
> index 8dc267977ae0..81aadbc61685 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -35,4 +35,24 @@ extern struct kmem_cache *lsm_inode_cache;
> int lsm_cred_alloc(struct cred *cred, gfp_t gfp);
> int lsm_task_alloc(struct task_struct *task);
>
> +/* LSM framework initializers */
> +
> +#ifdef CONFIG_MMU
> +int min_addr_init(void);
> +#else
> +static inline int min_addr_init(void)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_MMU */
> +
> +#ifdef CONFIG_SECURITYFS
> +int securityfs_init(void);
> +#else
> +static inline int securityfs_init(void)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITYFS */
> +
> #endif /* _LSM_H_ */
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index aacdac406ba5..0f668bca98f9 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -488,7 +488,12 @@ int __init security_init(void)
> */
> static int __init security_initcall_pure(void)
> {
> - return lsm_initcall(pure);
> + int rc_adr, rc_lsm;
> +
> + rc_adr = min_addr_init();
> + rc_lsm = lsm_initcall(pure);
> +
> + return (rc_adr ? rc_adr : rc_lsm);
> }
> pure_initcall(security_initcall_pure);
>
> @@ -506,7 +511,12 @@ early_initcall(security_initcall_early);
> */
> static int __init security_initcall_core(void)
> {
> - return lsm_initcall(core);
> + int rc_sfs, rc_lsm;
> +
> + rc_sfs = securityfs_init();
> + rc_lsm = lsm_initcall(core);
> +
> + return (rc_sfs ? rc_sfs : rc_lsm);
> }
> core_initcall(security_initcall_core);
>
> diff --git a/security/min_addr.c b/security/min_addr.c
> index c55bb84b8632..0fde5ec9abc8 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -5,6 +5,8 @@
> #include <linux/sysctl.h>
> #include <linux/minmax.h>
>
> +#include "lsm.h"
> +
> /* amount of vm to protect from userspace access by both DAC and the LSM*/
> unsigned long mmap_min_addr;
> /* amount of vm to protect from userspace using CAP_SYS_RAWIO (DAC) */
> @@ -52,11 +54,10 @@ static const struct ctl_table min_addr_sysctl_table[] = {
> },
> };
>
> -static int __init init_mmap_min_addr(void)
> +int __init min_addr_init(void)
> {
> register_sysctl_init("vm", min_addr_sysctl_table);
> update_mmap_min_addr();
>
> return 0;
> }
> -pure_initcall(init_mmap_min_addr);
> --
> 2.51.1.dirty
>
>
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Roberto Sassu @ 2026-01-29 16:18 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: Eric Biggers, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20260125192526.782202-2-jarkko@kernel.org>
On Sun, 2026-01-25 at 21:25 +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> generating the random numbers required.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> One might argue that TPM RNG should be used for the generated trusted keys,
> so that they have matching entropy with the TPM internally generated
> objects.
>
> This argument does have some weight into it but as far cryptography goes,
> FIPS certification sets the exact bar, not which exact FIPS certified RNG
> will be used. Thus, the rational choice is obviously to pick the lowest
> latency path, which is kernel RNG.
>
> Finally, there is an actual defence in depth benefit when using kernel RNG
> as it helps to mitigate TPM firmware bugs concerning RNG implementation,
> given the obfuscation by the other entropy sources.
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v7:
> - A new patch. Simplifies follow up patches.
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 636acb66a4f6..7ce7e31bcdfb 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -6,6 +6,16 @@
> * See Documentation/security/keys/trusted-encrypted.rst
> */
>
> +/**
> + * DOC: Random Number Generation
> + *
> + * tpm_get_random() was previously used here as the RNG in order to have equal
> + * entropy with the objects fully inside the TPM. However, as far as goes,
> + * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
> + * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
> + * associated with the RNG.
> + */
If we switch to the kernel RNG that is better, and the TPM one is
flawed, I guess we are going to have big problems anyway, since the TPM
random number generator is used by the TPM itself internally.
I think it makes sense to leave as it is.
Thanks
Roberto
> +
> #include <crypto/hash_info.h>
> #include <crypto/sha1.h>
> #include <crypto/utils.h>
> @@ -936,11 +946,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> return ret;
> }
>
> -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> -{
> - return tpm_get_random(chip, key, key_len);
> -}
> -
> static int __init init_digests(void)
> {
> int i;
> @@ -992,6 +997,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> .init = trusted_tpm_init,
> .seal = trusted_tpm_seal,
> .unseal = trusted_tpm_unseal,
> - .get_random = trusted_tpm_get_random,
> .exit = trusted_tpm_exit,
> };
^ permalink raw reply
* Re: [PATCH v4 13/17] module: Report signature type to users
From: Petr Pavlu @ 2026-01-29 14:44 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, Paul Moore, James Morris, Serge E. Hallyn,
Jonathan Corbet, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Schier,
Nicolas Bouchinet, Xiu Jianfeng, Fabian Grünbichler,
Arnout Engelen, Mattia Rizzolo, kpcyrd, Christian Heusel,
Câju Mihai-Drosi, Sebastian Andrzej Siewior, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20260113-module-hashes-v4-13-0b932db9b56b@weissschuh.net>
On 1/13/26 1:28 PM, Thomas Weißschuh wrote:
> The upcoming CONFIG_MODULE_HASHES will introduce a signature type.
> This needs to be handled by callers differently than PKCS7 signatures.
>
> Report the signature type to the caller and let them verify it.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> [...]
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d65bc300a78c..2a28a0ece809 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3348,19 +3348,24 @@ static int module_integrity_check(struct load_info *info, int flags)
> {
> bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> MODULE_INIT_IGNORE_VERMAGIC);
> + enum pkey_id_type sig_type;
> size_t sig_len;
> const u8 *sig;
> int err = 0;
>
> if (IS_ENABLED(CONFIG_MODULE_SIG_POLICY)) {
> err = mod_split_sig(info->hdr, &info->len, mangled_module,
> - &sig_len, &sig, "module");
> + &sig_type, &sig_len, &sig, "module");
> if (err)
> return err;
> }
>
> - if (IS_ENABLED(CONFIG_MODULE_SIG))
> + if (IS_ENABLED(CONFIG_MODULE_SIG) && sig_type == PKEY_ID_PKCS7) {
> err = module_sig_check(info, sig, sig_len);
> + } else {
> + pr_err("module: not signed with expected PKCS#7 message\n");
> + err = -ENOPKG;
> + }
The new else branch means that if the user chooses not to configure any
module integrity policy, they will no longer be able to load any
modules. I think this entire if-else part should be moved under the
IS_ENABLED(CONFIG_MODULE_SIG_POLICY) block above, as I'm mentioning on
patch #12.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v4 12/17] module: Move signature splitting up
From: Petr Pavlu @ 2026-01-29 14:41 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, Paul Moore, James Morris, Serge E. Hallyn,
Jonathan Corbet, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Schier,
Nicolas Bouchinet, Xiu Jianfeng, Fabian Grünbichler,
Arnout Engelen, Mattia Rizzolo, kpcyrd, Christian Heusel,
Câju Mihai-Drosi, Sebastian Andrzej Siewior, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20260113-module-hashes-v4-12-0b932db9b56b@weissschuh.net>
On 1/13/26 1:28 PM, Thomas Weißschuh wrote:
> The signature splitting will also be used by CONFIG_MODULE_HASHES.
>
> Move it up the callchain, so the result can be reused.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> [...]
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c09b25c0166a..d65bc300a78c 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3346,10 +3346,21 @@ static int early_mod_check(struct load_info *info, int flags)
>
> static int module_integrity_check(struct load_info *info, int flags)
> {
> + bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> + MODULE_INIT_IGNORE_VERMAGIC);
> + size_t sig_len;
> + const u8 *sig;
> int err = 0;
>
> + if (IS_ENABLED(CONFIG_MODULE_SIG_POLICY)) {
> + err = mod_split_sig(info->hdr, &info->len, mangled_module,
> + &sig_len, &sig, "module");
> + if (err)
> + return err;
> + }
> +
> if (IS_ENABLED(CONFIG_MODULE_SIG))
> - err = module_sig_check(info, flags);
> + err = module_sig_check(info, sig, sig_len);
>
> if (err)
> return err;
I suggest moving the IS_ENABLED(CONFIG_MODULE_SIG) block under the
new IS_ENABLED(CONFIG_MODULE_SIG_POLICY) section. I realize that
CONFIG_MODULE_SIG implies CONFIG_MODULE_SIG_POLICY, but I believe this
change makes it more apparent that this it the case. Otherwise, one
might for example wonder if sig_len in the module_sig_check() call can
be undefined.
if (IS_ENABLED(CONFIG_MODULE_SIG_POLICY)) {
err = mod_split_sig(info->hdr, &info->len, mangled_module,
&sig_len, &sig, "module");
if (err)
return err;
if (IS_ENABLED(CONFIG_MODULE_SIG))
err = module_sig_check(info, sig, sig_len);
}
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v5 0/6] Extend "trusted" keys to support a new trust source named the PowerVM Key Wrapping Module (PKWM)
From: Nayna Jain @ 2026-01-29 10:15 UTC (permalink / raw)
To: Srish Srinivasan, linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, rnsastry, linux-kernel, linux-security-module
In-Reply-To: <20260127145228.48320-1-ssrish@linux.ibm.com>
On 1/27/26 9:52 AM, Srish Srinivasan wrote:
> Power11 has introduced a feature called the PowerVM Key Wrapping Module
> (PKWM), where PowerVM in combination with Power LPAR Platform KeyStore
> (PLPKS) [1] supports a new feature called "Key Wrapping" [2] to protect
> user secrets by wrapping them using a hypervisor generated wrapping key.
> This wrapping key is an AES-GCM-256 symmetric key that is stored as an
> object in the PLPKS. It has policy based protections that prevents it from
> being read out or exposed to the user. This wrapping key can then be used
> by the OS to wrap or unwrap secrets via hypervisor calls.
>
> This patchset intends to add the PKWM, which is a combination of IBM
> PowerVM and PLPKS, as a new trust source for trusted keys. The wrapping key
> does not exist by default and its generation is requested by the kernel at
> the time of PKWM initialization. This key is then persisted by the PKWM and
> is used for wrapping any kernel provided key, and is never exposed to the
> user. The kernel is aware of only the label to this wrapping key.
>
> Along with the PKWM implementation, this patchset includes two preparatory
> patches: one fixing the kernel-doc inconsistencies in the PLPKS code and
> another reorganizing PLPKS config variables in the sysfs.
Tested the entire patch series. Seems to work as expected.
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [RFC][PATCH v2] ima: Add support for staging measurements for deletion and trimming
From: Roberto Sassu @ 2026-01-29 8:20 UTC (permalink / raw)
To: steven chen, corbet, zohar, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu
In-Reply-To: <6b36d6b3-34e8-4bdc-bd68-d71ddf44eba8@linux.microsoft.com>
On 1/28/2026 10:30 PM, steven chen wrote:
> On 12/12/2025 9:19 AM, Roberto Sassu wrote:
>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>
>> Introduce the ability of staging the entire (or a portion of the) IMA
>> measurement list for deletion. Staging means moving the current
>> content of
>> the measurement list to a separate location, and allowing users to
>> read and
>> delete it. This causes the measurement list to be atomically truncated
>> before new measurements can be added. Staging can be done only once at a
>> time. In the event of kexec(), staging is reverted and staged entries
>> will
>> be carried over to the new kernel.
>>
>> User space is responsible to concatenate the staged IMA measurements list
>> portions following the temporal order in which the operations were done,
>> together with the current measurement list. Then, it can send the
>> collected
>> data to the remote verifiers.
>>
>> Also introduce the ability of trimming N measurements entries from the
>> IMA
>> measurements list, provided that user space has already read them.
>> Trimming
>> combines staging and deletion in one operation.
>>
>> The benefit of these solutions is the ability to free precious kernel
>> memory, in exchange of delegating user space to reconstruct the full
>> measurement list from the chunks. No trust needs to be given to user
>> space,
>> since the integrity of the measurement list is protected by the TPM.
>>
>> By default, staging/trimming the measurements list does not alter the
>> hash
>> table. When staging/trimming are done, IMA is still able to detect
>> collisions on the staged and later deleted measurement entries, by
>> keeping
>> the entry digests (only template data are freed).
>>
>> However, since during the measurements list serialization only the SHA1
>> digest is passed, and since there are no template data to recalculate the
>> other digests from, the hash table is currently not populated with
>> digests
>> from staged/deleted entries after kexec().
>>
>> Introduce the new kernel option ima_flush_htable to decide whether or not
>> the digests of staged measurement entries are flushed from the hash
>> table.
>>
>> Then, introduce ascii_runtime_measurements_staged_<algo> and
>> binary_runtime_measurement_staged_<algo> interfaces to stage/trim/delete
>> the measurements. Use 'echo A > <IMA interface>' and
>> 'echo D > <IMA interface>' to respectively stage and delete the entire
>> measurements list. Use 'echo N > <IMA interface>', with N between 1 and
>> LONG_MAX, to stage the selected portion of the measurements list, and
>> 'echo -N > <IMA interface>' to trim N measurements entries.
>>
>> The ima_measure_users counter (protected by the ima_measure_lock
>> mutex) has
>> been introduced to protect access to the measurements list and the staged
>> part. The open method of all the measurement interfaces has been extended
>> to allow only one writer at a time or, in alternative, multiple readers.
>> The write permission is used to stage/trim/delete the measurements, the
>> read permission to read them. Write requires also the CAP_SYS_ADMIN
>> capability.
>>
>> Finally, introduce and maintain dedicate counters for the number of
>> measurement entries and binary size, for the current measurements list
>> (BINARY_SIZE), for the current measurements list plus staged entries
>> (BINARY_SIZE_STAGED) useful for kexec() segment allocation, and for the
>> entire measurement list without staging/trimming (BINARY_SIZE_FULL)
>> useful
>> for the kexec-related critical data records.
> Is the following possible race condition for staged list:
>
> Agent A: create staged list Staged list A1
> new measurement added Measurement list M1
> Two lists in kernel: A1 and M1
>
> Agent B: read staged list (A1) to do verification
> new measurement added Measurement list M2
> Two lists in kernel: A1 and M2
>
> Agent A: verified and remove staged list (A1)
> new measurement added Measurement list M3
> One list in kernel: M3
>
> Agent C: create staged list Staged list C1
> new measurement added Measurement list M4
> Two lists in kernel: C1 and M4
>
> Agent B: remove staged list (?), C1 removed ---this will cause problem
> new measurement added Measurement list M5
> One list in kernel: M5
>
> Agent C: try to remove staged list(?)
If you remember the patch, we added a read-write protection to the
measurements interfaces. As long as you keep the interface open for
write no one else can make change on the staging. Sure, you can drop the
write, and reopen for read, but then you should expect someone else to
operate on the interface.
If you want to be sure no one else changes the staged measurements, just
keep the interface open for write, read the staged measurements and
delete them.
Roberto
> Possible solution?
> Save the total number trimmed T or tag
>
> Trim request sync this parameter to trim the staged list
>
> Regards,
>
> Steven
>
>> Note: This code derives from the Alt-IMA Huawei project, and is being
>> released under the dual license model (GPL-2.0 OR MIT).
>>
>> Link: https://github.com/linux-integrity/linux/issues/1
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>> ---
>> .../admin-guide/kernel-parameters.txt | 4 +
>> security/integrity/ima/ima.h | 18 +-
>> security/integrity/ima/ima_fs.c | 240 +++++++++++++++++-
>> security/integrity/ima/ima_kexec.c | 42 ++-
>> security/integrity/ima/ima_queue.c | 169 +++++++++++-
>> 5 files changed, 439 insertions(+), 34 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/
>> Documentation/admin-guide/kernel-parameters.txt
>> index 6c42061ca20e..e5f1e11bd0a2 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2215,6 +2215,10 @@
>> Use the canonical format for the binary runtime
>> measurements, instead of host native format.
>> + ima_flush_htable [IMA]
>> + Flush the IMA hash table when staging for deletion or
>> + trimming measurement entries.
>> +
>> 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..8a6be4284210 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -28,6 +28,15 @@ enum ima_show_type { IMA_SHOW_BINARY,
>> IMA_SHOW_BINARY_NO_FIELD_LEN,
>> IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
>> enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>> +/*
>> + * BINARY_SIZE: size of the current measurements list
>> + * BINARY_SIZE_STAGED: size of current measurements list + staged
>> entries
>> + * BINARY_SIZE_FULL: size of measurements list since IMA initialization
>> + */
>> +enum binary_size_types {
>> + BINARY_SIZE, BINARY_SIZE_STAGED, BINARY_SIZE_FULL, BINARY__LAST
>> +};
>> +
>> /* digest size for IMA, fits SHA1 or MD5 */
>> #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
>> #define IMA_EVENT_NAME_LEN_MAX 255
>> @@ -117,6 +126,8 @@ struct ima_queue_entry {
>> struct ima_template_entry *entry;
>> };
>> extern struct list_head ima_measurements; /* list of all
>> measurements */
>> +extern struct list_head ima_measurements_staged; /* list of staged
>> meas. */
>> +extern bool ima_measurements_staged_exist; /* If there are staged
>> meas. */
>> /* Some details preceding the binary serialized measurement list */
>> struct ima_kexec_hdr {
>> @@ -281,10 +292,12 @@ struct ima_template_desc
>> *ima_template_desc_current(void);
>> struct ima_template_desc *ima_template_desc_buf(void);
>> struct ima_template_desc *lookup_template_desc(const char *name);
>> bool ima_template_has_modsig(const struct ima_template_desc
>> *ima_template);
>> +int ima_queue_stage_trim(unsigned long req_value, bool trim);
>> +int ima_queue_delete_staged_trimmed(bool staged_moved);
>> int ima_restore_measurement_entry(struct ima_template_entry *entry);
>> int ima_restore_measurement_list(loff_t bufsize, void *buf);
>> int ima_measurements_show(struct seq_file *m, void *v);
>> -unsigned long ima_get_binary_runtime_size(void);
>> +unsigned long ima_get_binary_runtime_size(enum binary_size_types type);
>> int ima_init_template(void);
>> void ima_init_template_list(void);
>> int __init ima_init_digests(void);
>> @@ -298,11 +311,12 @@ int ima_lsm_policy_change(struct notifier_block
>> *nb, unsigned long event,
>> extern spinlock_t ima_queue_lock;
>> struct ima_h_table {
>> - atomic_long_t len; /* number of stored measurements in the
>> list */
>> + atomic_long_t len[BINARY__LAST]; /* num of stored meas. in the
>> list */
>> atomic_long_t violations;
>> struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
>> };
>> extern struct ima_h_table ima_htable;
>> +extern struct mutex ima_extend_list_mutex;
>> static inline unsigned int ima_hash_key(u8 *digest)
>> {
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/
>> ima_fs.c
>> index 87045b09f120..a96f7c36b34a 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -24,7 +24,18 @@
>> #include "ima.h"
>> +/*
>> + * Requests:
>> + * 'A\n': stage the entire measurements list
>> + * '[1, LONG_MAX]\n' stage N measurements entries
>> + * '-[1, LONG_MAX]\n' trim N measurements entries
>> + * 'D\n': delete staged measurements
>> + */
>> +#define STAGED_REQ_LENGTH 21
>> +
>> static DEFINE_MUTEX(ima_write_mutex);
>> +static DEFINE_MUTEX(ima_measure_lock);
>> +static long ima_measure_users;
>> bool ima_canonical_fmt;
>> static int __init default_canonical_fmt_setup(char *str)
>> @@ -64,7 +75,8 @@ static ssize_t ima_show_measurements_count(struct
>> file *filp,
>> char __user *buf,
>> size_t count, loff_t *ppos)
>> {
>> - return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
>> + return ima_show_htable_value(buf, count, ppos,
>> + &ima_htable.len[BINARY_SIZE]);
>> }
>> @@ -74,14 +86,15 @@ static const struct file_operations
>> ima_measurements_count_ops = {
>> };
>> /* returns pointer to hlist_node */
>> -static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>> +static void *_ima_measurements_start(struct seq_file *m, loff_t *pos,
>> + struct list_head *head)
>> {
>> loff_t l = *pos;
>> struct ima_queue_entry *qe;
>> /* we need a lock since pos could point beyond last element */
>> rcu_read_lock();
>> - list_for_each_entry_rcu(qe, &ima_measurements, later) {
>> + list_for_each_entry_rcu(qe, head, later) {
>> if (!l--) {
>> rcu_read_unlock();
>> return qe;
>> @@ -91,7 +104,18 @@ static void *ima_measurements_start(struct
>> seq_file *m, loff_t *pos)
>> return NULL;
>> }
>> -static void *ima_measurements_next(struct seq_file *m, void *v,
>> loff_t *pos)
>> +static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>> +{
>> + return _ima_measurements_start(m, pos, &ima_measurements);
>> +}
>> +
>> +static void *ima_measurements_staged_start(struct seq_file *m, loff_t
>> *pos)
>> +{
>> + return _ima_measurements_start(m, pos, &ima_measurements_staged);
>> +}
>> +
>> +static void *_ima_measurements_next(struct seq_file *m, void *v,
>> loff_t *pos,
>> + struct list_head *head)
>> {
>> struct ima_queue_entry *qe = v;
>> @@ -103,7 +127,18 @@ static void *ima_measurements_next(struct
>> seq_file *m, void *v, loff_t *pos)
>> rcu_read_unlock();
>> (*pos)++;
>> - return (&qe->later == &ima_measurements) ? NULL : qe;
>> + return (&qe->later == head) ? NULL : qe;
>> +}
>> +
>> +static void *ima_measurements_next(struct seq_file *m, void *v,
>> loff_t *pos)
>> +{
>> + return _ima_measurements_next(m, v, pos, &ima_measurements);
>> +}
>> +
>> +static void *ima_measurements_staged_next(struct seq_file *m, void *v,
>> + loff_t *pos)
>> +{
>> + return _ima_measurements_next(m, v, pos, &ima_measurements_staged);
>> }
>> static void ima_measurements_stop(struct seq_file *m, void *v)
>> @@ -202,16 +237,147 @@ static const struct seq_operations
>> ima_measurments_seqops = {
>> .show = ima_measurements_show
>> };
>> +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,
>> +};
>> +
>> +static const struct seq_operations ima_measurments_staged_seqops = {
>> + .start = ima_measurements_staged_start,
>> + .next = ima_measurements_staged_next,
>> + .stop = ima_measurements_stop,
>> + .show = ima_measurements_show
>> +};
>> +
>> +static int ima_measurements_staged_open(struct inode *inode, struct
>> file *file)
>> +{
>> + return _ima_measurements_open(inode, file,
>> + &ima_measurments_staged_seqops);
>> +}
>> +
>> +static ssize_t ima_measurements_staged_read(struct file *file, char
>> __user *buf,
>> + size_t size, loff_t *ppos)
>> +{
>> + if (!ima_measurements_staged_exist)
>> + return -ENOENT;
>> +
>> + return seq_read(file, buf, size, ppos);
>> +}
>> +
>> +static ssize_t ima_measurements_staged_write(struct file *file,
>> + const char __user *buf,
>> + size_t datalen, loff_t *ppos)
>> +{
>> + char req[STAGED_REQ_LENGTH], *req_ptr = req;
>> + unsigned long req_value;
>> + bool trim = false;
>> + int ret;
>> +
>> + if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
>> + return -EINVAL;
>> +
>> + if (copy_from_user(req, buf, datalen) != 0)
>> + return -EFAULT;
>> +
>> + if (req[datalen - 1] != '\n')
>> + return -EINVAL;
>> +
>> + req[datalen - 1] = '\0';
>> + req_ptr = req;
>> +
>> + switch (req[0]) {
>> + case 'A':
>> + if (datalen != 2 || req[1] != '\0')
>> + return -EINVAL;
>> +
>> + ret = ima_queue_stage_trim(LONG_MAX, false);
>> + break;
>> + case 'D':
>> + if (datalen != 2 || req[1] != '\0')
>> + return -EINVAL;
>> +
>> + ret = ima_queue_delete_staged_trimmed(false);
>> + break;
>> + case '-':
>> + trim = true;
>> + req_ptr++;
>> + fallthrough;
>> + default:
>> + ret = kstrtoul(req_ptr, 0, &req_value);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = ima_queue_stage_trim(req_value, trim);
>> + }
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + return datalen;
>> +}
>> +
>> +static const struct file_operations ima_measurements_staged_ops = {
>> + .open = ima_measurements_staged_open,
>> + .read = ima_measurements_staged_read,
>> + .write = ima_measurements_staged_write,
>> + .llseek = seq_lseek,
>> + .release = ima_measurements_release,
>> };
>> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
>> @@ -279,14 +445,37 @@ 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 const struct seq_operations
>> ima_ascii_measurements_staged_seqops = {
>> + .start = ima_measurements_staged_start,
>> + .next = ima_measurements_staged_next,
>> + .stop = ima_measurements_stop,
>> + .show = ima_ascii_measurements_show
>> +};
>> +
>> +static int ima_ascii_measurements_staged_open(struct inode *inode,
>> + struct file *file)
>> +{
>> + return _ima_measurements_open(inode, file,
>> + &ima_ascii_measurements_staged_seqops);
>> +}
>> +
>> +static const struct file_operations ima_ascii_measurements_staged_ops
>> = {
>> + .open = ima_ascii_measurements_staged_open,
>> + .read = ima_measurements_staged_read,
>> + .write = ima_measurements_staged_write,
>> + .llseek = seq_lseek,
>> + .release = ima_measurements_release,
>> };
>> static ssize_t ima_read_policy(char *path)
>> @@ -419,6 +608,25 @@ static int __init
>> create_securityfs_measurement_lists(void)
>> &ima_measurements_ops);
>> if (IS_ERR(dentry))
>> return PTR_ERR(dentry);
>> +
>> + sprintf(file_name, "ascii_runtime_measurements_staged_%s",
>> + hash_algo_name[algo]);
>> + dentry = securityfs_create_file(file_name,
>> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
>> + ima_dir, (void *)(uintptr_t)i,
>> + &ima_ascii_measurements_staged_ops);
>> + if (IS_ERR(dentry))
>> + return PTR_ERR(dentry);
>> +
>> + sprintf(file_name, "binary_runtime_measurements_staged_%s",
>> + hash_algo_name[algo]);
>> + dentry = securityfs_create_file(file_name,
>> + S_IRUSR | S_IRGRP |
>> + S_IWUSR | S_IWGRP,
>> + ima_dir, (void *)(uintptr_t)i,
>> + &ima_measurements_staged_ops);
>> + if (IS_ERR(dentry))
>> + return PTR_ERR(dentry);
>> }
>> return 0;
>> @@ -528,6 +736,20 @@ int __init ima_fs_init(void)
>> goto out;
>> }
>> + dentry =
>> securityfs_create_symlink("binary_runtime_measurements_staged",
>> + ima_dir, "binary_runtime_measurements_staged_sha1", NULL);
>> + if (IS_ERR(dentry)) {
>> + ret = PTR_ERR(dentry);
>> + goto out;
>> + }
>> +
>> + dentry =
>> securityfs_create_symlink("ascii_runtime_measurements_staged",
>> + ima_dir, "ascii_runtime_measurements_staged_sha1", NULL);
>> + if (IS_ERR(dentry)) {
>> + ret = PTR_ERR(dentry);
>> + goto out;
>> + }
>> +
>> 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_kexec.c b/security/integrity/
>> ima/ima_kexec.c
>> index 7362f68f2d8b..13c7e78aeefd 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -40,8 +40,8 @@ void ima_measure_kexec_event(const char *event_name)
>> long len;
>> int n;
>> - buf_size = ima_get_binary_runtime_size();
>> - len = atomic_long_read(&ima_htable.len);
>> + buf_size = ima_get_binary_runtime_size(BINARY_SIZE_FULL);
>> + len = atomic_long_read(&ima_htable.len[BINARY_SIZE_FULL]);
>> n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>> "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
>> @@ -78,6 +78,17 @@ static int ima_alloc_kexec_file_buf(size_t
>> segment_size)
>> return 0;
>> }
>> +static int ima_dump_measurement(struct ima_kexec_hdr *khdr,
>> + struct ima_queue_entry *qe)
>> +{
>> + if (ima_kexec_file.count >= ima_kexec_file.size)
>> + return -EINVAL;
>> +
>> + khdr->count++;
>> + ima_measurements_show(&ima_kexec_file, qe);
>> + return 0;
>> +}
>> +
>> static int ima_dump_measurement_list(unsigned long *buffer_size,
>> void **buffer,
>> unsigned long segment_size)
>> {
>> @@ -93,17 +104,25 @@ static int ima_dump_measurement_list(unsigned
>> long *buffer_size, void **buffer,
>> memset(&khdr, 0, sizeof(khdr));
>> khdr.version = 1;
>> - /* This is an append-only list, no need to hold the RCU read lock */
>> - list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
>> - if (ima_kexec_file.count < ima_kexec_file.size) {
>> - khdr.count++;
>> - ima_measurements_show(&ima_kexec_file, qe);
>> - } else {
>> - ret = -EINVAL;
>> +
>> + /* It can race with ima_queue_stage_trim(). */
>> + mutex_lock(&ima_extend_list_mutex);
>> +
>> + list_for_each_entry(qe, &ima_measurements_staged, later) {
>> + ret = ima_dump_measurement(&khdr, qe);
>> + if (ret < 0)
>> + break;
>> + }
>> +
>> + list_for_each_entry(qe, &ima_measurements, later) {
>> + if (!ret)
>> + ret = ima_dump_measurement(&khdr, qe);
>> + if (ret < 0)
>> break;
>> - }
>> }
>> + mutex_unlock(&ima_extend_list_mutex);
>> +
>> /*
>> * fill in reserved space with some buffer details
>> * (eg. version, buffer size, number of measurements)
>> @@ -157,7 +176,8 @@ void ima_add_kexec_buffer(struct kimage *image)
>> else
>> extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
>> - binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
>> + binary_runtime_size =
>> ima_get_binary_runtime_size(BINARY_SIZE_STAGED) +
>> + extra_memory;
>> if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
>> kexec_segment_size = ULONG_MAX;
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/
>> ima/ima_queue.c
>> index 590637e81ad1..7dfa24b8ae31 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -22,19 +22,32 @@
>> #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;
>> LIST_HEAD(ima_measurements); /* list of all measurements */
>> +LIST_HEAD(ima_measurements_staged); /* list of staged measurements */
>> +static LIST_HEAD(ima_measurements_trim); /* list of measurements to
>> trim */
>> +bool ima_measurements_staged_exist; /* If there are staged
>> measurements */
>> #ifdef CONFIG_IMA_KEXEC
>> -static unsigned long binary_runtime_size;
>> +static unsigned long binary_runtime_size[BINARY__LAST];
>> #else
>> -static unsigned long binary_runtime_size = ULONG_MAX;
>> +static unsigned long binary_runtime_size[BINARY_SIZE] = ULONG_MAX;
>> +static unsigned long binary_runtime_size[BINARY_SIZE_FULL] = ULONG_MAX;
>> +static unsigned long binary_runtime_size[BINARY_SIZE_STAGED] =
>> ULONG_MAX;
>> #endif
>> /* key: inode (before secure-hashing a file) */
>> struct ima_h_table ima_htable = {
>> - .len = ATOMIC_LONG_INIT(0),
>> + .len = { ATOMIC_LONG_INIT(0) },
>> .violations = ATOMIC_LONG_INIT(0),
>> .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
>> };
>> @@ -43,7 +56,7 @@ struct ima_h_table ima_htable = {
>> * and extending the TPM PCR aggregate. Since tpm_extend can take
>> * long (and the tpm driver uses a mutex), we can't use the spinlock.
>> */
>> -static DEFINE_MUTEX(ima_extend_list_mutex);
>> +DEFINE_MUTEX(ima_extend_list_mutex);
>> /*
>> * Used internally by the kernel to suspend measurements.
>> @@ -101,7 +114,7 @@ static int ima_add_digest_entry(struct
>> ima_template_entry *entry,
>> bool update_htable)
>> {
>> struct ima_queue_entry *qe;
>> - unsigned int key;
>> + unsigned int i, key;
>> qe = kmalloc(sizeof(*qe), GFP_KERNEL);
>> if (qe == NULL) {
>> @@ -113,18 +126,23 @@ static int ima_add_digest_entry(struct
>> ima_template_entry *entry,
>> INIT_LIST_HEAD(&qe->later);
>> list_add_tail_rcu(&qe->later, &ima_measurements);
>> - atomic_long_inc(&ima_htable.len);
>> + for (i = 0; i < BINARY__LAST; i++)
>> + atomic_long_inc(&ima_htable.len[i]);
>> +
>> if (update_htable) {
>> key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
>> hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
>> }
>> - if (binary_runtime_size != ULONG_MAX) {
>> + if (binary_runtime_size[BINARY_SIZE_FULL] != ULONG_MAX) {
>> int size;
>> size = get_binary_runtime_size(entry);
>> - binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
>> - binary_runtime_size + size : ULONG_MAX;
>> +
>> + for (i = 0; i < BINARY__LAST; i++)
>> + binary_runtime_size[i] =
>> + (binary_runtime_size[i] < ULONG_MAX - size) ?
>> + binary_runtime_size[i] + size : ULONG_MAX;
>> }
>> return 0;
>> }
>> @@ -134,12 +152,18 @@ static int ima_add_digest_entry(struct
>> ima_template_entry *entry,
>> * entire binary_runtime_measurement list, including the ima_kexec_hdr
>> * structure.
>> */
>> -unsigned long ima_get_binary_runtime_size(void)
>> +unsigned long ima_get_binary_runtime_size(enum binary_size_types type)
>> {
>> - if (binary_runtime_size >= (ULONG_MAX - sizeof(struct
>> ima_kexec_hdr)))
>> + unsigned long val;
>> +
>> + mutex_lock(&ima_extend_list_mutex);
>> + val = binary_runtime_size[type];
>> + mutex_unlock(&ima_extend_list_mutex);
>> +
>> + if (val >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
>> return ULONG_MAX;
>> else
>> - return binary_runtime_size + sizeof(struct ima_kexec_hdr);
>> + return val + sizeof(struct ima_kexec_hdr);
>> }
>> static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
>> @@ -220,6 +244,127 @@ int ima_add_template_entry(struct
>> ima_template_entry *entry, int violation,
>> return result;
>> }
>> +int ima_queue_stage_trim(unsigned long req_value, bool trim)
>> +{
>> + unsigned long req_value_copy = req_value, to_remove = 0;
>> + struct list_head *moved = &ima_measurements_staged;
>> + struct ima_queue_entry *qe;
>> +
>> + if (req_value == 0 || req_value > LONG_MAX)
>> + return -EINVAL;
>> +
>> + if (ima_measurements_staged_exist)
>> + return -EEXIST;
>> +
>> + if (trim)
>> + moved = &ima_measurements_trim;
>> +
>> + mutex_lock(&ima_extend_list_mutex);
>> + if (list_empty(&ima_measurements)) {
>> + mutex_unlock(&ima_extend_list_mutex);
>> + return -ENOENT;
>> + }
>> +
>> + if (req_value == LONG_MAX) {
>> + list_replace(&ima_measurements, moved);
>> + INIT_LIST_HEAD(&ima_measurements);
>> + atomic_long_set(&ima_htable.len[BINARY_SIZE], 0);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE] = 0;
>> +
>> + if (trim) {
>> + atomic_long_set(&ima_htable.len[BINARY_SIZE_STAGED], 0);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE_STAGED] = 0;
>> + }
>> + } else {
>> + list_for_each_entry(qe, &ima_measurements, later) {
>> + to_remove += get_binary_runtime_size(qe->entry);
>> + if (--req_value_copy == 0)
>> + break;
>> + }
>> +
>> + if (req_value_copy > 0) {
>> + mutex_unlock(&ima_extend_list_mutex);
>> + return -ENOENT;
>> + }
>> +
>> + __list_cut_position(moved, &ima_measurements, &qe->later);
>> + atomic_long_sub(req_value, &ima_htable.len[BINARY_SIZE]);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE] -= to_remove;
>> +
>> + if (trim) {
>> + atomic_long_sub(req_value,
>> + &ima_htable.len[BINARY_SIZE_STAGED]);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE_STAGED] -=
>> + to_remove;
>> + }
>> + }
>> +
>> + if (ima_flush_htable)
>> + /* Either staged/trimmed entries are removed from hash table. */
>> + list_for_each_entry(qe, moved, later)
>> + /* It can race with ima_lookup_digest_entry(). */
>> + hlist_del_rcu(&qe->hnext);
>> +
>> + mutex_unlock(&ima_extend_list_mutex);
>> + ima_measurements_staged_exist = true;
>> +
>> + if (ima_flush_htable)
>> + synchronize_rcu();
>> +
>> + if (trim)
>> + return ima_queue_delete_staged_trimmed(true);
>> +
>> + return 0;
>> +}
>> +
>> +int ima_queue_delete_staged_trimmed(bool staged_moved)
>> +{
>> + struct ima_queue_entry *qe, *qe_tmp;
>> + unsigned int i;
>> +
>> + if (!ima_measurements_staged_exist)
>> + return -ENOENT;
>> +
>> + if (!staged_moved) {
>> + mutex_lock(&ima_extend_list_mutex);
>> + list_replace(&ima_measurements_staged, &ima_measurements_trim);
>> + INIT_LIST_HEAD(&ima_measurements_staged);
>> + atomic_long_set(&ima_htable.len[BINARY_SIZE_STAGED], 0);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE_STAGED] = 0;
>> +
>> + mutex_unlock(&ima_extend_list_mutex);
>> + }
>> +
>> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_trim,
>> later) {
>> + /*
>> + * Ok because after list delete qe is only accessed by
>> + * ima_lookup_digest_entry().
>> + */
>> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
>> + kfree(qe->entry->template_data[i].data);
>> + qe->entry->template_data[i].data = NULL;
>> + qe->entry->template_data[i].len = 0;
>> + }
>> +
>> + list_del(&qe->later);
>> +
>> + /* No leak if !ima_flush_htable, referenced by ima_htable. */
>> + if (ima_flush_htable) {
>> + kfree(qe->entry->digests);
>> + kfree(qe->entry);
>> + kfree(qe);
>> + }
>> + }
>> +
>> + ima_measurements_staged_exist = false;
>> + return 0;
>> +}
>> +
>> int ima_restore_measurement_entry(struct ima_template_entry *entry)
>> {
>> int result = 0;
>
>
^ permalink raw reply
* Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure
From: Günther Noack @ 2026-01-29 7:56 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260128.quaido7ia0Xu@digikod.net>
On Wed, Jan 28, 2026 at 10:34:02PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> > Tradeoffs:
> >
> > This change improves performance, at a slight size increase to the
> > layer masks data structure.
> >
> > At the moment, for the filesystem access rights, the data structure
> > has the same size as before, but once we introduce the 17th filesystem
> > access right, it will double in size (from 32 to 64 bytes), as
>
> ...for all access rights (e.g. even if there is no new network one)
Added.
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
> >
> > #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> >
> > +/* get_denied_layer - get the youngest layer that denied the access_request */
>
> /* Get the youngest layer that denied the access_request. */
OK, done. I also changed to non-docstring style for the
access_mask_subset() helper.
>
> > static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > access_mask_t *const access_request,
> > - const layer_mask_t (*const layer_masks)[],
> > - const size_t layer_masks_size)
> > + const struct layer_access_masks *masks)
> > {
> > - const unsigned long access_req = *access_request;
> > - unsigned long access_bit;
> > - access_mask_t missing = 0;
> > - long youngest_layer = -1;
> > -
> > - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > - long layer;
> > -
> > - if (!mask)
> > - continue;
> > -
> > - /* __fls(1) == 0 */
> > - layer = __fls(mask);
> > - if (layer > youngest_layer) {
> > - youngest_layer = layer;
> > - missing = BIT(access_bit);
> > - } else if (layer == youngest_layer) {
> > - missing |= BIT(access_bit);
> > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
>
> size_t i
This is one of the two places where this didn't work.
The loop goes from top to bottom here, and the "i >= 0" check would
always be true for a size_t.
If there is a more idiomatic way to write that loop, I can switch to
it, but would otherwise lean towards keeping it as it is?
> > + if (masks->access[i] & *access_request) {
> > + *access_request &= masks->access[i];
> > + return i;
> > }
> > }
> >
> > - for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
>
> size_t i
Ditto, the loop goes from top to bottom here.
> > + const access_mask_t denied = masks->access[i] & optional_access;
> > + const unsigned long newly_denied = denied & ~all_denied;
> >
> > -static bool
> > -scope_to_request(const access_mask_t access_request,
> > - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> > +static bool scope_to_request(const access_mask_t access_request,
> > + struct layer_access_masks *masks)
> > {
> > - const unsigned long access_req = access_request;
> > - unsigned long access_bit;
> > + bool saw_unfulfilled_access = false;
> >
> > - if (WARN_ON_ONCE(!layer_masks))
> > + if (WARN_ON_ONCE(!masks))
> > return true;
> >
> > - for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> > - (*layer_masks)[access_bit] = 0;
> > -
> > - return is_layer_masks_allowed(layer_masks);
> > + for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> > + masks->access[i] &= access_request;
> > + if (masks->access[i])
>
> {
>
> > + saw_unfulfilled_access = true;
>
> break;
> }
Two lines above, this loop mutates masks->access[...]:
masks->access[i] &= access_request
If we break the loop early, we would not actually scope it down to the
request entirely? Is this safe?
> > + }
> > + return !saw_unfulfilled_access;
> > }
–Günther
^ permalink raw reply
* Re: [PATCH v2 1/3] selftests/landlock: Add filesystem access benchmark
From: Mickaël Salaün @ 2026-01-28 21:31 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260125195853.109967-2-gnoack3000@gmail.com>
On Sun, Jan 25, 2026 at 08:58:51PM +0100, Günther Noack wrote:
> fs_bench benchmarks the performance of Landlock's path walk
> by exercising it in a scenario that amplifies Landlock's overhead:
>
> * Create a large number of nested directories
> * Enforce a Landlock policy in which a rule is associated with each of
> these subdirectories
> * Benchmark openat() applied to the deepest directory,
> forcing Landlock to walk the entire path.
>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> tools/testing/selftests/landlock/.gitignore | 1 +
> tools/testing/selftests/landlock/Makefile | 1 +
> tools/testing/selftests/landlock/fs_bench.c | 161 ++++++++++++++++++++
> 3 files changed, 163 insertions(+)
> create mode 100644 tools/testing/selftests/landlock/fs_bench.c
>
> diff --git a/tools/testing/selftests/landlock/.gitignore b/tools/testing/selftests/landlock/.gitignore
> index a820329cae0d..1974e17a2611 100644
> --- a/tools/testing/selftests/landlock/.gitignore
> +++ b/tools/testing/selftests/landlock/.gitignore
> @@ -1,4 +1,5 @@
> /*_test
> +/fs_bench
> /sandbox-and-launch
> /true
> /wait-pipe
> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> index 044b83bde16e..fc43225d319a 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -9,6 +9,7 @@ LOCAL_HDRS += $(wildcard *.h)
> src_test := $(wildcard *_test.c)
>
> TEST_GEN_PROGS := $(src_test:.c=)
> +TEST_GEN_PROGS += fs_bench
>
> TEST_GEN_PROGS_EXTENDED := \
> true \
> diff --git a/tools/testing/selftests/landlock/fs_bench.c b/tools/testing/selftests/landlock/fs_bench.c
> new file mode 100644
> index 000000000000..a3b686418bc5
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/fs_bench.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Landlock filesystem benchmark
You might want to add some copyright.
> + */
> +
> +#define _GNU_SOURCE
> +#include <err.h>
> +#include <fcntl.h>
> +#include <linux/landlock.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/prctl.h>
> +#include <sys/stat.h>
> +#include <sys/syscall.h>
> +#include <sys/times.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +void usage(const char *argv0)
const
> +{
> + printf("Usage:\n");
> + printf(" %s [OPTIONS]\n", argv0);
> + printf("\n");
> + printf(" Benchmark expensive Landlock checks for D nested dirs\n");
> + printf("\n");
> + printf("Options:\n");
> + printf(" -h help\n");
> + printf(" -L disable Landlock (as a baseline)\n");
> + printf(" -d D set directory depth to D\n");
> + printf(" -n N set number of benchmark iterations to N\n");
> +}
> +
> +/*
> + * Build a deep directory, enforce Landlock and return the FD to the
> + * deepest dir. On any failure, exit the process with an error.
> + */
> +int build_directory(size_t depth, bool use_landlock)
const
> +{
> + const char *path = "d"; /* directory name */
> + int abi, ruleset_fd, current, previous;
> +
> + if (use_landlock) {
> + abi = syscall(SYS_landlock_create_ruleset, NULL, 0,
> + LANDLOCK_CREATE_RULESET_VERSION);
Please include wrappers.h and use the related syscall helpers. One of
the benefit is to use __NR_* constants defined by the installed kernel
headers.
> + if (abi < 7)
> + err(1, "Landlock ABI too low: got %d, wanted 7+", abi);
> + }
> +
> + ruleset_fd = -1;
> + if (use_landlock) {
> + struct landlock_ruleset_attr attr = {
> + .handled_access_fs =
> + 0xffff, /* All FS access rights as of 2026-01 */
> + };
> + ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr,
> + sizeof(attr), 0U);
> + if (ruleset_fd < 0)
> + err(1, "landlock_create_ruleset");
> + }
> +
> + current = open(".", O_PATH);
> + if (current < 0)
> + err(1, "open(.)");
> +
> + while (depth--) {
> + if (use_landlock) {
> + struct landlock_path_beneath_attr attr = {
> + .allowed_access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .parent_fd = current,
> + };
> + if (syscall(SYS_landlock_add_rule, ruleset_fd,
> + LANDLOCK_RULE_PATH_BENEATH, &attr, 0) < 0)
> + err(1, "landlock_add_rule");
> + }
> +
> + if (mkdirat(current, path, 0700) < 0)
> + err(1, "mkdirat(%s)", path);
We should have a loop to build the directories, then start the timer and
have another loop to add Landlock rules.
> +
> + previous = current;
> + current = openat(current, path, O_PATH);
> + if (current < 0)
> + err(1, "open(%s)", path);
> +
> + close(previous);
> + }
> +
> + if (use_landlock) {
> + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0)
> + err(1, "prctl");
> +
> + if (syscall(SYS_landlock_restrict_self, ruleset_fd, 0) < 0)
> + err(1, "landlock_restrict_self");
> + }
> +
> + close(ruleset_fd);
> + return current;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + bool use_landlock = true;
> + size_t num_iterations = 100000;
> + size_t num_subdirs = 10000;
> + int c, current, fd;
> + struct tms start_time, end_time;
> +
> + setbuf(stdout, NULL);
> + while ((c = getopt(argc, argv, "hLd:n:")) != -1) {
> + switch (c) {
> + case 'h':
> + usage(argv[0]);
> + return EXIT_SUCCESS;
> + case 'L':
> + use_landlock = false;
> + break;
> + case 'd':
> + num_subdirs = atoi(optarg);
> + break;
> + case 'n':
> + num_iterations = atoi(optarg);
> + break;
> + default:
> + usage(argv[0]);
> + return EXIT_FAILURE;
> + }
> + }
> +
> + printf("*** Benchmark ***\n");
We should probably use ksft_*() helpers in main (see
seccomp_benchmark.c).
> + printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
> + num_iterations, use_landlock ? "with" : "without");
> +
> + if (times(&start_time) == -1)
> + err(1, "times");
> +
> + current = build_directory(num_subdirs, use_landlock);
> +
> + for (int i = 0; i < num_iterations; i++) {
> + fd = openat(current, ".", O_DIRECTORY);
We can use AT_EMPTY_PATH (with an empty path) instead of "."
I guess the benchmark should not change, but better to check again.
> + if (fd != -1) {
> + if (use_landlock)
> + errx(1, "openat succeeded, expected error");
> +
> + close(fd);
> + }
> + }
> +
> + if (times(&end_time) == -1)
> + err(1, "times");
The created directories should be removed here (setup and teardown).
> +
> + printf("*** Benchmark concluded ***\n");
> + printf("System: %ld clocks\n",
> + end_time.tms_stime - start_time.tms_stime);
> + printf("User : %ld clocks\n",
> + end_time.tms_utime - start_time.tms_utime);
> + printf("Clocks per second: %ld\n", CLOCKS_PER_SEC);
> +
> + close(current);
> +}
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v2 0/3] landlock: Refactor layer masks
From: Mickaël Salaün @ 2026-01-28 21:31 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260125195853.109967-1-gnoack3000@gmail.com>
On Sun, Jan 25, 2026 at 08:58:50PM +0100, Günther Noack wrote:
> Hello!
>
> This patch set "transposes" the layer masks matrix, which was
> previously modeled as a access-max-sized array of layer masks, and
> changes it to be a layer-max-sized array of access masks instead.
> (It is a pure refactoring, there are no user-visible changes.)
>
> This unlocks a few code simplifications and in multiple places it
> removes the need for loops and branches that deal with individual
> bits. Instead, the changed data structure now lends itself for more
> bitwise operations. The underlying hypothesis for me was that by
> using more bitwise operations and fewer branches, we would get an
> overall speedup even when the data structure size increases slightly
> in some cases.
>
> Tentative results with and without this patch set show that the
> hypothesis likely holds true. The benchmark I used exercises a "worst
> case" scenario that attempts to be bottlenecked on the affected code:
> constructs a large number of nested directories, with one "path
> beneath" rule each and then tries to open the innermost directory many
> times. The benchmark is intentionally unrealistic to amplify the
> amount of time used for the path walk logic and forces Landlock to
> walk the full path (eventually failing the open syscall). (I'll send
> the benchmark program in a reply to this mail for full transparency.)
>
> Measured with the benchmark program, the patch set results in a
> speedup of about -10%. The benchmark results are only tentative and
> have been produced in Qemu:
>
> With the patch, the benchmark runs in 6046 clocks (measured with
> times(3)):
>
> *** Benchmark ***
> 10000 dirs, 100000 iterations, with landlock
> *** Benchmark concluded ***
> System: 6046 clocks
> User : 1 clocks
> Clocks per second: 1000000
>
> Without the patch, we get 6713 clocks, which is 11% more
>
> *** Benchmark ***
> 10000 dirs, 100000 iterations, with landlock
> *** Benchmark concluded ***
> System: 6713 clocks
> User : 0 clocks
> Clocks per second: 1000000
>
> The base revision used for benchmarking was commit 7a51784da76d
> ("tools/sched_ext: update scx_show_state.py for scx_aborting change")
>
> In real-life scenarios, the speed improvement from this patch set will
> be less pronounced than in the artificial benchmark, as people do not
> usually stack directories that deeply and attach so many rules to
> them, and the EACCES error should also be the exception rather than
> the norm.
>
> I am looking forward to your feedback.
>
> P.S.: I am open to suggestions on what the "layer masks" variables
> should be called, because the name "layer masks" might be less
> appropriate after this change. I have not fixed up the name
> everywhere because fixing up the code took priority for now.
Could you please clarify your thoughts and explain why this name might
not be appropriate anymore? Any list of name proposals?
If we rename the variables, this should be done in the same refactoring
patch.
>
> ---
> Changes since previous versions:
>
> V2: (This patch set)
>
> * Remove the refactoring around the deny_mask_t type,
> it is better to send that as a separate patch (mic review)
Feel free to include the new dedicated patch in this series.
> * Added the benchmark program to the selftests
> * Fix unused variable report for "access_dom":
> https://lore.kernel.org/all/202601200900.wonk9M0m-lkp@intel.com/
> * Use size_t and ARRAY_SIZE to loop over the layers (mic review)
> * Documentation
> * Fixing up and adding back documentaiton (mic review)
> * Documented landlock_unmask_layers()
> * Fixed up kernel docs in a place where it was improperly updated
> (Spotted by Randy Dunlap
> https://lore.kernel.org/all/20260123025121.3713403-1-rdunlap@infradead.org/)
> * Minor
> * Const, some newlines (mic review)
>
> V1: (Initial version)
>
> https://lore.kernel.org/all/20251230103917.10549-3-gnoack3000@gmail.com/
>
> Günther Noack (3):
> selftests/landlock: Add filesystem access benchmark
> landlock: access_mask_subset() helper
> landlock: transpose the layer masks data structure
>
> security/landlock/access.h | 16 +-
> security/landlock/audit.c | 84 ++---
> security/landlock/audit.h | 3 +-
> security/landlock/domain.c | 45 +--
> security/landlock/domain.h | 4 +-
> security/landlock/fs.c | 354 +++++++++-----------
> security/landlock/net.c | 11 +-
> security/landlock/ruleset.c | 88 ++---
> security/landlock/ruleset.h | 21 +-
> tools/testing/selftests/landlock/.gitignore | 1 +
> tools/testing/selftests/landlock/Makefile | 1 +
> tools/testing/selftests/landlock/fs_bench.c | 161 +++++++++
> 12 files changed, 444 insertions(+), 345 deletions(-)
> create mode 100644 tools/testing/selftests/landlock/fs_bench.c
>
> --
> 2.52.0
>
>
^ 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