* [PATCH 5/6] selftests/landlock: Repurpose scoped_abstract_unix_test.c for pathname sockets too.
From: Tingmao Wang @ 2025-12-28 12:45 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, linux-security-module
In-Reply-To: <cover.1766925301.git.m@maowtm.org>
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
+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;
+ 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 =
+ 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 related
* [PATCH 6/6] selftests/landlock: Add pathname socket variants for more tests
From: Tingmao Wang @ 2025-12-28 12:45 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, linux-security-module
In-Reply-To: <cover.1766925301.git.m@maowtm.org>
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. */
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 related
* Re: [PATCH 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Justin Suess @ 2025-12-28 16:37 UTC (permalink / raw)
To: m; +Cc: demiobenour, fahimitahera, gnoack, hi, jannh,
linux-security-module, mic
In-Reply-To: <be315d8ff7544fd91bdb922e8afc7c8154e3594d.1766925301.git.m@maowtm.org>
On 12/28/25 07:45, 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>
> ---
>
> There is an argument that there should only really be one audit request
> type for both sockets, since the only difference is whether path= is
> followed by a normal path, or by a hex string starting with 00. But I'm
> not sure if we can change this at this point, so I have created a new
> request type.
>
> security/landlock/audit.c | 4 +++
> security/landlock/audit.h | 1 +
> security/landlock/task.c | 74 ++++++++++++++++++++++++++++++---------
> 3 files changed, 62 insertions(+), 17 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 6dfcc1860d6e..9fbb0ada440b 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -233,57 +233,84 @@ 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)
> +static bool sock_addr_is_abstract(const struct unix_address *const addr)
Nit: From the name sock_addr_is_abstract, it's unclear without reading
the parameter that this function only works with unix sockets, when
socket is an overloaded term that can refer to other kinds of sockets
(e.g tcp/udp/raw).
Maybe is_unix_sock_addr_abstract? or unix_sock_addr_is_abstract?
>
> {
> - struct unix_address *addr = unix_sk(sock)->addr;
> -
> - if (!addr)
> - return false;
> -
> - if (addr->len >= offsetof(struct sockaddr_un, sun_path) + 1 &&
> + if (addr && addr->len >= offsetof(struct sockaddr_un, sun_path) + 1 &&
> addr->name->sun_path[0] == '\0')
> return true;
>
> return false;
> }
>
> +/* Allow us to quickly test if the current domain scopes any form of socket */
> static const struct access_masks unix_scope = {
> - .scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET,
> + .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.
> + */
> static int hook_unix_stream_connect(struct sock *const sock,
> struct sock *const other,
> struct sock *const newsk)
> {
> size_t handle_layer;
> + access_mask_t scope;
> + enum landlock_request_type request_type;
> const struct landlock_cred_security *const subject =
> landlock_get_applicable_subject(current_cred(), unix_scope,
> &handle_layer);
> + const struct unix_address *addr;
>
> /* Quick return for non-landlocked tasks. */
> if (!subject)
> return 0;
>
> - if (!is_abstract_socket(other))
> + addr = unix_sk(other)->addr;
> + /* Unnamed sockets are not restricted. */
> + if (!addr)
> return 0;
>
> - if (!sock_is_scoped(other, subject->domain))
> + if (sock_addr_is_abstract(addr)) {
> + scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
> + request_type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET;
> + } else {
> + /* Pathname socket. */
> + scope = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> + request_type = LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET;
> + }
> +
> + if (!sock_is_scoped(other, subject->domain, scope))
> return 0;
>
> landlock_log_denial(subject, &(struct landlock_request) {
> - .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> + .type = request_type,
> .audit = {
> .type = LSM_AUDIT_DATA_NET,
> .u.net = &(struct lsm_network_audit) {
> @@ -299,9 +326,12 @@ static int hook_unix_may_send(struct socket *const sock,
> struct socket *const other)
> {
> size_t handle_layer;
> + access_mask_t scope;
> + enum landlock_request_type request_type;
> const struct landlock_cred_security *const subject =
> landlock_get_applicable_subject(current_cred(), unix_scope,
> &handle_layer);
> + const struct unix_address *addr;
>
> if (!subject)
> return 0;
> @@ -313,14 +343,24 @@ 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))
> + addr = unix_sk(other->sk)->addr;
> + /* Unnamed sockets are not restricted. */
> + if (!addr)
> return 0;
>
> - if (!sock_is_scoped(other->sk, subject->domain))
> + if (sock_addr_is_abstract(addr)) {
> + 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;
> + }
> +
> + if (!sock_is_scoped(other->sk, subject->domain, scope))
> return 0;
>
> landlock_log_denial(subject, &(struct landlock_request) {
> - .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> + .type = request_type,
> .audit = {
> .type = LSM_AUDIT_DATA_NET,
> .u.net = &(struct lsm_network_audit) {
^ permalink raw reply
* Re: [PATCH 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Mickaël Salaün @ 2025-12-28 18:15 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, linux-security-module, Justin Suess
In-Reply-To: <be315d8ff7544fd91bdb922e8afc7c8154e3594d.1766925301.git.m@maowtm.org>
On Sun, Dec 28, 2025 at 12:45:41PM +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>
Great, thanks!
> ---
>
> There is an argument that there should only really be one audit request
> type for both sockets, since the only difference is whether path= is
> followed by a normal path, or by a hex string starting with 00. But I'm
> not sure if we can change this at this point, so I have created a new
> request type.
It is the correct approach to add a dedicated request type, which
enables to filter on it, and doesn't have performance impact.
>
> security/landlock/audit.c | 4 +++
> security/landlock/audit.h | 1 +
> security/landlock/task.c | 74 ++++++++++++++++++++++++++++++---------
> 3 files changed, 62 insertions(+), 17 deletions(-)
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 6dfcc1860d6e..9fbb0ada440b 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> +/*
> + * 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.
> + */
> static int hook_unix_stream_connect(struct sock *const sock,
> struct sock *const other,
> struct sock *const newsk)
> {
> size_t handle_layer;
> + access_mask_t scope;
> + enum landlock_request_type request_type;
> const struct landlock_cred_security *const subject =
> landlock_get_applicable_subject(current_cred(), unix_scope,
> &handle_layer);
> + const struct unix_address *addr;
>
> /* Quick return for non-landlocked tasks. */
> if (!subject)
> return 0;
>
> - if (!is_abstract_socket(other))
> + addr = unix_sk(other)->addr;
> + /* Unnamed sockets are not restricted. */
> + if (!addr)
> return 0;
>
> - if (!sock_is_scoped(other, subject->domain))
> + if (sock_addr_is_abstract(addr)) {
> + scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
> + request_type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET;
> + } else {
> + /* Pathname socket. */
> + scope = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> + request_type = LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET;
> + }
> +
> + if (!sock_is_scoped(other, subject->domain, scope))
> return 0;
We should be able to factor out this hunk for both hooks, and then also
fold is_abstract_socket() in this new helper.
>
> landlock_log_denial(subject, &(struct landlock_request) {
> - .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> + .type = request_type,
> .audit = {
> .type = LSM_AUDIT_DATA_NET,
> .u.net = &(struct lsm_network_audit) {
> @@ -299,9 +326,12 @@ static int hook_unix_may_send(struct socket *const sock,
> struct socket *const other)
> {
> size_t handle_layer;
> + access_mask_t scope;
> + enum landlock_request_type request_type;
> const struct landlock_cred_security *const subject =
> landlock_get_applicable_subject(current_cred(), unix_scope,
> &handle_layer);
> + const struct unix_address *addr;
>
> if (!subject)
> return 0;
> @@ -313,14 +343,24 @@ 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))
> + addr = unix_sk(other->sk)->addr;
> + /* Unnamed sockets are not restricted. */
> + if (!addr)
> return 0;
>
> - if (!sock_is_scoped(other->sk, subject->domain))
> + if (sock_addr_is_abstract(addr)) {
> + 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;
> + }
> +
> + if (!sock_is_scoped(other->sk, subject->domain, scope))
> return 0;
>
> landlock_log_denial(subject, &(struct landlock_request) {
> - .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> + .type = request_type,
> .audit = {
> .type = LSM_AUDIT_DATA_NET,
> .u.net = &(struct lsm_network_audit) {
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH 3/5] Documentation/landlock: Fix missing case for ABI 6 in downgrade example
From: Mickaël Salaün @ 2025-12-28 18:16 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Günther Noack, Samasth Norway Ananda,
linux-security-module
In-Reply-To: <1175b37d065b49520cbc335a9a2d501513a6c6ef.1766885035.git.m@maowtm.org>
The subject for documentation should just start with "landlock: "
On Sun, Dec 28, 2025 at 01:27:33AM +0000, Tingmao Wang wrote:
> Note that this code is different from the one in sandboxer.c since
> sandboxer won't ever add LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF and
> LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF.
A more complet patch was sent a few days ago:
https://lore.kernel.org/r/3e21551d-24c3-459f-8cee-4d85c97c0120@oracle.com
>
> Fixes: 12bfcda73ac2 ("landlock: Add LANDLOCK_RESTRICT_SELF_LOG_*_EXEC_* flags")
Unless required by other parts of the patch, there is no need to
backport documentation changes, so there should not be any Fixes tag.
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> Documentation/userspace-api/landlock.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 1d0c2c15c22e..903d2ad11852 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -127,6 +127,12 @@ version, and only use the available subset of access rights:
> /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
> ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> LANDLOCK_SCOPE_SIGNAL);
> + __attribute__((fallthrough));
> + case 6:
> + /* Removes LANDLOCK_RESTRICT_SELF_LOG_* for ABI < 7 */
> + supported_restrict_flags &= ~(LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> + LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
> + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF);
As Günther pointed out, this switch/case might not be needed for this
part of the documentation.
> }
>
> This enables the creation of an inclusive ruleset that will contain our rules.
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH 0/5] Landlock: Test, comment and doc improvements
From: Mickaël Salaün @ 2025-12-28 18:16 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Samasth Norway Ananda, linux-security-module
In-Reply-To: <cover.1766885035.git.m@maowtm.org>
On Sun, Dec 28, 2025 at 01:27:30AM +0000, Tingmao Wang wrote:
> Hi,
>
> This series contains various unrelated improvements I stashed while
> working on pathname Unix socket scopes (I will send the series for that
> tomorrow). Only test/comment/doc changes.
Thanks!
>
> I was planning to include this in the upcoming Unix socket scope series
> but I thought maybe it would be better to send them separately, as they
> would be unrelated to the main topic of that series.
Yes, that's better.
>
> Tingmao Wang (5):
> selftests/landlock: Fix typo in fs_test
Applied
> selftests/landlock: Fix missing semicolon
Applied
> Documentation/landlock: Fix missing case for ABI 6 in downgrade
> example
Not applied, see review.
> selftests/landlock: Use scoped_base_variants.h for ptrace_test
Applied
> landlock: Improve the comment for domain_is_scoped
Applied with cosmetic changes.
>
> Documentation/userspace-api/landlock.rst | 6 +
> security/landlock/task.c | 9 +-
> tools/testing/selftests/landlock/fs_test.c | 4 +-
> .../testing/selftests/landlock/ptrace_test.c | 154 +-----------------
> .../landlock/scoped_abstract_unix_test.c | 2 +-
> .../selftests/landlock/scoped_base_variants.h | 9 +-
> 6 files changed, 26 insertions(+), 158 deletions(-)
>
>
> base-commit: 161db1810f3625e97ab414908dbcf4b2ab73c309
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [RFC PATCH 5/5] landlock: Improve the comment for domain_is_scoped
From: Mickaël Salaün @ 2025-12-28 18:16 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Samasth Norway Ananda, linux-security-module,
Tahera Fahimi
In-Reply-To: <06393bc18aee5bc278df5ef31c64a05b742ebc10.1766885035.git.m@maowtm.org>
On Sun, Dec 28, 2025 at 01:27:35AM +0000, Tingmao Wang wrote:
> Currently it is not obvious what "scoped" mean, and the fact that the
> function returns true when access should be denied is slightly surprising
> and in need of documentation.
>
> Cc: Tahera Fahimi <fahimitahera@gmail.com>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Open to discussion on whether this actually explains it better.
>
> security/landlock/task.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index bf4ed15a7f01..6dfcc1860d6e 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -166,15 +166,16 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> }
>
> /**
> - * domain_is_scoped - Checks if the client domain is scoped in the same
> - * domain as the server.
> + * domain_is_scoped - Check if an interaction from a client/sender to a
> + * server/receiver should be restricted based on scope controls.
> *
> * @client: IPC sender domain.
> * @server: IPC receiver domain.
> * @scope: The scope restriction criteria.
> *
> - * Returns: True if the @client domain is scoped to access the @server,
> - * unless the @server is also scoped in the same domain as @client.
> + * Returns: True if the @server is in a different domain from @client, and
> + * the @client domain is scoped to access the @server (i.e. access
> + * should be denied).
I removed the "the" and the heading spaces:
*
- * Returns: True if the @client domain is scoped to access the @server,
- * unless the @server is also scoped in the same domain as @client.
+ * Returns: True if @server is in a different domain from @client, and @client
+ * is scoped to access @server (i.e. access should be denied).
*/
> */
> static bool domain_is_scoped(const struct landlock_ruleset *const client,
> const struct landlock_ruleset *const server,
> --
> 2.52.0
>
^ permalink raw reply
* Re: commoncap KUnit tests v4
From: Ryan Foster @ 2025-12-28 19:45 UTC (permalink / raw)
To: linux-security-module; +Cc: linux-kernel, paul, serge
In-Reply-To: <20251204215610.148342-1-foster.ryan.r@gmail.com>
Hi all,
I've updated the patch after noticing the dependency on CONFIG_USER_NS=y
Thanks,
Ryan
^ permalink raw reply
* [PATCH v4] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Ryan Foster @ 2025-12-28 19:45 UTC (permalink / raw)
To: linux-security-module; +Cc: linux-kernel, paul, serge, Ryan Foster
In-Reply-To: <20251228194838.32214-1-foster.ryan.r@gmail.com>
Add comprehensive KUnit tests for the namespace-related capability
functions that Serge Hallyn refactored in commit 9891d2f79a9f
("Clarify the rootid_owns_currentns").
The tests verify:
- Basic functionality: UID 0 in init namespace, invalid vfsuid,
non-zero UIDs
- Actual namespace traversal: Creating user namespaces with different
UID mappings where uid 0 maps to different kuids (e.g., 1000, 2000,
3000)
- Hierarchy traversal: Testing multiple nested namespaces to verify
correct namespace hierarchy traversal
This addresses the feedback to "test the actual functionality" by
creating real user namespaces with different values for the
namespace's uid 0, rather than just basic input validation.
The test file is included at the end of commoncap.c when
CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, following the
standard kernel pattern (e.g., scsi_lib.c, ext4/mballoc.c). This
allows tests to access static functions in the same compilation unit
without modifying production code based on test configuration.
The tests require CONFIG_USER_NS to be enabled since they rely on user
namespace mapping functionality. The Kconfig dependency ensures the
tests only build when this requirement is met.
All 7 tests pass:
- test_vfsuid_root_in_currentns_init_ns
- test_vfsuid_root_in_currentns_invalid
- test_vfsuid_root_in_currentns_nonzero
- test_kuid_root_in_ns_init_ns_uid0
- test_kuid_root_in_ns_init_ns_nonzero
- test_kuid_root_in_ns_with_mapping
- test_kuid_root_in_ns_with_different_mappings
Updated MAINTAINER capabilities to include commoncap test
Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
---
MAINTAINERS | 1 +
security/Kconfig | 17 +++
security/commoncap.c | 4 +
security/commoncap_test.c | 290 ++++++++++++++++++++++++++++++++++++++
4 files changed, 312 insertions(+)
create mode 100644 security/commoncap_test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c0030e126fc8..6f162c736dfb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5682,6 +5682,7 @@ F: include/trace/events/capability.h
F: include/uapi/linux/capability.h
F: kernel/capability.c
F: security/commoncap.c
+F: security/commoncap_test.c
CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER
M: Kevin Tsai <ktsai@capellamicro.com>
diff --git a/security/Kconfig b/security/Kconfig
index 285f284dfcac..f47975284aab 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -284,6 +284,23 @@ config LSM
If unsure, leave this as the default.
+config SECURITY_COMMONCAP_KUNIT_TEST
+ bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
+ depends on KUNIT && USER_NS
+ default KUNIT_ALL_TESTS
+ help
+ This builds the commoncap KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
source "security/Kconfig.hardening"
endmenu
diff --git a/security/commoncap.c b/security/commoncap.c
index 8a23dfab7fac..3399535808fe 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1521,3 +1521,7 @@ DEFINE_LSM(capability) = {
};
#endif /* CONFIG_SECURITY */
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+#include "commoncap_test.c"
+#endif
diff --git a/security/commoncap_test.c b/security/commoncap_test.c
new file mode 100644
index 000000000000..1088364a54e6
--- /dev/null
+++ b/security/commoncap_test.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for commoncap.c security functions
+ *
+ * Tests for security-critical functions in the capability subsystem,
+ * particularly namespace-related capability checks.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+#include <linux/cred.h>
+#include <linux/mnt_idmapping.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/refcount.h>
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+
+/* Functions are static in commoncap.c, but we can call them since we're
+ * included in the same compilation unit when tests are enabled.
+ */
+
+/**
+ * test_vfsuid_root_in_currentns_init_ns - Test vfsuid_root_in_currentns with init ns
+ *
+ * Verifies that UID 0 in the init namespace correctly owns the current
+ * namespace when running in init_user_ns.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_init_ns(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create UID 0 in init namespace */
+ kuid = KUIDT_INIT(0);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* In init namespace, UID 0 should own current namespace */
+ KUNIT_EXPECT_TRUE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_invalid - Test vfsuid_root_in_currentns with invalid vfsuid
+ *
+ * Verifies that an invalid vfsuid correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_invalid(struct kunit *test)
+{
+ vfsuid_t invalid_vfsuid;
+
+ /* Use the predefined invalid vfsuid */
+ invalid_vfsuid = INVALID_VFSUID;
+
+ /* Invalid vfsuid should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(invalid_vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_nonzero - Test vfsuid_root_in_currentns with non-zero UID
+ *
+ * Verifies that a non-zero UID correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_nonzero(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create a non-zero UID */
+ kuid = KUIDT_INIT(1000);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* Non-zero UID should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_uid0 - Test kuid_root_in_ns with init namespace and UID 0
+ *
+ * Verifies that kuid_root_in_ns correctly identifies UID 0 in init namespace.
+ * This tests the core namespace traversal logic. In init namespace, UID 0
+ * maps to itself, so it should own the namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_uid0(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(0);
+ init_ns = &init_user_ns;
+
+ /* UID 0 should own init namespace */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_nonzero - Test kuid_root_in_ns with init namespace and non-zero UID
+ *
+ * Verifies that kuid_root_in_ns correctly rejects non-zero UIDs in init namespace.
+ * Only UID 0 should own a namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_nonzero(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(1000);
+ init_ns = &init_user_ns;
+
+ /* Non-zero UID should not own namespace */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * create_test_user_ns_with_mapping - Create a mock user namespace with UID mapping
+ *
+ * Creates a minimal user namespace structure for testing where uid 0 in the
+ * namespace maps to a specific kuid in the parent namespace.
+ *
+ * @test: KUnit test context
+ * @parent_ns: Parent namespace (typically init_user_ns)
+ * @mapped_kuid: The kuid that uid 0 in this namespace maps to in parent
+ *
+ * Returns: Pointer to allocated namespace, or NULL on failure
+ */
+static struct user_namespace *create_test_user_ns_with_mapping(struct kunit *test,
+ struct user_namespace *parent_ns,
+ kuid_t mapped_kuid)
+{
+ struct user_namespace *ns;
+ struct uid_gid_extent extent;
+
+ /* Allocate a test namespace - use kzalloc to zero all fields */
+ ns = kunit_kzalloc(test, sizeof(*ns), GFP_KERNEL);
+ if (!ns)
+ return NULL;
+
+ /* Initialize basic namespace structure fields */
+ ns->parent = parent_ns;
+ ns->level = parent_ns ? parent_ns->level + 1 : 0;
+ ns->owner = mapped_kuid;
+ ns->group = KGIDT_INIT(0);
+
+ /* Initialize ns_common structure */
+ refcount_set(&ns->ns.__ns_ref, 1);
+ ns->ns.inum = 0; /* Mock inum */
+
+ /* Set up uid mapping: uid 0 in this namespace maps to mapped_kuid in parent
+ * Format: first (uid in ns) : lower_first (kuid in parent) : count
+ * So: uid 0 in ns -> kuid mapped_kuid in parent
+ * This means from_kuid(ns, mapped_kuid) returns 0
+ */
+ extent.first = 0; /* uid 0 in this namespace */
+ extent.lower_first = __kuid_val(mapped_kuid); /* maps to this kuid in parent */
+ extent.count = 1;
+
+ ns->uid_map.extent[0] = extent;
+ ns->uid_map.nr_extents = 1;
+
+ /* Set up gid mapping: gid 0 maps to gid 0 in parent (simplified) */
+ extent.first = 0;
+ extent.lower_first = 0;
+ extent.count = 1;
+
+ ns->gid_map.extent[0] = extent;
+ ns->gid_map.nr_extents = 1;
+
+ return ns;
+}
+
+/**
+ * test_kuid_root_in_ns_with_mapping - Test kuid_root_in_ns with namespace where uid 0
+ * maps to different kuid
+ *
+ * Creates a user namespace where uid 0 maps to kuid 1000 in the parent namespace.
+ * Verifies that kuid_root_in_ns correctly identifies kuid 1000 as owning the namespace.
+ *
+ * Note: kuid_root_in_ns walks up the namespace hierarchy, so it checks the current
+ * namespace first, then parent, then parent's parent, etc. So:
+ * - kuid 1000 owns test_ns because from_kuid(test_ns, 1000) == 0
+ * - kuid 0 also owns test_ns because from_kuid(init_user_ns, 0) == 0
+ * (checked in parent)
+ *
+ * This tests the actual functionality as requested: creating namespaces with
+ * different values for the namespace's uid 0.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_mapping(struct kunit *test)
+{
+ struct user_namespace *test_ns;
+ struct user_namespace *parent_ns;
+ kuid_t mapped_kuid, other_kuid;
+
+ parent_ns = &init_user_ns;
+ mapped_kuid = KUIDT_INIT(1000);
+ other_kuid = KUIDT_INIT(2000);
+
+ test_ns = create_test_user_ns_with_mapping(test, parent_ns, mapped_kuid);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_ns);
+
+ /* kuid 1000 should own test_ns because it maps to uid 0 in test_ns */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(mapped_kuid, test_ns));
+
+ /* kuid 0 should also own test_ns (checked via parent init_user_ns) */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), test_ns));
+
+ /* Other kuids should not own test_ns */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(other_kuid, test_ns));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(500), test_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_with_different_mappings - Test with multiple namespaces
+ *
+ * Creates multiple user namespaces with different UID mappings to verify
+ * that kuid_root_in_ns correctly handles different namespace hierarchies.
+ *
+ * Since kuid_root_in_ns walks up the hierarchy, kuids that map to 0 in init_user_ns
+ * will own all namespaces, while kuids that only map to 0 in specific namespaces
+ * will only own those namespaces and their children.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_different_mappings(struct kunit *test)
+{
+ struct user_namespace *ns1, *ns2, *ns3;
+
+ /* Create ns1 where uid 0 maps to kuid 1000 */
+ ns1 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(1000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns1);
+
+ /* Create ns2 where uid 0 maps to kuid 2000 */
+ ns2 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(2000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns2);
+
+ /* Create ns3 as a child of ns1 where uid 0 maps to kuid 3000 */
+ ns3 = create_test_user_ns_with_mapping(test, ns1, KUIDT_INIT(3000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns3);
+
+ /* Test ns1: kuid 1000 owns it, kuid 0 owns it (via parent), kuid 2000 does not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns1));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns1));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns1));
+
+ /* Test ns2: kuid 2000 owns it, kuid 0 owns it (via parent), kuid 1000 does not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns2));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns2));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns2));
+
+ /* Test ns3: kuid 3000 owns it, kuid 1000 owns it (via parent ns1),
+ * kuid 0 owns it (via init_user_ns), kuid 2000 does not
+ */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns3));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns3));
+}
+
+static struct kunit_case commoncap_test_cases[] = {
+ KUNIT_CASE(test_vfsuid_root_in_currentns_init_ns),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_invalid),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_uid0),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_with_mapping),
+ KUNIT_CASE(test_kuid_root_in_ns_with_different_mappings),
+ {}
+};
+
+static struct kunit_suite commoncap_test_suite = {
+ .name = "commoncap",
+ .test_cases = commoncap_test_cases,
+};
+
+kunit_test_suite(commoncap_test_suite);
+
+MODULE_LICENSE("GPL");
+
+#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
--
2.52.0
^ permalink raw reply related
* Re: [PATCH 3/6] samples/landlock: Support LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Demi Marie Obenour @ 2025-12-29 2:48 UTC (permalink / raw)
To: Tingmao Wang, Mickaël Salaün
Cc: Günther Noack, Alyssa Ross, Jann Horn, Tahera Fahimi,
linux-security-module
In-Reply-To: <625fa7da41fa6abdebac9e48e824232465504c9f.1766925301.git.m@maowtm.org>
[-- Attachment #1.1.1: Type: text/plain, Size: 880 bytes --]
On 12/28/25 07:45, 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.
>
> 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 think almost all uses of Landlock would want to either scope pathname
sockets, or else only allow an allowlist of them to be accessed.
The latter is not currently possible.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Serge E. Hallyn @ 2025-12-29 4:14 UTC (permalink / raw)
To: Ryan Foster; +Cc: linux-security-module, linux-kernel, paul, serge
In-Reply-To: <20251228194838.32214-2-foster.ryan.r@gmail.com>
On Sun, Dec 28, 2025 at 11:45:37AM -0800, Ryan Foster wrote:
> Add comprehensive KUnit tests for the namespace-related capability
> functions that Serge Hallyn refactored in commit 9891d2f79a9f
> ("Clarify the rootid_owns_currentns").
>
> The tests verify:
> - Basic functionality: UID 0 in init namespace, invalid vfsuid,
> non-zero UIDs
> - Actual namespace traversal: Creating user namespaces with different
> UID mappings where uid 0 maps to different kuids (e.g., 1000, 2000,
> 3000)
> - Hierarchy traversal: Testing multiple nested namespaces to verify
> correct namespace hierarchy traversal
>
> This addresses the feedback to "test the actual functionality" by
> creating real user namespaces with different values for the
> namespace's uid 0, rather than just basic input validation.
>
> The test file is included at the end of commoncap.c when
> CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, following the
> standard kernel pattern (e.g., scsi_lib.c, ext4/mballoc.c). This
Thank you for pointing this out, as it seemed odd to me.
> allows tests to access static functions in the same compilation unit
> without modifying production code based on test configuration.
>
> The tests require CONFIG_USER_NS to be enabled since they rely on user
> namespace mapping functionality. The Kconfig dependency ensures the
> tests only build when this requirement is met.
>
> All 7 tests pass:
> - test_vfsuid_root_in_currentns_init_ns
> - test_vfsuid_root_in_currentns_invalid
> - test_vfsuid_root_in_currentns_nonzero
> - test_kuid_root_in_ns_init_ns_uid0
> - test_kuid_root_in_ns_init_ns_nonzero
> - test_kuid_root_in_ns_with_mapping
> - test_kuid_root_in_ns_with_different_mappings
>
> Updated MAINTAINER capabilities to include commoncap test
>
> Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
Thanks! A few comments below. But the set of tests looks great.
> ---
> MAINTAINERS | 1 +
> security/Kconfig | 17 +++
> security/commoncap.c | 4 +
> security/commoncap_test.c | 290 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 312 insertions(+)
> create mode 100644 security/commoncap_test.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c0030e126fc8..6f162c736dfb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5682,6 +5682,7 @@ F: include/trace/events/capability.h
> F: include/uapi/linux/capability.h
> F: kernel/capability.c
> F: security/commoncap.c
> +F: security/commoncap_test.c
>
> CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER
> M: Kevin Tsai <ktsai@capellamicro.com>
> diff --git a/security/Kconfig b/security/Kconfig
> index 285f284dfcac..f47975284aab 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -284,6 +284,23 @@ config LSM
>
> If unsure, leave this as the default.
>
> +config SECURITY_COMMONCAP_KUNIT_TEST
> + bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
> + depends on KUNIT && USER_NS
> + default KUNIT_ALL_TESTS
I might be misunderstanding the syntax, but given that this is
only enabled (iiuc) if KUNIT_ALL_TESTS=false, then wouldn't it
be the same, and simpler, to say default false here?
I'm probably missing something.
> + help
> + This builds the commoncap KUnit tests.
> +
> + KUnit tests run during boot and output the results to the debug log
> + in TAP format (https://testanything.org/). Only useful for kernel devs
> + running KUnit test harness and are not for inclusion into a
> + production build.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> source "security/Kconfig.hardening"
>
> endmenu
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 8a23dfab7fac..3399535808fe 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1521,3 +1521,7 @@ DEFINE_LSM(capability) = {
> };
>
> #endif /* CONFIG_SECURITY */
> +
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +#include "commoncap_test.c"
> +#endif
> diff --git a/security/commoncap_test.c b/security/commoncap_test.c
> new file mode 100644
> index 000000000000..1088364a54e6
> --- /dev/null
> +++ b/security/commoncap_test.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * KUnit tests for commoncap.c security functions
> + *
> + * Tests for security-critical functions in the capability subsystem,
> + * particularly namespace-related capability checks.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/user_namespace.h>
> +#include <linux/uidgid.h>
> +#include <linux/cred.h>
> +#include <linux/mnt_idmapping.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/refcount.h>
> +
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +
> +/* Functions are static in commoncap.c, but we can call them since we're
> + * included in the same compilation unit when tests are enabled.
> + */
> +
> +/**
> + * test_vfsuid_root_in_currentns_init_ns - Test vfsuid_root_in_currentns with init ns
> + *
> + * Verifies that UID 0 in the init namespace correctly owns the current
> + * namespace when running in init_user_ns.
> + *
> + * @test: KUnit test context
> + */
> +static void test_vfsuid_root_in_currentns_init_ns(struct kunit *test)
> +{
> + vfsuid_t vfsuid;
> + kuid_t kuid;
> +
> + /* Create UID 0 in init namespace */
> + kuid = KUIDT_INIT(0);
> + vfsuid = VFSUIDT_INIT(kuid);
> +
> + /* In init namespace, UID 0 should own current namespace */
> + KUNIT_EXPECT_TRUE(test, vfsuid_root_in_currentns(vfsuid));
> +}
> +
> +/**
> + * test_vfsuid_root_in_currentns_invalid - Test vfsuid_root_in_currentns with invalid vfsuid
> + *
> + * Verifies that an invalid vfsuid correctly returns false.
> + *
> + * @test: KUnit test context
> + */
> +static void test_vfsuid_root_in_currentns_invalid(struct kunit *test)
> +{
> + vfsuid_t invalid_vfsuid;
> +
> + /* Use the predefined invalid vfsuid */
> + invalid_vfsuid = INVALID_VFSUID;
> +
> + /* Invalid vfsuid should return false */
> + KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(invalid_vfsuid));
> +}
> +
> +/**
> + * test_vfsuid_root_in_currentns_nonzero - Test vfsuid_root_in_currentns with non-zero UID
> + *
> + * Verifies that a non-zero UID correctly returns false.
> + *
> + * @test: KUnit test context
> + */
> +static void test_vfsuid_root_in_currentns_nonzero(struct kunit *test)
> +{
> + vfsuid_t vfsuid;
> + kuid_t kuid;
> +
> + /* Create a non-zero UID */
> + kuid = KUIDT_INIT(1000);
> + vfsuid = VFSUIDT_INIT(kuid);
> +
> + /* Non-zero UID should return false */
> + KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(vfsuid));
> +}
> +
> +/**
> + * test_kuid_root_in_ns_init_ns_uid0 - Test kuid_root_in_ns with init namespace and UID 0
> + *
> + * Verifies that kuid_root_in_ns correctly identifies UID 0 in init namespace.
> + * This tests the core namespace traversal logic. In init namespace, UID 0
> + * maps to itself, so it should own the namespace.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_init_ns_uid0(struct kunit *test)
> +{
> + kuid_t kuid;
> + struct user_namespace *init_ns;
> +
> + kuid = KUIDT_INIT(0);
> + init_ns = &init_user_ns;
> +
> + /* UID 0 should own init namespace */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(kuid, init_ns));
> +}
> +
> +/**
> + * test_kuid_root_in_ns_init_ns_nonzero - Test kuid_root_in_ns with init namespace and non-zero UID
> + *
> + * Verifies that kuid_root_in_ns correctly rejects non-zero UIDs in init namespace.
> + * Only UID 0 should own a namespace.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_init_ns_nonzero(struct kunit *test)
> +{
> + kuid_t kuid;
> + struct user_namespace *init_ns;
> +
> + kuid = KUIDT_INIT(1000);
> + init_ns = &init_user_ns;
> +
> + /* Non-zero UID should not own namespace */
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(kuid, init_ns));
> +}
> +
> +/**
> + * create_test_user_ns_with_mapping - Create a mock user namespace with UID mapping
> + *
> + * Creates a minimal user namespace structure for testing where uid 0 in the
> + * namespace maps to a specific kuid in the parent namespace.
> + *
> + * @test: KUnit test context
> + * @parent_ns: Parent namespace (typically init_user_ns)
> + * @mapped_kuid: The kuid that uid 0 in this namespace maps to in parent
> + *
> + * Returns: Pointer to allocated namespace, or NULL on failure
> + */
> +static struct user_namespace *create_test_user_ns_with_mapping(struct kunit *test,
> + struct user_namespace *parent_ns,
> + kuid_t mapped_kuid)
> +{
> + struct user_namespace *ns;
> + struct uid_gid_extent extent;
> +
> + /* Allocate a test namespace - use kzalloc to zero all fields */
> + ns = kunit_kzalloc(test, sizeof(*ns), GFP_KERNEL);
> + if (!ns)
> + return NULL;
> +
> + /* Initialize basic namespace structure fields */
> + ns->parent = parent_ns;
> + ns->level = parent_ns ? parent_ns->level + 1 : 0;
> + ns->owner = mapped_kuid;
> + ns->group = KGIDT_INIT(0);
> +
> + /* Initialize ns_common structure */
> + refcount_set(&ns->ns.__ns_ref, 1);
> + ns->ns.inum = 0; /* Mock inum */
> +
> + /* Set up uid mapping: uid 0 in this namespace maps to mapped_kuid in parent
> + * Format: first (uid in ns) : lower_first (kuid in parent) : count
> + * So: uid 0 in ns -> kuid mapped_kuid in parent
> + * This means from_kuid(ns, mapped_kuid) returns 0
> + */
> + extent.first = 0; /* uid 0 in this namespace */
> + extent.lower_first = __kuid_val(mapped_kuid); /* maps to this kuid in parent */
> + extent.count = 1;
> +
> + ns->uid_map.extent[0] = extent;
> + ns->uid_map.nr_extents = 1;
> +
> + /* Set up gid mapping: gid 0 maps to gid 0 in parent (simplified) */
> + extent.first = 0;
> + extent.lower_first = 0;
> + extent.count = 1;
> +
> + ns->gid_map.extent[0] = extent;
> + ns->gid_map.nr_extents = 1;
> +
> + return ns;
> +}
> +
> +/**
> + * test_kuid_root_in_ns_with_mapping - Test kuid_root_in_ns with namespace where uid 0
> + * maps to different kuid
> + *
> + * Creates a user namespace where uid 0 maps to kuid 1000 in the parent namespace.
> + * Verifies that kuid_root_in_ns correctly identifies kuid 1000 as owning the namespace.
> + *
> + * Note: kuid_root_in_ns walks up the namespace hierarchy, so it checks the current
> + * namespace first, then parent, then parent's parent, etc. So:
> + * - kuid 1000 owns test_ns because from_kuid(test_ns, 1000) == 0
> + * - kuid 0 also owns test_ns because from_kuid(init_user_ns, 0) == 0
> + * (checked in parent)
> + *
> + * This tests the actual functionality as requested: creating namespaces with
> + * different values for the namespace's uid 0.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_with_mapping(struct kunit *test)
> +{
> + struct user_namespace *test_ns;
> + struct user_namespace *parent_ns;
> + kuid_t mapped_kuid, other_kuid;
> +
> + parent_ns = &init_user_ns;
> + mapped_kuid = KUIDT_INIT(1000);
> + other_kuid = KUIDT_INIT(2000);
> +
> + test_ns = create_test_user_ns_with_mapping(test, parent_ns, mapped_kuid);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_ns);
> +
> + /* kuid 1000 should own test_ns because it maps to uid 0 in test_ns */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(mapped_kuid, test_ns));
> +
> + /* kuid 0 should also own test_ns (checked via parent init_user_ns) */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), test_ns));
> +
> + /* Other kuids should not own test_ns */
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(other_kuid, test_ns));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(500), test_ns));
> +}
> +
> +/**
> + * test_kuid_root_in_ns_with_different_mappings - Test with multiple namespaces
> + *
> + * Creates multiple user namespaces with different UID mappings to verify
> + * that kuid_root_in_ns correctly handles different namespace hierarchies.
> + *
> + * Since kuid_root_in_ns walks up the hierarchy, kuids that map to 0 in init_user_ns
> + * will own all namespaces, while kuids that only map to 0 in specific namespaces
> + * will only own those namespaces and their children.
> + *
> + * @test: KUnit test context
> + */
> +static void test_kuid_root_in_ns_with_different_mappings(struct kunit *test)
> +{
> + struct user_namespace *ns1, *ns2, *ns3;
> +
> + /* Create ns1 where uid 0 maps to kuid 1000 */
> + ns1 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(1000));
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns1);
> +
> + /* Create ns2 where uid 0 maps to kuid 2000 */
> + ns2 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(2000));
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns2);
> +
> + /* Create ns3 as a child of ns1 where uid 0 maps to kuid 3000 */
> + ns3 = create_test_user_ns_with_mapping(test, ns1, KUIDT_INIT(3000));
This is not really a valid configuration, though. If ns3 is a child of ns1,
then any kuids mapped into ns3 must also have been mapped into ns1.
I think I'm ok with saying "it doesn't matter for this test", but it should
probably be documented here that we're ignoring that as irrelevant to the
test.
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns3);
> +
> + /* Test ns1: kuid 1000 owns it, kuid 0 owns it (via parent), kuid 2000 does not */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns1));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns1));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns1));
> +
> + /* Test ns2: kuid 2000 owns it, kuid 0 owns it (via parent), kuid 1000 does not */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns2));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns2));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns2));
> +
> + /* Test ns3: kuid 3000 owns it, kuid 1000 owns it (via parent ns1),
> + * kuid 0 owns it (via init_user_ns), kuid 2000 does not
> + */
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns3));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns3));
> + KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns3));
> + KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns3));
> +}
> +
> +static struct kunit_case commoncap_test_cases[] = {
> + KUNIT_CASE(test_vfsuid_root_in_currentns_init_ns),
> + KUNIT_CASE(test_vfsuid_root_in_currentns_invalid),
> + KUNIT_CASE(test_vfsuid_root_in_currentns_nonzero),
> + KUNIT_CASE(test_kuid_root_in_ns_init_ns_uid0),
> + KUNIT_CASE(test_kuid_root_in_ns_init_ns_nonzero),
> + KUNIT_CASE(test_kuid_root_in_ns_with_mapping),
> + KUNIT_CASE(test_kuid_root_in_ns_with_different_mappings),
> + {}
> +};
> +
> +static struct kunit_suite commoncap_test_suite = {
> + .name = "commoncap",
> + .test_cases = commoncap_test_cases,
> +};
> +
> +kunit_test_suite(commoncap_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +
> +#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
> --
> 2.52.0
^ permalink raw reply
* Re: [PATCH v4] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: kernel test robot @ 2025-12-29 13:56 UTC (permalink / raw)
To: Ryan Foster, linux-security-module
Cc: oe-kbuild-all, linux-kernel, paul, serge, Ryan Foster
In-Reply-To: <20251228194838.32214-2-foster.ryan.r@gmail.com>
Hi Ryan,
kernel test robot noticed the following build errors:
[auto build test ERROR on pcmoore-selinux/next]
[also build test ERROR on linus/master v6.19-rc3 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ryan-Foster/security-Add-KUnit-tests-for-kuid_root_in_ns-and-vfsuid_root_in_currentns/20251229-035056
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link: https://lore.kernel.org/r/20251228194838.32214-2-foster.ryan.r%40gmail.com
patch subject: [PATCH v4] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
config: i386-randconfig-011-20251229 (https://download.01.org/0day-ci/archive/20251229/202512292101.U530Lg4A-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251229/202512292101.U530Lg4A-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512292101.U530Lg4A-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: security/commoncap.o: in function `kunit_kmalloc':
>> include/kunit/test.h:501:(.text+0xb59): undefined reference to `kunit_kmalloc_array'
ld: security/commoncap.o: in function `test_kuid_root_in_ns_init_ns_nonzero':
>> security/commoncap_test.c:121:(.text+0x109c): undefined reference to `kunit_unary_assert_format'
>> ld: security/commoncap_test.c:121:(.text+0x10aa): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap.o: in function `test_kuid_root_in_ns_init_ns_uid0':
security/commoncap_test.c:101:(.text+0x1101): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:101:(.text+0x110f): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap.o: in function `test_vfsuid_root_in_currentns_nonzero':
security/commoncap_test.c:80:(.text+0x14aa): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:80:(.text+0x14b8): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap.o: in function `test_vfsuid_root_in_currentns_init_ns':
security/commoncap_test.c:42:(.text+0x1544): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:42:(.text+0x1552): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap.o: in function `test_kuid_root_in_ns_with_mapping':
security/commoncap_test.c:219:(.text+0x166a): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:219:(.text+0x1678): undefined reference to `__kunit_do_failed_assertion'
>> ld: security/commoncap_test.c:220:(.text+0x16ca): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:220:(.text+0x16d8): undefined reference to `__kunit_do_failed_assertion'
>> ld: security/commoncap_test.c:210:(.text+0x16f8): undefined reference to `kunit_ptr_not_err_assert_format'
ld: security/commoncap_test.c:210:(.text+0x1706): undefined reference to `__kunit_do_failed_assertion'
>> ld: security/commoncap_test.c:210:(.text+0x170d): undefined reference to `__kunit_abort'
ld: security/commoncap_test.c:213:(.text+0x172c): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:213:(.text+0x173a): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:216:(.text+0x175c): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:216:(.text+0x176a): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap.o: in function `test_kuid_root_in_ns_with_different_mappings':
security/commoncap_test.c:254:(.text+0x18fa): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:254:(.text+0x1908): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:259:(.text+0x19da): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:259:(.text+0x19e8): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:267:(.text+0x1af2): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:267:(.text+0x1b00): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:241:(.text+0x1b20): undefined reference to `kunit_ptr_not_err_assert_format'
ld: security/commoncap_test.c:241:(.text+0x1b2e): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:241:(.text+0x1b35): undefined reference to `__kunit_abort'
ld: security/commoncap_test.c:245:(.text+0x1b50): undefined reference to `kunit_ptr_not_err_assert_format'
ld: security/commoncap_test.c:245:(.text+0x1b61): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:245:(.text+0x1b68): undefined reference to `__kunit_abort'
ld: security/commoncap_test.c:249:(.text+0x1b80): undefined reference to `kunit_ptr_not_err_assert_format'
ld: security/commoncap_test.c:249:(.text+0x1b8e): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:249:(.text+0x1b95): undefined reference to `__kunit_abort'
ld: security/commoncap_test.c:252:(.text+0x1bb4): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:252:(.text+0x1bc5): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:266:(.text+0x1bec): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:266:(.text+0x1bfa): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:265:(.text+0x1c1c): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:265:(.text+0x1c2a): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:264:(.text+0x1c4c): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:264:(.text+0x1c5a): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:253:(.text+0x1c7c): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:253:(.text+0x1c8d): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:258:(.text+0x1cb4): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:258:(.text+0x1cc2): undefined reference to `__kunit_do_failed_assertion'
ld: security/commoncap_test.c:257:(.text+0x1ce4): undefined reference to `kunit_unary_assert_format'
ld: security/commoncap_test.c:257:(.text+0x1cf2): undefined reference to `__kunit_do_failed_assertion'
vim +501 include/kunit/test.h
7122debb4367ee Daniel Latypov 2021-05-03 487
7122debb4367ee Daniel Latypov 2021-05-03 488 /**
7122debb4367ee Daniel Latypov 2021-05-03 489 * kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
7122debb4367ee Daniel Latypov 2021-05-03 490 * @test: The test context object.
7122debb4367ee Daniel Latypov 2021-05-03 491 * @size: The size in bytes of the desired memory.
7122debb4367ee Daniel Latypov 2021-05-03 492 * @gfp: flags passed to underlying kmalloc().
7122debb4367ee Daniel Latypov 2021-05-03 493 *
7122debb4367ee Daniel Latypov 2021-05-03 494 * See kmalloc() and kunit_kmalloc_array() for more information.
57e3cded99e9c8 David Gow 2023-05-25 495 *
57e3cded99e9c8 David Gow 2023-05-25 496 * Note that some internal context data is also allocated with GFP_KERNEL,
57e3cded99e9c8 David Gow 2023-05-25 497 * regardless of the gfp passed in.
7122debb4367ee Daniel Latypov 2021-05-03 498 */
7122debb4367ee Daniel Latypov 2021-05-03 499 static inline void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
7122debb4367ee Daniel Latypov 2021-05-03 500 {
7122debb4367ee Daniel Latypov 2021-05-03 @501 return kunit_kmalloc_array(test, 1, size, gfp);
7122debb4367ee Daniel Latypov 2021-05-03 502 }
0a756853586ce1 Brendan Higgins 2019-09-23 503
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v5] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Ryan Foster @ 2025-12-29 14:23 UTC (permalink / raw)
To: serge; +Cc: linux-security-module, linux-kernel, paul, Ryan Foster
In-Reply-To: <aVIAFEUjPe6DhrKc@mail.hallyn.com>
Hi Serge,
Thanks for the review. I have updated the patch to address the feedback
and build error from the intel CI.
Changes:
- Fixed build error, changed dependency to KUNIT=y so always built in
and not a module
- Fixed test invalid namespace configuration. Made test ns children of
init_user_ns, simpler config and expected logic.
With regards to the kconfig default, I copied this pattern over without
fully understanding the design, e.g. from lib/Kconfig. I believe
it is used to auto enable tests when running full test suite by default,
otherwise allowing for individual opt-in tests.
Thanks,
Ryan
Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
---
MAINTAINERS | 1 +
security/Kconfig | 17 +++
security/commoncap.c | 4 +
security/commoncap_test.c | 288 ++++++++++++++++++++++++++++++++++++++
4 files changed, 310 insertions(+)
create mode 100644 security/commoncap_test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c0030e126fc8..6f162c736dfb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5682,6 +5682,7 @@ F: include/trace/events/capability.h
F: include/uapi/linux/capability.h
F: kernel/capability.c
F: security/commoncap.c
+F: security/commoncap_test.c
CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER
M: Kevin Tsai <ktsai@capellamicro.com>
diff --git a/security/Kconfig b/security/Kconfig
index 285f284dfcac..6a4393fce9a1 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -284,6 +284,23 @@ config LSM
If unsure, leave this as the default.
+config SECURITY_COMMONCAP_KUNIT_TEST
+ bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y && USER_NS
+ default KUNIT_ALL_TESTS
+ help
+ This builds the commoncap KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
source "security/Kconfig.hardening"
endmenu
diff --git a/security/commoncap.c b/security/commoncap.c
index 8a23dfab7fac..3399535808fe 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1521,3 +1521,7 @@ DEFINE_LSM(capability) = {
};
#endif /* CONFIG_SECURITY */
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+#include "commoncap_test.c"
+#endif
diff --git a/security/commoncap_test.c b/security/commoncap_test.c
new file mode 100644
index 000000000000..e9b278be37f1
--- /dev/null
+++ b/security/commoncap_test.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for commoncap.c security functions
+ *
+ * Tests for security-critical functions in the capability subsystem,
+ * particularly namespace-related capability checks.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+#include <linux/cred.h>
+#include <linux/mnt_idmapping.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/refcount.h>
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+
+/* Functions are static in commoncap.c, but we can call them since we're
+ * included in the same compilation unit when tests are enabled.
+ */
+
+/**
+ * test_vfsuid_root_in_currentns_init_ns - Test vfsuid_root_in_currentns with init ns
+ *
+ * Verifies that UID 0 in the init namespace correctly owns the current
+ * namespace when running in init_user_ns.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_init_ns(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create UID 0 in init namespace */
+ kuid = KUIDT_INIT(0);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* In init namespace, UID 0 should own current namespace */
+ KUNIT_EXPECT_TRUE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_invalid - Test vfsuid_root_in_currentns with invalid vfsuid
+ *
+ * Verifies that an invalid vfsuid correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_invalid(struct kunit *test)
+{
+ vfsuid_t invalid_vfsuid;
+
+ /* Use the predefined invalid vfsuid */
+ invalid_vfsuid = INVALID_VFSUID;
+
+ /* Invalid vfsuid should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(invalid_vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_nonzero - Test vfsuid_root_in_currentns with non-zero UID
+ *
+ * Verifies that a non-zero UID correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_nonzero(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create a non-zero UID */
+ kuid = KUIDT_INIT(1000);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* Non-zero UID should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_uid0 - Test kuid_root_in_ns with init namespace and UID 0
+ *
+ * Verifies that kuid_root_in_ns correctly identifies UID 0 in init namespace.
+ * This tests the core namespace traversal logic. In init namespace, UID 0
+ * maps to itself, so it should own the namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_uid0(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(0);
+ init_ns = &init_user_ns;
+
+ /* UID 0 should own init namespace */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_nonzero - Test kuid_root_in_ns with init namespace and non-zero UID
+ *
+ * Verifies that kuid_root_in_ns correctly rejects non-zero UIDs in init namespace.
+ * Only UID 0 should own a namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_nonzero(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(1000);
+ init_ns = &init_user_ns;
+
+ /* Non-zero UID should not own namespace */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * create_test_user_ns_with_mapping - Create a mock user namespace with UID mapping
+ *
+ * Creates a minimal user namespace structure for testing where uid 0 in the
+ * namespace maps to a specific kuid in the parent namespace.
+ *
+ * @test: KUnit test context
+ * @parent_ns: Parent namespace (typically init_user_ns)
+ * @mapped_kuid: The kuid that uid 0 in this namespace maps to in parent
+ *
+ * Returns: Pointer to allocated namespace, or NULL on failure
+ */
+static struct user_namespace *create_test_user_ns_with_mapping(struct kunit *test,
+ struct user_namespace *parent_ns,
+ kuid_t mapped_kuid)
+{
+ struct user_namespace *ns;
+ struct uid_gid_extent extent;
+
+ /* Allocate a test namespace - use kzalloc to zero all fields */
+ ns = kunit_kzalloc(test, sizeof(*ns), GFP_KERNEL);
+ if (!ns)
+ return NULL;
+
+ /* Initialize basic namespace structure fields */
+ ns->parent = parent_ns;
+ ns->level = parent_ns ? parent_ns->level + 1 : 0;
+ ns->owner = mapped_kuid;
+ ns->group = KGIDT_INIT(0);
+
+ /* Initialize ns_common structure */
+ refcount_set(&ns->ns.__ns_ref, 1);
+ ns->ns.inum = 0; /* Mock inum */
+
+ /* Set up uid mapping: uid 0 in this namespace maps to mapped_kuid in parent
+ * Format: first (uid in ns) : lower_first (kuid in parent) : count
+ * So: uid 0 in ns -> kuid mapped_kuid in parent
+ * This means from_kuid(ns, mapped_kuid) returns 0
+ */
+ extent.first = 0; /* uid 0 in this namespace */
+ extent.lower_first = __kuid_val(mapped_kuid); /* maps to this kuid in parent */
+ extent.count = 1;
+
+ ns->uid_map.extent[0] = extent;
+ ns->uid_map.nr_extents = 1;
+
+ /* Set up gid mapping: gid 0 maps to gid 0 in parent (simplified) */
+ extent.first = 0;
+ extent.lower_first = 0;
+ extent.count = 1;
+
+ ns->gid_map.extent[0] = extent;
+ ns->gid_map.nr_extents = 1;
+
+ return ns;
+}
+
+/**
+ * test_kuid_root_in_ns_with_mapping - Test kuid_root_in_ns with namespace where uid 0
+ * maps to different kuid
+ *
+ * Creates a user namespace where uid 0 maps to kuid 1000 in the parent namespace.
+ * Verifies that kuid_root_in_ns correctly identifies kuid 1000 as owning the namespace.
+ *
+ * Note: kuid_root_in_ns walks up the namespace hierarchy, so it checks the current
+ * namespace first, then parent, then parent's parent, etc. So:
+ * - kuid 1000 owns test_ns because from_kuid(test_ns, 1000) == 0
+ * - kuid 0 also owns test_ns because from_kuid(init_user_ns, 0) == 0
+ * (checked in parent)
+ *
+ * This tests the actual functionality as requested: creating namespaces with
+ * different values for the namespace's uid 0.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_mapping(struct kunit *test)
+{
+ struct user_namespace *test_ns;
+ struct user_namespace *parent_ns;
+ kuid_t mapped_kuid, other_kuid;
+
+ parent_ns = &init_user_ns;
+ mapped_kuid = KUIDT_INIT(1000);
+ other_kuid = KUIDT_INIT(2000);
+
+ test_ns = create_test_user_ns_with_mapping(test, parent_ns, mapped_kuid);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_ns);
+
+ /* kuid 1000 should own test_ns because it maps to uid 0 in test_ns */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(mapped_kuid, test_ns));
+
+ /* kuid 0 should also own test_ns (checked via parent init_user_ns) */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), test_ns));
+
+ /* Other kuids should not own test_ns */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(other_kuid, test_ns));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(500), test_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_with_different_mappings - Test with multiple namespaces
+ *
+ * Creates multiple user namespaces with different UID mappings to verify
+ * that kuid_root_in_ns correctly distinguishes between namespaces.
+ *
+ * Each namespace maps uid 0 to a different kuid, and we verify that each
+ * kuid only owns its corresponding namespace (plus kuid 0 owns all via
+ * init_user_ns parent).
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_different_mappings(struct kunit *test)
+{
+ struct user_namespace *ns1, *ns2, *ns3;
+
+ /* Create three independent namespaces, each mapping uid 0 to different kuids */
+ ns1 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(1000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns1);
+
+ ns2 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(2000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns2);
+
+ ns3 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(3000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns3);
+
+ /* Test ns1: kuid 1000 owns it, kuid 0 owns it (via parent), others do not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns1));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns1));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns1));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns1));
+
+ /* Test ns2: kuid 2000 owns it, kuid 0 owns it (via parent), others do not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns2));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns2));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns2));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns2));
+
+ /* Test ns3: kuid 3000 owns it, kuid 0 owns it (via parent), others do not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns3));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns3));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns3));
+}
+
+static struct kunit_case commoncap_test_cases[] = {
+ KUNIT_CASE(test_vfsuid_root_in_currentns_init_ns),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_invalid),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_uid0),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_with_mapping),
+ KUNIT_CASE(test_kuid_root_in_ns_with_different_mappings),
+ {}
+};
+
+static struct kunit_suite commoncap_test_suite = {
+ .name = "commoncap",
+ .test_cases = commoncap_test_cases,
+};
+
+kunit_test_suite(commoncap_test_suite);
+
+MODULE_LICENSE("GPL");
+
+#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
--
2.52.0
^ permalink raw reply related
* Re: [RFC 00/11] Reintroduce Hornet LSM
From: Ryan Foster @ 2025-12-29 17:12 UTC (permalink / raw)
To: bboscaccy
Cc: James.Bottomley, akpm, bpf, corbet, dhowells, foster.ryan.r,
gnoack, jmorris, linux-doc, linux-kernel, linux-security-module,
linux, mic, paul, serge
In-Reply-To: <87v7i4hpi4.fsf@microsoft.com>
Hi all,
I am considering how to reconcile the TOCTOU concern with the "don't touch BPF internals" feedback, I think a very small API might help:
Minimal API draft: BPF integrity measurement
Goal: kernel-generated measurement of the final relocated program + declared inputs, so attach/link can be enforced without poking internals.
1) New BPF cmd
- BPF_MEASURE_PROG (or BPF_PROG_MEASURE)
- Input: prog_fd
- Output: opaque measurement blob + metadata
struct bpf_prog_measure_opts {
__u32 size;
__u32 flags;
__u32 sig_len;
__u64 sig_ptr;
__u64 prog_id;
__u64 meas_id;
};
Semantics
- Kernel computes measurement over final relocated insns + inputs explicitly in the integrity contract (e.g., sealed maps).
- Measurement is kernel-owned and stable for a program state.
2) Per-prog integrity state
enum lsm_integrity_verdict {
LSM_INT_VERDICT_UNSIGNED,
LSM_INT_VERDICT_PARTIAL,
LSM_INT_VERDICT_OK,
LSM_INT_VERDICT_BADSIG,
};
struct bpf_prog_integrity {
__u64 meas_id;
enum lsm_integrity_verdict v;
};
- Attach/link allowed only if policy verdict passes.
- Any input mutation invalidates meas_id and resets verdict.
3) Input immutability
- Only sealed/frozen maps can be measured.
- Any write to a measured map invalidates the measurement.
4) LSM integration
- Hornet (or another integrity LSM) consumes the measurement blob, verifies signatures, stores verdict.
- SELinux/IPE/BPF LSMs can gate attach/link based on verdict.
Why this helps
- TOCTOU: verification tied to final relocated program + frozen inputs; mutations invalidate.
- No BPF internals: LSMs use a stable syscall API, not map internals.
- Minimal blast radius: one syscall + small per-prog state.
A thought for future iterations, happy to help refine if this seems useful.
Thanks,
Ryan
^ permalink raw reply
* [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Frederick Lawler @ 2025-12-29 17:52 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
Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
is no longer able to correctly track inode.i_version due to the struct
kstat.change_cookie no longer containing an updated i_version.
Introduce a fallback mechanism for IMA that instead tracks a
integrity_ctime_guard() in absence of or outdated i_version
for stacked file systems.
EVM is left alone since it mostly cares about the backing inode.
Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
The motivation behind this was that file systems that use the
cookie to set the i_version for stacked file systems may still do so.
Then add in the ctime_guard as a fallback if there's a detected change.
The assumption is that the ctime will be different if the i_version is
different anyway for non-stacked file systems.
I'm not too pleased with passing in struct file* to
integrity_inode_attrs_changed() since EVM doesn't currently use
that for now, but I couldn't come up with another idea to get the
stat without coming up with a new stat function to accommodate just
the file path, fully separate out IMA/EVM checks, or lastly add stacked
file system support to EVM (which doesn't make much sense to me
at the moment).
I plan on adding in self test infrastructure for the v1, but I would
like to get some early feedback on the approach first.
---
include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
security/integrity/evm/evm_crypto.c | 2 +-
security/integrity/evm/evm_main.c | 2 +-
security/integrity/ima/ima_api.c | 21 +++++++++++++++------
security/integrity/ima/ima_main.c | 17 ++++++++++-------
5 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
/* An inode's attributes for detection of changes */
struct integrity_inode_attributes {
+ u64 ctime_guard;
u64 version; /* track inode changes */
unsigned long ino;
dev_t dev;
};
+static inline u64 integrity_ctime_guard(struct kstat stat)
+{
+ return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
+}
+
/*
* On stacked filesystems the i_version alone is not enough to detect file data
* or metadata change. Additional metadata is required.
*/
static inline void
integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
- u64 i_version, const struct inode *inode)
+ u64 i_version, u64 ctime_guard,
+ const struct inode *inode)
{
+ attrs->ctime_guard = ctime_guard;
attrs->version = i_version;
attrs->dev = inode->i_sb->s_dev;
attrs->ino = inode->i_ino;
@@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
*/
static inline bool
integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
- const struct inode *inode)
+ struct file *file, struct inode *inode)
{
- return (inode->i_sb->s_dev != attrs->dev ||
- inode->i_ino != attrs->ino ||
- !inode_eq_iversion(inode, attrs->version));
+ struct kstat stat;
+
+ if (inode->i_sb->s_dev != attrs->dev ||
+ inode->i_ino != attrs->ino)
+ return true;
+
+ if (inode_eq_iversion(inode, attrs->version))
+ return false;
+
+ if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT))
+ return true;
+
+ return attrs->ctime_guard != integrity_ctime_guard(stat);
}
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (IS_I_VERSION(inode))
i_version = inode_query_iversion(inode);
integrity_inode_attrs_store(&iint->metadata_inode, i_version,
- inode);
+ 0, inode);
}
/* Portable EVM signatures must include an IMA hash */
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..0712802628fd6533383f9855687e19bef7b771c7 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
if (iint) {
ret = (!IS_I_VERSION(metadata_inode) ||
integrity_inode_attrs_changed(&iint->metadata_inode,
- metadata_inode));
+ NULL, metadata_inode));
if (ret)
iint->evm_status = INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..72bba8daa588a0f4e45e4249276edb54ca3d77ef 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
int length;
void *tmpbuf;
u64 i_version = 0;
+ u64 ctime_guard = 0;
/*
* Always collect the modsig, because IMA might have already collected
@@ -272,10 +273,16 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+
+ if (stat.result_mask & STATX_CTIME)
+ ctime_guard = integrity_ctime_guard(stat);
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
@@ -305,11 +312,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
iint->ima_hash = tmpbuf;
memcpy(iint->ima_hash, &hash, length);
- if (real_inode == inode)
+ if (real_inode == inode) {
iint->real_inode.version = i_version;
- else
+ iint->real_inode.ctime_guard = ctime_guard;
+ } else {
integrity_inode_attrs_store(&iint->real_inode, i_version,
- real_inode);
+ ctime_guard, real_inode);
+ }
/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
if (!result)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -22,6 +22,7 @@
#include <linux/mount.h>
#include <linux/mman.h>
#include <linux/slab.h>
+#include <linux/stat.h>
#include <linux/xattr.h>
#include <linux/ima.h>
#include <linux/fs.h>
@@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
{
fmode_t mode = file->f_mode;
bool update;
+ int ret;
if (!(mode & FMODE_WRITE))
return;
@@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
update = test_and_clear_bit(IMA_UPDATE_XATTR,
&iint->atomic_flags);
- if ((iint->flags & IMA_NEW_FILE) ||
- vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
- AT_STATX_SYNC_AS_STAT) ||
- !(stat.result_mask & STATX_CHANGE_COOKIE) ||
- stat.change_cookie != iint->real_inode.version) {
+ ret = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT);
+ if ((iint->flags & IMA_NEW_FILE) || ret ||
+ (!ret && stat.change_cookie != iint->real_inode.version) ||
+ (!ret && integrity_ctime_guard(stat) !=
+ iint->real_inode.ctime_guard)) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
@@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
(action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
if (!IS_I_VERSION(real_inode) ||
integrity_inode_attrs_changed(&iint->real_inode,
- real_inode)) {
+ file, real_inode)) {
iint->flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@cloudflare.com>
^ permalink raw reply related
* Re: [PATCH v4] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: kernel test robot @ 2025-12-29 18:35 UTC (permalink / raw)
To: Ryan Foster, linux-security-module
Cc: oe-kbuild-all, linux-kernel, paul, serge, Ryan Foster
In-Reply-To: <20251228194838.32214-2-foster.ryan.r@gmail.com>
Hi Ryan,
kernel test robot noticed the following build errors:
[auto build test ERROR on pcmoore-selinux/next]
[also build test ERROR on linus/master v6.19-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ryan-Foster/security-Add-KUnit-tests-for-kuid_root_in_ns-and-vfsuid_root_in_currentns/20251229-035056
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link: https://lore.kernel.org/r/20251228194838.32214-2-foster.ryan.r%40gmail.com
patch subject: [PATCH v4] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20251229/202512292252.1PPODroH-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251229/202512292252.1PPODroH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512292252.1PPODroH-lkp@intel.com/
All errors (new ones prefixed by >>):
nios2-linux-ld: security/commoncap.o: in function `create_test_user_ns_with_mapping':
commoncap.c:(.text+0xa3c): undefined reference to `kunit_kmalloc_array'
>> commoncap.c:(.text+0xa3c): relocation truncated to fit: R_NIOS2_CALL26 against `kunit_kmalloc_array'
nios2-linux-ld: security/commoncap.o: in function `test_kuid_root_in_ns_init_ns_uid0':
>> commoncap.c:(.text+0x1020): undefined reference to `kunit_unary_assert_format'
>> nios2-linux-ld: commoncap.c:(.text+0x102c): undefined reference to `kunit_unary_assert_format'
>> nios2-linux-ld: commoncap.c:(.text+0x1058): undefined reference to `__kunit_do_failed_assertion'
>> commoncap.c:(.text+0x1058): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
nios2-linux-ld: security/commoncap.o: in function `test_kuid_root_in_ns_init_ns_nonzero':
commoncap.c:(.text+0x10bc): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x10c4): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x10f0): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x10f0): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
nios2-linux-ld: security/commoncap.o: in function `test_vfsuid_root_in_currentns_nonzero':
commoncap.c:(.text+0x14c8): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x14d0): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x14fc): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x14fc): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
nios2-linux-ld: security/commoncap.o: in function `test_vfsuid_root_in_currentns_init_ns':
commoncap.c:(.text+0x1594): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x15a0): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x15cc): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x15cc): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
nios2-linux-ld: security/commoncap.o: in function `test_kuid_root_in_ns_with_mapping':
commoncap.c:(.text+0x170c): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1710): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1740): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x1740): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1748): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x174c): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x177c): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x177c): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1784): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x178c): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x17bc): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x17bc): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x17c4): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x17cc): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x17fc): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x17fc): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
>> nios2-linux-ld: commoncap.c:(.text+0x1820): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1824): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1854): undefined reference to `__kunit_do_failed_assertion'
commoncap.c:(.text+0x1854): relocation truncated to fit: R_NIOS2_CALL26 against `__kunit_do_failed_assertion'
>> nios2-linux-ld: commoncap.c:(.text+0x185c): undefined reference to `__kunit_abort'
commoncap.c:(.text+0x185c): additional relocation overflows omitted from the output
nios2-linux-ld: security/commoncap.o: in function `test_kuid_root_in_ns_with_different_mappings':
commoncap.c:(.text+0x1aec): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1af0): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1b20): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1b28): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1b30): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1b60): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1b68): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1b70): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1ba0): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1ba8): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1bb0): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1be0): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1be8): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1bec): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1c1c): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1c24): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1c2c): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1c5c): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1c64): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1c6c): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1c9c): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1ca4): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1ca8): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1cd8): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1ce0): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1ce8): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1d18): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1d20): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1d28): undefined reference to `kunit_unary_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1d58): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1d60): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1d64): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1d94): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1d9c): undefined reference to `__kunit_abort'
nios2-linux-ld: commoncap.c:(.text+0x1dbc): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1dc0): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1df0): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1df8): undefined reference to `__kunit_abort'
nios2-linux-ld: commoncap.c:(.text+0x1e50): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1e54): undefined reference to `kunit_ptr_not_err_assert_format'
nios2-linux-ld: commoncap.c:(.text+0x1e84): undefined reference to `__kunit_do_failed_assertion'
nios2-linux-ld: commoncap.c:(.text+0x1e8c): undefined reference to `__kunit_abort'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] ipv6: BUG() in pskb_expand_head() as part of calipso_skbuff_setattr()
From: patchwork-bot+netdevbpf @ 2025-12-29 18:40 UTC (permalink / raw)
To: Will Rosenberg
Cc: security, paul, davem, dsahern, edumazet, kuba, pabeni, horms,
huw, netdev, linux-security-module, linux-kernel
In-Reply-To: <20251219173637.797418-1-whrosenb@asu.edu>
Hello:
This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Fri, 19 Dec 2025 10:36:37 -0700 you wrote:
> There exists a kernel oops caused by a BUG_ON(nhead < 0) at
> net/core/skbuff.c:2232 in pskb_expand_head().
> This bug is triggered as part of the calipso_skbuff_setattr()
> routine when skb_cow() is passed headroom > INT_MAX
> (i.e. (int)(skb_headroom(skb) + len_delta) < 0).
>
> The root cause of the bug is due to an implicit integer cast in
> __skb_cow(). The check (headroom > skb_headroom(skb)) is meant to ensure
> that delta = headroom - skb_headroom(skb) is never negative, otherwise
> we will trigger a BUG_ON in pskb_expand_head(). However, if
> headroom > INT_MAX and delta <= -NET_SKB_PAD, the check passes, delta
> becomes negative, and pskb_expand_head() is passed a negative value for
> nhead.
>
> [...]
Here is the summary with links:
- ipv6: BUG() in pskb_expand_head() as part of calipso_skbuff_setattr()
https://git.kernel.org/netdev/net/c/58fc7342b529
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v3] ima: Fix stack-out-of-bounds in is_bprm_creds_for_exec()
From: Mimi Zohar @ 2025-12-29 21:13 UTC (permalink / raw)
To: Chris J Arges, roberto.sassu
Cc: kernel-team, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Mickaël Salaün,
Kees Cook, linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20251228031914.47618-1-carges@cloudflare.com>
On Sat, 2025-12-27 at 21:18 -0600, Chris J Arges wrote:
> KASAN reported a stack-out-of-bounds access in ima_appraise_measurement
> from is_bprm_creds_for_exec:
>
> BUG: KASAN: stack-out-of-bounds in ima_appraise_measurement+0x12dc/0x16a0
> Read of size 1 at addr ffffc9000160f940 by task sudo/550
> The buggy address belongs to stack of task sudo/550
> and is located at offset 24 in frame:
> ima_appraise_measurement+0x0/0x16a0
> This frame has 2 objects:
> [48, 56) 'file'
> [80, 148) 'hash'
>
> This is caused by using container_of on the *file pointer. This offset
> calculation is what triggers the stack-out-of-bounds error.
>
> In order to fix this, pass in a bprm_is_check boolean which can be set
> depending on how process_measurement is called. If the caller has a
> linux_binprm pointer and the function is BPRM_CHECK we can determine
> is_check and set it then. Otherwise set it to false.
>
> Fixes: 95b3cdafd7cb7 ("ima: instantiate the bprm_creds_for_exec() hook")
>
> Signed-off-by: Chris J Arges <carges@cloudflare.com>
Thanks, Chris. The patch is now queued in the linux-integrity/next-integrity
branch.
Mimi
^ permalink raw reply
* [RFC PATCH 0/2] landlock: Refactor layer masks
From: Günther Noack @ 2025-12-30 10:39 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, Günther Noack
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 -8.3%. The benchmark results are only tentative and have
been produced in Qemu:
With the patch, the benchmark runs in 5932 clocks (measured with
times(3)):
*** Benchmark ***
10000 dirs, 100000 iterations, with landlock
*** Benchmark concluded ***
System: 5932 clocks
User : 1 clocks
Clocks per second: 1000000
Without the patch, we get 6472 clocks, which is 9.1% more.
*** Benchmark ***
10000 dirs, 100000 iterations, with landlock
*** Benchmark concluded ***
System: 6472 clocks
User : 1 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.
Günther Noack (2):
landlock: access_mask_subset() helper
landlock: transpose the layer masks data structure
security/landlock/access.h | 10 +-
security/landlock/audit.c | 155 ++++++----------
security/landlock/audit.h | 3 +-
security/landlock/domain.c | 120 +++---------
security/landlock/domain.h | 6 +-
security/landlock/fs.c | 361 +++++++++++++++++-------------------
security/landlock/net.c | 10 +-
security/landlock/ruleset.c | 78 ++------
security/landlock/ruleset.h | 18 +-
9 files changed, 300 insertions(+), 461 deletions(-)
--
2.52.0
^ permalink raw reply
* [RFC PATCH 1/2] landlock: access_mask_subset() helper
From: Günther Noack @ 2025-12-30 10:39 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, Günther Noack
In-Reply-To: <20251230103917.10549-3-gnoack3000@gmail.com>
This helper function checks whether an access_mask_t has a subset of the
bits enabled than another one. This expresses the intent a bit smoother
in the code and does not cost us anything when it gets inlined.
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
security/landlock/fs.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index fe794875ad461..b4ce03bef4b8e 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -398,6 +398,15 @@ static const struct access_masks any_fs = {
.fs = ~0,
};
+/*
+ * Returns true iff a has a subset of the bits of b.
+ * It helps readability and gets inlined.
+ */
+static bool access_mask_subset(access_mask_t a, access_mask_t b)
+{
+ return (a | b) == b;
+}
+
/*
* Check that a destination file hierarchy has more restrictions than a source
* file hierarchy. This is only used for link and rename actions.
@@ -1696,7 +1705,7 @@ static int hook_file_open(struct file *const file)
ARRAY_SIZE(layer_masks));
#endif /* CONFIG_AUDIT */
- if ((open_access_request & allowed_access) == open_access_request)
+ if (access_mask_subset(open_access_request, allowed_access))
return 0;
/* Sets access to reflect the actual request. */
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Günther Noack @ 2025-12-30 10:39 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, Günther Noack
In-Reply-To: <20251230103917.10549-3-gnoack3000@gmail.com>
The layer masks data structure tracks the requested but unfulfilled
access rights during an operations 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)
* Code is ~160 lines smaller.
Other noteworthy changes:
* Clarify deny_mask_t and the code assembling it.
* Document what that value looks like
* Make writing and reading functions specific to file system rules.
(It only worked for FS rules before as well, but going all the way
simplifies the code logic more.)
* 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.
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
security/landlock/access.h | 10 +-
security/landlock/audit.c | 155 ++++++----------
security/landlock/audit.h | 3 +-
security/landlock/domain.c | 120 +++----------
security/landlock/domain.h | 6 +-
security/landlock/fs.c | 350 ++++++++++++++++--------------------
security/landlock/net.c | 10 +-
security/landlock/ruleset.c | 78 +++-----
security/landlock/ruleset.h | 18 +-
9 files changed, 290 insertions(+), 460 deletions(-)
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 7961c6630a2d7..aa0efa36a37db 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -61,14 +61,14 @@ union access_masks_all {
static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
sizeof(typeof_member(union access_masks_all, all)));
-typedef u16 layer_mask_t;
-
-/* Makes sure all layers can be checked. */
-static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
-
/*
* Tracks domains responsible of a denied access. This is required to avoid
* storing in each object the full layer_masks[] required by update_request().
+ *
+ * Each nibble represents the layer index of the newest layer which denied a
+ * certain access right. For file system access rights, the upper four bits are
+ * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
+ * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
*/
typedef u8 deny_masks_t;
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index c52d079cdb77b..650bd7f5cb6be 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -182,36 +182,18 @@ static void test_get_hierarchy(struct kunit *const test)
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 access_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 = LANDLOCK_MAX_NUM_LAYERS - 1; i >= 0; i--) {
+ if (masks->access[i] & *access_request) {
+ *access_request &= masks->access[i];
+ return i;
}
}
- *access_request = missing;
- if (youngest_layer == -1)
- return domain->num_layers - 1;
-
- return youngest_layer;
+ /* Not found - fall back to default values */
+ *access_request = 0;
+ return domain->num_layers - 1;
}
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
@@ -221,94 +203,82 @@ static void test_get_denied_layer(struct kunit *const test)
const struct landlock_ruleset dom = {
.num_layers = 5,
};
- const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
+ const struct layer_access_masks masks = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_READ_DIR,
+ .access[1] = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_READ_DIR,
+ .access[2] = LANDLOCK_ACCESS_FS_REMOVE_DIR,
};
access_mask_t access;
access = LANDLOCK_ACCESS_FS_EXECUTE;
- KUNIT_EXPECT_EQ(test, 0,
- get_denied_layer(&dom, &access, &layer_masks,
- sizeof(layer_masks)));
+ KUNIT_EXPECT_EQ(test, 0, get_denied_layer(&dom, &access, &masks));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
access = LANDLOCK_ACCESS_FS_READ_FILE;
- KUNIT_EXPECT_EQ(test, 1,
- get_denied_layer(&dom, &access, &layer_masks,
- sizeof(layer_masks)));
+ KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
access = LANDLOCK_ACCESS_FS_READ_DIR;
- KUNIT_EXPECT_EQ(test, 1,
- get_denied_layer(&dom, &access, &layer_masks,
- sizeof(layer_masks)));
+ KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
- KUNIT_EXPECT_EQ(test, 1,
- get_denied_layer(&dom, &access, &layer_masks,
- sizeof(layer_masks)));
+ KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
KUNIT_EXPECT_EQ(test, access,
LANDLOCK_ACCESS_FS_READ_FILE |
LANDLOCK_ACCESS_FS_READ_DIR);
access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
- KUNIT_EXPECT_EQ(test, 1,
- get_denied_layer(&dom, &access, &layer_masks,
- sizeof(layer_masks)));
+ KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
access = LANDLOCK_ACCESS_FS_WRITE_FILE;
- KUNIT_EXPECT_EQ(test, 4,
- get_denied_layer(&dom, &access, &layer_masks,
- sizeof(layer_masks)));
+ KUNIT_EXPECT_EQ(test, 4, get_denied_layer(&dom, &access, &masks));
KUNIT_EXPECT_EQ(test, access, 0);
}
#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
-static size_t
-get_layer_from_deny_masks(access_mask_t *const access_request,
- const access_mask_t all_existing_optional_access,
- const deny_masks_t deny_masks)
+/*
+ * get_layer_from_fs_deny_masks - get the layer which denied the access request
+ *
+ * As a side effect, stores the denied access rights from that layer(!) in
+ * *access_request.
+ */
+static size_t get_layer_from_fs_deny_masks(access_mask_t *const access_request,
+ const deny_masks_t deny_masks)
{
- const unsigned long access_opt = all_existing_optional_access;
- const unsigned long access_req = *access_request;
- access_mask_t missing = 0;
+ const access_mask_t access_req = *access_request;
size_t youngest_layer = 0;
- size_t access_index = 0;
- unsigned long access_bit;
+ access_mask_t missing = 0;
- /* This will require change with new object types. */
- WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
+ WARN_ON_ONCE((access_req | _LANDLOCK_ACCESS_FS_OPTIONAL) !=
+ _LANDLOCK_ACCESS_FS_OPTIONAL);
- for_each_set_bit(access_bit, &access_opt,
- BITS_PER_TYPE(access_mask_t)) {
- if (access_req & BIT(access_bit)) {
- const size_t layer =
- (deny_masks >> (access_index * 4)) &
- (LANDLOCK_MAX_NUM_LAYERS - 1);
+ if (access_req & LANDLOCK_ACCESS_FS_TRUNCATE) {
+ size_t layer = deny_masks & 0x0f;
- if (layer > youngest_layer) {
- youngest_layer = layer;
- missing = BIT(access_bit);
- } else if (layer == youngest_layer) {
- missing |= BIT(access_bit);
- }
- }
- access_index++;
+ missing |= LANDLOCK_ACCESS_FS_TRUNCATE;
+ youngest_layer = max(youngest_layer, layer);
}
+ if (access_req & LANDLOCK_ACCESS_FS_IOCTL_DEV) {
+ size_t layer = (deny_masks & 0xf0) >> 4;
+ if (layer > youngest_layer)
+ missing = 0;
+
+ missing |= LANDLOCK_ACCESS_FS_IOCTL_DEV;
+ youngest_layer = max(youngest_layer, layer);
+ }
*access_request = missing;
return youngest_layer;
}
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
-static void test_get_layer_from_deny_masks(struct kunit *const test)
+static void test_get_layer_from_fs_deny_masks(struct kunit *const test)
{
deny_masks_t deny_mask;
access_mask_t access;
@@ -318,16 +288,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
access = LANDLOCK_ACCESS_FS_TRUNCATE;
KUNIT_EXPECT_EQ(test, 0,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
KUNIT_EXPECT_EQ(test, 2,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
/* truncate:15 ioctl_dev:15 */
@@ -335,16 +301,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
access = LANDLOCK_ACCESS_FS_TRUNCATE;
KUNIT_EXPECT_EQ(test, 15,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
KUNIT_EXPECT_EQ(test, 15,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access,
LANDLOCK_ACCESS_FS_TRUNCATE |
LANDLOCK_ACCESS_FS_IOCTL_DEV);
@@ -361,18 +323,15 @@ static bool is_valid_request(const struct landlock_request *const request)
return false;
if (request->access) {
- if (WARN_ON_ONCE(!(!!request->layer_masks ^
+ if (WARN_ON_ONCE(!(!!request->masks ^
!!request->all_existing_optional_access)))
return false;
} else {
- if (WARN_ON_ONCE(request->layer_masks ||
+ if (WARN_ON_ONCE(request->masks ||
request->all_existing_optional_access))
return false;
}
- if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
- return false;
-
if (request->deny_masks) {
if (WARN_ON_ONCE(!request->all_existing_optional_access))
return false;
@@ -405,14 +364,12 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
missing = request->access;
if (missing) {
/* Gets the nearest domain that denies the request. */
- if (request->layer_masks) {
+ if (request->masks) {
youngest_layer = get_denied_layer(
- subject->domain, &missing, request->layer_masks,
- request->layer_masks_size);
+ subject->domain, &missing, request->masks);
} else {
- youngest_layer = get_layer_from_deny_masks(
- &missing, request->all_existing_optional_access,
- request->deny_masks);
+ youngest_layer = get_layer_from_fs_deny_masks(
+ &missing, request->deny_masks);
}
youngest_denied =
get_hierarchy(subject->domain, youngest_layer);
@@ -507,7 +464,7 @@ static struct kunit_case test_cases[] = {
/* clang-format off */
KUNIT_CASE(test_get_hierarchy),
KUNIT_CASE(test_get_denied_layer),
- KUNIT_CASE(test_get_layer_from_deny_masks),
+ KUNIT_CASE(test_get_layer_from_fs_deny_masks),
{}
/* clang-format on */
};
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d80..104472060ef5e 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -43,8 +43,7 @@ struct landlock_request {
access_mask_t access;
/* Required fields for requests with layer masks. */
- const layer_mask_t (*layer_masks)[];
- size_t layer_masks_size;
+ const struct layer_access_masks *masks;
/* Required fields for requests with deny masks. */
const access_mask_t all_existing_optional_access;
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index a647b68e8d060..e8e4ae5d075fe 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -23,6 +23,7 @@
#include "common.h"
#include "domain.h"
#include "id.h"
+#include "limits.h"
#ifdef CONFIG_AUDIT
@@ -133,111 +134,47 @@ int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy)
return 0;
}
-static deny_masks_t
-get_layer_deny_mask(const access_mask_t all_existing_optional_access,
- const unsigned long access_bit, const size_t layer)
-{
- unsigned long access_weight;
-
- /* This may require change with new object types. */
- WARN_ON_ONCE(all_existing_optional_access !=
- _LANDLOCK_ACCESS_FS_OPTIONAL);
-
- if (WARN_ON_ONCE(layer >= LANDLOCK_MAX_NUM_LAYERS))
- return 0;
-
- access_weight = hweight_long(all_existing_optional_access &
- GENMASK(access_bit, 0));
- if (WARN_ON_ONCE(access_weight < 1))
- return 0;
-
- return layer
- << ((access_weight - 1) * HWEIGHT(LANDLOCK_MAX_NUM_LAYERS - 1));
-}
-
-#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
-
-static void test_get_layer_deny_mask(struct kunit *const test)
-{
- const unsigned long truncate = BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE);
- const unsigned long ioctl_dev = BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV);
-
- KUNIT_EXPECT_EQ(test, 0,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- truncate, 0));
- KUNIT_EXPECT_EQ(test, 0x3,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- truncate, 3));
-
- KUNIT_EXPECT_EQ(test, 0,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- ioctl_dev, 0));
- KUNIT_EXPECT_EQ(test, 0xf0,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- ioctl_dev, 15));
-}
-
-#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
-
deny_masks_t
-landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
- const access_mask_t optional_access,
- const layer_mask_t (*const layer_masks)[],
- const size_t layer_masks_size)
+landlock_get_fs_deny_masks(const access_mask_t optional_access,
+ const struct layer_access_masks *layer_masks)
{
- const unsigned long access_opt = optional_access;
- unsigned long access_bit;
- deny_masks_t deny_masks = 0;
+ u8 truncate_layer = 0;
+ u8 ioctl_dev_layer = 0;
- /* This may require change with new object types. */
- WARN_ON_ONCE(access_opt !=
- (optional_access & all_existing_optional_access));
-
- if (WARN_ON_ONCE(!layer_masks))
- return 0;
-
- if (WARN_ON_ONCE(!access_opt))
- return 0;
-
- for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
- const layer_mask_t mask = (*layer_masks)[access_bit];
-
- if (!mask)
- continue;
-
- /* __fls(1) == 0 */
- deny_masks |= get_layer_deny_mask(all_existing_optional_access,
- access_bit, __fls(mask));
+ for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ if (layer_masks->access[i] & optional_access &
+ LANDLOCK_ACCESS_FS_TRUNCATE)
+ truncate_layer = i;
+ if (layer_masks->access[i] & optional_access &
+ LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ ioctl_dev_layer = i;
}
- return deny_masks;
+ return ((ioctl_dev_layer << 4) & 0xf0) | (truncate_layer & 0x0f);
}
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
-static void test_landlock_get_deny_masks(struct kunit *const test)
+static void test_landlock_get_fs_deny_masks(struct kunit *const test)
{
- const layer_mask_t layers1[BITS_PER_TYPE(access_mask_t)] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
- BIT_ULL(9),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = BIT_ULL(1),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = BIT_ULL(2) |
- BIT_ULL(0),
+ const struct layer_access_masks layers1 = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ .access[1] = LANDLOCK_ACCESS_FS_TRUNCATE,
+ .access[2] = LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ .access[9] = LANDLOCK_ACCESS_FS_EXECUTE,
};
KUNIT_EXPECT_EQ(test, 0x1,
- landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
- LANDLOCK_ACCESS_FS_TRUNCATE,
- &layers1, ARRAY_SIZE(layers1)));
+ landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE,
+ &layers1));
KUNIT_EXPECT_EQ(test, 0x20,
- landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
- LANDLOCK_ACCESS_FS_IOCTL_DEV,
- &layers1, ARRAY_SIZE(layers1)));
+ landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ &layers1));
KUNIT_EXPECT_EQ(
test, 0x21,
- landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
- LANDLOCK_ACCESS_FS_TRUNCATE |
- LANDLOCK_ACCESS_FS_IOCTL_DEV,
- &layers1, ARRAY_SIZE(layers1)));
+ landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE |
+ LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ &layers1));
}
#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
@@ -246,8 +183,7 @@ static void test_landlock_get_deny_masks(struct kunit *const test)
static struct kunit_case test_cases[] = {
/* clang-format off */
- KUNIT_CASE(test_get_layer_deny_mask),
- KUNIT_CASE(test_landlock_get_deny_masks),
+ KUNIT_CASE(test_landlock_get_fs_deny_masks),
{}
/* clang-format on */
};
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 7fb70b25f85a1..39600acb63897 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -120,10 +120,8 @@ struct landlock_hierarchy {
#ifdef CONFIG_AUDIT
deny_masks_t
-landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
- const access_mask_t optional_access,
- const layer_mask_t (*const layer_masks)[],
- size_t layer_masks_size);
+landlock_get_fs_deny_masks(const access_mask_t optional_access,
+ const struct layer_access_masks *layer_masks);
int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index b4ce03bef4b8e..1e765d22d8d49 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -407,57 +407,55 @@ static bool access_mask_subset(access_mask_t a, access_mask_t b)
return (a | b) == b;
}
+/*
+ * Returns true iff the child file with the given src_child access rights under
+ * src_parent would result in having the same or fewer access rights if it were
+ * moved under new_parent.
+ */
+static bool may_refer(const struct layer_access_masks *const src_parent,
+ const struct layer_access_masks *const src_child,
+ const struct layer_access_masks *const new_parent,
+ const bool child_is_dir)
+{
+ for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ access_mask_t child_access = src_parent->access[i] &
+ src_child->access[i];
+ access_mask_t parent_access = new_parent->access[i];
+
+ if (!child_is_dir) {
+ child_access &= ACCESS_FILE;
+ parent_access &= ACCESS_FILE;
+ }
+
+ if (!access_mask_subset(child_access, parent_access))
+ return false;
+ }
+ return true;
+}
+
/*
* Check that a destination file hierarchy has more restrictions than a source
* file hierarchy. This is only used for link and rename actions.
*
- * @layer_masks_child2: Optional child masks.
+ * Returns: true if child1 may be moved from parent1 to parent2 without
+ * increasing its access rights. If child2 is set, an additional condition is
+ * that child2 may be used from parent2 to parent1 without increasing its access
+ * rights.
*/
-static bool no_more_access(
- const layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
- const layer_mask_t (*const layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS],
- const bool child1_is_directory,
- const layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
- const layer_mask_t (*const layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS],
- const bool child2_is_directory)
+static bool no_more_access(const struct layer_access_masks *const parent1,
+ const struct layer_access_masks *const child1,
+ const bool child1_is_dir,
+ const struct layer_access_masks *const parent2,
+ const struct layer_access_masks *const child2,
+ const bool child2_is_dir)
{
- unsigned long access_bit;
+ if (!may_refer(parent1, child1, parent2, child1_is_dir))
+ return false;
- for (access_bit = 0; access_bit < ARRAY_SIZE(*layer_masks_parent2);
- access_bit++) {
- /* Ignores accesses that only make sense for directories. */
- const bool is_file_access =
- !!(BIT_ULL(access_bit) & ACCESS_FILE);
+ if (!child2)
+ return true;
- if (child1_is_directory || is_file_access) {
- /*
- * Checks if the destination restrictions are a
- * superset of the source ones (i.e. inherited access
- * rights without child exceptions):
- * restrictions(parent2) >= restrictions(child1)
- */
- if ((((*layer_masks_parent1)[access_bit] &
- (*layer_masks_child1)[access_bit]) |
- (*layer_masks_parent2)[access_bit]) !=
- (*layer_masks_parent2)[access_bit])
- return false;
- }
-
- if (!layer_masks_child2)
- continue;
- if (child2_is_directory || is_file_access) {
- /*
- * Checks inverted restrictions for RENAME_EXCHANGE:
- * restrictions(parent1) >= restrictions(child2)
- */
- if ((((*layer_masks_parent2)[access_bit] &
- (*layer_masks_child2)[access_bit]) |
- (*layer_masks_parent1)[access_bit]) !=
- (*layer_masks_parent1)[access_bit])
- return false;
- }
- }
- return true;
+ return may_refer(parent2, child2, parent1, child2_is_dir);
}
#define NMA_TRUE(...) KUNIT_EXPECT_TRUE(test, no_more_access(__VA_ARGS__))
@@ -467,25 +465,25 @@ static bool no_more_access(
static void test_no_more_access(struct kunit *const test)
{
- const layer_mask_t rx0[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT_ULL(0),
+ const struct layer_access_masks rx0 = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_READ_FILE,
};
- const layer_mask_t mx0[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = BIT_ULL(0),
+ const struct layer_access_masks mx0 = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
};
- const layer_mask_t x0[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
+ const struct layer_access_masks x0 = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
};
- const layer_mask_t x1[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(1),
+ const struct layer_access_masks x1 = {
+ .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
};
- const layer_mask_t x01[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
- BIT_ULL(1),
+ const struct layer_access_masks x01 = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
+ .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
};
- const layer_mask_t allows_all[LANDLOCK_NUM_ACCESS_FS] = {};
+ const struct layer_access_masks allows_all = {};
/* Checks without restriction. */
NMA_TRUE(&x0, &allows_all, false, &allows_all, NULL, false);
@@ -573,31 +571,30 @@ static void test_no_more_access(struct kunit *const test)
#undef NMA_TRUE
#undef NMA_FALSE
-static bool is_layer_masks_allowed(
- layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
+static bool is_layer_masks_allowed(const struct layer_access_masks *masks)
{
- return !memchr_inv(layer_masks, 0, sizeof(*layer_masks));
+ return !memchr_inv(&masks->access, 0, sizeof(masks->access));
}
/*
- * Removes @layer_masks accesses that are not requested.
+ * Removes @masks accesses that are not requested.
*
* Returns true if the request is allowed, false otherwise.
*/
-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 (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ masks->access[i] &= access_request;
+ if (masks->access[i])
+ saw_unfulfilled_access = true;
+ }
+ return !saw_unfulfilled_access;
}
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
@@ -605,48 +602,41 @@ scope_to_request(const access_mask_t access_request,
static void test_scope_to_request_with_exec_none(struct kunit *const test)
{
/* Allows everything. */
- layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+ struct layer_access_masks masks = {};
/* Checks and scopes with execute. */
- KUNIT_EXPECT_TRUE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
- &layer_masks));
- KUNIT_EXPECT_EQ(test, 0,
- layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
- KUNIT_EXPECT_EQ(test, 0,
- layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
+ KUNIT_EXPECT_TRUE(test,
+ scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE, &masks));
+ KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
}
static void test_scope_to_request_with_exec_some(struct kunit *const test)
{
/* Denies execute and write. */
- layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
+ struct layer_access_masks masks = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
+ .access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
};
/* Checks and scopes with execute. */
KUNIT_EXPECT_FALSE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
- &layer_masks));
- KUNIT_EXPECT_EQ(test, BIT_ULL(0),
- layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
- KUNIT_EXPECT_EQ(test, 0,
- layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
+ &masks));
+ KUNIT_EXPECT_EQ(test, LANDLOCK_ACCESS_FS_EXECUTE, masks.access[0]);
+ KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
}
static void test_scope_to_request_without_access(struct kunit *const test)
{
/* Denies execute and write. */
- layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
- [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
+ struct layer_access_masks masks = {
+ .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
+ .access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
};
/* Checks and scopes without access request. */
- KUNIT_EXPECT_TRUE(test, scope_to_request(0, &layer_masks));
- KUNIT_EXPECT_EQ(test, 0,
- layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
- KUNIT_EXPECT_EQ(test, 0,
- layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
+ KUNIT_EXPECT_TRUE(test, scope_to_request(0, &masks));
+ KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
+ KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
}
#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
@@ -655,20 +645,16 @@ static void test_scope_to_request_without_access(struct kunit *const test)
* Returns true if there is at least one access right different than
* LANDLOCK_ACCESS_FS_REFER.
*/
-static bool
-is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
- const access_mask_t access_request)
+static bool is_eacces(const struct layer_access_masks *masks,
+ const access_mask_t access_request)
{
- unsigned long access_bit;
- /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
- const unsigned long access_check = access_request &
- ~LANDLOCK_ACCESS_FS_REFER;
-
- if (!layer_masks)
+ if (!masks)
return false;
- for_each_set_bit(access_bit, &access_check, ARRAY_SIZE(*layer_masks)) {
- if ((*layer_masks)[access_bit])
+ for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
+ if (masks->access[i] & access_request &
+ ~LANDLOCK_ACCESS_FS_REFER)
return true;
}
return false;
@@ -681,37 +667,37 @@ is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
static void test_is_eacces_with_none(struct kunit *const test)
{
- const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+ const struct layer_access_masks masks = {};
- IE_FALSE(&layer_masks, 0);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
+ IE_FALSE(&masks, 0);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
}
static void test_is_eacces_with_refer(struct kunit *const test)
{
- const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = BIT_ULL(0),
+ const struct layer_access_masks masks = {
+ .access[0] = LANDLOCK_ACCESS_FS_REFER,
};
- IE_FALSE(&layer_masks, 0);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
+ IE_FALSE(&masks, 0);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
}
static void test_is_eacces_with_write(struct kunit *const test)
{
- const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
- [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(0),
+ const struct layer_access_masks masks = {
+ .access[0] = LANDLOCK_ACCESS_FS_WRITE_FILE,
};
- IE_FALSE(&layer_masks, 0);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
- IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
+ IE_FALSE(&masks, 0);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
+ IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
- IE_TRUE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
+ IE_TRUE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
}
#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
@@ -761,26 +747,25 @@ static void test_is_eacces_with_write(struct kunit *const test)
* - true if the access request is granted;
* - false otherwise.
*/
-static bool is_access_to_paths_allowed(
- const struct landlock_ruleset *const domain,
- const struct path *const path,
- const access_mask_t access_request_parent1,
- layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
- struct landlock_request *const log_request_parent1,
- struct dentry *const dentry_child1,
- const access_mask_t access_request_parent2,
- layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
- struct landlock_request *const log_request_parent2,
- struct dentry *const dentry_child2)
+static bool
+is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
+ const struct path *const path,
+ const access_mask_t access_request_parent1,
+ struct layer_access_masks *layer_masks_parent1,
+ struct landlock_request *const log_request_parent1,
+ struct dentry *const dentry_child1,
+ const access_mask_t access_request_parent2,
+ struct layer_access_masks *layer_masks_parent2,
+ struct landlock_request *const log_request_parent2,
+ struct dentry *const dentry_child2)
{
bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
child1_is_directory = true, child2_is_directory = true;
struct path walker_path;
access_mask_t access_masked_parent1, access_masked_parent2;
- layer_mask_t _layer_masks_child1[LANDLOCK_NUM_ACCESS_FS],
- _layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
- layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
- (*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
+ struct layer_access_masks _layer_masks_child1, _layer_masks_child2;
+ struct layer_access_masks *layer_masks_child1 = NULL,
+ *layer_masks_child2 = NULL;
if (!access_request_parent1 && !access_request_parent2)
return true;
@@ -820,22 +805,20 @@ static bool is_access_to_paths_allowed(
}
if (unlikely(dentry_child1)) {
- landlock_unmask_layers(
- find_rule(domain, dentry_child1),
- landlock_init_layer_masks(
- domain, LANDLOCK_MASK_ACCESS_FS,
- &_layer_masks_child1, LANDLOCK_KEY_INODE),
- &_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1));
+ if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
+ &_layer_masks_child1,
+ LANDLOCK_KEY_INODE))
+ landlock_unmask_layers(find_rule(domain, dentry_child1),
+ &_layer_masks_child1);
layer_masks_child1 = &_layer_masks_child1;
child1_is_directory = d_is_dir(dentry_child1);
}
if (unlikely(dentry_child2)) {
- landlock_unmask_layers(
- find_rule(domain, dentry_child2),
- landlock_init_layer_masks(
- domain, LANDLOCK_MASK_ACCESS_FS,
- &_layer_masks_child2, LANDLOCK_KEY_INODE),
- &_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2));
+ if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
+ &_layer_masks_child2,
+ LANDLOCK_KEY_INODE))
+ landlock_unmask_layers(find_rule(domain, dentry_child2),
+ &_layer_masks_child2);
layer_masks_child2 = &_layer_masks_child2;
child2_is_directory = d_is_dir(dentry_child2);
}
@@ -890,16 +873,12 @@ static bool is_access_to_paths_allowed(
}
rule = find_rule(domain, walker_path.dentry);
- allowed_parent1 = allowed_parent1 ||
- landlock_unmask_layers(
- rule, access_masked_parent1,
- layer_masks_parent1,
- ARRAY_SIZE(*layer_masks_parent1));
- allowed_parent2 = allowed_parent2 ||
- landlock_unmask_layers(
- rule, access_masked_parent2,
- layer_masks_parent2,
- ARRAY_SIZE(*layer_masks_parent2));
+ allowed_parent1 =
+ allowed_parent1 ||
+ landlock_unmask_layers(rule, layer_masks_parent1);
+ allowed_parent2 =
+ allowed_parent2 ||
+ landlock_unmask_layers(rule, layer_masks_parent2);
/* Stops when a rule from each layer grants access. */
if (allowed_parent1 && allowed_parent2)
@@ -953,9 +932,7 @@ static bool is_access_to_paths_allowed(
log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
log_request_parent1->audit.u.path = *path;
log_request_parent1->access = access_masked_parent1;
- log_request_parent1->layer_masks = layer_masks_parent1;
- log_request_parent1->layer_masks_size =
- ARRAY_SIZE(*layer_masks_parent1);
+ log_request_parent1->masks = layer_masks_parent1;
}
if (!allowed_parent2) {
@@ -963,9 +940,7 @@ static bool is_access_to_paths_allowed(
log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
log_request_parent2->audit.u.path = *path;
log_request_parent2->access = access_masked_parent2;
- log_request_parent2->layer_masks = layer_masks_parent2;
- log_request_parent2->layer_masks_size =
- ARRAY_SIZE(*layer_masks_parent2);
+ log_request_parent2->masks = layer_masks_parent2;
}
return allowed_parent1 && allowed_parent2;
}
@@ -978,7 +953,7 @@ static int current_check_access_path(const struct path *const path,
};
const struct landlock_cred_security *const subject =
landlock_get_applicable_subject(current_cred(), masks, NULL);
- layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+ struct layer_access_masks layer_masks;
struct landlock_request request = {};
if (!subject)
@@ -1053,10 +1028,10 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
* - true if all the domain access rights are allowed for @dir;
* - false if the walk reached @mnt_root.
*/
-static bool collect_domain_accesses(
- const struct landlock_ruleset *const domain,
- const struct dentry *const mnt_root, struct dentry *dir,
- layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS])
+static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
+ const struct dentry *const mnt_root,
+ struct dentry *dir,
+ struct layer_access_masks *layer_masks_dom)
{
unsigned long access_dom;
bool ret = false;
@@ -1075,9 +1050,8 @@ static bool collect_domain_accesses(
struct dentry *parent_dentry;
/* Gets all layers allowing all domain accesses. */
- if (landlock_unmask_layers(find_rule(domain, dir), access_dom,
- layer_masks_dom,
- ARRAY_SIZE(*layer_masks_dom))) {
+ if (landlock_unmask_layers(find_rule(domain, dir),
+ layer_masks_dom)) {
/*
* Stops when all handled accesses are allowed by at
* least one rule in each layer.
@@ -1165,8 +1139,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
access_mask_t access_request_parent1, access_request_parent2;
struct path mnt_dir;
struct dentry *old_parent;
- layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
- layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
+ struct layer_access_masks layer_masks_parent1 = {},
+ layer_masks_parent2 = {};
struct landlock_request request1 = {}, request2 = {};
if (!subject)
@@ -1323,7 +1297,8 @@ static void hook_sb_delete(struct super_block *const sb)
* second call to iput() for the same Landlock object. Also
* checks I_NEW because such inode cannot be tied to an object.
*/
- if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) {
+ if (inode_state_read(inode) &
+ (I_FREEING | I_WILL_FREE | I_NEW)) {
spin_unlock(&inode->i_lock);
continue;
}
@@ -1641,7 +1616,7 @@ static bool is_device(const struct file *const file)
static int hook_file_open(struct file *const file)
{
- layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+ struct layer_access_masks layer_masks = {};
access_mask_t open_access_request, full_access_request, allowed_access,
optional_access;
const struct landlock_cred_security *const subject =
@@ -1676,20 +1651,14 @@ static int hook_file_open(struct file *const file)
&layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
allowed_access = full_access_request;
} else {
- unsigned long access_bit;
- const unsigned long access_req = full_access_request;
-
/*
* Calculate the actual allowed access rights from layer_masks.
- * Add each access right to allowed_access which has not been
- * vetoed by any layer.
+ * Remove the access rights from the full access request which
+ * are still unfulfilled in any of the layers.
*/
- allowed_access = 0;
- for_each_set_bit(access_bit, &access_req,
- ARRAY_SIZE(layer_masks)) {
- if (!layer_masks[access_bit])
- allowed_access |= BIT_ULL(access_bit);
- }
+ allowed_access = full_access_request;
+ for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++)
+ allowed_access &= ~layer_masks.access[i];
}
/*
@@ -1700,9 +1669,8 @@ static int hook_file_open(struct file *const file)
*/
landlock_file(file)->allowed_access = allowed_access;
#ifdef CONFIG_AUDIT
- landlock_file(file)->deny_masks = landlock_get_deny_masks(
- _LANDLOCK_ACCESS_FS_OPTIONAL, optional_access, &layer_masks,
- ARRAY_SIZE(layer_masks));
+ landlock_file(file)->deny_masks =
+ landlock_get_fs_deny_masks(optional_access, &layer_masks);
#endif /* CONFIG_AUDIT */
if (access_mask_subset(open_access_request, allowed_access))
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 1f3915a90a808..2a5456f4f017e 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -47,7 +47,7 @@ static int current_check_access_socket(struct socket *const sock,
access_mask_t access_request)
{
__be16 port;
- layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+ struct layer_access_masks layer_masks = {};
const struct landlock_rule *rule;
struct landlock_id id = {
.type = LANDLOCK_KEY_NET_PORT,
@@ -178,8 +178,9 @@ static int current_check_access_socket(struct socket *const sock,
access_request = landlock_init_layer_masks(subject->domain,
access_request, &layer_masks,
LANDLOCK_KEY_NET_PORT);
- if (landlock_unmask_layers(rule, access_request, &layer_masks,
- ARRAY_SIZE(layer_masks)))
+ if (!access_request)
+ return 0;
+ if (landlock_unmask_layers(rule, &layer_masks))
return 0;
audit_net.family = address->sa_family;
@@ -189,8 +190,7 @@ static int current_check_access_socket(struct socket *const sock,
.audit.type = LSM_AUDIT_DATA_NET,
.audit.u.net = &audit_net,
.access = access_request,
- .layer_masks = &layer_masks,
- .layer_masks_size = ARRAY_SIZE(layer_masks),
+ .masks = &layer_masks,
});
return -EACCES;
}
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index dfcdc19ea2683..d20e28d38e9c9 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -622,49 +622,24 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
* request are empty).
*/
bool landlock_unmask_layers(const struct landlock_rule *const rule,
- const access_mask_t access_request,
- layer_mask_t (*const layer_masks)[],
- const size_t masks_array_size)
+ struct layer_access_masks *masks)
{
- size_t layer_level;
-
- if (!access_request || !layer_masks)
+ if (!masks)
return true;
if (!rule)
return false;
- /*
- * An access is granted if, for each policy layer, at least one rule
- * encountered on the pathwalk grants the requested access,
- * regardless of its position in the layer stack. We must then check
- * the remaining layers for each inode, from the first added layer to
- * the last one. When there is multiple requested accesses, for each
- * policy layer, the full set of requested accesses may not be granted
- * by only one rule, but by the union (binary OR) of multiple rules.
- * E.g. /a/b <execute> + /a <read> => /a/b <execute + read>
- */
- for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
- const struct landlock_layer *const layer =
- &rule->layers[layer_level];
- const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
- const unsigned long access_req = access_request;
- unsigned long access_bit;
- bool is_empty;
+ for (int i = 0; i < rule->num_layers; i++) {
+ const struct landlock_layer *l = &rule->layers[i];
- /*
- * Records in @layer_masks which layer grants access to each requested
- * access: bit cleared if the related layer grants access.
- */
- is_empty = true;
- for_each_set_bit(access_bit, &access_req, masks_array_size) {
- if (layer->access & BIT_ULL(access_bit))
- (*layer_masks)[access_bit] &= ~layer_bit;
- is_empty = is_empty && !(*layer_masks)[access_bit];
- }
- if (is_empty)
- return true;
+ masks->access[l->level - 1] &= ~l->access;
}
- return false;
+
+ for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ if (masks->access[i])
+ return false;
+ }
+ return true;
}
typedef access_mask_t
@@ -679,8 +654,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
*
* @domain: The domain that defines the current restrictions.
* @access_request: The requested access rights to check.
- * @layer_masks: It must contain %LANDLOCK_NUM_ACCESS_FS or
- * %LANDLOCK_NUM_ACCESS_NET elements according to @key_type.
+ * @masks: Layer access masks to populate.
* @key_type: The key type to switch between access masks of different types.
*
* Returns: An access mask where each access right bit is set which is handled
@@ -689,23 +663,20 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
access_mask_t
landlock_init_layer_masks(const struct landlock_ruleset *const domain,
const access_mask_t access_request,
- layer_mask_t (*const layer_masks)[],
+ struct layer_access_masks *masks,
const enum landlock_key_type key_type)
{
access_mask_t handled_accesses = 0;
- size_t layer_level, num_access;
get_access_mask_t *get_access_mask;
switch (key_type) {
case LANDLOCK_KEY_INODE:
get_access_mask = landlock_get_fs_access_mask;
- num_access = LANDLOCK_NUM_ACCESS_FS;
break;
#if IS_ENABLED(CONFIG_INET)
case LANDLOCK_KEY_NET_PORT:
get_access_mask = landlock_get_net_access_mask;
- num_access = LANDLOCK_NUM_ACCESS_NET;
break;
#endif /* IS_ENABLED(CONFIG_INET) */
@@ -714,27 +685,18 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
return 0;
}
- memset(layer_masks, 0,
- array_size(sizeof((*layer_masks)[0]), num_access));
-
/* An empty access request can happen because of O_WRONLY | O_RDWR. */
if (!access_request)
return 0;
- /* Saves all handled accesses per layer. */
- for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
- const unsigned long access_req = access_request;
- const access_mask_t access_mask =
- get_access_mask(domain, layer_level);
- unsigned long access_bit;
+ for (int i = 0; i < domain->num_layers; i++) {
+ const access_mask_t handled = get_access_mask(domain, i);
- for_each_set_bit(access_bit, &access_req, num_access) {
- if (BIT_ULL(access_bit) & access_mask) {
- (*layer_masks)[access_bit] |=
- BIT_ULL(layer_level);
- handled_accesses |= BIT_ULL(access_bit);
- }
- }
+ masks->access[i] = access_request & handled;
+ handled_accesses |= masks->access[i];
}
+ for (int i = domain->num_layers; i < LANDLOCK_MAX_NUM_LAYERS; i++)
+ masks->access[i] = 0;
+
return handled_accesses;
}
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 1a78cba662b24..f7b80b18c2a70 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -301,15 +301,25 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
return ruleset->access_masks[layer_level].scope;
}
+/**
+ * struct layer_accesses - A boolean matrix of layers and access rights
+ *
+ * This has a bit for each combination of layer numbers and access rights.
+ * During access checks, it is used to represent the access rights for each
+ * layer which still need to be fulfilled. When all bits are 0, the access
+ * request is considered to be fulfilled.
+ */
+struct layer_access_masks {
+ access_mask_t access[LANDLOCK_MAX_NUM_LAYERS];
+};
+
bool landlock_unmask_layers(const struct landlock_rule *const rule,
- const access_mask_t access_request,
- layer_mask_t (*const layer_masks)[],
- const size_t masks_array_size);
+ struct layer_access_masks *masks);
access_mask_t
landlock_init_layer_masks(const struct landlock_ruleset *const domain,
const access_mask_t access_request,
- layer_mask_t (*const layer_masks)[],
+ struct layer_access_masks *masks,
const enum landlock_key_type key_type);
#endif /* _SECURITY_LANDLOCK_RULESET_H */
--
2.52.0
^ permalink raw reply related
* Re: [RFC PATCH 0/2] landlock: Refactor layer masks
From: Günther Noack @ 2025-12-30 10:48 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
In-Reply-To: <20251230103917.10549-3-gnoack3000@gmail.com>
On Tue, Dec 30, 2025 at 11:39:17AM +0100, Günther Noack wrote:
> 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.)
Please see the benchmark program below.
To compile it, use:
cc -o benchmark_worsecase benchmark_worsecase.c
Source code:
```
#define _GNU_SOURCE
#include <err.h>
#include <fcntl.h>
#include <linux/landlock.h>
#include <stdbool.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>
/* Flags */
bool use_landlock = true;
size_t num_iterations = 100000;
size_t num_subdirs = 10000;
void usage() { puts("Usage: benchmark_worstcase [-no-landlock]"); }
/*
* 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) {
const char *path = "d"; /* directory name */
if (use_landlock) {
int abi = syscall(SYS_landlock_create_ruleset, NULL, 0,
LANDLOCK_CREATE_RULESET_VERSION);
if (abi < 7)
err(1, "Landlock ABI too low: got %d, wanted 7+", abi);
}
int ruleset_fd = -1;
if (use_landlock) {
if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0)
err(1, "prctl");
struct landlock_ruleset_attr attr = {
.handled_access_fs = 0xffff, /* All FS access rights as of 2025-12 */
};
ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr, sizeof(attr), 0U);
if (ruleset_fd < 0)
err(1, "landlock_create_ruleset");
}
int 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);
int previous = current;
current = openat(current, path, O_PATH);
if (current < 0)
err(1, "open(%s)", path);
close(previous);
}
if (use_landlock) {
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[]) {
for (int i = 1; i < argc; i++) {
if (!strcmp(argv[i], "-no-landlock")) {
use_landlock = false;
} else if (!strcmp(argv[i], "-d")) {
i++;
if (i < argc)
err(1, "expected number of subdirs after -d");
num_subdirs = atoi(argv[i]);
} else if (!strcmp(argv[i], "-n")) {
i++;
if (i < argc)
err(1, "expected number of iterations after -n");
num_iterations = atoi(argv[i]);
} else {
usage();
errx(1, "unknown argument: %s", argv[i]);
}
}
printf("*** Benchmark ***\n");
printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
num_iterations, use_landlock ? "with" : "without");
struct tms start_time;
if (times(&start_time) == -1)
err(1, "times");
int current = build_directory(num_subdirs);
for (int i = 0; i < num_iterations; i++) {
int fd = openat(current, ".", O_DIRECTORY);
if (fd != -1)
errx(1, "openat succeeded, expected error");
}
struct tms end_time;
if (times(&end_time) == -1)
err(1, "times");
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: %d\n", CLOCKS_PER_SEC);
close(current);
}
```
^ permalink raw reply
* [PATCH v5] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Ryan Foster @ 2025-12-30 15:13 UTC (permalink / raw)
To: foster.ryan.r; +Cc: linux-kernel, linux-security-module, paul, selinux, serge
In-Reply-To: <20251110143748.4144288-1-foster.ryan.r@gmail.com>
Hi all,
Sorry for the spam, this aims to fix both issues. Attempted to reproduce
CI config build locally.
Thanks, Ryan
---
Add comprehensive KUnit tests for the namespace-related capability
functions that Serge Hallyn refactored in commit 9891d2f79a9f
("Clarify the rootid_owns_currentns").
The tests verify:
- Basic functionality: UID 0 in init namespace, invalid vfsuid,
non-zero UIDs
- Actual namespace traversal: Creating user namespaces with different
UID mappings where uid 0 maps to different kuids (e.g., 1000, 2000,
3000)
- Hierarchy traversal: Testing multiple nested namespaces to verify
correct namespace hierarchy traversal
This addresses the feedback to "test the actual functionality" by
creating real user namespaces with different values for the
namespace's uid 0, rather than just basic input validation.
The test file is included at the end of commoncap.c when
CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, following the
standard kernel pattern (e.g., scsi_lib.c, ext4/mballoc.c). This
allows tests to access static functions in the same compilation unit
without modifying production code based on test configuration.
The tests require CONFIG_USER_NS to be enabled since they rely on user
namespace mapping functionality. The Kconfig dependency ensures the
tests only build when this requirement is met.
All 7 tests pass:
- test_vfsuid_root_in_currentns_init_ns
- test_vfsuid_root_in_currentns_invalid
- test_vfsuid_root_in_currentns_nonzero
- test_kuid_root_in_ns_init_ns_uid0
- test_kuid_root_in_ns_init_ns_nonzero
- test_kuid_root_in_ns_with_mapping
- test_kuid_root_in_ns_with_different_mappings
Updated MAINTAINER capabilities to include commoncap test
Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
---
MAINTAINERS | 1 +
security/Kconfig | 17 +++
security/commoncap.c | 4 +
security/commoncap_test.c | 290 ++++++++++++++++++++++++++++++++++++++
4 files changed, 312 insertions(+)
create mode 100644 security/commoncap_test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c0030e126fc8..6f162c736dfb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5682,6 +5682,7 @@ F: include/trace/events/capability.h
F: include/uapi/linux/capability.h
F: kernel/capability.c
F: security/commoncap.c
+F: security/commoncap_test.c
CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER
M: Kevin Tsai <ktsai@capellamicro.com>
diff --git a/security/Kconfig b/security/Kconfig
index 285f284dfcac..6a4393fce9a1 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -284,6 +284,23 @@ config LSM
If unsure, leave this as the default.
+config SECURITY_COMMONCAP_KUNIT_TEST
+ bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y && USER_NS
+ default KUNIT_ALL_TESTS
+ help
+ This builds the commoncap KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
source "security/Kconfig.hardening"
endmenu
diff --git a/security/commoncap.c b/security/commoncap.c
index 8a23dfab7fac..3399535808fe 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1521,3 +1521,7 @@ DEFINE_LSM(capability) = {
};
#endif /* CONFIG_SECURITY */
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+#include "commoncap_test.c"
+#endif
diff --git a/security/commoncap_test.c b/security/commoncap_test.c
new file mode 100644
index 000000000000..1088364a54e6
--- /dev/null
+++ b/security/commoncap_test.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for commoncap.c security functions
+ *
+ * Tests for security-critical functions in the capability subsystem,
+ * particularly namespace-related capability checks.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+#include <linux/cred.h>
+#include <linux/mnt_idmapping.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/refcount.h>
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+
+/* Functions are static in commoncap.c, but we can call them since we're
+ * included in the same compilation unit when tests are enabled.
+ */
+
+/**
+ * test_vfsuid_root_in_currentns_init_ns - Test vfsuid_root_in_currentns with init ns
+ *
+ * Verifies that UID 0 in the init namespace correctly owns the current
+ * namespace when running in init_user_ns.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_init_ns(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create UID 0 in init namespace */
+ kuid = KUIDT_INIT(0);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* In init namespace, UID 0 should own current namespace */
+ KUNIT_EXPECT_TRUE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_invalid - Test vfsuid_root_in_currentns with invalid vfsuid
+ *
+ * Verifies that an invalid vfsuid correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_invalid(struct kunit *test)
+{
+ vfsuid_t invalid_vfsuid;
+
+ /* Use the predefined invalid vfsuid */
+ invalid_vfsuid = INVALID_VFSUID;
+
+ /* Invalid vfsuid should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(invalid_vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_nonzero - Test vfsuid_root_in_currentns with non-zero UID
+ *
+ * Verifies that a non-zero UID correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_nonzero(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create a non-zero UID */
+ kuid = KUIDT_INIT(1000);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* Non-zero UID should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_uid0 - Test kuid_root_in_ns with init namespace and UID 0
+ *
+ * Verifies that kuid_root_in_ns correctly identifies UID 0 in init namespace.
+ * This tests the core namespace traversal logic. In init namespace, UID 0
+ * maps to itself, so it should own the namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_uid0(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(0);
+ init_ns = &init_user_ns;
+
+ /* UID 0 should own init namespace */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_nonzero - Test kuid_root_in_ns with init namespace and non-zero UID
+ *
+ * Verifies that kuid_root_in_ns correctly rejects non-zero UIDs in init namespace.
+ * Only UID 0 should own a namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_nonzero(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(1000);
+ init_ns = &init_user_ns;
+
+ /* Non-zero UID should not own namespace */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * create_test_user_ns_with_mapping - Create a mock user namespace with UID mapping
+ *
+ * Creates a minimal user namespace structure for testing where uid 0 in the
+ * namespace maps to a specific kuid in the parent namespace.
+ *
+ * @test: KUnit test context
+ * @parent_ns: Parent namespace (typically init_user_ns)
+ * @mapped_kuid: The kuid that uid 0 in this namespace maps to in parent
+ *
+ * Returns: Pointer to allocated namespace, or NULL on failure
+ */
+static struct user_namespace *create_test_user_ns_with_mapping(struct kunit *test,
+ struct user_namespace *parent_ns,
+ kuid_t mapped_kuid)
+{
+ struct user_namespace *ns;
+ struct uid_gid_extent extent;
+
+ /* Allocate a test namespace - use kzalloc to zero all fields */
+ ns = kunit_kzalloc(test, sizeof(*ns), GFP_KERNEL);
+ if (!ns)
+ return NULL;
+
+ /* Initialize basic namespace structure fields */
+ ns->parent = parent_ns;
+ ns->level = parent_ns ? parent_ns->level + 1 : 0;
+ ns->owner = mapped_kuid;
+ ns->group = KGIDT_INIT(0);
+
+ /* Initialize ns_common structure */
+ refcount_set(&ns->ns.__ns_ref, 1);
+ ns->ns.inum = 0; /* Mock inum */
+
+ /* Set up uid mapping: uid 0 in this namespace maps to mapped_kuid in parent
+ * Format: first (uid in ns) : lower_first (kuid in parent) : count
+ * So: uid 0 in ns -> kuid mapped_kuid in parent
+ * This means from_kuid(ns, mapped_kuid) returns 0
+ */
+ extent.first = 0; /* uid 0 in this namespace */
+ extent.lower_first = __kuid_val(mapped_kuid); /* maps to this kuid in parent */
+ extent.count = 1;
+
+ ns->uid_map.extent[0] = extent;
+ ns->uid_map.nr_extents = 1;
+
+ /* Set up gid mapping: gid 0 maps to gid 0 in parent (simplified) */
+ extent.first = 0;
+ extent.lower_first = 0;
+ extent.count = 1;
+
+ ns->gid_map.extent[0] = extent;
+ ns->gid_map.nr_extents = 1;
+
+ return ns;
+}
+
+/**
+ * test_kuid_root_in_ns_with_mapping - Test kuid_root_in_ns with namespace where uid 0
+ * maps to different kuid
+ *
+ * Creates a user namespace where uid 0 maps to kuid 1000 in the parent namespace.
+ * Verifies that kuid_root_in_ns correctly identifies kuid 1000 as owning the namespace.
+ *
+ * Note: kuid_root_in_ns walks up the namespace hierarchy, so it checks the current
+ * namespace first, then parent, then parent's parent, etc. So:
+ * - kuid 1000 owns test_ns because from_kuid(test_ns, 1000) == 0
+ * - kuid 0 also owns test_ns because from_kuid(init_user_ns, 0) == 0
+ * (checked in parent)
+ *
+ * This tests the actual functionality as requested: creating namespaces with
+ * different values for the namespace's uid 0.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_mapping(struct kunit *test)
+{
+ struct user_namespace *test_ns;
+ struct user_namespace *parent_ns;
+ kuid_t mapped_kuid, other_kuid;
+
+ parent_ns = &init_user_ns;
+ mapped_kuid = KUIDT_INIT(1000);
+ other_kuid = KUIDT_INIT(2000);
+
+ test_ns = create_test_user_ns_with_mapping(test, parent_ns, mapped_kuid);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_ns);
+
+ /* kuid 1000 should own test_ns because it maps to uid 0 in test_ns */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(mapped_kuid, test_ns));
+
+ /* kuid 0 should also own test_ns (checked via parent init_user_ns) */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), test_ns));
+
+ /* Other kuids should not own test_ns */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(other_kuid, test_ns));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(500), test_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_with_different_mappings - Test with multiple namespaces
+ *
+ * Creates multiple user namespaces with different UID mappings to verify
+ * that kuid_root_in_ns correctly handles different namespace hierarchies.
+ *
+ * Since kuid_root_in_ns walks up the hierarchy, kuids that map to 0 in init_user_ns
+ * will own all namespaces, while kuids that only map to 0 in specific namespaces
+ * will only own those namespaces and their children.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_different_mappings(struct kunit *test)
+{
+ struct user_namespace *ns1, *ns2, *ns3;
+
+ /* Create ns1 where uid 0 maps to kuid 1000 */
+ ns1 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(1000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns1);
+
+ /* Create ns2 where uid 0 maps to kuid 2000 */
+ ns2 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(2000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns2);
+
+ /* Create ns3 as a child of ns1 where uid 0 maps to kuid 3000 */
+ ns3 = create_test_user_ns_with_mapping(test, ns1, KUIDT_INIT(3000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns3);
+
+ /* Test ns1: kuid 1000 owns it, kuid 0 owns it (via parent), kuid 2000 does not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns1));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns1));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns1));
+
+ /* Test ns2: kuid 2000 owns it, kuid 0 owns it (via parent), kuid 1000 does not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns2));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns2));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns2));
+
+ /* Test ns3: kuid 3000 owns it, kuid 1000 owns it (via parent ns1),
+ * kuid 0 owns it (via init_user_ns), kuid 2000 does not
+ */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns3));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns3));
+}
+
+static struct kunit_case commoncap_test_cases[] = {
+ KUNIT_CASE(test_vfsuid_root_in_currentns_init_ns),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_invalid),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_uid0),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_with_mapping),
+ KUNIT_CASE(test_kuid_root_in_ns_with_different_mappings),
+ {}
+};
+
+static struct kunit_suite commoncap_test_suite = {
+ .name = "commoncap",
+ .test_cases = commoncap_test_cases,
+};
+
+kunit_test_suite(commoncap_test_suite);
+
+MODULE_LICENSE("GPL");
+
+#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
--
2.52.0
^ permalink raw reply related
* Re: [PATCH 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Tingmao Wang @ 2025-12-30 15:52 UTC (permalink / raw)
To: Justin Suess, Mickaël Salaün
Cc: demiobenour, fahimitahera, gnoack, hi, jannh,
linux-security-module
In-Reply-To: <20251228163750.451028-1-utilityemal77@gmail.com>
On 12/28/25 16:37, Justin Suess wrote:
> On 12/28/25 07:45, Tingmao Wang wrote:
>> [...]
>> diff --git a/security/landlock/task.c b/security/landlock/task.c
>> index 6dfcc1860d6e..9fbb0ada440b 100644
>> --- a/security/landlock/task.c
>> +++ b/security/landlock/task.c
>> @@ -233,57 +233,84 @@ 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)
>> +static bool sock_addr_is_abstract(const struct unix_address *const addr)
>
> Nit: From the name sock_addr_is_abstract, it's unclear without reading
> the parameter that this function only works with unix sockets, when
> socket is an overloaded term that can refer to other kinds of sockets
> (e.g tcp/udp/raw).
>
> Maybe is_unix_sock_addr_abstract? or unix_sock_addr_is_abstract?
I guess sock_addr_is_abstract is indeed a bit of a weird name, but it
helps that this function is static to this file and also very short.
Maybe is_unix_addr_abstract?
^ permalink raw reply
* Re: [PATCH 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Tingmao Wang @ 2025-12-30 15:56 UTC (permalink / raw)
To: Justin Suess, Mickaël Salaün
Cc: demiobenour, fahimitahera, gnoack, hi, jannh,
linux-security-module
In-Reply-To: <09f721d0-2644-44ba-b2e8-93626a17fa55@maowtm.org>
On 12/30/25 15:52, Tingmao Wang wrote:
> On 12/28/25 16:37, Justin Suess wrote:
>> On 12/28/25 07:45, Tingmao Wang wrote:
>>> [...]
>>> diff --git a/security/landlock/task.c b/security/landlock/task.c
>>> index 6dfcc1860d6e..9fbb0ada440b 100644
>>> --- a/security/landlock/task.c
>>> +++ b/security/landlock/task.c
>>> @@ -233,57 +233,84 @@ 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)
>>> +static bool sock_addr_is_abstract(const struct unix_address *const addr)
>>
>> Nit: From the name sock_addr_is_abstract, it's unclear without reading
>> the parameter that this function only works with unix sockets, when
>> socket is an overloaded term that can refer to other kinds of sockets
>> (e.g tcp/udp/raw).
>>
>> Maybe is_unix_sock_addr_abstract? or unix_sock_addr_is_abstract?
>
> I guess sock_addr_is_abstract is indeed a bit of a weird name, but it
> helps that this function is static to this file and also very short.
> Maybe is_unix_addr_abstract?
>
(nevermind I didn't read Mickaël's reply before sending this, if we can
get rid of this extra function then the name doesn't matter)
^ 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