* [RFC PATCH v3 4/8] selftests/landlock: Add UDP bind/connect tests
From: Matthieu Buffet @ 2025-12-12 16:37 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
konstantin.meskhidze, netdev, Matthieu Buffet
In-Reply-To: <20251212163704.142301-1-matthieu@buffet.re>
Make basic changes to the existing bind() and connect() test suite to
also encompass testing UDP access control.
Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/net_test.c | 389 ++++++++++++++-----
2 files changed, 303 insertions(+), 88 deletions(-)
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 7b69002239d7..f4b1a275d8d9 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -76,7 +76,7 @@ TEST(abi_version)
const struct landlock_ruleset_attr ruleset_attr = {
.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
};
- ASSERT_EQ(7, landlock_create_ruleset(NULL, 0,
+ ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
LANDLOCK_CREATE_RULESET_VERSION));
ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index b34b139b3f89..977d82eb9934 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -35,6 +35,7 @@ enum sandbox_type {
NO_SANDBOX,
/* This may be used to test rules that allow *and* deny accesses. */
TCP_SANDBOX,
+ UDP_SANDBOX,
};
static int set_service(struct service_fixture *const srv,
@@ -93,11 +94,20 @@ static bool prot_is_tcp(const struct protocol_variant *const prot)
(prot->protocol == IPPROTO_TCP || prot->protocol == IPPROTO_IP);
}
+static bool prot_is_udp(const struct protocol_variant *const prot)
+{
+ return (prot->domain == AF_INET || prot->domain == AF_INET6) &&
+ prot->type == SOCK_DGRAM &&
+ (prot->protocol == IPPROTO_UDP || prot->protocol == IPPROTO_IP);
+}
+
static bool is_restricted(const struct protocol_variant *const prot,
const enum sandbox_type sandbox)
{
if (sandbox == TCP_SANDBOX)
return prot_is_tcp(prot);
+ else if (sandbox == UDP_SANDBOX)
+ return prot_is_udp(prot);
return false;
}
@@ -271,10 +281,9 @@ FIXTURE_VARIANT(protocol)
FIXTURE_SETUP(protocol)
{
- const struct protocol_variant prot_unspec = {
- .domain = AF_UNSPEC,
- .type = SOCK_STREAM,
- };
+ struct protocol_variant prot_unspec = variant->prot;
+
+ prot_unspec.domain = AF_UNSPEC;
disable_caps(_metadata);
@@ -510,6 +519,92 @@ FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_unix_datagram) {
},
};
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv4_udp1) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET,
+ .type = SOCK_DGRAM,
+ .protocol = IPPROTO_UDP,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv4_udp2) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET,
+ .type = SOCK_DGRAM,
+ /* IPPROTO_IP == 0 */
+ .protocol = IPPROTO_IP,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv6_udp1) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET6,
+ .type = SOCK_DGRAM,
+ .protocol = IPPROTO_UDP,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv6_udp2) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET6,
+ .type = SOCK_DGRAM,
+ /* IPPROTO_IP == 0 */
+ .protocol = IPPROTO_IP,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv4_tcp) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET,
+ .type = SOCK_STREAM,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv6_tcp) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET6,
+ .type = SOCK_STREAM,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_unix_stream) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_UNIX,
+ .type = SOCK_STREAM,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_unix_datagram) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_UNIX,
+ .type = SOCK_DGRAM,
+ },
+};
+
static void test_bind_and_connect(struct __test_metadata *const _metadata,
const struct service_fixture *const srv,
const bool deny_bind, const bool deny_connect)
@@ -602,7 +697,7 @@ static void test_bind_and_connect(struct __test_metadata *const _metadata,
ret = connect_variant(connect_fd, srv);
if (deny_connect) {
EXPECT_EQ(-EACCES, ret);
- } else if (deny_bind) {
+ } else if (deny_bind && srv->protocol.type == SOCK_STREAM) {
/* No listening server. */
EXPECT_EQ(-ECONNREFUSED, ret);
} else {
@@ -641,18 +736,25 @@ static void test_bind_and_connect(struct __test_metadata *const _metadata,
TEST_F(protocol, bind)
{
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
+ const __u64 bind_access =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_BIND_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP);
+ const __u64 connect_access =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_CONNECT_TCP :
+ LANDLOCK_ACCESS_NET_CONNECT_UDP);
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_net = bind_access | connect_access,
};
- const struct landlock_net_port_attr tcp_bind_connect_p0 = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ const struct landlock_net_port_attr bind_connect_p0 = {
+ .allowed_access = bind_access | connect_access,
.port = self->srv0.port,
};
- const struct landlock_net_port_attr tcp_connect_p1 = {
- .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ const struct landlock_net_port_attr connect_p1 = {
+ .allowed_access = connect_access,
.port = self->srv1.port,
};
int ruleset_fd;
@@ -664,12 +766,12 @@ TEST_F(protocol, bind)
/* Allows connect and bind for the first port. */
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_bind_connect_p0, 0));
+ &bind_connect_p0, 0));
/* Allows connect and denies bind for the second port. */
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_connect_p1, 0));
+ &connect_p1, 0));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
@@ -691,18 +793,25 @@ TEST_F(protocol, bind)
TEST_F(protocol, connect)
{
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
+ const __u64 bind_access =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_BIND_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP);
+ const __u64 connect_access =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_CONNECT_TCP :
+ LANDLOCK_ACCESS_NET_CONNECT_UDP);
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_net = bind_access | connect_access,
};
- const struct landlock_net_port_attr tcp_bind_connect_p0 = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ const struct landlock_net_port_attr bind_connect_p0 = {
+ .allowed_access = bind_access | connect_access,
.port = self->srv0.port,
};
- const struct landlock_net_port_attr tcp_bind_p1 = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
+ const struct landlock_net_port_attr bind_p1 = {
+ .allowed_access = bind_access,
.port = self->srv1.port,
};
int ruleset_fd;
@@ -714,12 +823,12 @@ TEST_F(protocol, connect)
/* Allows connect and bind for the first port. */
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_bind_connect_p0, 0));
+ &bind_connect_p0, 0));
/* Allows bind and denies connect for the second port. */
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_bind_p1, 0));
+ &bind_p1, 0));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
@@ -737,16 +846,24 @@ TEST_F(protocol, connect)
TEST_F(protocol, bind_unspec)
{
- const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP,
- };
- const struct landlock_net_port_attr tcp_bind = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
- .port = self->srv0.port,
- };
+ const int bind_access_right = (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_BIND_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP);
int bind_fd, ret;
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_net = bind_access_right,
+ };
+ const struct landlock_net_port_attr bind = {
+ .allowed_access =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_BIND_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP),
+ .port = self->srv0.port,
+ };
+
const int ruleset_fd = landlock_create_ruleset(
&ruleset_attr, sizeof(ruleset_attr), 0);
ASSERT_LE(0, ruleset_fd);
@@ -754,7 +871,7 @@ TEST_F(protocol, bind_unspec)
/* Allows bind. */
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_bind, 0));
+ &bind, 0));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
}
@@ -782,7 +899,11 @@ TEST_F(protocol, bind_unspec)
}
EXPECT_EQ(0, close(bind_fd));
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_net = bind_access_right,
+ };
const int ruleset_fd = landlock_create_ruleset(
&ruleset_attr, sizeof(ruleset_attr), 0);
ASSERT_LE(0, ruleset_fd);
@@ -829,10 +950,12 @@ TEST_F(protocol, bind_unspec)
TEST_F(protocol, connect_unspec)
{
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_net = LANDLOCK_ACCESS_NET_CONNECT_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_UDP,
};
- const struct landlock_net_port_attr tcp_connect = {
- .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ const struct landlock_net_port_attr connect = {
+ .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_UDP,
.port = self->srv0.port,
};
int bind_fd, client_fd, status;
@@ -865,7 +988,8 @@ TEST_F(protocol, connect_unspec)
EXPECT_EQ(0, ret);
}
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
const int ruleset_fd = landlock_create_ruleset(
&ruleset_attr, sizeof(ruleset_attr), 0);
ASSERT_LE(0, ruleset_fd);
@@ -873,7 +997,7 @@ TEST_F(protocol, connect_unspec)
/* Allows connect. */
ASSERT_EQ(0, landlock_add_rule(ruleset_fd,
LANDLOCK_RULE_NET_PORT,
- &tcp_connect, 0));
+ &connect, 0));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
}
@@ -896,7 +1020,8 @@ TEST_F(protocol, connect_unspec)
EXPECT_EQ(0, ret);
}
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
const int ruleset_fd = landlock_create_ruleset(
&ruleset_attr, sizeof(ruleset_attr), 0);
ASSERT_LE(0, ruleset_fd);
@@ -975,6 +1100,13 @@ FIXTURE_VARIANT_ADD(ipv4, tcp_sandbox_with_tcp) {
.type = SOCK_STREAM,
};
+/* clang-format off */
+FIXTURE_VARIANT_ADD(ipv4, udp_sandbox_with_tcp) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .type = SOCK_STREAM,
+};
+
/* clang-format off */
FIXTURE_VARIANT_ADD(ipv4, no_sandbox_with_udp) {
/* clang-format on */
@@ -989,6 +1121,13 @@ FIXTURE_VARIANT_ADD(ipv4, tcp_sandbox_with_udp) {
.type = SOCK_DGRAM,
};
+/* clang-format off */
+FIXTURE_VARIANT_ADD(ipv4, udp_sandbox_with_udp) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .type = SOCK_DGRAM,
+};
+
FIXTURE_SETUP(ipv4)
{
const struct protocol_variant prot = {
@@ -1010,16 +1149,21 @@ FIXTURE_TEARDOWN(ipv4)
TEST_F(ipv4, from_unix_to_inet)
{
+ const int access_rights =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_BIND_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP |
+ LANDLOCK_ACCESS_NET_CONNECT_UDP);
int unix_stream_fd, unix_dgram_fd;
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_net = access_rights,
};
- const struct landlock_net_port_attr tcp_bind_connect_p0 = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ const struct landlock_net_port_attr bind_connect_p0 = {
+ .allowed_access = access_rights,
.port = self->srv0.port,
};
int ruleset_fd;
@@ -1032,7 +1176,7 @@ TEST_F(ipv4, from_unix_to_inet)
/* Allows connect and bind for srv0. */
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_bind_connect_p0, 0));
+ &bind_connect_p0, 0));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
@@ -1326,11 +1470,13 @@ FIXTURE_TEARDOWN(mini)
/* clang-format off */
-#define ACCESS_LAST LANDLOCK_ACCESS_NET_CONNECT_TCP
+#define ACCESS_LAST LANDLOCK_ACCESS_NET_CONNECT_UDP
#define ACCESS_ALL ( \
LANDLOCK_ACCESS_NET_BIND_TCP | \
- LANDLOCK_ACCESS_NET_CONNECT_TCP)
+ LANDLOCK_ACCESS_NET_CONNECT_TCP | \
+ LANDLOCK_ACCESS_NET_BIND_UDP | \
+ LANDLOCK_ACCESS_NET_CONNECT_UDP)
/* clang-format on */
@@ -1697,7 +1843,7 @@ FIXTURE_VARIANT_ADD(port_specific, no_sandbox_with_ipv4) {
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(port_specific, sandbox_with_ipv4) {
+FIXTURE_VARIANT_ADD(port_specific, tcp_sandbox_with_ipv4) {
/* clang-format on */
.sandbox = TCP_SANDBOX,
.prot = {
@@ -1706,6 +1852,16 @@ FIXTURE_VARIANT_ADD(port_specific, sandbox_with_ipv4) {
},
};
+/* clang-format off */
+FIXTURE_VARIANT_ADD(port_specific, udp_sandbox_with_ipv4) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET,
+ .type = SOCK_DGRAM,
+ },
+};
+
/* clang-format off */
FIXTURE_VARIANT_ADD(port_specific, no_sandbox_with_ipv6) {
/* clang-format on */
@@ -1717,7 +1873,7 @@ FIXTURE_VARIANT_ADD(port_specific, no_sandbox_with_ipv6) {
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(port_specific, sandbox_with_ipv6) {
+FIXTURE_VARIANT_ADD(port_specific, tcp_sandbox_with_ipv6) {
/* clang-format on */
.sandbox = TCP_SANDBOX,
.prot = {
@@ -1726,6 +1882,16 @@ FIXTURE_VARIANT_ADD(port_specific, sandbox_with_ipv6) {
},
};
+/* clang-format off */
+FIXTURE_VARIANT_ADD(port_specific, udp_sandbox_with_ipv6) {
+ /* clang-format on */
+ .sandbox = UDP_SANDBOX,
+ .prot = {
+ .domain = AF_INET6,
+ .type = SOCK_DGRAM,
+ },
+};
+
FIXTURE_SETUP(port_specific)
{
disable_caps(_metadata);
@@ -1745,14 +1911,19 @@ TEST_F(port_specific, bind_connect_zero)
uint16_t port;
/* Adds a rule layer with bind and connect actions. */
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
+ const int access_rights =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_BIND_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP |
+ LANDLOCK_ACCESS_NET_CONNECT_UDP);
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP
+ .handled_access_net = access_rights,
};
const struct landlock_net_port_attr tcp_bind_connect_zero = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .allowed_access = access_rights,
.port = 0,
};
int ruleset_fd;
@@ -1785,11 +1956,13 @@ TEST_F(port_specific, bind_connect_zero)
ret = bind_variant(bind_fd, &self->srv0);
EXPECT_EQ(0, ret);
- EXPECT_EQ(0, listen(bind_fd, backlog));
+ if (variant->prot.type == SOCK_STREAM) {
+ EXPECT_EQ(0, listen(bind_fd, backlog));
- /* Connects on port 0. */
- ret = connect_variant(connect_fd, &self->srv0);
- EXPECT_EQ(-ECONNREFUSED, ret);
+ /* Connects on port 0. */
+ ret = connect_variant(connect_fd, &self->srv0);
+ EXPECT_EQ(-ECONNREFUSED, ret);
+ }
/* Sets binded port for both protocol families. */
port = get_binded_port(bind_fd, &variant->prot);
@@ -1813,21 +1986,25 @@ TEST_F(port_specific, bind_connect_1023)
int bind_fd, connect_fd, ret;
/* Adds a rule layer with bind and connect actions. */
- if (variant->sandbox == TCP_SANDBOX) {
+ if (variant->sandbox == TCP_SANDBOX ||
+ variant->sandbox == UDP_SANDBOX) {
+ const int access_rights =
+ (variant->sandbox == TCP_SANDBOX ?
+ LANDLOCK_ACCESS_NET_BIND_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP |
+ LANDLOCK_ACCESS_NET_CONNECT_UDP);
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP
+ .handled_access_net = access_rights,
};
/* A rule with port value less than 1024. */
- const struct landlock_net_port_attr tcp_bind_connect_low_range = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ const struct landlock_net_port_attr bind_connect_low_range = {
+ .allowed_access = access_rights,
.port = 1023,
};
/* A rule with 1024 port. */
- const struct landlock_net_port_attr tcp_bind_connect = {
- .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ const struct landlock_net_port_attr bind_connect = {
+ .allowed_access = access_rights,
.port = 1024,
};
int ruleset_fd;
@@ -1838,10 +2015,10 @@ TEST_F(port_specific, bind_connect_1023)
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_bind_connect_low_range, 0));
+ &bind_connect_low_range, 0));
ASSERT_EQ(0,
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &tcp_bind_connect, 0));
+ &bind_connect, 0));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
@@ -1865,7 +2042,8 @@ TEST_F(port_specific, bind_connect_1023)
ret = bind_variant(bind_fd, &self->srv0);
clear_cap(_metadata, CAP_NET_BIND_SERVICE);
EXPECT_EQ(0, ret);
- EXPECT_EQ(0, listen(bind_fd, backlog));
+ if (variant->prot.type == SOCK_STREAM)
+ EXPECT_EQ(0, listen(bind_fd, backlog));
/* Connects on the binded port 1023. */
ret = connect_variant(connect_fd, &self->srv0);
@@ -1885,7 +2063,8 @@ TEST_F(port_specific, bind_connect_1023)
/* Binds on port 1024. */
ret = bind_variant(bind_fd, &self->srv0);
EXPECT_EQ(0, ret);
- EXPECT_EQ(0, listen(bind_fd, backlog));
+ if (variant->prot.type == SOCK_STREAM)
+ EXPECT_EQ(0, listen(bind_fd, backlog));
/* Connects on the binded port 1024. */
ret = connect_variant(connect_fd, &self->srv0);
@@ -1895,9 +2074,9 @@ TEST_F(port_specific, bind_connect_1023)
EXPECT_EQ(0, close(bind_fd));
}
-static int matches_log_tcp(const int audit_fd, const char *const blockers,
- const char *const dir_addr, const char *const addr,
- const char *const dir_port)
+static int matches_log_prot(const int audit_fd, const char *const blockers,
+ const char *const dir_addr, const char *const addr,
+ const char *const dir_port)
{
static const char log_template[] = REGEX_LANDLOCK_PREFIX
" blockers=%s %s=%s %s=1024$";
@@ -1933,7 +2112,7 @@ FIXTURE_VARIANT(audit)
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(audit, ipv4) {
+FIXTURE_VARIANT_ADD(audit, ipv4_tcp) {
/* clang-format on */
.addr = "127\\.0\\.0\\.1",
.prot = {
@@ -1943,7 +2122,17 @@ FIXTURE_VARIANT_ADD(audit, ipv4) {
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(audit, ipv6) {
+FIXTURE_VARIANT_ADD(audit, ipv4_udp) {
+ /* clang-format on */
+ .addr = "127\\.0\\.0\\.1",
+ .prot = {
+ .domain = AF_INET,
+ .type = SOCK_DGRAM,
+ },
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(audit, ipv6_tcp) {
/* clang-format on */
.addr = "::1",
.prot = {
@@ -1952,6 +2141,16 @@ FIXTURE_VARIANT_ADD(audit, ipv6) {
},
};
+/* clang-format off */
+FIXTURE_VARIANT_ADD(audit, ipv6_udp) {
+ /* clang-format on */
+ .addr = "::1",
+ .prot = {
+ .domain = AF_INET6,
+ .type = SOCK_DGRAM,
+ },
+};
+
FIXTURE_SETUP(audit)
{
ASSERT_EQ(0, set_service(&self->srv0, variant->prot, 0));
@@ -1972,9 +2171,17 @@ FIXTURE_TEARDOWN(audit)
TEST_F(audit, bind)
{
+ const char *audit_evt = (variant->prot.type == SOCK_STREAM ?
+ "net\\.bind_tcp" :
+ "net\\.bind_udp");
+ const int access_rights =
+ (variant->prot.type == SOCK_STREAM ?
+ LANDLOCK_ACCESS_NET_BIND_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP |
+ LANDLOCK_ACCESS_NET_CONNECT_UDP);
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_net = access_rights,
};
struct audit_records records;
int ruleset_fd, sock_fd;
@@ -1988,8 +2195,8 @@ TEST_F(audit, bind)
sock_fd = socket_variant(&self->srv0);
ASSERT_LE(0, sock_fd);
EXPECT_EQ(-EACCES, bind_variant(sock_fd, &self->srv0));
- EXPECT_EQ(0, matches_log_tcp(self->audit_fd, "net\\.bind_tcp", "saddr",
- variant->addr, "src"));
+ EXPECT_EQ(0, matches_log_prot(self->audit_fd, audit_evt, "saddr",
+ variant->addr, "src"));
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
EXPECT_EQ(0, records.access);
@@ -2000,9 +2207,17 @@ TEST_F(audit, bind)
TEST_F(audit, connect)
{
+ const char *audit_evt = (variant->prot.type == SOCK_STREAM ?
+ "net\\.connect_tcp" :
+ "net\\.connect_udp");
+ const int access_rights =
+ (variant->prot.type == SOCK_STREAM ?
+ LANDLOCK_ACCESS_NET_BIND_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_TCP :
+ LANDLOCK_ACCESS_NET_BIND_UDP |
+ LANDLOCK_ACCESS_NET_CONNECT_UDP);
const struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_net = access_rights,
};
struct audit_records records;
int ruleset_fd, sock_fd;
@@ -2016,8 +2231,8 @@ TEST_F(audit, connect)
sock_fd = socket_variant(&self->srv0);
ASSERT_LE(0, sock_fd);
EXPECT_EQ(-EACCES, connect_variant(sock_fd, &self->srv0));
- EXPECT_EQ(0, matches_log_tcp(self->audit_fd, "net\\.connect_tcp",
- "daddr", variant->addr, "dest"));
+ EXPECT_EQ(0, matches_log_prot(self->audit_fd, audit_evt, "daddr",
+ variant->addr, "dest"));
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
EXPECT_EQ(0, records.access);
--
2.47.3
^ permalink raw reply related
* [RFC PATCH v3 5/8] landlock: Add UDP sendmsg access control
From: Matthieu Buffet @ 2025-12-12 16:37 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
konstantin.meskhidze, netdev, Matthieu Buffet
In-Reply-To: <20251212163704.142301-1-matthieu@buffet.re>
Add support for a LANDLOCK_ACCESS_NET_SENDTO_UDP access right, providing
control over specifying a UDP datagram's destination address explicitly
in sendto(), sendmsg(), and sendmmsg().
This complements the previous control of connect() via
LANDLOCK_ACCESS_NET_CONNECT_UDP.
Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
include/uapi/linux/landlock.h | 13 ++++++++
security/landlock/audit.c | 1 +
security/landlock/limits.h | 2 +-
security/landlock/net.c | 61 +++++++++++++++++++++++++++++++++--
4 files changed, 74 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 8f748fcf79dd..c43586e02216 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -352,12 +352,25 @@ struct landlock_net_port_attr {
* - %LANDLOCK_ACCESS_NET_CONNECT_UDP: Connect UDP sockets to remote
* addresses with the given remote port. Support added in Landlock ABI
* version 8.
+ * - %LANDLOCK_ACCESS_NET_SENDTO_UDP: Send datagrams on UDP sockets with
+ * an explicit destination address set to the given remote port.
+ * Support added in Landlock ABI version 8. Note: this access right
+ * does not control sending datagrams with no explicit destination
+ * (e.g. via :manpage:`send(2)` or ``sendto(..., NULL, 0)``, so this
+ * access right is not necessary when specifying a destination address
+ * once and for all in :manpage:`connect(2)`.
+ *
+ * Note: sending datagrams to an explicit ``AF_UNSPEC`` destination
+ * address family is not supported. For IPv4 sockets, you will need to
+ * use an ``AF_INET`` address instead, and for IPv6 sockets, you will
+ * need to use a ``NULL`` address.
*/
/* clang-format off */
#define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0)
#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1)
#define LANDLOCK_ACCESS_NET_BIND_UDP (1ULL << 2)
#define LANDLOCK_ACCESS_NET_CONNECT_UDP (1ULL << 3)
+#define LANDLOCK_ACCESS_NET_SENDTO_UDP (1ULL << 4)
/* clang-format on */
/**
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 23d8dee320ef..e0c030727dab 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -46,6 +46,7 @@ static const char *const net_access_strings[] = {
[BIT_INDEX(LANDLOCK_ACCESS_NET_CONNECT_TCP)] = "net.connect_tcp",
[BIT_INDEX(LANDLOCK_ACCESS_NET_BIND_UDP)] = "net.bind_udp",
[BIT_INDEX(LANDLOCK_ACCESS_NET_CONNECT_UDP)] = "net.connect_udp",
+ [BIT_INDEX(LANDLOCK_ACCESS_NET_SENDTO_UDP)] = "net.sendto_udp",
};
static_assert(ARRAY_SIZE(net_access_strings) == LANDLOCK_NUM_ACCESS_NET);
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 13dd5503e471..b6d26bc5c49e 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -23,7 +23,7 @@
#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
-#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_CONNECT_UDP
+#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_SENDTO_UDP
#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET)
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 9bddcf466ce9..061a531339de 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -121,6 +121,34 @@ static int current_check_access_socket(struct socket *const sock,
else
return -EAFNOSUPPORT;
}
+ } else if (access_request == LANDLOCK_ACCESS_NET_SENDTO_UDP) {
+ /*
+ * We cannot allow LANDLOCK_ACCESS_NET_SENDTO_UDP on an
+ * explicit AF_UNSPEC address. That's because semantics
+ * of AF_UNSPEC change between socket families (e.g.
+ * IPv6 treat it as "no address" in the sendmsg()
+ * syscall family, so we should always allow, whilst
+ * IPv4 treat it as AF_INET, so we should filter based
+ * on port, and future address families might even do
+ * something else), and the socket's family can change
+ * under our feet due to setsockopt(IPV6_ADDRFORM).
+ */
+ audit_net.family = AF_UNSPEC;
+ landlock_init_layer_masks(subject->domain,
+ access_request, &layer_masks,
+ LANDLOCK_KEY_NET_PORT);
+ landlock_log_denial(
+ subject,
+ &(struct landlock_request){
+ .type = LANDLOCK_REQUEST_NET_ACCESS,
+ .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),
+ });
+ return -EACCES;
} else {
WARN_ON_ONCE(1);
}
@@ -136,7 +164,8 @@ static int current_check_access_socket(struct socket *const sock,
port = addr4->sin_port;
if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
- access_request == LANDLOCK_ACCESS_NET_CONNECT_UDP) {
+ access_request == LANDLOCK_ACCESS_NET_CONNECT_UDP ||
+ access_request == LANDLOCK_ACCESS_NET_SENDTO_UDP) {
audit_net.dport = port;
audit_net.v4info.daddr = addr4->sin_addr.s_addr;
} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP ||
@@ -160,7 +189,8 @@ static int current_check_access_socket(struct socket *const sock,
port = addr6->sin6_port;
if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
- access_request == LANDLOCK_ACCESS_NET_CONNECT_UDP) {
+ access_request == LANDLOCK_ACCESS_NET_CONNECT_UDP ||
+ access_request == LANDLOCK_ACCESS_NET_SENDTO_UDP) {
audit_net.dport = port;
audit_net.v6info.daddr = addr6->sin6_addr;
} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP ||
@@ -248,9 +278,36 @@ static int hook_socket_connect(struct socket *const sock,
access_request);
}
+static int hook_socket_sendmsg(struct socket *const sock,
+ struct msghdr *const msg, const int size)
+{
+ struct sockaddr *const address = msg->msg_name;
+ const int addrlen = msg->msg_namelen;
+ access_mask_t access_request;
+
+ /*
+ * If there is no explicit address in the message, we have no
+ * policy to enforce here because either:
+ * - the socket has a remote address assigned, so the appropriate
+ * access check has already been done back then at assignment time;
+ * - or, we can let the networking stack reply -EDESTADDRREQ.
+ */
+ if (!address)
+ return 0;
+
+ if (sk_is_udp(sock->sk))
+ access_request = LANDLOCK_ACCESS_NET_SENDTO_UDP;
+ else
+ return 0;
+
+ return current_check_access_socket(sock, address, addrlen,
+ access_request);
+}
+
static struct security_hook_list landlock_hooks[] __ro_after_init = {
LSM_HOOK_INIT(socket_bind, hook_socket_bind),
LSM_HOOK_INIT(socket_connect, hook_socket_connect),
+ LSM_HOOK_INIT(socket_sendmsg, hook_socket_sendmsg),
};
__init void landlock_add_net_hooks(void)
--
2.47.3
^ permalink raw reply related
* Re: An opinion about Linux security
From: Stephen Smalley @ 2025-12-12 16:11 UTC (permalink / raw)
To: Timur Chernykh; +Cc: Casey Schaufler, torvalds, linux-security-module, greg
In-Reply-To: <CABZOZnRQ=b4K5jpNM8Z=Zr=+66COwLpC8gynzw88==mL0FCrOw@mail.gmail.com>
On Fri, Dec 12, 2025 at 9:47 AM Timur Chernykh <tim.cherry.co@gmail.com> wrote:
>
> Thanks for the answer!
>
> > eBPF has been accepted because of these restrictions. Without them
> > an eBPF program could very easily reek havoc on the system's behavior.
>
> Unfortunately, these restrictions also make it impossible to use eBPF
> effectively for security. We can certainly discuss the limitations
> themselves if needed, but that’s not the main point I’m trying to make
> here.
There was no shortage of talks about using eBPF for security at LPC this week,
https://lpc.events/event/19/sessions/235/#all
If eBPF can't do what you want today, I suspect it will in the
not-too-distant future.
The best way to make it so would be to try implementing your desired
functionality using it, find the cases where it is currently
insufficient and then take those to LPC or other venues to argue for
enhancements to enable it to do so. Doing so may require re-thinking
how you implement that functionality to fit within the required safety
constraints but that doesn't mean it is impossible.
> > Please propose the interfaces you want. An API definition would be
> > good, and patches even better.
>
> I’m lucky enough to have already built a working prototype, which I
> once offered for review:
>
> https://github.com/Linux-Endpoint-Security-Framework/linux/tree/esf/main/security/esf
> https://github.com/Linux-Endpoint-Security-Framework/linux/tree/esf/main/include/uapi/linux/esf
>
> Less lucky was the reaction I received. Paul Moore was strongly
> opposed, as far as I remember. Dr. Greg once said that heat death of
> the universe is more likely than this approach being accepted into the
> kernel.
Not seeing an actual response from Paul in the archives, but did you
ever actually post patches to the list? Posting a GitHub URL is NOT a
real request for review.
> I don’t want to “poke old wounds”, but the design I proposed was
> largely inspired by Apple’s Endpoint Security Framework (originally
> “mac policies”):
> https://developer.apple.com/documentation/endpointsecurity
>
> In my view, it is an exemplary model. The kernel provides a consistent
> stream of events to userspace, hides the complexity of data collection
> and transport, and ensures fault-tolerance if userspace fails to
> process events quickly enough.
> Audit, for comparison, simply hangs the system if its buffer
> overflows. ESF does not have that problem and even lets userspace
> control the event stream.
>
> > There are numerous cases where LSMs have mitigated attacks. One case:
>
> And there are thousands of cases where LSMs did not. Most companies
> never disclose successful breaches because of NDAs and fear of
> investor backlash. The few disclosures we see typically appear only
> when it’s safe for the company, or when they specifically want to
> appear transparent.
>
> > Your assertion that LSMs don't provide "real" value is not substantiated
> > in the literature.
>
> Which literature do you mean?
> As I mentioned above, public reports about successful attacks are rare.
>
> Maybe I poorly delivered my point. I'll try again.
>
> The strongest protection on Linux today is provided by proprietary
> security vendors. Unfortunately, they often achieve this using
> kernel-module “hacks” rather than LSMs. It’s not because they prefer
> hacks — it’s because Linux doesn’t offer proper APIs to implement the
> required functionality.
>
> > Each of the existing LSMs addresses one or more real world issues. I would
> > hardly call any of the Linux security mechanisms, from mode bits and setuid
> > through SELinux, landlock and EVM, "theoretical".
>
> In practice, most LSM modules are either disabled or misconfigured on
> real servers, and the responsibility falls on AV/EDR vendors. As
> someone who has been building such systems professionally for years, I
> can say with confidence: there is no clean, reliable mechanism
> available today.
> As much as I love Linux, its security and its friendliness toward
> security vendors are its weak side.
>
> In the real world, deploying a modern security product means dealing
> with endless pain: missing kernel headers (sometimes there isn’t even
> Internet access to download them), unstable kernel APIs, lack of BTF,
> constant fights with the eBPF verifier, and all the other limitations
> of eBPF.
>
> To be honest, many developers — and I’m taking the liberty to speak
> broadly here — would simply like to see something similar to Apple’s
> ESF available on Linux.
>
> Best regards,
> Timur Chernykh
>
> On Wed, Dec 10, 2025 at 11:08 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > On 12/9/2025 4:15 PM, Timur Chernykh wrote:
> > > Hello Linus,
> > >
> > > I’m writing to ask for your opinion. What do you think about Linux’s
> > > current readiness for security-focused commercial products? I’m
> > > particularly interested in several areas.
> > >
> > > First, in today’s 3rd-party (out-of-tree) EDR development — EDR being
> > > the most common commercial class of security products — eBPF has
> > > effectively become the main option. Yet eBPF is extremely restrictive.
> >
> > eBPF has been accepted because of these restrictions. Without them
> > an eBPF program could very easily reek havoc on the system's behavior.
> >
> > > It is not possible to write fully expressive real-time analysis code:
> > > the verifier is overly strict,
> >
> > This is of necessity. It is the only protection against malicious and
> > badly written eBPF programs.
> >
> > > non-deterministic loops are not
> > > allowed,
> >
> > Without this restriction denial of service attacks become trivial.
> >
> > > and older kernels lack BTF support.
> >
> > Yes. That's true for many kernel features.
> >
> > > These issues create real
> > > limitations.
> > >
> > > Second, the removal of the out-of-tree LSM API in the 4.x kernel
> > > series caused significant problems for many AV/EDR vendors. I was
> > > unable to find an explanation in the mailing lists that convincingly
> > > justified that decision.
> >
> > To the best of my understanding, and I have been working with LSMs
> > and the infrastructure for quite some time, there has never been an
> > out-of-tree LSM API. We have been very clear that the LSM interfaces
> > are fluid. We have also been very clear that supporting out-of-tree
> > security modules is not a requirement. There are members of the
> > community who would prefer that they be completely prohibited.
> >
> >
> > > The next closest mechanism, fanotify, was a genuine improvement.
> > > However, it does not allow an AV/EDR vendor to protect the integrity
> > > of its own product. Is Linux truly expecting modern AV/EDR solutions
> > > to rely on fanotify alone?
> >
> > You will need to provide more detail about why you believe that the
> > integrity of an AV/EDR product cannot be protected.
> >
> > > My main question is: what are the future plans?
> >
> > Security remains a dynamic technology. There are quite a few plans,
> > from a variety of sources, with no shortage of security goals. Trying
> > to keep up with the concern/crisis of the day is more than sufficient
> > to consume the resources committed to Linux security.
> >
> > > Linux provides very
> > > few APIs for security and dynamic analysis.
> >
> > What APIs do you want? It's possible that some exist that you haven't
> > discovered.
> >
> > > eBPF is still immature,
> >
> > And it will be for some time. That is, until it is mature.
> >
> > > fanotify is insufficient,
> >
> > Without specifics it is quite difficult to do anything about that.
> > And, as we like to say, patches are welcome.
> >
> > > and driver workarounds that bypass kernel
> > > restrictions are risky — they introduce both stability and security
> > > problems.
> >
> > Yes. Don't do that.
> >
> > > At the same time, properly implemented in-tree LSMs are not
> > > inherently dangerous and remain the safer, supported path for
> > > extending security functionality. Without safe, supported interfaces,
> > > however, commercial products struggle to be competitive. At the
> > > moment, macOS with its Endpoint Security Framework is significantly
> > > ahead.
> >
> > Please propose the interfaces you want. An API definition would be
> > good, and patches even better.
> >
> > > Yes, the kernel includes multiple in-tree LSM modules, but in practice
> > > SELinux does not simplify operations — it often complicates them,
> > > despite its long-standing presence. Many of the other LSMs are rarely
> > > used in production. As an EDR developer, I seldom encounter them, and
> > > when I do, they usually provide little practical value. Across
> > > numerous real-world server intrusions, none of these LSM modules have
> > > meaningfully prevented attacks, despite many years of kernel
> > > development.
> >
> > There are numerous cases where LSMs have mitigated attacks. One case:
> >
> > https://mihail-milev.medium.com/mitigating-malware-risks-with-selinux-e37cf1db7c56
> >
> > Your assertion that LSMs don't provide "real" value is not substantiated
> > in the literature.
> >
> > > Perhaps it is time for Linux to focus on more than a theoretical model
> > > of security.
> >
> > Each of the existing LSMs addresses one or more real world issues. I would
> > hardly call any of the Linux security mechanisms, from mode bits and setuid
> > through SELinux, landlock and EVM, "theoretical".
> >
> > > P.S.
> > > Everything above reflects only my personal opinion. I would greatly
> > > appreciate your response and any criticism you may have.
> >
> > We are always ready to improve the security infrastructure of Linux.
> > If it does not meet your needs, help us improve it. The best way to
> > accomplish this is to provide the changes required.
> >
> > > Best regards,
> > > Timur Chernykh
> > >
>
^ permalink raw reply
* Re: An opinion about Linux security
From: Timur Chernykh @ 2025-12-12 14:47 UTC (permalink / raw)
To: Casey Schaufler; +Cc: torvalds, linux-security-module, greg
In-Reply-To: <a3d28a4b-ee63-463e-8c5c-2597e2dcba98@schaufler-ca.com>
Thanks for the answer!
> eBPF has been accepted because of these restrictions. Without them
> an eBPF program could very easily reek havoc on the system's behavior.
Unfortunately, these restrictions also make it impossible to use eBPF
effectively for security. We can certainly discuss the limitations
themselves if needed, but that’s not the main point I’m trying to make
here.
> Yes. That's true for many kernel features.
Right, and that’s a well-known issue that has no clean solution other
than backporting. I completely agree with that.
> To the best of my understanding, and I have been working with LSMs
> and the infrastructure for quite some time, there has never been an
> out-of-tree LSM API.
I was under the impression that there was something of that sort.
For example: https://elixir.bootlin.com/linux/v3.10/source/include/linux/security.h#L1688
It may not have been “officially” exported, but people who needed it
simply resolved the symbol through `kallsyms` or a kprobe and used it.
I’m not claiming that this is good practice, but it did work.
> You will need to provide more detail about why you believe that the
> integrity of an AV/EDR product cannot be protected.
Consider the case where an attacker already has root access. The first
thing they typically do is disable or tamper with security tools.
After that, they establish persistence. Access-control models alone
are not enough here — sometimes they need to be adjusted dynamically,
based on complex real-time analysis.
> What APIs do you want? It's possible that some exist that you haven't
> discovered.
I’m confident I’ve studied all the mechanisms available for real-time
system protection. I’ve even given talks and published material on
approaches to securing Linux.
I may not have covered every LSM module in depth, but in practice they
simply don’t provide the capabilities required for modern security
workloads.
And I don’t want to sound overly categorical, but the current path of
security development feels like a dead end to me, for several reasons.
Modern security relies on analyzing hundreds of thousands of events
per second. eBPF isn’t particularly fast to begin with, and on top of
that it imposes strict technological limitations that prevent
meaningful in-kernel analysis. Every EDR vendor ends up doing the same
thing: collecting raw data (which is already difficult) and pushing it
into userspace for processing. That adds transport overhead on top of
everything else.
Or, of course, writes a dirty and unsafe kernel module. For me in the
perfect world seems to it's better to provide an API for userspace.
It's safe enough and doesn't contain a restriction that eBPF has.
> Please propose the interfaces you want. An API definition would be
> good, and patches even better.
I’m lucky enough to have already built a working prototype, which I
once offered for review:
https://github.com/Linux-Endpoint-Security-Framework/linux/tree/esf/main/security/esf
https://github.com/Linux-Endpoint-Security-Framework/linux/tree/esf/main/include/uapi/linux/esf
Less lucky was the reaction I received. Paul Moore was strongly
opposed, as far as I remember. Dr. Greg once said that heat death of
the universe is more likely than this approach being accepted into the
kernel.
I don’t want to “poke old wounds”, but the design I proposed was
largely inspired by Apple’s Endpoint Security Framework (originally
“mac policies”):
https://developer.apple.com/documentation/endpointsecurity
In my view, it is an exemplary model. The kernel provides a consistent
stream of events to userspace, hides the complexity of data collection
and transport, and ensures fault-tolerance if userspace fails to
process events quickly enough.
Audit, for comparison, simply hangs the system if its buffer
overflows. ESF does not have that problem and even lets userspace
control the event stream.
> There are numerous cases where LSMs have mitigated attacks. One case:
And there are thousands of cases where LSMs did not. Most companies
never disclose successful breaches because of NDAs and fear of
investor backlash. The few disclosures we see typically appear only
when it’s safe for the company, or when they specifically want to
appear transparent.
> Your assertion that LSMs don't provide "real" value is not substantiated
> in the literature.
Which literature do you mean?
As I mentioned above, public reports about successful attacks are rare.
Maybe I poorly delivered my point. I'll try again.
The strongest protection on Linux today is provided by proprietary
security vendors. Unfortunately, they often achieve this using
kernel-module “hacks” rather than LSMs. It’s not because they prefer
hacks — it’s because Linux doesn’t offer proper APIs to implement the
required functionality.
> Each of the existing LSMs addresses one or more real world issues. I would
> hardly call any of the Linux security mechanisms, from mode bits and setuid
> through SELinux, landlock and EVM, "theoretical".
In practice, most LSM modules are either disabled or misconfigured on
real servers, and the responsibility falls on AV/EDR vendors. As
someone who has been building such systems professionally for years, I
can say with confidence: there is no clean, reliable mechanism
available today.
As much as I love Linux, its security and its friendliness toward
security vendors are its weak side.
In the real world, deploying a modern security product means dealing
with endless pain: missing kernel headers (sometimes there isn’t even
Internet access to download them), unstable kernel APIs, lack of BTF,
constant fights with the eBPF verifier, and all the other limitations
of eBPF.
To be honest, many developers — and I’m taking the liberty to speak
broadly here — would simply like to see something similar to Apple’s
ESF available on Linux.
Best regards,
Timur Chernykh
On Wed, Dec 10, 2025 at 11:08 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 12/9/2025 4:15 PM, Timur Chernykh wrote:
> > Hello Linus,
> >
> > I’m writing to ask for your opinion. What do you think about Linux’s
> > current readiness for security-focused commercial products? I’m
> > particularly interested in several areas.
> >
> > First, in today’s 3rd-party (out-of-tree) EDR development — EDR being
> > the most common commercial class of security products — eBPF has
> > effectively become the main option. Yet eBPF is extremely restrictive.
>
> eBPF has been accepted because of these restrictions. Without them
> an eBPF program could very easily reek havoc on the system's behavior.
>
> > It is not possible to write fully expressive real-time analysis code:
> > the verifier is overly strict,
>
> This is of necessity. It is the only protection against malicious and
> badly written eBPF programs.
>
> > non-deterministic loops are not
> > allowed,
>
> Without this restriction denial of service attacks become trivial.
>
> > and older kernels lack BTF support.
>
> Yes. That's true for many kernel features.
>
> > These issues create real
> > limitations.
> >
> > Second, the removal of the out-of-tree LSM API in the 4.x kernel
> > series caused significant problems for many AV/EDR vendors. I was
> > unable to find an explanation in the mailing lists that convincingly
> > justified that decision.
>
> To the best of my understanding, and I have been working with LSMs
> and the infrastructure for quite some time, there has never been an
> out-of-tree LSM API. We have been very clear that the LSM interfaces
> are fluid. We have also been very clear that supporting out-of-tree
> security modules is not a requirement. There are members of the
> community who would prefer that they be completely prohibited.
>
>
> > The next closest mechanism, fanotify, was a genuine improvement.
> > However, it does not allow an AV/EDR vendor to protect the integrity
> > of its own product. Is Linux truly expecting modern AV/EDR solutions
> > to rely on fanotify alone?
>
> You will need to provide more detail about why you believe that the
> integrity of an AV/EDR product cannot be protected.
>
> > My main question is: what are the future plans?
>
> Security remains a dynamic technology. There are quite a few plans,
> from a variety of sources, with no shortage of security goals. Trying
> to keep up with the concern/crisis of the day is more than sufficient
> to consume the resources committed to Linux security.
>
> > Linux provides very
> > few APIs for security and dynamic analysis.
>
> What APIs do you want? It's possible that some exist that you haven't
> discovered.
>
> > eBPF is still immature,
>
> And it will be for some time. That is, until it is mature.
>
> > fanotify is insufficient,
>
> Without specifics it is quite difficult to do anything about that.
> And, as we like to say, patches are welcome.
>
> > and driver workarounds that bypass kernel
> > restrictions are risky — they introduce both stability and security
> > problems.
>
> Yes. Don't do that.
>
> > At the same time, properly implemented in-tree LSMs are not
> > inherently dangerous and remain the safer, supported path for
> > extending security functionality. Without safe, supported interfaces,
> > however, commercial products struggle to be competitive. At the
> > moment, macOS with its Endpoint Security Framework is significantly
> > ahead.
>
> Please propose the interfaces you want. An API definition would be
> good, and patches even better.
>
> > Yes, the kernel includes multiple in-tree LSM modules, but in practice
> > SELinux does not simplify operations — it often complicates them,
> > despite its long-standing presence. Many of the other LSMs are rarely
> > used in production. As an EDR developer, I seldom encounter them, and
> > when I do, they usually provide little practical value. Across
> > numerous real-world server intrusions, none of these LSM modules have
> > meaningfully prevented attacks, despite many years of kernel
> > development.
>
> There are numerous cases where LSMs have mitigated attacks. One case:
>
> https://mihail-milev.medium.com/mitigating-malware-risks-with-selinux-e37cf1db7c56
>
> Your assertion that LSMs don't provide "real" value is not substantiated
> in the literature.
>
> > Perhaps it is time for Linux to focus on more than a theoretical model
> > of security.
>
> Each of the existing LSMs addresses one or more real world issues. I would
> hardly call any of the Linux security mechanisms, from mode bits and setuid
> through SELinux, landlock and EVM, "theoretical".
>
> > P.S.
> > Everything above reflects only my personal opinion. I would greatly
> > appreciate your response and any criticism you may have.
>
> We are always ready to improve the security infrastructure of Linux.
> If it does not meet your needs, help us improve it. The best way to
> accomplish this is to provide the changes required.
>
> > Best regards,
> > Timur Chernykh
> >
^ permalink raw reply
* Re: [PATCH v4 1/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Mickaël Salaün @ 2025-12-12 13:27 UTC (permalink / raw)
To: Justin Suess
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <20251207015132.800576-2-utilityemal77@gmail.com>
Thanks for this patch series. This feature would be useful, but the
related patches add a lot of new code. We should try to minimize that
as much as possible, especially when similar/close logic already exist
(e.g. path walk).
On Sat, Dec 06, 2025 at 08:51:27PM -0500, Justin Suess wrote:
> Implements a flag to prevent access grant inheritance within the filesystem
> hierarchy for landlock rules.
>
> If a landlock rule on an inode has this flag, any access grants on parent
> inodes will be ignored. Moreover, operations that involve altering the
> ancestors of the subject with LANDLOCK_ADD_RULE_NO_INHERIT will be
> denied up to the VFS root (new in v4).
>
> For example, if /a/b/c/ = read only + LANDLOCK_ADD_RULE_NO_INHERIT and
> / = read write, writes to files in /a/b/c will be denied. Moreover,
> moving /a to /bad, removing /a/b/c, or creating links to /a will be
> prohibited.
>
> Parent flag inheritance is automatically suppressed by the permission
> harvesting logic, which will finish processing early if all relevant
> layers are tagged with NO_INHERIT.
>
> And if / has LANDLOCK_ADD_RULE_QUIET, /a/b/c will still audit (handled)
> accesses. This is because LANDLOCK_ADD_RULE_NO_INHERIT also
> suppresses flag inheritance from parent objects.
>
> The parent directory restrictions mitigate sandbox-restart attacks. For
> example, if a sandboxed program is able to move a
> LANDLOCK_ADD_RULE_NO_INHERIT restricted directory, upon sandbox restart,
> the policy applied naively on the same filenames would be invalid.
> Preventing these operations mitigates these attacks.
>
Your Signed-off-by and Cc should be here, followed by "---", followed by
the changelog (to exclude it from the git message e.g., when using
git am).
> v3..v4 changes:
>
> * Rebased on v6 of Tingmao Wang's "quiet flag" series.
> * Removed unnecessary mask_no_inherit_descendant_layers and related
> code at Tingmao Wang's suggestion, simplifying patch.
> * Updated to use new disconnected directory handling.
> * Improved WARN_ON_ONCE usage. (Thanks Tingmao Wang!)
> * Removed redundant loop for single-layer rulesets (again thanks Tingmao
> Wang!)
> * Protections now apply up to the VFS root, not just the mountpoint.
> * Indentation fixes.
> * Removed redundant flag marker blocked_flag_masks.
>
> v2..v3 changes:
>
> * Parent directory topology protections now work by lazily
> inserting blank rules on parent inodes if they do not
> exist. This replaces the previous xarray implementation
> with simplified logic.
> * Added an optimization to skip further processing if all layers
> collected have no inherit.
> * Added support to block flag inheritance.
>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> security/landlock/fs.c | 389 +++++++++++++++++++++++++++++++++++-
> security/landlock/ruleset.c | 19 +-
> security/landlock/ruleset.h | 29 ++-
> 3 files changed, 433 insertions(+), 4 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 0b589263ea42..7b0b77859778 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -317,6 +317,207 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> LANDLOCK_ACCESS_FS_IOCTL_DEV)
> /* clang-format on */
>
> +static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
> + const struct dentry *const dentry);
> +
> +/**
> + * landlock_domain_layers_mask - Build a mask covering all layers of a domain
> + * @domain: The ruleset (domain) to inspect.
> + *
> + * Return a layer mask with a 1 bit for each existing layer of @domain.
> + * If @domain has no layers 0 is returned. If the number of layers is
> + * greater than or equal to the number of bits in layer_mask_t, all bits
> + * are set.
> + */
> +static layer_mask_t landlock_domain_layers_mask(const struct landlock_ruleset
> + *const domain)
> +{
> + if (!domain || !domain->num_layers)
> + return 0;
> +
> + if (domain->num_layers >= sizeof(layer_mask_t) * BITS_PER_BYTE)
> + return (layer_mask_t)~0ULL;
> +
> + return GENMASK_ULL(domain->num_layers - 1, 0);
> +}
> +
> +/**
> + * rule_blocks_all_layers_no_inherit - check whether a rule disables inheritance
> + * @domain_layers_mask: Mask describing the domain's active layers.
> + * @rule: Rule to inspect.
> + *
> + * Return true if every layer present in @rule has its no_inherit flag set
> + * and the set of layers covered by the rule equals @domain_layers_mask.
> + * This indicates that the rule prevents inheritance on all layers of the
> + * domain and thus further walking for inheritance checks can stop.
> + */
> +static bool rule_blocks_all_layers_no_inherit(const layer_mask_t domain_layers_mask,
> + const struct landlock_rule *const rule)
> +{
> + layer_mask_t rule_layers = 0;
> + u32 layer_index;
> +
> + if (!domain_layers_mask || !rule)
> + return false;
> +
> + for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
> + const struct landlock_layer *const layer =
> + &rule->layers[layer_index];
> + const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
> +
> + if (!layer->flags.no_inherit)
> + return false;
> +
> + rule_layers |= layer_bit;
> + }
> +
> + return rule_layers && rule_layers == domain_layers_mask;
> +}
> +
> +/**
> + * ensure_rule_for_dentry - ensure a ruleset contains a rule entry for dentry,
> + * inserting a blank rule if needed.
> + * @ruleset: Ruleset to modify/inspect. Caller must hold @ruleset->lock.
> + * @dentry: Dentry to ensure a rule exists for.
> + *
> + * If no rule is currently associated with @dentry, insert an empty rule
> + * (with zero access) tied to the backing inode. Returns a pointer to the
> + * rule associated with @dentry on success, NULL when @dentry is negative, or
> + * an ERR_PTR()-encoded error if the rule cannot be created.
> + *
> + * This is useful for LANDLOCK_ADD_RULE_NO_INHERIT processing, where a rule
> + * may need to be created for an ancestor dentry that does not yet have one
> + * to properly track no_inherit flags.
> + *
> + * The flags are set to zero if a rule is newly created, and the caller
> + * is responsible for setting them appropriately.
> + *
> + * The returned rule pointer's lifetime is tied to @ruleset.
> + */
> +static const struct landlock_rule *
> +ensure_rule_for_dentry(struct landlock_ruleset *const ruleset,
> + struct dentry *const dentry)
> +{
> + struct landlock_id id = {
> + .type = LANDLOCK_KEY_INODE,
> + };
> + const struct landlock_rule *rule;
> + int err;
> +
> + if (WARN_ON_ONCE(!ruleset || !dentry || d_is_negative(dentry)))
> + return NULL;
> +
> + lockdep_assert_held(&ruleset->lock);
> +
> + rule = find_rule(ruleset, dentry);
> + if (rule)
> + return rule;
> +
> + id.key.object = get_inode_object(d_backing_inode(dentry));
> + if (IS_ERR(id.key.object))
> + return ERR_CAST(id.key.object);
> +
> + err = landlock_insert_rule(ruleset, id, 0, 0);
> + landlock_put_object(id.key.object);
> + if (err)
> + return ERR_PTR(err);
> +
> + rule = find_rule(ruleset, dentry);
> + if (WARN_ON_ONCE(!rule))
> + return ERR_PTR(-ENOENT);
> + return rule;
> +}
> +
> +/**
> + * mark_no_inherit_ancestors - mark ancestors as having no_inherit descendants
> + * @ruleset: Ruleset to modify. Caller must hold @ruleset->lock.
> + * @path: Path representing the descendant that carries no_inherit bits.
> + * @descendant_layers: Mask of layers from the descendant that should be
> + * advertised to ancestors via has_no_inherit_descendant.
> + *
> + * Walks upward from @dentry and ensures that any ancestor rule contains the
> + * has_no_inherit_descendant marker for the specified @descendant_layers so
> + * parent lookups can quickly detect descendant no_inherit influence.
> + *
> + * Returns 0 on success or a negative errno if ancestor bookkeeping fails.
> + */
> +static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
> + const struct path *const path,
> + layer_mask_t descendant_layers)
> +{
> + struct dentry *cursor;
> + struct path walk_path;
> + int err = 0;
> +
> + if (WARN_ON_ONCE(!ruleset || !path || !path->dentry || !path->mnt ||
> + !descendant_layers))
> + return -EINVAL;
> +
> + lockdep_assert_held(&ruleset->lock);
> +
> + walk_path.mnt = path->mnt;
> + walk_path.dentry = path->dentry;
> + path_get(&walk_path);
> +
> + cursor = dget(walk_path.dentry);
> + while (cursor) {
> + struct dentry *parent;
> + const struct landlock_rule *rule;
> +
> + /* Follow mounts all the way up to the root. */
> + if (IS_ROOT(cursor)) {
> + dput(cursor);
> + if (!follow_up(&walk_path)) {
This path walk is inconsistent and a duplicate of the (correct)
is_access_to_paths_allowed() one. Please add a patch to factor out the
common parts as much as possible. Ditto for the
collect_domain_accesses() and collect_topology_sealed_layers().
> + cursor = NULL;
> + continue;
> + }
> + cursor = dget(walk_path.dentry);
> + }
> +
> + parent = dget_parent(cursor);
> + dput(cursor);
> + if (!parent)
> + break;
> +
> + if (WARN_ON_ONCE(d_is_negative(parent))) {
> + dput(parent);
> + break;
> + }
> + /*
> + * Ensures a rule exists for the parent dentry,
> + * inserting a blank one if needed.
> + */
> + rule = ensure_rule_for_dentry(ruleset, parent);
> + if (IS_ERR(rule)) {
> + err = PTR_ERR(rule);
> + dput(parent);
> + cursor = NULL;
> + break;
> + }
> + if (rule) {
> + struct landlock_rule *mutable_rule =
> + (struct landlock_rule *)rule;
> + /*
> + * Unmerged rulesets should only have one layer.
> + */
> + if (WARN_ON_ONCE(mutable_rule->num_layers != 1)) {
> + dput(parent);
> + err = -EINVAL;
> + cursor = NULL;
> + break;
> + }
> +
> + if (descendant_layers & BIT_ULL(0))
> + mutable_rule->layers[0]
> + .flags.has_no_inherit_descendant = true;
> + }
> +
> + cursor = parent;
> + }
> + path_put(&walk_path);
> + return err;
> +}
> +/**
> + * collect_topology_sealed_layers - collect layers sealed against topology changes
> + * @domain: Ruleset to consult.
> + * @dentry: Starting dentry for the upward walk.
> + * @override_layers: Optional out parameter filled with layers that are
> + * present on ancestors but considered overrides (not
> + * sealing the topology for descendants).
> + *
> + * Walk upwards from @dentry and return a mask of layers where either the
> + * visited dentry contains a no_inherit rule or ancestors were previously
> + * marked as having a descendant with no_inherit. @override_layers, if not
> + * NULL, is filled with layers that would normally be overridden by more
> + * specific descendant rules.
> + *
> + * Returns a layer mask where set bits indicate layers that are "sealed"
> + * (topology changes like rename/rmdir are denied) for the subtree rooted at
> + * @dentry.
> + *
> + * Useful for LANDLOCK_ADD_RULE_NO_INHERIT parent directory enforcement to ensure
> + * that topology changes do not violate the no_inherit constraints.
> + */
> +static layer_mask_t
> +collect_topology_sealed_layers(const struct landlock_ruleset *const domain,
> + struct dentry *dentry,
> + layer_mask_t *const override_layers)
> +{
> + struct dentry *cursor, *parent;
> + bool include_descendants = true;
> + layer_mask_t sealed_layers = 0;
> +
> + if (override_layers)
> + *override_layers = 0;
> +
> + if (WARN_ON_ONCE(!domain || !dentry || d_is_negative(dentry)))
> + return 0;
> +
> + cursor = dget(dentry);
> + while (cursor) {
> + const struct landlock_rule *rule;
> + u32 layer_index;
> +
> + rule = find_rule(domain, cursor);
> + if (rule) {
> + for (layer_index = 0; layer_index < rule->num_layers;
> + layer_index++) {
> + const struct landlock_layer *layer =
> + &rule->layers[layer_index];
> + layer_mask_t layer_bit =
> + BIT_ULL(layer->level - 1);
> +
> + if (include_descendants &&
> + (layer->flags.no_inherit ||
> + layer->flags.has_no_inherit_descendant)) {
> + sealed_layers |= layer_bit;
> + } else if (override_layers) {
> + *override_layers |= layer_bit;
> + }
> + }
> + }
> +
> + if (sealed_layers || IS_ROOT(cursor))
> + break;
> +
> + parent = dget_parent(cursor);
> + dput(cursor);
> + if (!parent)
> + return sealed_layers;
> +
> + cursor = parent;
> + include_descendants = false;
> + }
> + dput(cursor);
> + return sealed_layers;
> +}
^ permalink raw reply
* Re: [PATCH v6 04/10] landlock: Fix wrong type usage
From: Mickaël Salaün @ 2025-12-12 11:13 UTC (permalink / raw)
To: Günther Noack
Cc: Tingmao Wang, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <aTvqrOomA3v52sIg@google.com>
On Fri, Dec 12, 2025 at 11:13:19AM +0100, Günther Noack wrote:
> On Sat, Dec 06, 2025 at 05:11:06PM +0000, Tingmao Wang wrote:
> > I think, based on my best understanding, that this type is likely a typo
> > (even though in the end both are u16)
> >
> > Signed-off-by: Tingmao Wang <m@maowtm.org>
> > Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
> > ---
> >
> > Changes in v2:
> > - Added Fixes tag
> >
> > security/landlock/audit.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> > index 1a9d3f4e3369..d51563712325 100644
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -191,7 +191,7 @@ static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > long youngest_layer = -1;
> >
> > for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > - const access_mask_t mask = (*layer_masks)[access_bit];
> > + const layer_mask_t mask = (*layer_masks)[access_bit];
> > long layer;
> >
> > if (!mask)
> > --
> > 2.52.0
>
> Agreed, thanks!
>
> Reviewed-by: Günther Noack <gnoack@google.com>
Thanks, I pushed it to the next branch.
>
> —Günther
>
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Peter Zijlstra @ 2025-12-12 11:09 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CANpmjNP=s33L6LgYWHygEuLtWTq-s2n4yFDvvGcF3HjbGH+hqw@mail.gmail.com>
On Fri, Dec 12, 2025 at 11:15:29AM +0100, Marco Elver wrote:
> On Fri, 12 Dec 2025 at 10:43, Peter Zijlstra <peterz@infradead.org> wrote:
> [..]
> > > Correct. We're trading false negatives over false positives at this
> > > point, just to get things to compile cleanly.
> >
> > Right, and this all 'works' right up to the point someone sticks a
> > must_not_hold somewhere.
> >
> > > > > Better support for Linux's scoped guard design could be added in
> > > > > future if deemed critical.
> > > >
> > > > I would think so, per the above I don't think this is 'right'.
> > >
> > > It's not sound, but we'll avoid false positives for the time being.
> > > Maybe we can wrangle the jigsaw of macros to let it correctly acquire
> > > and then release (via a 2nd cleanup function), it might be as simple
> > > as marking the 'constructor' with the right __acquires(..), and then
> > > have a 2nd __attribute__((cleanup)) variable that just does a no-op
> > > release via __release(..) so we get the already supported pattern
> > > above.
> >
> > Right, like I mentioned in my previous email; it would be lovely if at
> > the very least __always_inline would get a *very* early pass such that
> > the above could be resolved without inter-procedural bits. I really
> > don't consider an __always_inline as another procedure.
> >
> > Because as I already noted yesterday, cleanup is now all
> > __always_inline, and as such *should* all end up in the one function.
> >
> > But yes, if we can get a magical mash-up of __cleanup and __release (let
> > it be knows as __release_on_cleanup ?) that might also work I suppose.
> > But I vastly prefer __always_inline actually 'working' ;-)
>
> The truth is that __always_inline working in this way is currently
> infeasible. Clang and LLVM's architecture simply disallow this today:
> the semantic analysis that -Wthread-safety does happens over the AST,
> whereas always_inline is processed by early passes in the middle-end
> already within LLVM's pipeline, well after semantic analysis. There's
> a complexity budget limit for semantic analysis (type checking,
> warnings, assorted other errors), and path-sensitive &
> intra-procedural analysis over the plain AST is outside that budget.
> Which is why tools like clang-analyzer exist (symbolic execution),
> where it's possible to afford that complexity since that's not
> something that runs for a normal compile.
>
> I think I've pushed the current version of Clang's -Wthread-safety
> already far beyond what folks were thinking is possible (a variant of
> alias analysis), but even my healthy disregard for the impossible
> tells me that making path-sensitive intra-procedural analysis even if
> just for __always_inline functions is quite possibly a fool's errand.
Well, I had to propose it. Gotta push the envelope :-)
> So either we get it to work with what we have, or give up.
So I think as is, we can start. But I really do want the cleanup thing
sorted, even if just with that __release_on_cleanup mashup or so.
^ permalink raw reply
* Re: [PATCH v4 07/35] lockdep: Annotate lockdep assertions for context analysis
From: Marco Elver @ 2025-12-12 10:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251212095943.GM3911114@noisy.programming.kicks-ass.net>
On Fri, 12 Dec 2025 at 10:59, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Dec 11, 2025 at 02:24:57PM +0100, Marco Elver wrote:
>
> > > It is *NOT* (as the clang naming suggests) an assertion of holding the
> > > lock (which is requires_ctx), but rather an annotation that forces the
> > > ctx to be considered held.
> >
> > Noted. I'll add some appropriate wording above the
> > __assumes_ctx_guard() attribute, so this is not lost in the commit
> > logs.
>
> On IRC you stated:
>
> <melver> peterz: 'assume' just forces the compiler to think something is
> held, whether or not it is then becomes the programmer's problem. we
> need it in 2 places at least: for the runtime assertions (to help
> patterns beyond the compiler's static reasoning abilities), and for
> initialization (so we can access guarded variables right after
> initialization; nobody should hold the lock yet)
>
> I'm really not much a fan of that init hack either ;-)
>
> Once we get the scope crap working sanely, I would much rather we move
> to something like:
>
> scoped_guard (spinlock_init, &foo->lock) {
> // init foo fields
> }
>
> or perhaps:
>
> guard(mutex_init)(&bar->lock);
> // init until end of current scope
>
> Where this latter form is very similar to the current semantics where
> mutex_init() will implicitly 'leak' the holding of the lock. But the
> former gives more control where we need it.
I like it. It would also more clearly denote where initialization
start+ends if not confined to a dedicated function.
^ permalink raw reply
* Re: [PATCH v4 02/35] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Marco Elver @ 2025-12-12 10:37 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251212093149.GJ3911114@noisy.programming.kicks-ass.net>
On Fri, 12 Dec 2025 at 10:32, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Dec 11, 2025 at 02:12:19PM +0100, Marco Elver wrote:
>
> > What's a better name?
>
> That must be the hardest question in programming; screw this P-vs-NP
> debate :-)
>
> > context_lock_struct -> and call it "context lock" rather than "context
> > guard"; it might work also for things like RCU, PREEMPT, BH, etc. that
> > aren't normal "locks", but could claim they are "context locks".
> >
> > context_handle_struct -> "context handle" ...
>
> Both work for me I suppose, although I think I have a slight preference
> to the former: 'context_lock_struct'.
>
> One other possibility is wrapping things like so:
>
> #define define_context_struct(name) ... // the big thing
>
> #define define_lock_struct(name) define_context_struct(name)
Note that 'context_lock_struct' (assuming that's the new name) can be
used to just forward declare structs, too, so 'define' in the name is
probably incorrect. And to avoid more levels of indirection I'd just
stick with one name; if 'context_lock_struct' isn't too offensive to
anyone, that'd be the name for the next version.
Thanks,
-- Marco
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2025-12-12 10:15 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251212094352.GL3911114@noisy.programming.kicks-ass.net>
On Fri, 12 Dec 2025 at 10:43, Peter Zijlstra <peterz@infradead.org> wrote:
[..]
> > Correct. We're trading false negatives over false positives at this
> > point, just to get things to compile cleanly.
>
> Right, and this all 'works' right up to the point someone sticks a
> must_not_hold somewhere.
>
> > > > Better support for Linux's scoped guard design could be added in
> > > > future if deemed critical.
> > >
> > > I would think so, per the above I don't think this is 'right'.
> >
> > It's not sound, but we'll avoid false positives for the time being.
> > Maybe we can wrangle the jigsaw of macros to let it correctly acquire
> > and then release (via a 2nd cleanup function), it might be as simple
> > as marking the 'constructor' with the right __acquires(..), and then
> > have a 2nd __attribute__((cleanup)) variable that just does a no-op
> > release via __release(..) so we get the already supported pattern
> > above.
>
> Right, like I mentioned in my previous email; it would be lovely if at
> the very least __always_inline would get a *very* early pass such that
> the above could be resolved without inter-procedural bits. I really
> don't consider an __always_inline as another procedure.
>
> Because as I already noted yesterday, cleanup is now all
> __always_inline, and as such *should* all end up in the one function.
>
> But yes, if we can get a magical mash-up of __cleanup and __release (let
> it be knows as __release_on_cleanup ?) that might also work I suppose.
> But I vastly prefer __always_inline actually 'working' ;-)
The truth is that __always_inline working in this way is currently
infeasible. Clang and LLVM's architecture simply disallow this today:
the semantic analysis that -Wthread-safety does happens over the AST,
whereas always_inline is processed by early passes in the middle-end
already within LLVM's pipeline, well after semantic analysis. There's
a complexity budget limit for semantic analysis (type checking,
warnings, assorted other errors), and path-sensitive &
intra-procedural analysis over the plain AST is outside that budget.
Which is why tools like clang-analyzer exist (symbolic execution),
where it's possible to afford that complexity since that's not
something that runs for a normal compile.
I think I've pushed the current version of Clang's -Wthread-safety
already far beyond what folks were thinking is possible (a variant of
alias analysis), but even my healthy disregard for the impossible
tells me that making path-sensitive intra-procedural analysis even if
just for __always_inline functions is quite possibly a fool's errand.
So either we get it to work with what we have, or give up.
Thanks,
-- Marco
^ permalink raw reply
* Re: [PATCH v6 04/10] landlock: Fix wrong type usage
From: Günther Noack @ 2025-12-12 10:13 UTC (permalink / raw)
To: Tingmao Wang
Cc: Mickaël Salaün, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <7339ad7b47f998affd84ca629a334a71f913616d.1765040503.git.m@maowtm.org>
On Sat, Dec 06, 2025 at 05:11:06PM +0000, Tingmao Wang wrote:
> I think, based on my best understanding, that this type is likely a typo
> (even though in the end both are u16)
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
> ---
>
> Changes in v2:
> - Added Fixes tag
>
> security/landlock/audit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index 1a9d3f4e3369..d51563712325 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -191,7 +191,7 @@ static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> long youngest_layer = -1;
>
> for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> - const access_mask_t mask = (*layer_masks)[access_bit];
> + const layer_mask_t mask = (*layer_masks)[access_bit];
> long layer;
>
> if (!mask)
> --
> 2.52.0
Agreed, thanks!
Reviewed-by: Günther Noack <gnoack@google.com>
—Günther
^ permalink raw reply
* Re: [PATCH v4 07/35] lockdep: Annotate lockdep assertions for context analysis
From: Peter Zijlstra @ 2025-12-12 9:59 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CANpmjNObaGarY1_niCkgEXMNm2bLAVwKwQsLVYekE=Ce6y3ehQ@mail.gmail.com>
On Thu, Dec 11, 2025 at 02:24:57PM +0100, Marco Elver wrote:
> > It is *NOT* (as the clang naming suggests) an assertion of holding the
> > lock (which is requires_ctx), but rather an annotation that forces the
> > ctx to be considered held.
>
> Noted. I'll add some appropriate wording above the
> __assumes_ctx_guard() attribute, so this is not lost in the commit
> logs.
On IRC you stated:
<melver> peterz: 'assume' just forces the compiler to think something is
held, whether or not it is then becomes the programmer's problem. we
need it in 2 places at least: for the runtime assertions (to help
patterns beyond the compiler's static reasoning abilities), and for
initialization (so we can access guarded variables right after
initialization; nobody should hold the lock yet)
I'm really not much a fan of that init hack either ;-)
Once we get the scope crap working sanely, I would much rather we move
to something like:
scoped_guard (spinlock_init, &foo->lock) {
// init foo fields
}
or perhaps:
guard(mutex_init)(&bar->lock);
// init until end of current scope
Where this latter form is very similar to the current semantics where
mutex_init() will implicitly 'leak' the holding of the lock. But the
former gives more control where we need it.
^ permalink raw reply
* Re: [RFC 04/11] crypto: pkcs7: add flag for validated trust on a signed info block
From: David Howells @ 2025-12-12 9:45 UTC (permalink / raw)
To: Blaise Boscaccy
Cc: dhowells, Jonathan Corbet, Paul Moore, James Morris,
Serge E. Hallyn, Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley,
linux-security-module, linux-doc, linux-kernel, bpf
In-Reply-To: <20251211021257.1208712-5-bboscaccy@linux.microsoft.com>
Note that there are two other potentially conflicting sets of changes to the
PKCS#7 code that will need to be coordinated: ML-DSA support and RSASSA-PSS
support. The former wants to do the hashing itself, the latter requires
signature parameters.
David
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Peter Zijlstra @ 2025-12-12 9:43 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CANpmjNOmAYFj518rH0FdPp=cqK8EeKEgh1ok_zFUwHU5Fu92=w@mail.gmail.com>
On Thu, Dec 11, 2025 at 02:19:28PM +0100, Marco Elver wrote:
> On Thu, 11 Dec 2025 at 13:17, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Thu, Nov 20, 2025 at 04:09:31PM +0100, Marco Elver wrote:
> > > Introduce basic compatibility with cleanup.h infrastructure: introduce
> > > DECLARE_LOCK_GUARD_*_ATTRS() helpers to add attributes to constructors
> > > and destructors respectively.
> > >
> > > Note: Due to the scoped cleanup helpers used for lock guards wrapping
> > > acquire and release around their own constructors/destructors that store
> > > pointers to the passed locks in a separate struct, we currently cannot
> > > accurately annotate *destructors* which lock was released. While it's
> > > possible to annotate the constructor to say which lock was acquired,
> > > that alone would result in false positives claiming the lock was not
> > > released on function return.
> > >
> > > Instead, to avoid false positives, we can claim that the constructor
> > > "assumes" that the taken lock is held via __assumes_ctx_guard().
>
>
> > Moo, so the alias analysis didn't help here?
>
> Unfortunately no, because intra-procedural alias analysis for these
> kinds of diagnostics is infeasible. The compiler can only safely
> perform alias analysis for local variables that do not escape the
> function. The layers of wrapping here make this a bit tricky.
>
> The compiler (unlike before) is now able to deal with things like:
> {
> spinlock_t *lock_scope __attribute__((cleanup(spin_unlock))) = &lock;
> spin_lock(&lock); // lock through &lock
> ... critical section ...
> } // unlock through lock_scope (alias -> &lock)
>
> > What is the scope of this __assumes_ctx stuff? The way it is used in the
> > lock initializes seems to suggest it escapes scope. But then something
> > like:
>
> It escapes scope.
>
> > scoped_guard (mutex, &foo) {
> > ...
> > }
> > // context analysis would still assume foo held
> >
> > is somewhat sub-optimal, no?
>
> Correct. We're trading false negatives over false positives at this
> point, just to get things to compile cleanly.
Right, and this all 'works' right up to the point someone sticks a
must_not_hold somewhere.
> > > Better support for Linux's scoped guard design could be added in
> > > future if deemed critical.
> >
> > I would think so, per the above I don't think this is 'right'.
>
> It's not sound, but we'll avoid false positives for the time being.
> Maybe we can wrangle the jigsaw of macros to let it correctly acquire
> and then release (via a 2nd cleanup function), it might be as simple
> as marking the 'constructor' with the right __acquires(..), and then
> have a 2nd __attribute__((cleanup)) variable that just does a no-op
> release via __release(..) so we get the already supported pattern
> above.
Right, like I mentioned in my previous email; it would be lovely if at
the very least __always_inline would get a *very* early pass such that
the above could be resolved without inter-procedural bits. I really
don't consider an __always_inline as another procedure.
Because as I already noted yesterday, cleanup is now all
__always_inline, and as such *should* all end up in the one function.
But yes, if we can get a magical mash-up of __cleanup and __release (let
it be knows as __release_on_cleanup ?) that might also work I suppose.
But I vastly prefer __always_inline actually 'working' ;-)
^ permalink raw reply
* Re: [PATCH v4 16/35] kref: Add context-analysis annotations
From: Peter Zijlstra @ 2025-12-12 9:33 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CANpmjNN+zafzhvUBBmjyy+TL1ecqJUHQNRX3bo9fBJi2nFUt=A@mail.gmail.com>
On Thu, Dec 11, 2025 at 02:54:06PM +0100, Marco Elver wrote:
> Wrappers will need their own annotations; for this kind of static
> analysis (built-in warning diagnostic), inferring things like
> __cond_acquires(true, lock) is far too complex (requires
> intra-procedural control-flow analysis), and would likely be
> incomplete too.
>
> It might also be reasonable to argue that the explicit annotation is
> good for documentation.
>
> Aside: There's other static analysis tooling, like clang-analyzer that
> can afford to do more complex flow-sensitive intra-procedural
> analysis. But that has its own limitations, requires separate
> invocation, and is pretty slow in comparison.
I was sorta hoping that (perhaps only for __always_inline) the thing
would indeed do an early inline pass on the AST such that these cases
would not in fact require inter-procedural analysis.
^ permalink raw reply
* Re: [PATCH v4 02/35] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Peter Zijlstra @ 2025-12-12 9:31 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CANpmjNOyDW7-G5Op5nw722ecPEv=Ys5TPbJnVBB1_WGiM2LeWQ@mail.gmail.com>
On Thu, Dec 11, 2025 at 02:12:19PM +0100, Marco Elver wrote:
> What's a better name?
That must be the hardest question in programming; screw this P-vs-NP
debate :-)
> context_lock_struct -> and call it "context lock" rather than "context
> guard"; it might work also for things like RCU, PREEMPT, BH, etc. that
> aren't normal "locks", but could claim they are "context locks".
>
> context_handle_struct -> "context handle" ...
Both work for me I suppose, although I think I have a slight preference
to the former: 'context_lock_struct'.
One other possibility is wrapping things like so:
#define define_context_struct(name) ... // the big thing
#define define_lock_struct(name) define_context_struct(name)
^ permalink raw reply
* Re: An opinion about Linux security
From: Dr. Greg @ 2025-12-12 5:45 UTC (permalink / raw)
To: Timur Chernykh; +Cc: torvalds, linux-security-module
In-Reply-To: <CABZOZnS4im-wNK4jtGKvp3YT9hPobA503rgiptutOF8rZEwt_w@mail.gmail.com>
On Wed, Dec 10, 2025 at 03:15:39AM +0300, Timur Chernykh wrote:
Good morning Timur, I hope this note finds your week having gone well.
> Hello Linus,
>
> I'm writing to ask for your opinion. What do you think about Linux's
> current readiness for security-focused commercial products? I'm
> particularly interested in several areas.
I don't expect you will receive an answer.
Based on his previous comments and long standing position on this
issue, I believe it can be fairly stated that he looks at the LSM as
an unnecessary evil.
So in his absence, some 'in loco parentis' reflections on the issues
you raise.
I've been advised, more than once, that in this day and age, no one is
interested in reading more than a two sentence paragraph, so a short
response to your issues here and a bit more detail for anyone who
wants to read more, at the end.
There is active art available to address the shortcomings you outline
in your post below. Our TSEM LSM was designed to service the
realitities of the modern security environment and where it is going.
In a manner that doesn't provide any restrictions on how 'security'
can be implemented.
We've done four releases over three years and we believe an unbiased
observer would conclude they have received no substantive technical
review that would support interest in upstream integration.
The challenge is that the security gatekeepers view LSM submissions
through a lens of whether or not the LSM implements security
consistent with what they believe is security. Those views are
inconsistent with the realities of the modern security market, a
market that that is now predicated on detection rather than
enforcement. A trend that will only accelerate with advancements in
machine learning and AI.
It is worth noting that the history of the technology industry is
littered with examples of technology incumbents usually missing
disruptive innovation.
This restriction on suitability is actually inconsistent with Linus'
stated position on how Linux sub-systems can be used, as expressed in
his comment in the following post.
https://lore.kernel.org/lkml/CAHk-=wgLbz1Bm8QhmJ4dJGSmTuV5w_R0Gwvg5kHrYr4Ko9dUHQ@mail.gmail.com/
So the problem is not technical, it is a political eco-system problem.
So, big picture, that is the challenge facing resolution of your
concerns.
Apologies to everyone about the paragraph/sentence overflow and any
afront to sensibilities.
More detail below if anyone is interested.
> First, in today's 3rd-party (out-of-tree) EDR development EDR
> being the most common commercial class of security products eBPF
> has effectively become the main option. Yet eBPF is extremely
> restrictive. It is not possible to write fully expressive real-time
> analysis code: the verifier is overly strict, non-deterministic
> loops are not allowed, and older kernels lack BTF support. These
> issues create real limitations.
>
> Second, the removal of the out-of-tree LSM API in the 4.x kernel
> series caused significant problems for many AV/EDR vendors. I was
> unable to find an explanation in the mailing lists that convincingly
> justified that decision.
>
> The next closest mechanism, fanotify, was a genuine improvement.
> However, it does not allow an AV/EDR vendor to protect the integrity
> of its own product. Is Linux truly expecting modern AV/EDR solutions
> to rely on fanotify alone?
>
> My main question is: what are the future plans? Linux provides very
> few APIs for security and dynamic analysis. eBPF is still immature,
> fanotify is insufficient, and driver workarounds that bypass kernel
> restrictions are risky they introduce both stability and security
> problems. At the same time, properly implemented in-tree LSMs are not
> inherently dangerous and remain the safer, supported path for
> extending security functionality. Without safe, supported interfaces,
> however, commercial products struggle to be competitive. At the
> moment, macOS with its Endpoint Security Framework is significantly
> ahead.
>
> Yes, the kernel includes multiple in-tree LSM modules, but in
> practice SELinux does not simplify operations it often complicates
> them, despite its long-standing presence. Many of the other LSMs are
> rarely used in production. As an EDR developer, I seldom encounter
> them, and when I do, they usually provide little practical
> value. Across numerous real-world server intrusions, none of these
> LSM modules have meaningfully prevented attacks, despite many years
> of kernel development.
>
> Perhaps it is time for Linux to focus on more than a theoretical model
> of security.
The heart of the political eco-system challenge is best expressed by a
quote from Kyle Moffett, in which he stated that security should only
be developed and implemented by experts. Unfortunately that view is
inconsistent with the current state of the technology industry.
Classical security practititioners will defend complex subject/object
architectures with: "Google uses SeLinux for Android security".
Our response to that is that the world doesn't have a security problem
because Google lacks sufficient resources to implement anything it
desires to implement, regardless of the development and maintenance
input costs.
Unfortunately, that luxury is inconsistent with the rest of the
software development world that doesn't enjoy a 3.8 trillion dollar
market capitalization.
The world simply lacks enough experts to make the 'security only by
experts' model work.
Today, the fastest way to a product is to grab Linux and a development
team and write software for hardware that is now completely
commoditized. Everyone knows that security is not one of the
fundamental project predicates in this model.
Both NIST and DHS/CISA are officially on record as indicating that
security needs to start with and be baked in through the development
process. One of the objectives of TSEM was to provide a framework for
enabling this concept for the implementation of analysis and mandatory
behavior controls for software workloads.
A second fundamental problem is that the world has moved, in large
part, to containerized execution workloads. The Linux LSM, in its
current form, doesn't effectively support the application of workload
specific security policies.
Further complicating this issue is the fact that LSM 'stacking'
requires reasoning as to what a final security policy will be when
multiple different security architectures/policies get to decide on
the outcome of a security event/hook. The concept of least surprise
would suggest the need for stacking to have idempotency, in other
words, the order in which LSM event consumers are called shouldn't
influence the effective policy, but this is generally acknowledged as
not being the case with 'stacking'.
So we designed TSEM to provide an alternative, not a replacement, but
an alternative to how developers and system administrators can develop
and apply security policy, including integrity controls.
TSEM is an LSM that implements containerized security infrastructure
rather than security policy. It is designed around the concept of a
security orchestrator that can execute security isolated workloads and
receive the LSM events and their parameters from that workload and
process them in any manner it wishes.
For example: A Docker/Kubernetes container can be run and all of the
security events by that workload exported up into an OpenSearch or
ElasticSearch instance for anomaly detection and analysis.
So an EDR implemented on top of this has visibility into all of the
security events and their characteristics that are deemed security
relevant by the kernel developers.
One of the pushbacks is that this can lead to asynchronous security
decisions, but as you note, that is the model that the commercial
security industry and the consumers of its products has embraced,
particularly in light of the advancements coming out of the AI
industry, detection rather than enforcement.
If synchronous enforcement is required TSEM provides that as well,
including the use of standard kernel modules to implement analysis and
response to the LSM hooks. Internally we have implemented other LSM's
such as Tomoyo and IMA as loadable modules that can support multiple
and independent workload policies.
If you or other EDR vendors are interested, we would be more than
happy to engage in conversations as to how to improve the capabilities
of this type of architecture, as an alternative to what is currently
available in Linux, which as you note, has significant limitations.
> Everything above reflects only my personal opinion. I would greatly
> appreciate your response and any criticism you may have.
As I mentioned at the outset, you are unlikely to hear anything.
For the necessary Linux infrastructure improvements to emerge we
believe there is the need to develop and engage a community effort
that independently pursues the advancements that are necessary,
particularly those that enable Linux to implement first class AI based
security controls.
We believe that only this will result in sufficient 'market pull' at the
distribution level to help shape upstreaming decisions.
Absent that, it is likely that Linux will continue to implement what
has failed to work in the past in the hope that it will somehow work
in the future.
Comments and criticism welcome, we have had plenty of experience with
the latter.... :-)
> Best regards,
> Timur Chernykh
Best wishes for the success of your work and a pleasant holiday season.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: steven chen @ 2025-12-11 23:38 UTC (permalink / raw)
To: Roberto Sassu, Gregory Lumen
Cc: corbet, zohar, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-doc, linux-kernel, linux-integrity,
linux-security-module, nramas, Roberto Sassu, steven chen
In-Reply-To: <2f550d4cd860022e990d1de62049df85a6a86df8.camel@huaweicloud.com>
On 12/11/2025 6:50 AM, Roberto Sassu wrote:
> On Thu, 2025-12-11 at 10:56 +0100, Roberto Sassu wrote:
>> On Wed, 2025-12-10 at 11:12 -0800, Gregory Lumen wrote:
>>> Roberto,
>>>
>>> The proposed approach appears to be workable. However, if our primary goal
>>> here is to enable UM to free kernel memory consumed by the IMA log with an
>>> absolute minimum of kernel functionality/change, then I would argue that
>>> the proposed Stage-then-delete approach still represents unnecessary
>>> complexity when compared to a trim-to-N solution. Specifically:
> The benefit of the Stage-then-delete is that you don't need to scan the
> IMA measurements list in advance to determine what to trim, you just
> trim everything by swapping list head (very fast) and then you can read
> and delete the measurements out of the hot path.
>
> [...]
Trim N entries proposal can do the same speed as staged proposal or
faster than staged proposal.
Steven
>>> - There exists a potential UM measurement-loss race condition introduced
>>> by the staging functionality that would not exist with a trim-to-N
>>> approach. (Occurs if a kexec call occurs after a UM agent has staged
>>> measurements for deletion, but has not completed copying them to
>>> userspace). This could be avoided by persisting staged measurements across
>>> kexec calls at the cost of making the proposed change larger.
>> The solution is to coordinate the staging with kexec in user space.
> To avoid requiring coordination in user space, I will try to see if I
> could improve my patch to prepend the staged entries to the current
> measurement list, before serializing them for kexec().
Trim N entries proposal does not need this at all.
Steven
> Roberto
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Jeff Layton @ 2025-12-11 22:50 UTC (permalink / raw)
To: Frederick Lawler
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <aTtF3BPUcd9crhAm@CMGLRV3>
On Thu, 2025-12-11 at 16:29 -0600, Frederick Lawler wrote:
> On Fri, Dec 12, 2025 at 06:41:00AM +0900, Jeff Layton wrote:
> > On Thu, 2025-12-11 at 15:12 -0600, Frederick Lawler wrote:
> > > On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> > > > On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > > > > Hi Jeff,
> > > > >
> > > > > While testing 6.18, I think I found a regression with
> > > > > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > > > > where IMA is no longer able to properly cache i_version when we overlay
> > > > > tmpfs on top of XFS. Each measurement diff check in function
> > > > > process_measurement() reports that the i_version is
> > > > > always set to zero for iint->real_inode.version.
> > > > >
> > > > > The function ima_collect_measurement() is looking to extract the version
> > > > > from the cookie on next measurement to cache i_version.
> > > > >
> > > > > I'm unclear from the commit description what the right approach here is:
> > > > > update in IMA land by checking for time changes, or do
> > > > > something else such as adding the cookie back.
> > > > >
> > > > >
> > > >
> > > > What we probably want to do is switch to using the ctime to manufacture
> > > > a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> > > > reply.
> > > >
> > > > IIRC, IMA doesn't need to persist these values across reboot, so
> > > > something like this (completely untested) might work, but it may be
> > > > better to lift nfsd4_change_attribute() into a common header and use
> > > > the same mechanism for both:
> > >
> > > I agree lifting nfsd4_change_attribute(), if anything else, a consistent
> > > place to fetch the i_version from. Am I correct in my understanding that
> > > the XOR on the times will cancel out and result in just the i_version?
> >
> > No. I was just using the XOR to mix the tv_sec and tv_nsec fields
> > together in a way that (hopefully) wouldn't generate collisions. It's
> > quite not as robust as what nfsd4_change_attribute() does, but might be
> > sane enough for IMA.
> >
> > > IMA is calling into inode_eq_iversion() to perform the comparison
> > > between the cached value and inode.i_version.
> >
> > That just looks at the i_version field directly without going through -
> > > getattr, so that would need to be switched over as well. Could
> > integrity_inode_attrs_changed() use vfs_getattr_nosec() and compare the
> > result?
>
> That makes sense to me. I'll look through it a bit more, roll a patch, and
> see what the IMA folks have to say (unless they comment here first).
>
Great, thanks. Looks like ima_check_last_writer() might also need a
similar change.
I'm not sure, but It's possible that vfs_getattr_nosec() may not be
callable from all the contexts where integrity_inode_attrs_changed() is
called. If that is the case, then you may just need to convert IMA back
to accessing the i_version field directly instead of going through
vfs_getattr_nosec().
That's less than ideal though, as it will limit the filesystems where
IMA can be implemented. You'll need to come up with a way to deal with
the fact that XFS's i_version field can change on atime updates.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Frederick Lawler @ 2025-12-11 22:29 UTC (permalink / raw)
To: Jeff Layton
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <fab68913274be1cb2a629372eafd52205a51b74e.camel@kernel.org>
On Fri, Dec 12, 2025 at 06:41:00AM +0900, Jeff Layton wrote:
> On Thu, 2025-12-11 at 15:12 -0600, Frederick Lawler wrote:
> > On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> > > On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > > > Hi Jeff,
> > > >
> > > > While testing 6.18, I think I found a regression with
> > > > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > > > where IMA is no longer able to properly cache i_version when we overlay
> > > > tmpfs on top of XFS. Each measurement diff check in function
> > > > process_measurement() reports that the i_version is
> > > > always set to zero for iint->real_inode.version.
> > > >
> > > > The function ima_collect_measurement() is looking to extract the version
> > > > from the cookie on next measurement to cache i_version.
> > > >
> > > > I'm unclear from the commit description what the right approach here is:
> > > > update in IMA land by checking for time changes, or do
> > > > something else such as adding the cookie back.
> > > >
> > > >
> > >
> > > What we probably want to do is switch to using the ctime to manufacture
> > > a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> > > reply.
> > >
> > > IIRC, IMA doesn't need to persist these values across reboot, so
> > > something like this (completely untested) might work, but it may be
> > > better to lift nfsd4_change_attribute() into a common header and use
> > > the same mechanism for both:
> >
> > I agree lifting nfsd4_change_attribute(), if anything else, a consistent
> > place to fetch the i_version from. Am I correct in my understanding that
> > the XOR on the times will cancel out and result in just the i_version?
>
> No. I was just using the XOR to mix the tv_sec and tv_nsec fields
> together in a way that (hopefully) wouldn't generate collisions. It's
> quite not as robust as what nfsd4_change_attribute() does, but might be
> sane enough for IMA.
>
> > IMA is calling into inode_eq_iversion() to perform the comparison
> > between the cached value and inode.i_version.
>
> That just looks at the i_version field directly without going through -
> >getattr, so that would need to be switched over as well. Could
> integrity_inode_attrs_changed() use vfs_getattr_nosec() and compare the
> result?
That makes sense to me. I'll look through it a bit more, roll a patch, and
see what the IMA folks have to say (unless they comment here first).
Thanks Jeff
>
>
> > >
> > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > > index c35ea613c9f8..5a71845f579e 100644
> > > --- a/security/integrity/ima/ima_api.c
> > > +++ b/security/integrity/ima/ima_api.c
> > > @@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > * to an initial measurement/appraisal/audit, but was modified to
> > > * assume the file changed.
> > > */
> > > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > > + result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
> > > AT_STATX_SYNC_AS_STAT);
> > > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > > - i_version = stat.change_cookie;
> > > + if (!result) {
> > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > + i_version = stat.change_cookie;
> > > + else if (stat.result_mask & STATX_CTIME)
> > > + i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > > + }
> > > hash.hdr.algo = algo;
> > > hash.hdr.length = hash_digest_size[algo];
> > >
>
> --
> Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: steven chen @ 2025-12-11 22:06 UTC (permalink / raw)
To: Roberto Sassu, corbet, zohar, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <b701686d-212e-4152-9db9-0c56f21e1fdc@linux.microsoft.com>
On 12/11/2025 11:20 AM, steven chen wrote:
> On 12/11/2025 2:18 AM, Roberto Sassu wrote:
>> On Wed, 2025-12-10 at 16:03 -0800, steven chen wrote:
>>> On 12/9/2025 2:17 AM, Roberto Sassu wrote:
>>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>>
>>>> Introduce the ability of staging the entire of the IMA measurement
>>>> list, or
>>>> a portion, for deletion. Staging means moving the current content
>>>> of the
>>>> measurement list to a separate location, and allowing users to read
>>>> and
>>>> delete it. This causes the measurement list to be atomically truncated
>>>> before new measurements can be added. Staging can be done only once
>>>> at a
>>>> time.
>>>>
>>>> User space is responsible to concatenate the staged IMA
>>>> measurements list
>>>> portions following the temporal order in which the operations were
>>>> done,
>>>> together with the current measurement list. Then, it can send the
>>>> collected
>>>> data to the remote verifiers.
>>>>
>>>> The benefit of this solution is the ability to free precious kernel
>>>> memory,
>>>> in exchange of delegating user space to reconstruct the full
>>>> measurement
>>>> list from the chunks. No trust needs to be given to user space,
>>>> since the
>>>> integrity of the measurement list is protected by the TPM.
>>>>
>>>> By default, staging the measurements list for deletion does not
>>>> alter the
>>>> hash table. When staging is done, IMA is still able to detect
>>>> collisions on
>>>> the staged and later deleted measurement entries, by keeping the entry
>>>> digests (only template data are freed).
>>>>
>>>> However, since during the measurements list serialization only the
>>>> SHA1
>>>> digest is passed, and since there are no template data to
>>>> recalculate the
>>>> other digests from, the hash table is currently not populated with
>>>> digests
>>>> from staged/deleted entries after kexec().
>>>>
>>>> Introduce the new kernel option ima_flush_htable to decide whether
>>>> or not
>>>> the digests of staged measurement entries are flushed from the hash
>>>> table.
>>>>
>>>> Then, introduce ascii_runtime_measurements_staged_<algo> and
>>>> binary_runtime_measurement_staged_<algo> interfaces to stage/delete
>>>> the
>>>> measurements. Use 'echo A > <IMA interface>' and 'echo D > <IMA
>>>> interface>'
>>>> to respectively stage and delete the entire measurements list. Use
>>>> 'echo N > <IMA interface>', with N between 1 and ULONG_MAX, to
>>>> stage the
>>>> selected portion of the measurements list.
>>>>
>>>> The ima_measure_users counter (protected by the ima_measure_lock
>>>> mutex) has
>>>> been introduced to protect access to the measurement list and the
>>>> staged
>>>> part. The open method of all the measurement interfaces has been
>>>> extended
>>>> to allow only one writer at a time or, in alternative, multiple
>>>> readers.
>>>> The write permission is used to stage/delete the measurements, the
>>>> read
>>>> permission to read them. Write requires also the CAP_SYS_ADMIN
>>>> capability.
>>> Hi Roberto,
>>>
>>> I released version 2 of trim N entries patch as bellow:
>>>
>>> [PATCH v2 0/1] Trim N entries of IMA event logs
>>> <https://lore.kernel.org/linux-integrity/20251210235314.3341-1-chenste@linux.microsoft.com/T/#t>
>>>
>>>
>>> I adapted some of your idea and I think trim N has following
>>> advantages:
>>> 1: less measurement list hold time than your current implementation
>>> 2. operation much simple for user space
>>> 3. less kernel code change
>>> 4. no potential issue as Gregory mentioned.
>> Please have a look at:
>>
>> https://marc.info/?l=linux-integrity&m=176545085325473&w=2
>>
>> and let me know if I'm missing something.
>>
>> Thanks
>>
>> Roberto
>
> Hi Roberto,
>
> what does this staging solution do that's not achieved by trim N
> entries solution?
>
> You did not address all my comments and your other idea make things
> more complex.
The following are steps for both proposals:
the steps for trim N solution:
1. User space reads list without lock
2. User space decides to trim N entries and send command to kernel
3. Kernel will lock the list use the same or less time as
staged solution use(we can improve this together)
the steps for staged N solution:
1. User space reads list without lock
2. User space stages list with lock
3. User space decides to trim N entries and send command to kernel
4. Kernel trim staged list (staged list may not empty after trim)
5. kexec save the staged list during soft reboot
6. kexec restore the staged list during soft reboot
>
> Also, Trim N solution is simple and will bring following two good points:
> easy for future IMA development
will be easier for future "Kexec Measurement List Passing" project
> easy for code maintenance
>
> Could you also add your comments on the trim N solution?
>
> Thanks,
>
> Steven
>
>>
>>> Thanks,
>>>
>>> Steven
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Jeff Layton @ 2025-12-11 21:41 UTC (permalink / raw)
To: Frederick Lawler
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <aTszzVJkIqBpYLst@CMGLRV3>
On Thu, 2025-12-11 at 15:12 -0600, Frederick Lawler wrote:
> On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> > On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > > Hi Jeff,
> > >
> > > While testing 6.18, I think I found a regression with
> > > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > > where IMA is no longer able to properly cache i_version when we overlay
> > > tmpfs on top of XFS. Each measurement diff check in function
> > > process_measurement() reports that the i_version is
> > > always set to zero for iint->real_inode.version.
> > >
> > > The function ima_collect_measurement() is looking to extract the version
> > > from the cookie on next measurement to cache i_version.
> > >
> > > I'm unclear from the commit description what the right approach here is:
> > > update in IMA land by checking for time changes, or do
> > > something else such as adding the cookie back.
> > >
> > >
> >
> > What we probably want to do is switch to using the ctime to manufacture
> > a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> > reply.
> >
> > IIRC, IMA doesn't need to persist these values across reboot, so
> > something like this (completely untested) might work, but it may be
> > better to lift nfsd4_change_attribute() into a common header and use
> > the same mechanism for both:
>
> I agree lifting nfsd4_change_attribute(), if anything else, a consistent
> place to fetch the i_version from. Am I correct in my understanding that
> the XOR on the times will cancel out and result in just the i_version?
No. I was just using the XOR to mix the tv_sec and tv_nsec fields
together in a way that (hopefully) wouldn't generate collisions. It's
quite not as robust as what nfsd4_change_attribute() does, but might be
sane enough for IMA.
> IMA is calling into inode_eq_iversion() to perform the comparison
> between the cached value and inode.i_version.
That just looks at the i_version field directly without going through -
>getattr, so that would need to be switched over as well. Could
integrity_inode_attrs_changed() use vfs_getattr_nosec() and compare the
result?
> >
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8..5a71845f579e 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > + else if (stat.result_mask & STATX_CTIME)
> > + i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Frederick Lawler @ 2025-12-11 21:12 UTC (permalink / raw)
To: Jeff Layton
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <2b193b5ccd696420196ae9059f83dcc8b3f06473.camel@kernel.org>
On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > Hi Jeff,
> >
> > While testing 6.18, I think I found a regression with
> > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > where IMA is no longer able to properly cache i_version when we overlay
> > tmpfs on top of XFS. Each measurement diff check in function
> > process_measurement() reports that the i_version is
> > always set to zero for iint->real_inode.version.
> >
> > The function ima_collect_measurement() is looking to extract the version
> > from the cookie on next measurement to cache i_version.
> >
> > I'm unclear from the commit description what the right approach here is:
> > update in IMA land by checking for time changes, or do
> > something else such as adding the cookie back.
> >
> >
>
> What we probably want to do is switch to using the ctime to manufacture
> a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> reply.
>
> IIRC, IMA doesn't need to persist these values across reboot, so
> something like this (completely untested) might work, but it may be
> better to lift nfsd4_change_attribute() into a common header and use
> the same mechanism for both:
I agree lifting nfsd4_change_attribute(), if anything else, a consistent
place to fetch the i_version from. Am I correct in my understanding that
the XOR on the times will cancel out and result in just the i_version?
IMA is calling into inode_eq_iversion() to perform the comparison
between the cached value and inode.i_version.
>
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8..5a71845f579e 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + else if (stat.result_mask & STATX_CTIME)
> + i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Jeff Layton @ 2025-12-11 20:55 UTC (permalink / raw)
To: Frederick Lawler
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <aTspr4_h9IU4EyrR@CMGLRV3>
On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> Hi Jeff,
>
> While testing 6.18, I think I found a regression with
> commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> where IMA is no longer able to properly cache i_version when we overlay
> tmpfs on top of XFS. Each measurement diff check in function
> process_measurement() reports that the i_version is
> always set to zero for iint->real_inode.version.
>
> The function ima_collect_measurement() is looking to extract the version
> from the cookie on next measurement to cache i_version.
>
> I'm unclear from the commit description what the right approach here is:
> update in IMA land by checking for time changes, or do
> something else such as adding the cookie back.
>
>
What we probably want to do is switch to using the ctime to manufacture
a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
reply.
IIRC, IMA doesn't need to persist these values across reboot, so
something like this (completely untested) might work, but it may be
better to lift nfsd4_change_attribute() into a common header and use
the same mechanism for both:
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8..5a71845f579e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ else if (stat.result_mask & STATX_CTIME)
+ i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
^ permalink raw reply related
* xfs/ima: Regression caching i_version
From: Frederick Lawler @ 2025-12-11 20:29 UTC (permalink / raw)
To: Jeff Layton
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
Hi Jeff,
While testing 6.18, I think I found a regression with
commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
where IMA is no longer able to properly cache i_version when we overlay
tmpfs on top of XFS. Each measurement diff check in function
process_measurement() reports that the i_version is
always set to zero for iint->real_inode.version.
The function ima_collect_measurement() is looking to extract the version
from the cookie on next measurement to cache i_version.
I'm unclear from the commit description what the right approach here is:
update in IMA land by checking for time changes, or do
something else such as adding the cookie back.
Thanks,
Fred
^ 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