* [RFC PATCH v4 07/19] selftests/landlock: Test basic socket restriction
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add `protocol` fixture to test biggest part of communication protocol
variants. Add config options required to create sockets for each of these
protocols. Support CAP_NET_RAW capability which is required by some
of the tested protocols.
Add protocols_define.h file containing definitions of tested protocols.
Add test that validates Landlock ability to control creation of sockets
via socket(2) syscall.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Removes some of the protocols from testing (SMC, KMC and XDP).
* Rewrites commit description.
* Changes test name to "restrict_socket_creation".
* Minor changes.
Changes since v2:
* Extends variants of `protocol` fixture with every socket protocol
that can be used to create user space sockets.
* Adds `SYS_ADMIN`, `NET_ADMIN` and `NET_RAW` capabilities required for
some socket protocols.
* Removes network namespace creation in `protocol` fixture setup.
Sockets of some protocols can be created only in initial network
namespace. This shouldn't cause any issues until `protocol` fixture
is used in connection or binding tests.
* Extends config file with a set of options required by socket protocols.
* Adds CAP_NET_RAW capability to landlock selftests which is required
to create sockets of some protocols.
* Adds protocol field to the `protocol` fixture.
* Adds test_socket_variant() helper and changes the signature of
test_socket() helper.
* Checks socket(2) when ruleset is not established.
* Removes checks for AF_UNSPEC. This is moved to unsupported_af_and_prot
test.
* Removes `service_fixture` struct.
* Minor fixes.
* Refactors commit message and title.
Changes since v1:
* Replaces test_socket_create() and socket_variant() helpers
with test_socket().
* Renames domain to family in protocol fixture.
* Remove AF_UNSPEC fixture entry and add unspec_srv0 fixture field to
check AF_UNSPEC socket creation case.
* Formats code with clang-format.
* Refactors commit message.
---
tools/testing/selftests/landlock/common.h | 1 +
tools/testing/selftests/landlock/config | 47 +++++
.../selftests/landlock/protocols_define.h | 169 ++++++++++++++++++
.../testing/selftests/landlock/socket_test.c | 132 ++++++++++++++
4 files changed, 349 insertions(+)
create mode 100644 tools/testing/selftests/landlock/protocols_define.h
diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
index 88a3c78f5d98..e9378a229a4c 100644
--- a/tools/testing/selftests/landlock/common.h
+++ b/tools/testing/selftests/landlock/common.h
@@ -47,6 +47,7 @@ static void _init_caps(struct __test_metadata *const _metadata, bool drop_all)
CAP_SETUID,
CAP_SYS_ADMIN,
CAP_SYS_CHROOT,
+ CAP_NET_RAW,
/* clang-format on */
};
const unsigned int noroot = SECBIT_NOROOT | SECBIT_NOROOT_LOCKED;
diff --git a/tools/testing/selftests/landlock/config b/tools/testing/selftests/landlock/config
index 8fe9b461b1fd..98b3996c36a8 100644
--- a/tools/testing/selftests/landlock/config
+++ b/tools/testing/selftests/landlock/config
@@ -17,3 +17,50 @@ CONFIG_SHMEM=y
CONFIG_SYSFS=y
CONFIG_TMPFS=y
CONFIG_TMPFS_XATTR=y
+
+#
+# Support of socket protocols for socket_test
+#
+CONFIG_AF_RXRPC=y
+CONFIG_ATALK=y
+CONFIG_ATM=y
+CONFIG_AX25=y
+CONFIG_BT=y
+CONFIG_CAIF=y
+CONFIG_CAN_BCM=y
+CONFIG_CAN=y
+CONFIG_CRYPTO_USER_API_AEAD=y
+CONFIG_CRYPTO=y
+CONFIG_HAMRADIO=y
+CONFIG_IEEE802154_SOCKET=y
+CONFIG_IEEE802154=y
+CONFIG_INET=y
+CONFIG_INFINIBAND=y
+CONFIG_IP_SCTP=y
+CONFIG_ISDN=y
+CONFIG_LLC2=y
+CONFIG_LLC=y
+CONFIG_MPTCP=y
+CONFIG_MPTCP_IPV6=y
+CONFIG_MCTP=y
+CONFIG_MISDN=y
+CONFIG_NETDEVICES=y
+CONFIG_NET_KEY=y
+CONFIG_NETROM=y
+CONFIG_NFC=y
+CONFIG_PACKET=y
+CONFIG_PCI=y
+CONFIG_PHONET=y
+CONFIG_PPPOE=y
+CONFIG_PPP=y
+CONFIG_QRTR=y
+CONFIG_RDS=y
+CONFIG_ROSE=y
+CONFIG_SMC=y
+CONFIG_TIPC=y
+CONFIG_UNIX=y
+CONFIG_VMWARE_VMCI_VSOCKETS=y
+CONFIG_VMWARE_VMCI=y
+CONFIG_VSOCKETS=y
+CONFIG_X25=y
+CONFIG_XDP_SOCKETS=y
diff --git a/tools/testing/selftests/landlock/protocols_define.h b/tools/testing/selftests/landlock/protocols_define.h
new file mode 100644
index 000000000000..e44d2278d289
--- /dev/null
+++ b/tools/testing/selftests/landlock/protocols_define.h
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock tests - Tested socket protocols definitions
+ *
+ * Copyright © 2025 Huawei Tech. Co., Ltd.
+ */
+
+/* Almost every protocol that can be used to create socket using create() method
+ * of net_proto_family structure is tested (e.g. this method is used to
+ * create socket with socket(2)).
+ *
+ * List of address families that are not tested:
+ * - Protocol families requiring CAP_SYS_ADMIN or CAP_NET_RAW (eg. AF_PACKET,
+ * AF_CAIF).
+ * - AF_SMC, AF_KMC, AF_XDP.
+ * - AF_ASH, AF_SNA, AF_WANPIPE, AF_NETBEUI, AF_IPX, AF_DECNET, AF_ECONET
+ * and AF_IRDA are not implemented in kernel.
+ * - AF_BRIDGE, AF_MPLS can't be used for creating sockets.
+ * - AF_SECURITY - pseudo AF (Cf. socket.h).
+ * - AF_IB is reserved by infiniband.
+ */
+
+/* Cf. unix_create */
+PROTOCOL_VARIANT_ADD(UNIX, STREAM, 0);
+PROTOCOL_VARIANT_ADD(UNIX, RAW, 0);
+PROTOCOL_VARIANT_ADD(UNIX, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(UNIX, SEQPACKET, 0);
+
+/* Cf. inet_create */
+PROTOCOL_VARIANT_ADD(INET, STREAM, 0);
+PROTOCOL_VARIANT_ADD(INET, STREAM, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(INET, DGRAM, IPPROTO_UDP);
+PROTOCOL_VARIANT_ADD_CAPS(INET, RAW, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET, SEQPACKET, IPPROTO_SCTP);
+PROTOCOL_VARIANT_ADD(INET, STREAM, IPPROTO_MPTCP);
+
+/* Cf. ax25_create */
+PROTOCOL_VARIANT_ADD(AX25, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(AX25, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD_CAPS(AX25, RAW, 0);
+
+/* Cf. atalk_create */
+PROTOCOL_VARIANT_ADD_CAPS(APPLETALK, RAW, 0);
+PROTOCOL_VARIANT_ADD(APPLETALK, DGRAM, 0);
+
+/* Cf. nr_create */
+PROTOCOL_VARIANT_ADD(NETROM, SEQPACKET, 0);
+
+/* Cf. pvc_create */
+PROTOCOL_VARIANT_ADD(ATMPVC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, RAW, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, RDM, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, DCCP, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, PACKET, 0);
+
+/* Cf. x25_create */
+PROTOCOL_VARIANT_ADD(X25, SEQPACKET, 0);
+
+/* Cf. inet6_create */
+PROTOCOL_VARIANT_ADD(INET6, STREAM, 0);
+PROTOCOL_VARIANT_ADD(INET6, STREAM, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET6, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(INET6, DGRAM, IPPROTO_UDP);
+PROTOCOL_VARIANT_ADD_CAPS(INET6, RAW, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET6, SEQPACKET, IPPROTO_SCTP);
+PROTOCOL_VARIANT_ADD(INET6, STREAM, IPPROTO_MPTCP);
+
+/* Cf. rose_create */
+PROTOCOL_VARIANT_ADD(ROSE, SEQPACKET, 0);
+
+/* Cf. pfkey_create */
+PROTOCOL_VARIANT_ADD_CAPS(KEY, RAW, PF_KEY_V2);
+
+/* Cf. netlink_create */
+PROTOCOL_VARIANT_ADD(NETLINK, RAW, 0);
+PROTOCOL_VARIANT_ADD(NETLINK, DGRAM, 0);
+
+/* Cf. packet_create */
+PROTOCOL_VARIANT_ADD_CAPS(PACKET, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(PACKET, RAW, 0);
+PROTOCOL_VARIANT_ADD_CAPS(PACKET, PACKET, 0);
+
+/* Cf. svc_create */
+PROTOCOL_VARIANT_ADD(ATMSVC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, RAW, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, RDM, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, DCCP, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, PACKET, 0);
+
+/* Cf. rds_create */
+PROTOCOL_VARIANT_ADD(RDS, SEQPACKET, 0);
+
+/* Cf. pppox_create + pppoe_create */
+PROTOCOL_VARIANT_ADD(PPPOX, STREAM, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, RAW, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, RDM, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, DCCP, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, PACKET, 0);
+
+/* Cf. llc_ui_create */
+PROTOCOL_VARIANT_ADD_CAPS(LLC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(LLC, STREAM, 0);
+
+/* Cf. can_create */
+PROTOCOL_VARIANT_ADD(CAN, DGRAM, CAN_BCM);
+
+/* Cf. tipc_sk_create */
+PROTOCOL_VARIANT_ADD(TIPC, STREAM, 0);
+PROTOCOL_VARIANT_ADD(TIPC, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(TIPC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(TIPC, RDM, 0);
+
+/* Cf. l2cap_sock_create */
+#ifndef __s390x__
+PROTOCOL_VARIANT_ADD(BLUETOOTH, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(BLUETOOTH, STREAM, 0);
+PROTOCOL_VARIANT_ADD(BLUETOOTH, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(BLUETOOTH, RAW, 0);
+#endif
+
+/* Cf. iucv_sock_create */
+#ifdef __s390x__
+PROTOCOL_VARIANT_ADD(IUCV, STREAM, 0);
+PROTOCOL_VARIANT_ADD(IUCV, SEQPACKET, 0);
+#endif
+
+/* Cf. rxrpc_create */
+PROTOCOL_VARIANT_ADD(RXRPC, DGRAM, PF_INET);
+
+/* Cf. mISDN_sock_create */
+#define ISDN_P_BASE 0 /* Cf. linux/mISDNif.h */
+#define ISDN_P_TE_S0 0x01 /* Cf. linux/mISDNif.h */
+PROTOCOL_VARIANT_ADD_CAPS(ISDN, RAW, ISDN_P_BASE);
+PROTOCOL_VARIANT_ADD(ISDN, DGRAM, ISDN_P_TE_S0);
+
+/* Cf. pn_socket_create */
+PROTOCOL_VARIANT_ADD_CAPS(PHONET, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(PHONET, SEQPACKET, 0);
+
+/* Cf. ieee802154_create */
+PROTOCOL_VARIANT_ADD_CAPS(IEEE802154, RAW, 0);
+PROTOCOL_VARIANT_ADD(IEEE802154, DGRAM, 0);
+
+/* Cf. caif_create */
+PROTOCOL_VARIANT_ADD_CAPS(CAIF, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD_CAPS(CAIF, STREAM, 0);
+
+/* Cf. alg_create */
+PROTOCOL_VARIANT_ADD(ALG, SEQPACKET, 0);
+
+/* Cf. nfc_sock_create + rawsock_create */
+PROTOCOL_VARIANT_ADD(NFC, SEQPACKET, 0);
+
+/* Cf. vsock_create */
+#if defined(__x86_64__) || defined(__aarch64__)
+PROTOCOL_VARIANT_ADD(VSOCK, STREAM, 0);
+PROTOCOL_VARIANT_ADD(VSOCK, SEQPACKET, 0);
+#endif
+
+/* Cf. qrtr_create */
+PROTOCOL_VARIANT_ADD(QIPCRTR, DGRAM, 0);
+
+/* Cf. mctp_pf_create */
+PROTOCOL_VARIANT_ADD(MCTP, DGRAM, 0);
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 16477614dfed..1b6c709d2893 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -9,6 +9,9 @@
#include <linux/landlock.h>
#include <sys/prctl.h>
+#include <linux/pfkeyv2.h>
+#include <linux/kcm.h>
+#include <linux/can.h>
#include "common.h"
@@ -379,4 +382,133 @@ TEST_F(prot_outside_range, add_rule)
ASSERT_EQ(0, close(ruleset_fd));
}
+FIXTURE(protocol)
+{
+ struct protocol_variant prot;
+ bool requires_caps;
+};
+
+FIXTURE_VARIANT(protocol)
+{
+ struct protocol_variant prot;
+ bool requires_caps;
+};
+
+FIXTURE_SETUP(protocol)
+{
+ disable_caps(_metadata);
+
+ self->prot = variant->prot;
+ self->requires_caps = variant->requires_caps;
+};
+
+FIXTURE_TEARDOWN(protocol)
+{
+}
+
+#define _PROTOCOL_VARIANT_ADD(family_, type_, protocol_, caps_) \
+ FIXTURE_VARIANT_ADD(protocol, family_##_##type_##_##protocol_) \
+ { \
+ .prot = { \
+ .domain = AF_##family_, \
+ .type = SOCK_##type_, \
+ .protocol = protocol_, \
+ }, \
+ .requires_caps = caps_, \
+ }
+
+#define PROTOCOL_VARIANT_ADD(family, type, protocol) \
+ _PROTOCOL_VARIANT_ADD(family, type, protocol, false)
+
+#define PROTOCOL_VARIANT_ADD_CAPS(family, type, protocol) \
+ _PROTOCOL_VARIANT_ADD(family, type, protocol, true)
+
+#include "protocols_define.h"
+
+#undef _PROTOCOL_VARIANT_ADD
+#undef PROTOCOL_VARIANT_ADD
+#undef PROTOCOL_VARIANT_ADD_CAPS
+
+static int test_socket(int family, int type, int protocol)
+{
+ int fd;
+
+ fd = socket(family, type | SOCK_CLOEXEC, protocol);
+ if (fd < 0)
+ return errno;
+ /*
+ * Mixing error codes from close(2) and socket(2) should not lead to
+ * any (access type) confusion for this tests.
+ */
+ if (close(fd) != 0)
+ return errno;
+ return 0;
+}
+
+static int test_socket_variant(struct __test_metadata *const _metadata,
+ const struct protocol_variant *const prot,
+ bool requires_caps)
+{
+ int err;
+
+ if (requires_caps) {
+ set_cap(_metadata, CAP_NET_RAW);
+ set_cap(_metadata, CAP_SYS_ADMIN);
+ set_cap(_metadata, CAP_NET_ADMIN);
+ }
+
+ err = test_socket(prot->domain, prot->type, prot->protocol);
+
+ if (requires_caps) {
+ clear_cap(_metadata, CAP_NET_RAW);
+ clear_cap(_metadata, CAP_SYS_ADMIN);
+ clear_cap(_metadata, CAP_NET_ADMIN);
+ }
+
+ return err;
+}
+
+TEST_F(protocol, restrict_socket)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ int ruleset_fd;
+ const struct landlock_socket_attr create_socket_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = self->prot.domain,
+ .type = self->prot.type,
+ .protocol = self->prot.protocol,
+ };
+
+ /* Verifies default socket creation. */
+ ASSERT_EQ(0, test_socket_variant(_metadata, &self->prot,
+ self->requires_caps));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &create_socket_attr, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create socket when protocol is allowed. */
+ EXPECT_EQ(0, test_socket_variant(_metadata, &self->prot,
+ self->requires_caps));
+
+ /* Denies creation. */
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create a socket when protocol is restricted. */
+ EXPECT_EQ(EACCES, test_socket_variant(_metadata, &self->prot,
+ self->requires_caps));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 12/19] selftests/landlock: Test socketpair(2) restriction
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test that checks the restriction on socket creation using
socketpair(2).
Add `socket_creation` fixture to configure sandboxing in tests in
which different socket creation actions are tested.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Removes socket_creation fixture.
---
.../testing/selftests/landlock/socket_test.c | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index e22e10edb103..d1a004c2e0f5 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -866,4 +866,59 @@ TEST_F(tcp_protocol, alias_restriction)
}
}
+static int test_socketpair(int family, int type, int protocol)
+{
+ int fds[2];
+ int err;
+
+ err = socketpair(family, type | SOCK_CLOEXEC, protocol, fds);
+ if (err)
+ return errno;
+ /*
+ * Mixing error codes from close(2) and socketpair(2) should not lead to
+ * any (access type) confusion for this test.
+ */
+ if (close(fds[0]) != 0)
+ return errno;
+ if (close(fds[1]) != 0)
+ return errno;
+ return 0;
+}
+
+TEST_F(mini, socketpair)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_socket_attr unix_socket_create = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_UNIX,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+ int ruleset_fd;
+
+ /* Tries to create socket when ruleset is not established. */
+ ASSERT_EQ(0, test_socketpair(AF_UNIX, SOCK_STREAM, 0));
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &unix_socket_create, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create socket when protocol is allowed */
+ EXPECT_EQ(0, test_socketpair(AF_UNIX, SOCK_STREAM, 0));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create socket when protocol is restricted. */
+ EXPECT_EQ(EACCES, test_socketpair(AF_UNIX, SOCK_STREAM, 0));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 02/19] selftests/landlock: Test creating a ruleset with unknown access
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test that validates behaviour of Landlock after ruleset with
unknown access is created.
Reviewed-by: Günther Noack <gnoack@google.com>
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Adds fixture `mini`. Socket creation should be tested with capabilities
disabled.
Changes since v2:
* Removes fixture `mini`. Network namespace is not used, so this
fixture has become useless.
* Changes commit title and message.
Changes since v1:
* Refactors commit message.
---
.../testing/selftests/landlock/socket_test.c | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 tools/testing/selftests/landlock/socket_test.c
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
new file mode 100644
index 000000000000..d5716149d03f
--- /dev/null
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock tests - Socket
+ *
+ * Copyright © 2025 Huawei Tech. Co., Ltd.
+ */
+
+#define _GNU_SOURCE
+
+#include <linux/landlock.h>
+#include <sys/prctl.h>
+
+#include "common.h"
+
+#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
+#define ACCESS_ALL LANDLOCK_ACCESS_SOCKET_CREATE
+
+/* clang-format off */
+FIXTURE(mini) {};
+/* clang-format on */
+
+FIXTURE_SETUP(mini)
+{
+ disable_caps(_metadata);
+};
+
+FIXTURE_TEARDOWN(mini)
+{
+}
+
+TEST_F(mini, ruleset_with_unknown_access)
+{
+ __u64 access_mask;
+
+ for (access_mask = 1ULL << 63; access_mask != ACCESS_LAST;
+ access_mask >>= 1) {
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = access_mask,
+ };
+
+ ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0));
+ ASSERT_EQ(EINVAL, errno);
+ }
+}
+
+TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 09/19] selftests/landlock: Test overlapped rulesets with rules of protocol ranges
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test that validates Landlock behaviour with overlapped socket
restriction.
Add test that validates behaviour of using multiple layers that
define access for protocol ranges using wildcard values.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Adds test "ruleset_with_wildcards_overlap".
Changes since v2:
* Removes `tcp_layers` fixture and replaces it with `protocol` fixture
for this test. protocol.ruleset_overlap tests every layers depth
in a single run.
* Adds add_ruleset_layer() helper that enforces ruleset and allows access
if such is given.
* Replaces EXPECT_EQ with ASSERT_EQ for close().
* Refactors commit message and title.
Changes since v1:
* Replaces test_socket_create() with test_socket().
* Formats code with clang-format.
* Refactors commit message.
* Minor fixes.
---
.../testing/selftests/landlock/socket_test.c | 92 +++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index ebb39cbf9211..8b8913290a64 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -578,4 +578,96 @@ TEST_F(mini, unsupported_af_and_prot)
EXPECT_EQ(EACCES, test_socket(AF_UNIX, SOCK_STREAM, PF_UNIX + 1));
}
+static void add_ruleset_layer(struct __test_metadata *const _metadata,
+ const struct landlock_socket_attr *socket_attr)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ int ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ if (socket_attr) {
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ socket_attr, 0));
+ }
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(mini, ruleset_overlap)
+{
+ const struct landlock_socket_attr create_socket_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+
+ /* socket(2) is allowed if there are no restrictions. */
+ ASSERT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+
+ /* Creates ruleset with socket(2) allowed. */
+ add_ruleset_layer(_metadata, &create_socket_attr);
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+
+ /* Adds ruleset layer with socket(2) restricted. */
+ add_ruleset_layer(_metadata, NULL);
+ EXPECT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
+
+ /*
+ * Adds ruleset layer with socket(2) allowed. socket(2) is restricted
+ * by second layer of the ruleset.
+ */
+ add_ruleset_layer(_metadata, &create_socket_attr);
+ EXPECT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
+}
+
+TEST_F(mini, ruleset_with_wildcards_overlap)
+{
+ const struct landlock_socket_attr create_socket_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = (-1),
+ .protocol = (-1),
+ };
+
+ /* socket(2) is allowed if there are no restrictions. */
+ ASSERT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+ ASSERT_EQ(0, test_socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP));
+ ASSERT_EQ(0, test_socket(AF_INET, SOCK_DGRAM, 0));
+
+ /* Creates ruleset with AF_INET allowed. */
+ add_ruleset_layer(_metadata, &create_socket_attr);
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP));
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_DGRAM, 0));
+
+ const struct landlock_socket_attr create_socket_attr2 = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = (-1),
+ };
+ /* Creates layer with AF_INET + SOCK_STREAM allowed. */
+ add_ruleset_layer(_metadata, &create_socket_attr2);
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP));
+ EXPECT_EQ(EACCES, test_socket(AF_INET, SOCK_DGRAM, 0));
+
+ const struct landlock_socket_attr create_socket_attr3 = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+ /* Creates layer with AF_INET + SOCK_STREAM + 0 allowed. */
+ add_ruleset_layer(_metadata, &create_socket_attr3);
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+ EXPECT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP));
+ EXPECT_EQ(EACCES, test_socket(AF_INET, SOCK_DGRAM, 0));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 04/19] selftests/landlock: Testing adding rule with wildcard value
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test that validates behaviour of Landlock when rule with
wildcard value is added.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
.../testing/selftests/landlock/socket_test.c | 39 +++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index fcc0f92075a7..abcef11aaf39 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -151,4 +151,43 @@ TEST_F(mini, rule_with_empty_access)
ASSERT_EQ(0, close(ruleset_fd));
}
+/* Validates landlock behaviour when using wildcard (-1) values in socket rules. */
+TEST_F(mini, rule_with_wildcard)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_socket_attr create_family_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = -1,
+ .protocol = -1,
+ };
+ const struct landlock_socket_attr create_family_type_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = -1,
+ };
+ const struct landlock_socket_attr create_family_protocol_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = -1,
+ .protocol = 0,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &create_family_attr, 0));
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &create_family_type_attr, 0));
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &create_family_protocol_attr, 0));
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 05/19] selftests/landlock: Test acceptable ranges of socket rule key
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Create fixture "protocol_inside_range" and "protocol_outside_range"
examining acceptable limits of family, type and protocol values
supported by Landlock ruleset.
Add test validating Landlock behaviour of adding rule with values
corresponding to the limits of the acceptable range and with values
beyond the acceptable ranges.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
.../testing/selftests/landlock/socket_test.c | 189 ++++++++++++++++++
1 file changed, 189 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index abcef11aaf39..16477614dfed 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -190,4 +190,193 @@ TEST_F(mini, rule_with_wildcard)
ASSERT_EQ(0, close(ruleset_fd));
}
+/* clang-format off */
+FIXTURE(prot_inside_range) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(prot_inside_range)
+{
+ int family, type, protocol;
+};
+
+FIXTURE_SETUP(prot_inside_range)
+{
+ disable_caps(_metadata);
+};
+
+FIXTURE_TEARDOWN(prot_inside_range)
+{
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_inside_range, family_upper) {
+ /* clang-format on */
+ .family = UINT8_MAX - 1,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_inside_range, type_upper) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = UINT8_MAX - 1,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_inside_range, protocol_upper) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = UINT16_MAX - 1,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_inside_range, family_lower) {
+ /* clang-format on */
+ .family = 0,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_inside_range, type_lower) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = 0,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_inside_range, protocol_lower) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+};
+
+/*
+ * Verifies acceptable range of family, type and protocol values. Specific
+ * case of adding rule with masked fields checked in "rule_with_wildcard"
+ * test.
+ *
+ * Acceptable ranges are [0, UINT8_MAX) for family and type,
+ * [0, UINT16_MAX) for protocol field.
+ */
+TEST_F(prot_inside_range, add_rule)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_socket_attr create_socket_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = variant->family,
+ .type = variant->type,
+ .protocol = variant->protocol,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &create_socket_attr, 0));
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
+/* clang-format off */
+FIXTURE(prot_outside_range) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(prot_outside_range)
+{
+ int family, type, protocol;
+};
+
+FIXTURE_SETUP(prot_outside_range)
+{
+ disable_caps(_metadata);
+};
+
+FIXTURE_TEARDOWN(prot_outside_range)
+{
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_outside_range, family_upper) {
+ /* clang-format on */
+ .family = UINT8_MAX,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_outside_range, type_upper) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = UINT8_MAX,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_outside_range, protocol_upper) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = UINT16_MAX,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_outside_range, family_lower) {
+ /* clang-format on */
+ .family = -1,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_outside_range, type_lower) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = -2,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(prot_outside_range, protocol_lower) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = -2,
+};
+
+/*
+ * Acceptable ranges are [0, UINT8_MAX) for family and type,
+ * [0, UINT16_MAX) for protocol field. Also type and protocol
+ * can be set with specific -1 wildcard value.
+ */
+TEST_F(prot_outside_range, add_rule)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_socket_attr create_socket_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = variant->family,
+ .type = variant->type,
+ .protocol = variant->protocol,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ EXPECT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &create_socket_attr, 0));
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 01/19] landlock: Support socket access-control
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Landlock implements the `LANDLOCK_RULE_NET_PORT` rule type, which provides
fine-grained control of actions for a specific protocol. Any action or
protocol that is not supported by this rule can not be controlled. As a
result, protocols for which fine-grained control is not supported can be
used in a sandboxed system and lead to vulnerabilities or unexpected
behavior.
Controlling the protocols used will allow to use only those that are
necessary for the system and/or which have fine-grained Landlock control
through others types of rules (e.g. TCP bind/connect control with
`LANDLOCK_RULE_NET_PORT`, UNIX bind control with
`LANDLOCK_RULE_PATH_BENEATH`). Consider following examples:
* Server may want to use only TCP sockets for which there is fine-grained
control of bind(2) and connect(2) actions [1].
* System that does not need a network or that may want to disable network
for security reasons (e.g. [2]) can achieve this by restricting the use
of all possible protocols.
This patch implements such control by restricting socket creation in a
sandboxed process. Introduced changes are listed below.
Add `LANDLOCK_RULE_SOCKET` rule type that restricts actions on sockets.
This rule uses values of protocol family, protocol number and socket type
(cf. socket(2)) to determine sockets that should be restricted. This is
represented in a struct landlock_socket_attr:
struct landlock_socket_attr {
__u64 allowed_access;
__s32 family; /* same as domain in socket(2) */
__s32 type; /* see socket(2) */
__s32 protocol;
};
Add `LANDLOCK_ACCESS_SOCKET_CREATE` access right that corresponds to the
creation of user space sockets (eg. by calling socket(2) system call).
In the case of connection-based socket types, this does not restrict the
actions that result in creation of sockets used for messaging between
already existing endpoints (e.g. accept(2), SCTP_SOCKOPT_PEELOFF).
Also, this does not restrict any other socket-related actions such as
bind(2) or send(2). All restricted actions are enlisted in the
documentation of this access right.
As with all other access rights, using `LANDLOCK_ACCESS_SOCKET_CREATE`
does not affect the actions on sockets which were created before
sandboxing.
In some cases it may be useful to define a single rule that will enable
entire protocol family or all protocols which correspond to specified
family and type.
Add "wildcard" values for type, protocol fields to make it possible to
define rule for a set of protocols. Setting wildcard means that rule
allows every socket type or protocol inside protocol family. For example,
following rule allows creating sockets of each protocol corresponding to
INET family (ie. TCP, SCTP, UDP, ..):
struct landlock_socket_attr allow_inet {
.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
.family = AF_INET,
.type = -1,
.protocol = -1,
};
It is possible to create sockets of the same protocol with different
protocol number values. For example, TCP sockets can be created using one
of the following commands:
1. fd = socket(AF_INET, SOCK_STREAM, 0);
2. fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
Whereas IPPROTO_TCP = 6. Protocol number 0 correspond to the default
protocol of the given protocol family and can be mapped to another
value.
Socket rules do not perform such mappings to not increase complexity
of rules definition and their maintenance.
Add socket.c file that will contain socket rules management and LSM hooks.
Support socket rule storage in landlock ruleset. Acceptable ranges for
family and type fields are [0, U8_MAX) and for the protocol field it is
[0, U16_MAX).
Implement helper pack_socket_key() to convert 32-bit family, type and
protocol values into uintptr_t. This is possible due to the fact that
family and type values are limited to AF_MAX (= 46), SOCK_MAX (= 11)
constants. These assumption is checked in build-time. Protocol value is
expected to be less than UINT16_MAX. If user tries to define a rule with
values outside ranges, landlock_add_rule returns with -EINVAL.
Support socket rules in landlock syscalls. Change ABI version to 8.
[1] https://lore.kernel.org/all/ZJvy2SViorgc+cZI@google.com/
[2] https://cr.yp.to/unix/disablenetwork.html
Closes: https://github.com/landlock-lsm/linux/issues/6
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Changes ABI version from 6 to 8.
* landlock_socket_attr changes:
* Supports protocol field.
* Supports wildcard values for type, protocol fields.
* Changes data type of family and type fields from int to __s32
* Rewrites commit message.
* Fixes grammar.
* Minor fixes.
Changes since v2:
* Refactors access_mask for `LANDLOCK_RULE_SOCKET`.
* Changes type of 'socket_key.packed' from 'uintptr_t' to 'unsigned int'
in order to fix UB in pack_socket_key().
* Accepts (AF_INET, SOCK_PACKET) as an alias for (AF_PACKET, SOCK_PACKET)
in landlock_append_socket_rule().
* Fixes documentation.
* Rewrites commit message.
* Fixes grammar.
* Minor fixes.
Changes since v1:
* Reverts landlock_key.data type from u64 to uinptr_t.
* Adds helper to pack domain and type values into uintptr_t.
* Denies inserting socket rule with invalid family and type.
* Renames 'domain' to 'family' in landlock_socket_attr.
* Updates ABI version to 6 since ioctl patches changed it to 5.
* Formats code with clang-format.
* Minor fixes.
---
include/uapi/linux/landlock.h | 60 ++++++++++-
security/landlock/Makefile | 2 +-
security/landlock/access.h | 3 +
security/landlock/limits.h | 4 +
security/landlock/ruleset.c | 37 +++++--
security/landlock/ruleset.h | 46 ++++++--
security/landlock/socket.c | 105 +++++++++++++++++++
security/landlock/socket.h | 18 ++++
security/landlock/syscalls.c | 61 ++++++++++-
tools/testing/selftests/landlock/base_test.c | 2 +-
10 files changed, 319 insertions(+), 19 deletions(-)
create mode 100644 security/landlock/socket.c
create mode 100644 security/landlock/socket.h
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f030adc462ee..030c96cb5d25 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -45,6 +45,11 @@ struct landlock_ruleset_attr {
* flags`_).
*/
__u64 handled_access_net;
+ /**
+ * @handled_access_socket: Bitmask of handled actions performed on sockets
+ * (cf. `Socket flags`).
+ */
+ __u64 handled_access_socket;
/**
* @scoped: Bitmask of scopes (cf. `Scope flags`_)
* restricting a Landlock domain from accessing outside
@@ -140,6 +145,11 @@ enum landlock_rule_type {
* landlock_net_port_attr .
*/
LANDLOCK_RULE_NET_PORT,
+ /**
+ * @LANDLOCK_RULE_SOCKET: Type of a &struct
+ * landlock_socket_attr.
+ */
+ LANDLOCK_RULE_SOCKET,
};
/**
@@ -191,6 +201,33 @@ struct landlock_net_port_attr {
__u64 port;
};
+/**
+ * struct landlock_socket_attr - Socket protocol definition
+ *
+ * Argument of sys_landlock_add_rule().
+ */
+struct landlock_socket_attr {
+ /**
+ * @allowed_access: Bitmask of allowed access for a socket protocol
+ * (cf. `Socket flags`_).
+ */
+ __u64 allowed_access;
+ /**
+ * @family: Protocol family used for communication
+ * (cf. include/linux/socket.h).
+ */
+ __s32 family;
+ /**
+ * @type: Socket type (cf. include/linux/net.h)
+ */
+ __s32 type;
+ /**
+ * @protocol: Communication protocol specific to protocol family set in
+ * @family field.
+ */
+ __s32 protocol;
+} __attribute__((packed));
+
/**
* DOC: fs_access
*
@@ -327,7 +364,7 @@ struct landlock_net_port_attr {
* DOC: net_access
*
* Network flags
- * ~~~~~~~~~~~~~~~~
+ * ~~~~~~~~~~~~~
*
* These flags enable to restrict a sandboxed process to a set of network
* actions.
@@ -345,6 +382,27 @@ struct landlock_net_port_attr {
#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1)
/* clang-format on */
+/**
+ * DOC: socket_access
+ *
+ * Socket flags
+ * ~~~~~~~~~~~~
+ *
+ * These flags restrict actions on sockets for a sandboxed process (e.g. creation
+ * of a socket via socket(2)). Sockets opened before sandboxing are not subject
+ * to these restrictions. This is supported since the Landlock ABI version 8.
+ *
+ * The following access right applies only to sockets:
+ *
+ * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a user space socket. This access
+ * right restricts the following operations: :manpage:`socket(2)`,
+ * :manpage:`socketpair(2)`, ``IORING_OP_SOCKET`` (cf.
+ * :manpage:`io_uring_enter(2)`).
+ */
+/* clang-format off */
+#define LANDLOCK_ACCESS_SOCKET_CREATE (1ULL << 0)
+/* clang-format on */
+
/**
* DOC: scope
*
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 3160c2bdac1d..89f0d12d3af1 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,7 +1,7 @@
obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
landlock-y := setup.o syscalls.o object.o ruleset.o \
- cred.o task.o fs.o
+ cred.o task.o fs.o socket.o
landlock-$(CONFIG_INET) += net.o
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 7961c6630a2d..03ccd6fbfe83 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -40,6 +40,8 @@ typedef u16 access_mask_t;
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
/* Makes sure all network access rights can be stored. */
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
+/* Makes sure all socket access rights can be stored. */
+static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_SOCKET);
/* Makes sure all scoped rights can be stored. */
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE);
/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
@@ -49,6 +51,7 @@ static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
struct access_masks {
access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
+ access_mask_t socket : LANDLOCK_NUM_ACCESS_SOCKET;
access_mask_t scope : LANDLOCK_NUM_SCOPE;
};
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 65b5ff051674..f87f2e8f2644 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -27,6 +27,10 @@
#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET)
+#define LANDLOCK_LAST_ACCESS_SOCKET LANDLOCK_ACCESS_SOCKET_CREATE
+#define LANDLOCK_MASK_ACCESS_SOCKET ((LANDLOCK_LAST_ACCESS_SOCKET << 1) - 1)
+#define LANDLOCK_NUM_ACCESS_SOCKET __const_hweight64(LANDLOCK_MASK_ACCESS_SOCKET)
+
#define LANDLOCK_LAST_SCOPE LANDLOCK_SCOPE_SIGNAL
#define LANDLOCK_MASK_SCOPE ((LANDLOCK_LAST_SCOPE << 1) - 1)
#define LANDLOCK_NUM_SCOPE __const_hweight64(LANDLOCK_MASK_SCOPE)
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index dfcdc19ea268..a34d2dbe3954 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -45,6 +45,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
#if IS_ENABLED(CONFIG_INET)
new_ruleset->root_net_port = RB_ROOT;
#endif /* IS_ENABLED(CONFIG_INET) */
+ new_ruleset->root_socket = RB_ROOT;
new_ruleset->num_layers = num_layers;
/*
@@ -55,15 +56,15 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
return new_ruleset;
}
-struct landlock_ruleset *
-landlock_create_ruleset(const access_mask_t fs_access_mask,
- const access_mask_t net_access_mask,
- const access_mask_t scope_mask)
+struct landlock_ruleset *landlock_create_ruleset(
+ const access_mask_t fs_access_mask, const access_mask_t net_access_mask,
+ const access_mask_t socket_access_mask, const access_mask_t scope_mask)
{
struct landlock_ruleset *new_ruleset;
/* Informs about useless ruleset. */
- if (!fs_access_mask && !net_access_mask && !scope_mask)
+ if (!fs_access_mask && !net_access_mask && !socket_access_mask &&
+ !scope_mask)
return ERR_PTR(-ENOMSG);
new_ruleset = create_ruleset(1);
if (IS_ERR(new_ruleset))
@@ -72,6 +73,9 @@ landlock_create_ruleset(const access_mask_t fs_access_mask,
landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
if (net_access_mask)
landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
+ if (socket_access_mask)
+ landlock_add_socket_access_mask(new_ruleset, socket_access_mask,
+ 0);
if (scope_mask)
landlock_add_scope_mask(new_ruleset, scope_mask, 0);
return new_ruleset;
@@ -101,6 +105,8 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
return false;
#endif /* IS_ENABLED(CONFIG_INET) */
+ case LANDLOCK_KEY_SOCKET:
+ return false;
default:
WARN_ON_ONCE(1);
return false;
@@ -158,6 +164,8 @@ static struct rb_root *get_root(struct landlock_ruleset *const ruleset,
return &ruleset->root_net_port;
#endif /* IS_ENABLED(CONFIG_INET) */
+ case LANDLOCK_KEY_SOCKET:
+ return &ruleset->root_socket;
default:
WARN_ON_ONCE(1);
return ERR_PTR(-EINVAL);
@@ -396,6 +404,9 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
goto out_unlock;
#endif /* IS_ENABLED(CONFIG_INET) */
+ err = merge_tree(dst, src, LANDLOCK_KEY_SOCKET);
+ if (err)
+ goto out_unlock;
out_unlock:
mutex_unlock(&src->lock);
mutex_unlock(&dst->lock);
@@ -459,6 +470,11 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
goto out_unlock;
#endif /* IS_ENABLED(CONFIG_INET) */
+ /* Copies the @parent socket tree. */
+ err = inherit_tree(parent, child, LANDLOCK_KEY_SOCKET);
+ if (err)
+ goto out_unlock;
+
if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
err = -EINVAL;
goto out_unlock;
@@ -495,6 +511,9 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
free_rule(freeme, LANDLOCK_KEY_NET_PORT);
#endif /* IS_ENABLED(CONFIG_INET) */
+ rbtree_postorder_for_each_entry_safe(freeme, next,
+ &ruleset->root_socket, node)
+ free_rule(freeme, LANDLOCK_KEY_SOCKET);
landlock_put_hierarchy(ruleset->hierarchy);
kfree(ruleset);
}
@@ -679,8 +698,8 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
*
* @domain: The domain that defines the current restrictions.
* @access_request: The requested access rights to check.
- * @layer_masks: It must contain %LANDLOCK_NUM_ACCESS_FS or
- * %LANDLOCK_NUM_ACCESS_NET elements according to @key_type.
+ * @layer_masks: It must contain %LANDLOCK_NUM_ACCESS_{FS,NET,SOCKET}
+ * elements according to @key_type.
* @key_type: The key type to switch between access masks of different types.
*
* Returns: An access mask where each access right bit is set which is handled
@@ -709,6 +728,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
break;
#endif /* IS_ENABLED(CONFIG_INET) */
+ case LANDLOCK_KEY_SOCKET:
+ get_access_mask = landlock_get_socket_access_mask;
+ num_access = LANDLOCK_NUM_ACCESS_SOCKET;
+ break;
default:
WARN_ON_ONCE(1);
return 0;
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 1a78cba662b2..a60ede2fc2a5 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -66,6 +66,11 @@ enum landlock_key_type {
* node keys.
*/
LANDLOCK_KEY_NET_PORT,
+ /**
+ * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
+ * node keys.
+ */
+ LANDLOCK_KEY_SOCKET,
};
/**
@@ -135,6 +140,14 @@ struct landlock_ruleset {
struct rb_root root_net_port;
#endif /* IS_ENABLED(CONFIG_INET) */
+ /**
+ * @root_socket: Root of a red-black tree containing &struct
+ * landlock_rule nodes with socket protocol definition. Once a
+ * ruleset is tied to a process (i.e. as a domain), this tree is
+ * immutable until @usage reaches zero.
+ */
+ struct rb_root root_socket;
+
/**
* @hierarchy: Enables hierarchy identification even when a parent
* domain vanishes. This is needed for the ptrace protection.
@@ -173,8 +186,10 @@ struct landlock_ruleset {
*/
u32 num_layers;
/**
- * @access_masks: Contains the subset of filesystem and
- * network actions that are restricted by a ruleset.
+ * @access_masks: Contains the subset of filesystem,
+ * network and socket actions that are restricted by
+ * a ruleset.
+ *
* A domain saves all layers of merged rulesets in a
* stack (FAM), starting from the first layer to the
* last one. These layers are used when merging
@@ -189,10 +204,9 @@ struct landlock_ruleset {
};
};
-struct landlock_ruleset *
-landlock_create_ruleset(const access_mask_t access_mask_fs,
- const access_mask_t access_mask_net,
- const access_mask_t scope_mask);
+struct landlock_ruleset *landlock_create_ruleset(
+ const access_mask_t access_mask_fs, const access_mask_t access_mask_net,
+ const access_mask_t access_mask_socket, const access_mask_t scope_mask);
void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
@@ -267,6 +281,19 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset,
ruleset->access_masks[layer_level].net |= net_mask;
}
+static inline void
+landlock_add_socket_access_mask(struct landlock_ruleset *const ruleset,
+ const access_mask_t socket_access_mask,
+ const u16 layer_level)
+{
+ access_mask_t socket_mask = socket_access_mask &
+ LANDLOCK_MASK_ACCESS_SOCKET;
+
+ /* Should already be checked in sys_landlock_create_ruleset(). */
+ WARN_ON_ONCE(socket_access_mask != socket_mask);
+ ruleset->access_masks[layer_level].socket |= socket_mask;
+}
+
static inline void
landlock_add_scope_mask(struct landlock_ruleset *const ruleset,
const access_mask_t scope_mask, const u16 layer_level)
@@ -294,6 +321,13 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset,
return ruleset->access_masks[layer_level].net;
}
+static inline access_mask_t
+landlock_get_socket_access_mask(const struct landlock_ruleset *const ruleset,
+ const u16 layer_level)
+{
+ return ruleset->access_masks[layer_level].socket;
+}
+
static inline access_mask_t
landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
const u16 layer_level)
diff --git a/security/landlock/socket.c b/security/landlock/socket.c
new file mode 100644
index 000000000000..28a80dcad629
--- /dev/null
+++ b/security/landlock/socket.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - Socket management and hooks
+ *
+ * Copyright © 2025 Huawei Tech. Co., Ltd.
+ */
+
+#include <linux/net.h>
+#include <linux/socket.h>
+#include <linux/stddef.h>
+#include <net/ipv6.h>
+
+#include "limits.h"
+#include "ruleset.h"
+#include "socket.h"
+#include "cred.h"
+
+#define TYPE_ALL (-1)
+#define PROTOCOL_ALL (-1)
+
+static int pack_socket_key(const s32 family, const s32 type, const s32 protocol,
+ uintptr_t *val)
+{
+ int err = -EINVAL;
+ union {
+ struct {
+ u8 family;
+ u8 type;
+ u16 protocol;
+ } __packed data;
+ u32 packed;
+ } socket_key;
+
+ /* Checks that socket_key content can be stored in struct landlock_key. */
+ BUILD_BUG_ON(sizeof(socket_key.data) > sizeof(socket_key.packed));
+ BUILD_BUG_ON(sizeof(socket_key.packed) >
+ sizeof_field(union landlock_key, data));
+
+ /*
+ * Checks that all supported protocol families and socket types can be
+ * stored in socket_key fields.
+ */
+ BUILD_BUG_ON(AF_MAX - 1 > U8_MAX);
+ BUILD_BUG_ON(SOCK_MAX - 1 > U8_MAX);
+
+ /* Checks ranges and handles wildcard type and protocol value mapping. */
+ if (family >= 0 && family < U8_MAX)
+ socket_key.data.family = family;
+ else
+ goto out;
+
+ BUILD_BUG_ON(TYPE_ALL != -1);
+ if (type == TYPE_ALL)
+ socket_key.data.type = U8_MAX;
+ else if (type >= 0 && type < U8_MAX)
+ socket_key.data.type = type;
+ else
+ goto out;
+
+ BUILD_BUG_ON(PROTOCOL_ALL != -1);
+ if (protocol == PROTOCOL_ALL)
+ socket_key.data.protocol = U16_MAX;
+ else if (protocol >= 0 && protocol < U16_MAX)
+ socket_key.data.protocol = protocol;
+ else
+ goto out;
+
+ *val = socket_key.packed;
+ err = 0;
+out:
+ return err;
+}
+
+int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
+ s32 family, s32 type, s32 protocol,
+ access_mask_t access_rights)
+{
+ int err;
+ uintptr_t key;
+ /*
+ * (AF_INET, SOCK_PACKET) is an alias for (AF_PACKET, SOCK_PACKET)
+ * (cf. __sock_create).
+ */
+ if (family == AF_INET && type == SOCK_PACKET)
+ family = AF_PACKET;
+
+ err = pack_socket_key(family, type, protocol, &key);
+ if (err)
+ return err;
+
+ const struct landlock_id id = {
+ .key.data = key,
+ .type = LANDLOCK_KEY_SOCKET,
+ };
+
+ /* Transforms relative access rights to absolute ones. */
+ access_rights |= LANDLOCK_MASK_ACCESS_SOCKET &
+ ~landlock_get_socket_access_mask(ruleset, 0);
+
+ mutex_lock(&ruleset->lock);
+ err = landlock_insert_rule(ruleset, id, access_rights);
+ mutex_unlock(&ruleset->lock);
+
+ return err;
+}
diff --git a/security/landlock/socket.h b/security/landlock/socket.h
new file mode 100644
index 000000000000..bd0ac74c39e2
--- /dev/null
+++ b/security/landlock/socket.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - Socket management and hooks
+ *
+ * Copyright © 2025 Huawei Tech. Co., Ltd.
+ */
+
+#ifndef _SECURITY_LANDLOCK_SOCKET_H
+#define _SECURITY_LANDLOCK_SOCKET_H
+
+#include "ruleset.h"
+
+int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
+ const s32 family, const s32 type,
+ const s32 protocol,
+ access_mask_t access_rights);
+
+#endif /* _SECURITY_LANDLOCK_SOCKET_H */
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 33eafb71e4f3..e9f500f97c86 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -27,6 +27,7 @@
#include <linux/syscalls.h>
#include <linux/types.h>
#include <linux/uaccess.h>
+#include <linux/net.h>
#include <uapi/linux/landlock.h>
#include "cred.h"
@@ -34,6 +35,7 @@
#include "fs.h"
#include "limits.h"
#include "net.h"
+#include "socket.h"
#include "ruleset.h"
#include "setup.h"
@@ -92,7 +94,8 @@ static void build_check_abi(void)
struct landlock_ruleset_attr ruleset_attr;
struct landlock_path_beneath_attr path_beneath_attr;
struct landlock_net_port_attr net_port_attr;
- size_t ruleset_size, path_beneath_size, net_port_size;
+ struct landlock_socket_attr socket_attr;
+ size_t ruleset_size, path_beneath_size, net_port_size, socket_size;
/*
* For each user space ABI structures, first checks that there is no
@@ -101,9 +104,10 @@ static void build_check_abi(void)
*/
ruleset_size = sizeof(ruleset_attr.handled_access_fs);
ruleset_size += sizeof(ruleset_attr.handled_access_net);
+ ruleset_size += sizeof(ruleset_attr.handled_access_socket);
ruleset_size += sizeof(ruleset_attr.scoped);
BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
- BUILD_BUG_ON(sizeof(ruleset_attr) != 24);
+ BUILD_BUG_ON(sizeof(ruleset_attr) != 32);
path_beneath_size = sizeof(path_beneath_attr.allowed_access);
path_beneath_size += sizeof(path_beneath_attr.parent_fd);
@@ -114,6 +118,13 @@ static void build_check_abi(void)
net_port_size += sizeof(net_port_attr.port);
BUILD_BUG_ON(sizeof(net_port_attr) != net_port_size);
BUILD_BUG_ON(sizeof(net_port_attr) != 16);
+
+ socket_size = sizeof(socket_attr.allowed_access);
+ socket_size += sizeof(socket_attr.family);
+ socket_size += sizeof(socket_attr.type);
+ socket_size += sizeof(socket_attr.protocol);
+ BUILD_BUG_ON(sizeof(socket_attr) != socket_size);
+ BUILD_BUG_ON(sizeof(socket_attr) != 20);
}
/* Ruleset handling */
@@ -161,7 +172,7 @@ static const struct file_operations ruleset_fops = {
* Documentation/userspace-api/landlock.rst should be updated to reflect the
* UAPI change.
*/
-const int landlock_abi_version = 7;
+const int landlock_abi_version = 8;
/**
* sys_landlock_create_ruleset - Create a new ruleset
@@ -237,6 +248,11 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
LANDLOCK_MASK_ACCESS_NET)
return -EINVAL;
+ /* Checks socket content (and 32-bits cast). */
+ if ((ruleset_attr.handled_access_socket |
+ LANDLOCK_MASK_ACCESS_SOCKET) != LANDLOCK_MASK_ACCESS_SOCKET)
+ return -EINVAL;
+
/* Checks IPC scoping content (and 32-bits cast). */
if ((ruleset_attr.scoped | LANDLOCK_MASK_SCOPE) != LANDLOCK_MASK_SCOPE)
return -EINVAL;
@@ -244,6 +260,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
/* Checks arguments and transforms to kernel struct. */
ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs,
ruleset_attr.handled_access_net,
+ ruleset_attr.handled_access_socket,
ruleset_attr.scoped);
if (IS_ERR(ruleset))
return PTR_ERR(ruleset);
@@ -383,6 +400,40 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
net_port_attr.allowed_access);
}
+static int add_rule_socket(struct landlock_ruleset *ruleset,
+ const void __user *const rule_attr)
+{
+ struct landlock_socket_attr socket_attr;
+ int res;
+ access_mask_t mask;
+ s32 family, type, protocol;
+
+ /* Copies raw user space buffer. */
+ res = copy_from_user(&socket_attr, rule_attr, sizeof(socket_attr));
+ if (res)
+ return -EFAULT;
+
+ /*
+ * Informs about useless rule: empty allowed_access (i.e. deny rules)
+ * are ignored by socket actions.
+ */
+ if (!socket_attr.allowed_access)
+ return -ENOMSG;
+
+ /* Checks that allowed_access matches the @ruleset constraints. */
+ mask = landlock_get_socket_access_mask(ruleset, 0);
+ if ((socket_attr.allowed_access | mask) != mask)
+ return -EINVAL;
+
+ family = socket_attr.family;
+ type = socket_attr.type;
+ protocol = socket_attr.protocol;
+
+ /* Imports the new rule. */
+ return landlock_append_socket_rule(ruleset, family, type, protocol,
+ socket_attr.allowed_access);
+}
+
/**
* sys_landlock_add_rule - Add a new rule to a ruleset
*
@@ -407,6 +458,8 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
* &landlock_net_port_attr.allowed_access is not a subset of the ruleset
* handled accesses)
* - %EINVAL: &landlock_net_port_attr.port is greater than 65535;
+ * - %EINVAL: &landlock_socket_attr.{family, type} are greater than 254 or
+ * &landlock_socket_attr.protocol is greater than 65534;
* - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access is
* 0);
* - %EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
@@ -439,6 +492,8 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
return add_rule_path_beneath(ruleset, rule_attr);
case LANDLOCK_RULE_NET_PORT:
return add_rule_net_port(ruleset, rule_attr);
+ case LANDLOCK_RULE_SOCKET:
+ return add_rule_socket(ruleset, rule_attr);
default:
return -EINVAL;
}
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,
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 03/19] selftests/landlock: Test adding a socket rule
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test that validates behaviour of Landlock after rule of
LANDLOCK_RULE_SOCKET type is added
* with all possible access rights;
* with unknown access rights;
* with unhandled access rights;
* with empty access.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Squashes commits related to checking rule adding
* Removes "protocol" fixture dependency. Use single TCP protocol for
testing instead.
* Fixes semantics of test "rule_with_unhandled_access".
Changes since v2:
* Replaces EXPECT_EQ with ASSERT_EQ for close().
* Refactors commit message and title.
Changes since v1:
* Formats code with clang-format.
* Refactors commit message.
---
.../testing/selftests/landlock/socket_test.c | 107 ++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index d5716149d03f..fcc0f92075a7 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -44,4 +44,111 @@ TEST_F(mini, ruleset_with_unknown_access)
}
}
+TEST_F(mini, rule_with_supported_access)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = ACCESS_ALL,
+ };
+ struct landlock_socket_attr protocol = {
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+ int ruleset_fd;
+ __u64 access;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ for (access = 1; access <= ACCESS_LAST; access <<= 1) {
+ protocol.allowed_access = access;
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &protocol, 0))
+ {
+ TH_LOG("Failed to add rule with access 0x%llx: %s",
+ access, strerror(errno));
+ }
+ }
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(mini, rule_with_unknown_access)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = ACCESS_ALL,
+ };
+ struct landlock_socket_attr protocol = { .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0 };
+ int ruleset_fd;
+ __u64 access;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
+ protocol.allowed_access = access;
+ EXPECT_EQ(-1,
+ landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &protocol, 0));
+ EXPECT_EQ(EINVAL, errno);
+ }
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(mini, rule_with_unhandled_access)
+{
+ /* Prepares ruleset that handles network access instead of socket access. */
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP,
+ };
+ struct landlock_socket_attr protocol = { .family = AF_UNIX,
+ .type = SOCK_STREAM,
+ .protocol = 0 };
+ int ruleset_fd;
+ __u64 access;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ for (access = ACCESS_LAST; access > 0; access <<= 1) {
+ int err;
+
+ protocol.allowed_access = access;
+ err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &protocol, 0);
+ EXPECT_EQ(-1, err);
+ EXPECT_EQ(EINVAL, errno);
+ }
+
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(mini, rule_with_empty_access)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE
+ };
+ const struct landlock_socket_attr protocol_denied = {
+ .allowed_access = 0,
+ .family = AF_UNIX,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Checks zero access value. */
+ EXPECT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &protocol_denied, 0));
+ EXPECT_EQ(ENOMSG, errno);
+ ASSERT_EQ(0, close(ruleset_fd));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 00/19] Support socket access-control
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
Hello! This is v4 RFC patch dedicated to socket protocols restriction.
It is based on the landlock's mic-next branch on top of Linux 6.16-rc2
kernel version.
Objective
=========
Extend Landlock with a mechanism to restrict any set of protocols in
a sandboxed process.
Closes: https://github.com/landlock-lsm/linux/issues/6
Motivation
==========
Landlock implements the `LANDLOCK_RULE_NET_PORT` rule type, which provides
fine-grained control of actions for a specific protocol. Any action or
protocol that is not supported by this rule can not be controlled. As a
result, protocols for which fine-grained control is not supported can be
used in a sandboxed system and lead to vulnerabilities or unexpected
behavior.
Controlling the protocols used will allow to use only those that are
necessary for the system and/or which have fine-grained Landlock control
through others types of rules (e.g. TCP bind/connect control with
`LANDLOCK_RULE_NET_PORT`, UNIX bind control with
`LANDLOCK_RULE_PATH_BENEATH`).
Consider following examples:
* Server may want to use only TCP sockets for which there is fine-grained
control of bind(2) and connect(2) actions [1].
* System that does not need a network or that may want to disable network
for security reasons (e.g. [2]) can achieve this by restricting the use
of all possible protocols.
[1] https://lore.kernel.org/all/ZJvy2SViorgc+cZI@google.com/
[2] https://cr.yp.to/unix/disablenetwork.html
Implementation
==============
This patchset adds control over the protocols used by implementing a
restriction of socket creation. This is possible thanks to the new type
of rule - `LANDLOCK_RULE_SOCKET`, that allows to restrict actions on
sockets, and a new access right - `LANDLOCK_ACCESS_SOCKET_CREATE`, that
corresponds to user space sockets creation. The key in this rule
corresponds to communication protocol signature from socket(2) syscall.
The right to create a socket is checked in the LSM hook which is called
in the __sock_create method. The following user space operations are
subject to this check: socket(2), socketpair(2), io_uring(7).
`LANDLOCK_ACCESS_SOCKET_CREATE` does not restrict socket creation
performed by accept(2), because created socket is used for messaging
between already existing endpoints.
Design discussion
===================
1. Should `SCTP_SOCKOPT_PEELOFF` and socketpair(2) be restricted?
SCTP socket can be connected to a multiple endpoints (one-to-many
relation). Calling setsockopt(2) on such socket with option
`SCTP_SOCKOPT_PEELOFF` detaches one of existing connections to a separate
UDP socket. This detach is currently restrictable.
Same applies for the socketpair(2) syscall. It was noted that denying
usage of socketpair(2) in sandboxed environment may be not meaninful [1].
Currently both operations use general socket interface to create sockets.
Therefore it's not possible to distinguish between socket(2) and those
operations inside security_socket_create LSM hook which is currently
used for protocols restriction. Providing such separation may require
changes in socket layer (eg. in __sock_create) interface which may not be
acceptable.
[1] https://lore.kernel.org/all/ZurZ7nuRRl0Zf2iM@google.com/
Code coverage
=============
Code coverage(gcov) report with the launch of all the landlock selftests:
* security/landlock:
lines......: 94.0% (1200 of 1276 lines)
functions..: 95.0% (134 of 141 functions)
* security/landlock/socket.c:
lines......: 100.0% (56 of 56 lines)
functions..: 100.0% (5 of 5 functions)
Currently landlock-test-tools fails on mini.kernel_socket test due to lack
of SMC protocol support.
General changes v3->v4
======================
* Implementation
* Adds protocol field to landlock_socket_attr.
* Adds protocol masks support via wildcards values in
landlock_socket_attr.
* Changes LSM hook used from socket_post_create to socket_create.
* Changes protocol ranges acceptable by socket rules.
* Adds audit support.
* Changes ABI version to 8.
* Tests
* Adds 5 new tests:
* mini.rule_with_wildcard, protocol_wildcard.access,
mini.ruleset_with_wildcards_overlap:
verify rulesets containing rules with wildcard values.
* tcp_protocol.alias_restriction: verify that Landlock doesn't
perform protocol mappings.
* audit.socket_create: tests audit denial logging.
* Squashes tests corresponding to Landlock rule adding to a single commit.
* Documentation
* Refactors Documentation/userspace-api/landlock.rst.
* Commits
* Rebases on mic-next.
* Refactors commits.
Previous versions
=================
v3: https://lore.kernel.org/all/20240904104824.1844082-1-ivanov.mikhail1@huawei-partners.com/
v2: https://lore.kernel.org/all/20240524093015.2402952-1-ivanov.mikhail1@huawei-partners.com/
v1: https://lore.kernel.org/all/20240408093927.1759381-1-ivanov.mikhail1@huawei-partners.com/
Mikhail Ivanov (19):
landlock: Support socket access-control
selftests/landlock: Test creating a ruleset with unknown access
selftests/landlock: Test adding a socket rule
selftests/landlock: Testing adding rule with wildcard value
selftests/landlock: Test acceptable ranges of socket rule key
landlock: Add hook on socket creation
selftests/landlock: Test basic socket restriction
selftests/landlock: Test network stack error code consistency
selftests/landlock: Test overlapped rulesets with rules of protocol
ranges
selftests/landlock: Test that kernel space sockets are not restricted
selftests/landlock: Test protocol mappings
selftests/landlock: Test socketpair(2) restriction
selftests/landlock: Test SCTP peeloff restriction
selftests/landlock: Test that accept(2) is not restricted
lsm: Support logging socket common data
landlock: Log socket creation denials
selftests/landlock: Test socket creation denial log for audit
samples/landlock: Support socket protocol restrictions
landlock: Document socket rule type support
Documentation/userspace-api/landlock.rst | 48 +-
include/linux/lsm_audit.h | 8 +
include/uapi/linux/landlock.h | 60 +-
samples/landlock/sandboxer.c | 118 +-
security/landlock/Makefile | 2 +-
security/landlock/access.h | 3 +
security/landlock/audit.c | 12 +
security/landlock/audit.h | 1 +
security/landlock/limits.h | 4 +
security/landlock/ruleset.c | 37 +-
security/landlock/ruleset.h | 46 +-
security/landlock/setup.c | 2 +
security/landlock/socket.c | 198 +++
security/landlock/socket.h | 20 +
security/landlock/syscalls.c | 61 +-
security/lsm_audit.c | 4 +
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/common.h | 14 +
tools/testing/selftests/landlock/config | 47 +
tools/testing/selftests/landlock/net_test.c | 11 -
.../selftests/landlock/protocols_define.h | 169 +++
.../testing/selftests/landlock/socket_test.c | 1169 +++++++++++++++++
22 files changed, 1990 insertions(+), 46 deletions(-)
create mode 100644 security/landlock/socket.c
create mode 100644 security/landlock/socket.h
create mode 100644 tools/testing/selftests/landlock/protocols_define.h
create mode 100644 tools/testing/selftests/landlock/socket_test.c
base-commit: 6dde339a3df80a57ac3d780d8cfc14d9262e2acd
--
2.34.1
^ permalink raw reply
* [RFC PATCH v4 14/19] selftests/landlock: Test that accept(2) is not restricted
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test validating that socket creation with accept(2) is not restricted
by Landlock.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Minor fixes.
---
.../testing/selftests/landlock/socket_test.c | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index e9f56a86f456..ea1590e555b7 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -1049,4 +1049,66 @@ TEST_F(connection_restriction, sctp_peeloff)
ASSERT_EQ(0, close(server_fd));
}
+TEST_F(connection_restriction, accept)
+{
+ int status;
+ pid_t child;
+ struct sockaddr_in addr;
+ int server_fd, client_fd;
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+
+ server_fd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ ASSERT_LE(0, server_fd);
+
+ addr.sin_family = AF_INET;
+ addr.sin_port = htons(loopback_port);
+ addr.sin_addr.s_addr = inet_addr(loopback_ipv4);
+
+ ASSERT_EQ(0, bind(server_fd, &addr, sizeof(addr)));
+ ASSERT_EQ(0, listen(server_fd, backlog));
+
+ child = fork();
+ ASSERT_LE(0, child);
+ if (child == 0) {
+ /* Connects to server once and exits. */
+
+ /* Closes listening socket for the child. */
+ ASSERT_EQ(0, close(server_fd));
+
+ client_fd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ ASSERT_LE(0, client_fd);
+
+ ASSERT_EQ(0, connect(client_fd, &addr, sizeof(addr)));
+
+ ASSERT_EQ(0, close(client_fd));
+ _exit(_metadata->exit_code);
+ return;
+ }
+
+ if (variant->sandboxed) {
+ int ruleset_fd;
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+ }
+
+ client_fd = accept(server_fd, NULL, 0);
+
+ /* accept(2) should not be restricted by Landlock. */
+ ASSERT_LE(0, client_fd);
+
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ ASSERT_EQ(1, WIFEXITED(status));
+ ASSERT_EQ(EXIT_SUCCESS, WEXITSTATUS(status));
+
+ ASSERT_EQ(0, close(server_fd));
+ ASSERT_EQ(0, close(client_fd));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: Helge Deller @ 2025-11-18 12:49 UTC (permalink / raw)
To: John Paul Adrian Glaubitz, Helge Deller
Cc: linux-kernel, apparmor, John Johansen, linux-security-module
In-Reply-To: <90513f85cc8d060ebccd3972cc7709e4b6f13f34.camel@physik.fu-berlin.de>
Hi Adrian,
On 11/18/25 12:43, John Paul Adrian Glaubitz wrote:
> On Tue, 2025-11-18 at 12:09 +0100, Helge Deller wrote:
>> My patch fixed two call sites, but I suspect you see another call site which
>> hasn't been fixed yet.
>>
>> Can you try attached patch? It might indicate the caller of the function and
>> maybe prints the struct name/address which isn't aligned.
>>
>> Helge
>>
>>
>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>> index c5a91600842a..b477430c07eb 100644
>> --- a/security/apparmor/match.c
>> +++ b/security/apparmor/match.c
>> @@ -313,6 +313,9 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
>> if (size < sizeof(struct table_set_header))
>> goto fail;
>>
>> + if (WARN_ON(((unsigned long)data) & (BITS_PER_LONG/8 - 1)))
>> + pr_warn("dfa blob stream %pS not aligned.\n", data);
>> +
>> if (ntohl(*(__be32 *) data) != YYTH_MAGIC)
>> goto fail;
>
> Here is the relevant output with the patch applied:
>
> [ 73.840639] ------------[ cut here ]------------
> [ 73.901376] WARNING: CPU: 0 PID: 341 at security/apparmor/match.c:316 aa_dfa_unpack+0x6cc/0x720
> [ 74.015867] Modules linked in: binfmt_misc evdev flash sg drm drm_panel_orientation_quirks backlight i2c_core configfs nfnetlink autofs4 ext4 crc16 mbcache jbd2 hid_generic usbhid sr_mod hid cdrom
> sd_mod ata_generic ohci_pci ehci_pci ehci_hcd ohci_hcd pata_ali libata sym53c8xx scsi_transport_spi tg3 scsi_mod usbcore libphy scsi_common mdio_bus usb_common
> [ 74.428977] CPU: 0 UID: 0 PID: 341 Comm: apparmor_parser Not tainted 6.18.0-rc6+ #9 NONE
> [ 74.536543] Call Trace:
> [ 74.568561] [<0000000000434c24>] dump_stack+0x8/0x18
> [ 74.633757] [<0000000000476438>] __warn+0xd8/0x100
> [ 74.696664] [<00000000004296d4>] warn_slowpath_fmt+0x34/0x74
> [ 74.771006] [<00000000008db28c>] aa_dfa_unpack+0x6cc/0x720
> [ 74.843062] [<00000000008e643c>] unpack_pdb+0xbc/0x7e0
> [ 74.910545] [<00000000008e7740>] unpack_profile+0xbe0/0x1300
> [ 74.984888] [<00000000008e82e0>] aa_unpack+0xe0/0x6a0
> [ 75.051226] [<00000000008e3ec4>] aa_replace_profiles+0x64/0x1160
> [ 75.130144] [<00000000008d4d90>] policy_update+0xf0/0x280
> [ 75.201057] [<00000000008d4fc8>] profile_replace+0xa8/0x100
> [ 75.274258] [<0000000000766bd0>] vfs_write+0x90/0x420
> [ 75.340594] [<00000000007670cc>] ksys_write+0x4c/0xe0
> [ 75.406932] [<0000000000767174>] sys_write+0x14/0x40
> [ 75.472126] [<0000000000406174>] linux_sparc_syscall+0x34/0x44
> [ 75.548802] ---[ end trace 0000000000000000 ]---
> [ 75.609503] dfa blob stream 0xfff0000008926b96 not aligned.
> [ 75.682695] Kernel unaligned access at TPC[8db2a8] aa_dfa_unpack+0x6e8/0x720
The non-8-byte-aligned address (0xfff0000008926b96) is coming from userspace
(via the write syscall).
Some apparmor userspace tool writes into the apparmor ".replace" virtual file with
a source address which is not correctly aligned.
You should be able to debug/find the problematic code with strace from userspace.
Maybe someone with apparmor knowledge here on the list has an idea?
Helge
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-18 12:19 UTC (permalink / raw)
To: Coiby Xu
Cc: Paul Moore, linux-integrity, linux-security-module, Karel Srot,
James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <42qcfcxxlmwphctzvji76hy5tycfabiiv5u6zw6lgg2p3e2jwv@fp4g2y7ecf2y>
On Thu, 2025-11-13 at 12:06 +0800, Coiby Xu wrote:
> On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
> > On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
> > > On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> > > > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > > > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> > > > [...]
> > > > >
> > > > > Hi Coiby,
> > > > >
> > > > > Based on the conversation with Paul, there is no reason to remove the existing
> > > > > security_kernel_post_read_file() call.
> > > > >
> > > > > The changes are similar to the 2nd link, but a bit different.
> > > > > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > > > >
> > > > > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > > > > after decompressing the kernel module. Like a previous version of this patch,
> > > > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > > > > based on MODULE_INIT_COMPRESSED_FILE.
> > > > >
> > > > > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > > > > READING_MODULE_COMPRESSED. (No need for a new function ima_read_kernel_module.)
> > > >
> > > > Hi Mimi,
> > > >
> > > > Thanks for summarizing your conversation with Paul! I can confirm Paul's
> > > > approach works
> > > > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> > > >
> > > > While testing the patch today, I realized there is another
> > > > issue/challenge introduced by in-kernel module decompression. IMA
> > > > appraisal is to verify the digest of compressed kernel module but
> > > > currently the passed buffer is uncompressed module. When IMA uses
> > > > uncompressed module data to calculate the digest, xattr signature
> > > > verification will fail. If we always make IMA read the original kernel
> > > > module data again to calculate the digest, does it look like a
> > > > quick-and-dirty fix? If we can assume people won't load kernel module so
> > > > often, the performance impact is negligible. Otherwise we may have to
> > > > introduce a new LSM hook so IMA can access uncompressed and original
> > > > module data one time.
> > >
> > > ima_collect_measurement() stores the file hash info in the iint and uses that
> > > information to verify the signature as stored in the security xattr.
> > > Decompressing the kernel module shouldn't affect the xattr signature
> > > verification.
> >
> > In the case when the compressed kernel module hasn't previously been measured or
> > appraised before loading the kernel module, we need to "collect" the file data
> > hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
> >
> > An alternative to your suggestion of re-reading the original kernel module data
> > to calculate the digest or defining a new hook, would be to define "collect" as
> > a new "action" and pass the kernel_read_file_id enumeration to
> > process_measurement(). IMA_COLLECTED already exists. Only IMA_COLLECT would
> > need to be defined. The new collect "action" should be limited to
> > func=MODULE_CHECK.
> >
> > The downside of this alternative is that it requires a new collect rule:
> > collect func=MODULE_CHECK mask=MAY_READ uid=0
> > appraise func=MODULE_CHECK appraise_type=imasig|modsig
As it turns out, the "collect" rule is unnecessary. On
READING_MODULE_COMPRESSED, process_measurement() should calculate the compressed
file hash. Extending the IMA measurement list and verifying the signature can
then be differed to READING_MODULE.
>
> Thank for suggesting an alternative! I've implemented the idea in
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect
>
> Note besides a new collect rule, another change is needed. Currently,
> process_measurement only accepts enum ima_hooks thus it can't tell if
> it's READING_MODULE_COMPRESSED so to only do collect action. So I
> create a fake MODULE_COMPRESSED_CHECK func.
Correct, either extending process_measurement() with the read_idmap enum or
defining the fake hook would work.
>
> And for the idea of re-reading the original kernel module data, it has
> been implemented in
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>
> Both branches have applied your requested three changes including
> respecting the 80 char line limit. Additionally, I made a change to the
> IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
> enumerate.
>
> After comparing the two implementations, personally I prefer re-reading
> the original kernel module data because the change is smaller and it's
> more user-friendly. But if there are other reasons I don't know, I'll
> post the patches of the new collect action approach to the mailing list.
The "re-reading" option fails some of the tests. As the "collect" rule isn't
needed, let's stick with the first option.
--
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Paul Adrian Glaubitz @ 2025-11-18 11:43 UTC (permalink / raw)
To: Helge Deller
Cc: linux-kernel, apparmor, John Johansen, linux-security-module,
Helge Deller
In-Reply-To: <aRxT78fdN5v2Ajyl@p100>
Hi Helge,
On Tue, 2025-11-18 at 12:09 +0100, Helge Deller wrote:
> My patch fixed two call sites, but I suspect you see another call site which
> hasn't been fixed yet.
>
> Can you try attached patch? It might indicate the caller of the function and
> maybe prints the struct name/address which isn't aligned.
>
> Helge
>
>
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index c5a91600842a..b477430c07eb 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -313,6 +313,9 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
> if (size < sizeof(struct table_set_header))
> goto fail;
>
> + if (WARN_ON(((unsigned long)data) & (BITS_PER_LONG/8 - 1)))
> + pr_warn("dfa blob stream %pS not aligned.\n", data);
> +
> if (ntohl(*(__be32 *) data) != YYTH_MAGIC)
> goto fail;
Here is the relevant output with the patch applied:
[ 73.840639] ------------[ cut here ]------------
[ 73.901376] WARNING: CPU: 0 PID: 341 at security/apparmor/match.c:316 aa_dfa_unpack+0x6cc/0x720
[ 74.015867] Modules linked in: binfmt_misc evdev flash sg drm drm_panel_orientation_quirks backlight i2c_core configfs nfnetlink autofs4 ext4 crc16 mbcache jbd2 hid_generic usbhid sr_mod hid cdrom
sd_mod ata_generic ohci_pci ehci_pci ehci_hcd ohci_hcd pata_ali libata sym53c8xx scsi_transport_spi tg3 scsi_mod usbcore libphy scsi_common mdio_bus usb_common
[ 74.428977] CPU: 0 UID: 0 PID: 341 Comm: apparmor_parser Not tainted 6.18.0-rc6+ #9 NONE
[ 74.536543] Call Trace:
[ 74.568561] [<0000000000434c24>] dump_stack+0x8/0x18
[ 74.633757] [<0000000000476438>] __warn+0xd8/0x100
[ 74.696664] [<00000000004296d4>] warn_slowpath_fmt+0x34/0x74
[ 74.771006] [<00000000008db28c>] aa_dfa_unpack+0x6cc/0x720
[ 74.843062] [<00000000008e643c>] unpack_pdb+0xbc/0x7e0
[ 74.910545] [<00000000008e7740>] unpack_profile+0xbe0/0x1300
[ 74.984888] [<00000000008e82e0>] aa_unpack+0xe0/0x6a0
[ 75.051226] [<00000000008e3ec4>] aa_replace_profiles+0x64/0x1160
[ 75.130144] [<00000000008d4d90>] policy_update+0xf0/0x280
[ 75.201057] [<00000000008d4fc8>] profile_replace+0xa8/0x100
[ 75.274258] [<0000000000766bd0>] vfs_write+0x90/0x420
[ 75.340594] [<00000000007670cc>] ksys_write+0x4c/0xe0
[ 75.406932] [<0000000000767174>] sys_write+0x14/0x40
[ 75.472126] [<0000000000406174>] linux_sparc_syscall+0x34/0x44
[ 75.548802] ---[ end trace 0000000000000000 ]---
[ 75.609503] dfa blob stream 0xfff0000008926b96 not aligned.
[ 75.682695] Kernel unaligned access at TPC[8db2a8] aa_dfa_unpack+0x6e8/0x720
[ 75.775337] Kernel unaligned access at TPC[8dac18] aa_dfa_unpack+0x58/0x720
[ 75.866840] Kernel unaligned access at TPC[8dad00] aa_dfa_unpack+0x140/0x720
[ 75.959481] Kernel unaligned access at TPC[8dad00] aa_dfa_unpack+0x140/0x720
[ 76.052125] Kernel unaligned access at TPC[8dad00] aa_dfa_unpack+0x140/0x720
[ 76.146188] ------------[ cut here ]------------
[ 76.206858] WARNING: CPU: 0 PID: 341 at security/apparmor/match.c:316 aa_dfa_unpack+0x6cc/0x720
[ 76.321326] Modules linked in: binfmt_misc evdev flash sg drm drm_panel_orientation_quirks backlight i2c_core configfs nfnetlink autofs4 ext4 crc16 mbcache jbd2 hid_generic usbhid sr_mod hid cdrom
sd_mod ata_generic ohci_pci ehci_pci ehci_hcd ohci_hcd pata_ali libata sym53c8xx scsi_transport_spi tg3 scsi_mod usbcore libphy scsi_common mdio_bus usb_common
[ 76.734360] CPU: 0 UID: 0 PID: 341 Comm: apparmor_parser Tainted: G W 6.18.0-rc6+ #9 NONE
[ 76.862518] Tainted: [W]=WARN
[ 76.901396] Call Trace:
[ 76.933421] [<0000000000434c24>] dump_stack+0x8/0x18
[ 76.998616] [<0000000000476438>] __warn+0xd8/0x100
[ 77.061522] [<00000000004296d4>] warn_slowpath_fmt+0x34/0x74
[ 77.135867] [<00000000008db28c>] aa_dfa_unpack+0x6cc/0x720
[ 77.207923] [<00000000008e643c>] unpack_pdb+0xbc/0x7e0
[ 77.275405] [<00000000008e71dc>] unpack_profile+0x67c/0x1300
[ 77.349749] [<00000000008e82e0>] aa_unpack+0xe0/0x6a0
[ 77.416084] [<00000000008e3ec4>] aa_replace_profiles+0x64/0x1160
[ 77.495003] [<00000000008d4d90>] policy_update+0xf0/0x280
[ 77.565915] [<00000000008d4fc8>] profile_replace+0xa8/0x100
[ 77.639116] [<0000000000766bd0>] vfs_write+0x90/0x420
[ 77.705454] [<00000000007670cc>] ksys_write+0x4c/0xe0
[ 77.771792] [<0000000000767174>] sys_write+0x14/0x40
[ 77.836986] [<0000000000406174>] linux_sparc_syscall+0x34/0x44
[ 77.913633] ---[ end trace 0000000000000000 ]---
The message is repeated multiple times.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: Helge Deller @ 2025-11-18 11:09 UTC (permalink / raw)
To: John Paul Adrian Glaubitz
Cc: linux-kernel, apparmor, John Johansen, linux-security-module,
Helge Deller
In-Reply-To: <bc21bee14ca44077ae9323bfc251ad12390fa841.camel@physik.fu-berlin.de>
Hi Adrian,
* John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>:
> On Sat, 2025-05-31 at 17:08 +0200, deller@kernel.org wrote:
> > From: Helge Deller <deller@gmx.de>
> >
> > Two patches which fix unaligned memory accesses in apparmor.
> > Both triggered on the parisc platform, which is much more
> > memory alignment sensitive and will report violations.
> > Please check and apply.
> >
> > Helge
> >
> > Helge Deller (2):
> > apparmor: Fix 8-byte alignment for initial dfa blob streams
> > apparmor: Fix unaligned memory accesses in KUnit test
> >
> > security/apparmor/lsm.c | 4 ++--
> > security/apparmor/policy_unpack_test.c | 6 ++++--
> > 2 files changed, 6 insertions(+), 4 deletions(-)
>
> Thanks for looking into this!
>
> Unfortunately, the problem still persists on SPARC even with v6.18-rc6:
>
> [ 76.209476] Kernel unaligned access at TPC[8dabfc] aa_dfa_unpack+0x3c/0x6e0
> [ 76.301115] Kernel unaligned access at TPC[8dac0c] aa_dfa_unpack+0x4c/0x6e0
> [ 76.392697] Kernel unaligned access at TPC[8dacf0] aa_dfa_unpack+0x130/0x6e0
> [ 76.485440] Kernel unaligned access at TPC[8dacf0] aa_dfa_unpack+0x130/0x6e0
> [ 76.578179] Kernel unaligned access at TPC[8dacf0] aa_dfa_unpack+0x130/0x6e0
>
> I have documented the problem here [1].
> [1] https://github.com/sparclinux/issues/issues/30
>
> So, I suspect that your fix is incomplete.
My patch fixed two call sites, but I suspect you see another call site which
hasn't been fixed yet.
Can you try attached patch? It might indicate the caller of the function and
maybe prints the struct name/address which isn't aligned.
Helge
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index c5a91600842a..b477430c07eb 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -313,6 +313,9 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
if (size < sizeof(struct table_set_header))
goto fail;
+ if (WARN_ON(((unsigned long)data) & (BITS_PER_LONG/8 - 1)))
+ pr_warn("dfa blob stream %pS not aligned.\n", data);
+
if (ntohl(*(__be32 *) data) != YYTH_MAGIC)
goto fail;
^ permalink raw reply related
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Paul Adrian Glaubitz @ 2025-11-18 9:04 UTC (permalink / raw)
To: deller, linux-kernel, apparmor, John Johansen,
linux-security-module
Cc: Helge Deller
In-Reply-To: <20250531150822.135803-1-deller@kernel.org>
Hi Helge,
On Sat, 2025-05-31 at 17:08 +0200, deller@kernel.org wrote:
> From: Helge Deller <deller@gmx.de>
>
> Two patches which fix unaligned memory accesses in apparmor.
> Both triggered on the parisc platform, which is much more
> memory alignment sensitive and will report violations.
> Please check and apply.
>
> Helge
>
> Helge Deller (2):
> apparmor: Fix 8-byte alignment for initial dfa blob streams
> apparmor: Fix unaligned memory accesses in KUnit test
>
> security/apparmor/lsm.c | 4 ++--
> security/apparmor/policy_unpack_test.c | 6 ++++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
Thanks for looking into this!
Unfortunately, the problem still persists on SPARC even with v6.18-rc6:
[ 76.209476] Kernel unaligned access at TPC[8dabfc] aa_dfa_unpack+0x3c/0x6e0
[ 76.301115] Kernel unaligned access at TPC[8dac0c] aa_dfa_unpack+0x4c/0x6e0
[ 76.392697] Kernel unaligned access at TPC[8dacf0] aa_dfa_unpack+0x130/0x6e0
[ 76.485440] Kernel unaligned access at TPC[8dacf0] aa_dfa_unpack+0x130/0x6e0
[ 76.578179] Kernel unaligned access at TPC[8dacf0] aa_dfa_unpack+0x130/0x6e0
I have documented the problem here [1].
So, I suspect that your fix is incomplete.
Adrian
> [1] https://github.com/sparclinux/issues/issues/30
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: [PATCH v3 00/14] Remove redundant rcu_read_lock/unlock() in spin_lock
From: Sebastian Andrzej Siewior @ 2025-11-18 7:24 UTC (permalink / raw)
To: pengdonglin
Cc: tj, tony.luck, jani.nikula, ap420073, jv, freude, bcrl, trondmy,
longman, kees, hdanton, paulmck, linux-kernel, linux-rt-devel,
linux-nfs, linux-aio, linux-fsdevel, linux-security-module,
netdev, intel-gfx, linux-wireless, linux-acpi, linux-s390,
cgroups
In-Reply-To: <20250916044735.2316171-1-dolinux.peng@gmail.com>
On 2025-09-16 12:47:21 [+0800], pengdonglin wrote:
Hi,
> There is no need no explicitly start a RCU read section if one has already
> been started implicitly by spin_lock().
>
> Simplify the code and remove the inner rcu_read_lock() invocation.
I'm not going argue if this is a good or not but: If you intend to get
this merged I suggest you rebase your series (or what is left since I
think a few patches got merged) on top of current tree and resend them
individually targeting the relevant tree/ list. Otherwise everyone might
think someone else is in charge of this big series.
Sebastian
^ permalink raw reply
* Re: [PATCH v6 12/15] Add start_renaming_two_dentries()
From: Paul Moore @ 2025-11-17 23:04 UTC (permalink / raw)
To: NeilBrown
Cc: Alexander Viro, Christian Brauner, Amir Goldstein, Jan Kara,
linux-fsdevel, Jeff Layton, Chris Mason, David Sterba,
David Howells, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Tyler Hicks, Miklos Szeredi, Chuck Lever,
Olga Kornievskaia, Dai Ngo, Namjae Jeon, Steve French,
Sergey Senozhatsky, Carlos Maiolino, John Johansen, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Mateusz Guzik,
Lorenzo Stoakes, Stefan Berger, Darrick J. Wong, linux-kernel,
netfs, ecryptfs, linux-nfs, linux-unionfs, linux-cifs, linux-xfs,
linux-security-module, selinux
In-Reply-To: <20251113002050.676694-13-neilb@ownmail.net>
On Wed, Nov 12, 2025 at 7:42 PM NeilBrown <neilb@ownmail.net> wrote:
>
> From: NeilBrown <neil@brown.name>
>
> A few callers want to lock for a rename and already have both dentries.
> Also debugfs does want to perform a lookup but doesn't want permission
> checking, so start_renaming_dentry() cannot be used.
>
> This patch introduces start_renaming_two_dentries() which is given both
> dentries. debugfs performs one lookup itself. As it will only continue
> with a negative dentry and as those cannot be renamed or unlinked, it is
> safe to do the lookup before getting the rename locks.
>
> overlayfs uses start_renaming_two_dentries() in three places and selinux
> uses it twice in sel_make_policy_nodes().
>
> In sel_make_policy_nodes() we now lock for rename twice instead of just
> once so the combined operation is no longer atomic w.r.t the parent
> directory locks. As selinux_state.policy_mutex is held across the whole
> operation this does not open up any interesting races.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: NeilBrown <neil@brown.name>
>
> ---
> changes since v5:
> - sel_make_policy_nodes now uses "goto out" on error from start_renaming_two_dentries()
>
> changes since v3:
> added missing assignment to rd.mnt_idmap in ovl_cleanup_and_whiteout
> ---
> fs/debugfs/inode.c | 48 ++++++++++++--------------
> fs/namei.c | 65 ++++++++++++++++++++++++++++++++++++
> fs/overlayfs/dir.c | 43 ++++++++++++++++--------
> include/linux/namei.h | 2 ++
> security/selinux/selinuxfs.c | 15 +++++++--
> 5 files changed, 131 insertions(+), 42 deletions(-)
...
> diff --git a/fs/namei.c b/fs/namei.c
> index 4b740048df97..7f0384ceb976 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3877,6 +3877,71 @@ int start_renaming_dentry(struct renamedata *rd, int lookup_flags,
> }
> EXPORT_SYMBOL(start_renaming_dentry);
>
> +/**
> + * start_renaming_two_dentries - Lock to dentries in given parents for rename
I'm guessing you meant this to read "Lock *two* dentries ...".
Otherwise the SELinux changes look fine to me.
Acked-by: Paul Moore <paul@paul-moore.com> (SELinux)
> + * @rd: rename data containing parent
> + * @old_dentry: dentry of name to move
> + * @new_dentry: dentry to move to
> + *
> + * Ensure locks are in place for rename and check parentage is still correct.
> + *
> + * On success the two dentries are stored in @rd.old_dentry and
> + * @rd.new_dentry and @rd.old_parent and @rd.new_parent are confirmed to
> + * be the parents of the dentries.
> + *
> + * References and the lock can be dropped with end_renaming()
> + *
> + * Returns: zero or an error.
> + */
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Bernd Edlinger @ 2025-11-17 20:08 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Christian Brauner, Alexander Viro, Alexey Dobriyan, Kees Cook,
Andy Lutomirski, Will Drewry, Andrew Morton, Michal Hocko,
Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <aRs4zYDhddBQFiXZ@redhat.com>
On 11/17/25 16:01, Oleg Nesterov wrote:
> On 11/17, Bernd Edlinger wrote:
>>
>> On 11/11/25 10:21, Christian Brauner wrote:
>>> On Wed, Nov 05, 2025 at 03:32:10PM +0100, Oleg Nesterov wrote:
>>
>>>> But this is minor. Why do we need "bool unsafe_execve_in_progress" ?
>>>> If this patch is correct, de_thread() can drop/reacquire cred_guard_mutex
>>>> unconditionally.
>>>>
>>
>> I would not like to drop the mutex when no absolutely necessary for performance reasons.
>
> OK, I won't insist... But I don't really understand how this can help to
> improve the performance. If nothing else, this adds another for_other_threads()
> loop.
>
If no dead-lock is possible it is better to complete the de_thread without
releasing the mutex. For the debugger it is also the better experience,
no matter when the ptrace_attack happens it will succeed rather quickly either
before the execve or after the execve.
> And again, the unsafe_execve_in_progress == T case is unlikely. I'm afraid this
> case (de_thread() without cred_guard_mutex) won't have enough testing.
>
Same is the case of a multi-threaded application that does execve while other threads
are still alive. But I have test cases, they are pretty good at reproducing the
dead-locks.
> In any case, why you dislike the suggestion to add this unsafe_execve_in_progress
> logic in a separate patch?
>
I do not want to regress use cases where there is no dead-lock possible.
The saying "if it ain't broke, don't fix it" means you shouldn't try to change something
that is already working well, because meddling with it could potentially make it worse.
>>>>> + if (unlikely(unsafe_execve_in_progress)) {
>>>>> + spin_unlock_irq(lock);
>>>>> + sig->exec_bprm = bprm;
>>>>> + mutex_unlock(&sig->cred_guard_mutex);
>>>>> + spin_lock_irq(lock);
>>>>
>>>> I don't think spin_unlock_irq() + spin_lock_irq() makes any sense...
>>>>
>>
>> Since the spin lock was acquired while holding the mutex, both should be
>> unlocked in reverse sequence and the spin lock re-acquired after releasing
>> the mutex.
>
> Why?
>
It is generally more safe when each thread acquires its mutexes in order and
releases them in reverse order.
Consider this:
Thread A:
holds spin_lock_irq(siglock);
does mutes_unlock(cred_guard_mutex); with irq disabled.
task switch happens to Thread B which has irq enabled.
and is waiting for cred_guard_mutex.
Thrad B:
does mutex_lock(cred_guard_mutex);
but is interrupted this point and the interrupt handler I executes
now iterrupt handler I wants to take siglock and is blocked,
because the system one single CPU core.
>> I'd expect the scheduler to do a task switch after the cred_guard_mutex is
>> unlocked, at least in the RT-linux variant, while the spin lock is not yet
>> unlocked.
>
> I must have missed something, but I still don't understand why this would
> be wrong...
>
>>>>> @@ -1114,13 +1139,31 @@ int begin_new_exec(struct linux_binprm * bprm)
>>>>> */
>>>>> trace_sched_prepare_exec(current, bprm);
>>>>>
>>>>> + /* If the binary is not readable then enforce mm->dumpable=0 */
>>>>> + would_dump(bprm, bprm->file);
>>>>> + if (bprm->have_execfd)
>>>>> + would_dump(bprm, bprm->executable);
>>>>> +
>>>>> + /*
>>>>> + * Figure out dumpability. Note that this checking only of current
>>>>> + * is wrong, but userspace depends on it. This should be testing
>>>>> + * bprm->secureexec instead.
>>>>> + */
>>>>> + if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
>>>>> + is_dumpability_changed(current_cred(), bprm->cred) ||
>>>>> + !(uid_eq(current_euid(), current_uid()) &&
>>>>> + gid_eq(current_egid(), current_gid())))
>>>>> + set_dumpable(bprm->mm, suid_dumpable);
>>>>> + else
>>>>> + set_dumpable(bprm->mm, SUID_DUMP_USER);
>>>>> +
>>>>
>>>> OK, we need to do this before de_thread() drops cred_guard_mutex.
>>>> But imo this too should be done in a separate patch, the changelog should
>>>> explain this change.
>>>>
>>
>> The dumpability need to be determined before de_thread, because ptrace_may_access
>> needs this information to determine if the tracer is allowed to ptrace. That is
>> part of the core of the patch, it would not work without that.
>
> Yes,
>
>> I will add more comments to make that more easy to understand.
>
> But again, why this change can't come in a separate patch? Before the patch which
> drops cred_guard_mutex in de_thread().
>
I did this mistake already, the end result was that a patch was split in 8 different
parts, but one of them was not accepted, and therefore we have now the test failure
in the vmaccess since 5 years now, because the test was designed to test the complete
puzzle, but one puzzle part was missing...
>>>> int lock_current_cgm(void)
>>>> {
>>>> if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
>>>> return -ERESTARTNOINTR;
>>>>
>>>> if (!current->signal->group_exec_task)
>>>> return 0;
>>>>
>>>> WARN_ON(!fatal_signal_pending(current));
>>>> mutex_unlock(¤t->signal->cred_guard_mutex);
>>>> return -ERESTARTNOINTR;
>>>> }
>>>>
>>>> ?
>>>>
>>
>> Some use mutex_lock_interruptible and some use mutex_lock_killable here,
>> so it wont work for all of them. I would not consider this a new kind
>> of dead-lock free mutex, but just an open-coded state machine, handling
>> the state that the tasks have whild de_thread is running.
>
> OK. and we don't have mutex_lock_state(). I think that all users could
> use mutex_lock_killable(), but you are right anyway, and this is minor.
>
>>>> Note that it checks ->group_exec_task, not ->exec_bprm. So this change can
>>>> come in a separate patch too, but I won't insist.
>
> Yes. Although this is minor too ;)
>
>>>> This is the most problematic change which I can't review...
>>>>
>>>> Firstly, it changes task->mm/real_cred for __ptrace_may_access() and this
>>>> looks dangerous to me.
>>>
>>> Yeah, that is not ok. This is effectively override_creds for real_cred
>>> and that is not a pattern I want to see us establish at all! Temporary
>>> credential overrides for the subjective credentials is already terrible
>>> but at least we have the explicit split between real_cred and cred
>>> expressely for that. So no, that's not an acceptable solution.
>>>
>>
>> Okay I understand your point.
>> I did this originally just to avoid to have to change the interface to all
>> the security engines, but instead I could add a flag PTRACE_MODE_BPRMCREDS to
>> the ptrace_may_access which must be handled in all security engines, to use
>> child->signal->exec_bprm->creds instead of __task_cred(child).
>
> Can't comment... I don't understand your idea, but this is my fault. I guess
> this needs more changes, in particular __ptrace_may_access_mm_cred(), but
> most probably I misunderstood your idea.
>
>>
>>>> Or. check_unsafe_exec() sets LSM_UNSAFE_PTRACE if ptrace. Is it safe to
>>>> ptrace the execing task after that? I have no idea what the security hooks
>>>> can do...
>>
>> That means the tracee is already ptraced before the execve, and SUID-bits
>> do not work as usual, and are more or less ignored. But in this patch
>> the tracee is not yet ptraced.
>
> Well. I meant that if LSM_UNSAFE_PTRACE is not set, then currently (say)
> security_bprm_committing_creds() has all rights to assume that the execing
> task is not ptraced. Yes, I don't see any potential problem right now, but
> still.
>
> And just in case... Lets look at this code
>
> + rcu_assign_pointer(task->real_cred, bprm->cred);
> + task->mm = bprm->mm;
> + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> + rcu_assign_pointer(task->real_cred, old_cred);
> + task->mm = old_mm;
>
> again.
>
> This is mostly theoretical, but what if begin_new_exec() fails after de_thread()
> and before exec_mmap() and/or commit_creds(bprm->cred) ? In this case the execing
> thread will report SIGSEGV to debugger which can (say) read old_mm.
>
> No?
>
Yes, and that is the reason why the debugger has to prove the possession of access rights
to the process before the execve which are necessary in case exeve fails, yes the debugger
may inspect the result, and as well the debugger's access rights must be also sufficient
to ptrace the process after execve succeeds, moreover the debugged process shall stop
right at the first instruction where the new process starts.
> I am starting to think that ptrace_attach() should simply fail with -EWOULDBLOCK
> if it detects "unsafe_execve_in_progress" ... And perhaps this is what you already
> tried to do in the past, I can't recall :/
>
This was indeed the previous attempt, but I changed my mind in the mean time,
as the return code -EAGAIN from the ptrace_attach is not documented, and is therefore
an API change, but also the debugger might misunderstand that hint, and try the same
ptrace_attach in a loop, instead of calling wait for the pending SIGCHILD signal.
I know Linus pointed out that the tracer would better use a signal hander, to avoid
the problem, but I think that no debugger wants to implement the state machine that
handles the ptrace events in a signal handler.
Thanks
Bernd.
> Oleg.
>
^ permalink raw reply
* Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Oleg Nesterov @ 2025-11-17 15:01 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Christian Brauner, Alexander Viro, Alexey Dobriyan, Kees Cook,
Andy Lutomirski, Will Drewry, Andrew Morton, Michal Hocko,
Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE4FE6E639B899D01D8870E4C9A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
On 11/17, Bernd Edlinger wrote:
>
> On 11/11/25 10:21, Christian Brauner wrote:
> > On Wed, Nov 05, 2025 at 03:32:10PM +0100, Oleg Nesterov wrote:
>
> >> But this is minor. Why do we need "bool unsafe_execve_in_progress" ?
> >> If this patch is correct, de_thread() can drop/reacquire cred_guard_mutex
> >> unconditionally.
> >>
>
> I would not like to drop the mutex when no absolutely necessary for performance reasons.
OK, I won't insist... But I don't really understand how this can help to
improve the performance. If nothing else, this adds another for_other_threads()
loop.
And again, the unsafe_execve_in_progress == T case is unlikely. I'm afraid this
case (de_thread() without cred_guard_mutex) won't have enough testing.
In any case, why you dislike the suggestion to add this unsafe_execve_in_progress
logic in a separate patch?
> >>> + if (unlikely(unsafe_execve_in_progress)) {
> >>> + spin_unlock_irq(lock);
> >>> + sig->exec_bprm = bprm;
> >>> + mutex_unlock(&sig->cred_guard_mutex);
> >>> + spin_lock_irq(lock);
> >>
> >> I don't think spin_unlock_irq() + spin_lock_irq() makes any sense...
> >>
>
> Since the spin lock was acquired while holding the mutex, both should be
> unlocked in reverse sequence and the spin lock re-acquired after releasing
> the mutex.
Why?
> I'd expect the scheduler to do a task switch after the cred_guard_mutex is
> unlocked, at least in the RT-linux variant, while the spin lock is not yet
> unlocked.
I must have missed something, but I still don't understand why this would
be wrong...
> >>> @@ -1114,13 +1139,31 @@ int begin_new_exec(struct linux_binprm * bprm)
> >>> */
> >>> trace_sched_prepare_exec(current, bprm);
> >>>
> >>> + /* If the binary is not readable then enforce mm->dumpable=0 */
> >>> + would_dump(bprm, bprm->file);
> >>> + if (bprm->have_execfd)
> >>> + would_dump(bprm, bprm->executable);
> >>> +
> >>> + /*
> >>> + * Figure out dumpability. Note that this checking only of current
> >>> + * is wrong, but userspace depends on it. This should be testing
> >>> + * bprm->secureexec instead.
> >>> + */
> >>> + if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> >>> + is_dumpability_changed(current_cred(), bprm->cred) ||
> >>> + !(uid_eq(current_euid(), current_uid()) &&
> >>> + gid_eq(current_egid(), current_gid())))
> >>> + set_dumpable(bprm->mm, suid_dumpable);
> >>> + else
> >>> + set_dumpable(bprm->mm, SUID_DUMP_USER);
> >>> +
> >>
> >> OK, we need to do this before de_thread() drops cred_guard_mutex.
> >> But imo this too should be done in a separate patch, the changelog should
> >> explain this change.
> >>
>
> The dumpability need to be determined before de_thread, because ptrace_may_access
> needs this information to determine if the tracer is allowed to ptrace. That is
> part of the core of the patch, it would not work without that.
Yes,
> I will add more comments to make that more easy to understand.
But again, why this change can't come in a separate patch? Before the patch which
drops cred_guard_mutex in de_thread().
> >> int lock_current_cgm(void)
> >> {
> >> if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> >> return -ERESTARTNOINTR;
> >>
> >> if (!current->signal->group_exec_task)
> >> return 0;
> >>
> >> WARN_ON(!fatal_signal_pending(current));
> >> mutex_unlock(¤t->signal->cred_guard_mutex);
> >> return -ERESTARTNOINTR;
> >> }
> >>
> >> ?
> >>
>
> Some use mutex_lock_interruptible and some use mutex_lock_killable here,
> so it wont work for all of them. I would not consider this a new kind
> of dead-lock free mutex, but just an open-coded state machine, handling
> the state that the tasks have whild de_thread is running.
OK. and we don't have mutex_lock_state(). I think that all users could
use mutex_lock_killable(), but you are right anyway, and this is minor.
> >> Note that it checks ->group_exec_task, not ->exec_bprm. So this change can
> >> come in a separate patch too, but I won't insist.
Yes. Although this is minor too ;)
> >> This is the most problematic change which I can't review...
> >>
> >> Firstly, it changes task->mm/real_cred for __ptrace_may_access() and this
> >> looks dangerous to me.
> >
> > Yeah, that is not ok. This is effectively override_creds for real_cred
> > and that is not a pattern I want to see us establish at all! Temporary
> > credential overrides for the subjective credentials is already terrible
> > but at least we have the explicit split between real_cred and cred
> > expressely for that. So no, that's not an acceptable solution.
> >
>
> Okay I understand your point.
> I did this originally just to avoid to have to change the interface to all
> the security engines, but instead I could add a flag PTRACE_MODE_BPRMCREDS to
> the ptrace_may_access which must be handled in all security engines, to use
> child->signal->exec_bprm->creds instead of __task_cred(child).
Can't comment... I don't understand your idea, but this is my fault. I guess
this needs more changes, in particular __ptrace_may_access_mm_cred(), but
most probably I misunderstood your idea.
>
> >> Or. check_unsafe_exec() sets LSM_UNSAFE_PTRACE if ptrace. Is it safe to
> >> ptrace the execing task after that? I have no idea what the security hooks
> >> can do...
>
> That means the tracee is already ptraced before the execve, and SUID-bits
> do not work as usual, and are more or less ignored. But in this patch
> the tracee is not yet ptraced.
Well. I meant that if LSM_UNSAFE_PTRACE is not set, then currently (say)
security_bprm_committing_creds() has all rights to assume that the execing
task is not ptraced. Yes, I don't see any potential problem right now, but
still.
And just in case... Lets look at this code
+ rcu_assign_pointer(task->real_cred, bprm->cred);
+ task->mm = bprm->mm;
+ retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
+ rcu_assign_pointer(task->real_cred, old_cred);
+ task->mm = old_mm;
again.
This is mostly theoretical, but what if begin_new_exec() fails after de_thread()
and before exec_mmap() and/or commit_creds(bprm->cred) ? In this case the execing
thread will report SIGSEGV to debugger which can (say) read old_mm.
No?
I am starting to think that ptrace_attach() should simply fail with -EWOULDBLOCK
if it detects "unsafe_execve_in_progress" ... And perhaps this is what you already
tried to do in the past, I can't recall :/
Oleg.
^ permalink raw reply
* Re: [PATCH v3 14/14] wifi: ath9k: Remove redundant rcu_read_lock/unlock() in spin_lock
From: Toke Høiland-Jørgensen @ 2025-11-17 11:47 UTC (permalink / raw)
To: pengdonglin, tj, tony.luck, jani.nikula, ap420073, jv, freude,
bcrl, trondmy, longman, kees
Cc: bigeasy, hdanton, paulmck, linux-kernel, linux-rt-devel,
linux-nfs, linux-aio, linux-fsdevel, linux-security-module,
netdev, intel-gfx, linux-wireless, linux-acpi, linux-s390,
cgroups, pengdonglin, Jakub Kicinski, pengdonglin
In-Reply-To: <20250916044735.2316171-15-dolinux.peng@gmail.com>
pengdonglin <dolinux.peng@gmail.com> writes:
> From: pengdonglin <pengdonglin@xiaomi.com>
>
> Since commit a8bb74acd8efe ("rcu: Consolidate RCU-sched update-side function definitions")
> there is no difference between rcu_read_lock(), rcu_read_lock_bh() and
> rcu_read_lock_sched() in terms of RCU read section and the relevant grace
> period. That means that spin_lock(), which implies rcu_read_lock_sched(),
> also implies rcu_read_lock().
>
> There is no need no explicitly start a RCU read section if one has already
> been started implicitly by spin_lock().
>
> Simplify the code and remove the inner rcu_read_lock() invocation.
>
> Cc: "Toke" <toke@toke.dk>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
> Signed-off-by: pengdonglin <dolinux.peng@gmail.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
^ permalink raw reply
* [PATCH v13 4/4] rust: Add `OwnableRefCounted`
From: Oliver Mangold @ 2025-11-17 10:08 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Benno Lossin, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
Serge Hallyn, Asahi Lina
Cc: rust-for-linux, linux-kernel, linux-block, dri-devel,
linux-fsdevel, linux-mm, linux-pm, linux-pci,
linux-security-module, Oliver Mangold
In-Reply-To: <20251117-unique-ref-v13-0-b5b243df1250@pm.me>
Types implementing one of these traits can safely convert between an
`ARef<T>` and an `Owned<T>`.
This is useful for types which generally are accessed through an `ARef`
but have methods which can only safely be called when the reference is
unique, like e.g. `block::mq::Request::end_ok()`.
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/owned.rs | 138 ++++++++++++++++++++++++++++++++++++++++++++---
rust/kernel/sync/aref.rs | 11 +++-
rust/kernel/types.rs | 2 +-
3 files changed, 141 insertions(+), 10 deletions(-)
diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
index a26747cbc13b..26ab2b00ada0 100644
--- a/rust/kernel/owned.rs
+++ b/rust/kernel/owned.rs
@@ -5,6 +5,7 @@
//! These pointer types are useful for C-allocated objects which by API-contract
//! are owned by Rust, but need to be freed through the C API.
+use crate::sync::aref::{ARef, RefCounted};
use core::{
mem::ManuallyDrop,
ops::{Deref, DerefMut},
@@ -14,14 +15,16 @@
/// Type allocated and destroyed on the C side, but owned by Rust.
///
-/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type. This
-/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather
-/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is
-/// executed when the owned reference [`Owned<Self>`] pointing to the object is dropped.
+/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type.
+/// - This is useful when it is desirable to tie the lifetime of an object reference to an owned
+/// object, rather than pass around a bare reference.
+/// - [`Ownable`] types can define custom drop logic that is executed when the owned reference
+/// of type [`Owned<_>`] pointing to the object is dropped.
///
/// Note: The underlying object is not required to provide internal reference counting, because it
/// represents a unique, owned reference. If reference counting (on the Rust side) is required,
-/// [`RefCounted`](crate::types::RefCounted) should be implemented.
+/// [`RefCounted`] should be implemented. [`OwnableRefCounted`] should be implemented if conversion
+/// between unique and shared (reference counted) ownership is needed.
///
/// # Safety
///
@@ -143,9 +146,7 @@ impl<T: Ownable> Owned<T> {
/// mutable reference requirements. That is, the kernel will not mutate or free the underlying
/// object and is okay with it being modified by Rust code.
pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
- Self {
- ptr,
- }
+ Self { ptr }
}
/// Consumes the [`Owned`], returning a raw pointer.
@@ -193,3 +194,124 @@ fn drop(&mut self) {
unsafe { T::release(self.ptr) };
}
}
+
+/// A trait for objects that can be wrapped in either one of the reference types [`Owned`] and
+/// [`ARef`].
+///
+/// # Examples
+///
+/// A minimal example implementation of [`OwnableRefCounted`], [`Ownable`] and its usage with
+/// [`ARef`] and [`Owned`] looks like this:
+///
+/// ```
+/// # #![expect(clippy::disallowed_names)]
+/// # use core::cell::Cell;
+/// # use core::ptr::NonNull;
+/// # use kernel::alloc::{flags, kbox::KBox, AllocError};
+/// # use kernel::sync::aref::{ARef, RefCounted};
+/// # use kernel::types::{Owned, Ownable, OwnableRefCounted};
+///
+/// // Example internally refcounted struct.
+/// //
+/// // # Invariants
+/// //
+/// // - `refcount` is always non-zero for a valid object.
+/// // - `refcount` is >1 if there are more than 1 Rust reference to it.
+/// //
+/// struct Foo {
+/// refcount: Cell<usize>,
+/// }
+///
+/// impl Foo {
+/// fn new() -> Result<Owned<Self>, AllocError> {
+/// // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is
+/// // not actually a C-allocated object.
+/// let result = KBox::new(
+/// Foo {
+/// refcount: Cell::new(1),
+/// },
+/// flags::GFP_KERNEL,
+/// )?;
+/// let result = NonNull::new(KBox::into_raw(result))
+/// .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
+/// // SAFETY: We just allocated the `Self`, thus it is valid and there cannot be any other
+/// // Rust references. Calling `into_raw()` makes us responsible for ownership and
+/// // we won't use the raw pointer anymore, thus we can transfer ownership to the `Owned`.
+/// Ok(unsafe { Owned::from_raw(result) })
+/// }
+/// }
+///
+/// // SAFETY: We increment and decrement each time the respective function is called and only free
+/// // the `Foo` when the refcount reaches zero.
+/// unsafe impl RefCounted for Foo {
+/// fn inc_ref(&self) {
+/// self.refcount.replace(self.refcount.get() + 1);
+/// }
+///
+/// unsafe fn dec_ref(this: NonNull<Self>) {
+/// // SAFETY: By requirement on calling this function, the refcount is non-zero,
+/// // implying the underlying object is valid.
+/// let refcount = unsafe { &this.as_ref().refcount };
+/// let new_refcount = refcount.get() - 1;
+/// if new_refcount == 0 {
+/// // The `Foo` will be dropped when `KBox` goes out of scope.
+/// // SAFETY: The [`KBox<Foo>`] is still alive as the old refcount is 1. We can pass
+/// // ownership to the [`KBox`] as by requirement on calling this function,
+/// // the `Self` will no longer be used by the caller.
+/// unsafe { KBox::from_raw(this.as_ptr()) };
+/// } else {
+/// refcount.replace(new_refcount);
+/// }
+/// }
+/// }
+///
+/// impl OwnableRefCounted for Foo {
+/// fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
+/// if this.refcount.get() == 1 {
+/// // SAFETY: The `Foo` is still alive and has no other Rust references as the refcount
+/// // is 1.
+/// Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
+/// } else {
+/// Err(this)
+/// }
+/// }
+/// }
+///
+/// // SAFETY: This implementation of `release()` is safe for any valid `Self`.
+/// unsafe impl Ownable for Foo {
+/// unsafe fn release(this: NonNull<Self>) {
+/// // SAFETY: Using `dec_ref()` from [`RefCounted`] to release is okay, as the refcount is
+/// // always 1 for an [`Owned<Foo>`].
+/// unsafe{ Foo::dec_ref(this) };
+/// }
+/// }
+///
+/// let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
+/// let mut foo = ARef::from(foo);
+/// {
+/// let bar = foo.clone();
+/// assert!(Owned::try_from(bar).is_err());
+/// }
+/// assert!(Owned::try_from(foo).is_ok());
+/// ```
+pub trait OwnableRefCounted: RefCounted + Ownable + Sized {
+ /// Checks if the [`ARef`] is unique and convert it to an [`Owned`] it that is that case.
+ /// Otherwise it returns again an [`ARef`] to the same underlying object.
+ fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>>;
+
+ /// Converts the [`Owned`] into an [`ARef`].
+ fn into_shared(this: Owned<Self>) -> ARef<Self> {
+ // SAFETY: Safe by the requirements on implementing the trait.
+ unsafe { ARef::from_raw(Owned::into_raw(this)) }
+ }
+}
+
+impl<T: OwnableRefCounted> TryFrom<ARef<T>> for Owned<T> {
+ type Error = ARef<T>;
+ /// Tries to convert the [`ARef`] to an [`Owned`] by calling
+ /// [`try_from_shared()`](OwnableRefCounted::try_from_shared). In case the [`ARef`] is not
+ /// unique, it returns again an [`ARef`] to the same underlying object.
+ fn try_from(b: ARef<T>) -> Result<Owned<T>, Self::Error> {
+ T::try_from_shared(b)
+ }
+}
diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index 937dcf6ed5de..2dbffe2ed1b8 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -30,7 +30,10 @@
/// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
/// internal reference count and provides only shared references. If unique references are required
/// [`Ownable`](crate::types::Ownable) should be implemented which allows types to be wrapped in an
-/// [`Owned<Self>`](crate::types::Owned).
+/// [`Owned<Self>`](crate::types::Owned). Implementing the trait
+/// [`OwnableRefCounted`](crate::types::OwnableRefCounted) allows to convert between unique and
+/// shared references (i.e. [`Owned<Self>`](crate::types::Owned) and
+/// [`ARef<Self>`](crate::types::Owned)).
///
/// # Safety
///
@@ -180,6 +183,12 @@ fn from(b: &T) -> Self {
}
}
+impl<T: crate::types::OwnableRefCounted> From<crate::types::Owned<T>> for ARef<T> {
+ fn from(b: crate::types::Owned<T>) -> Self {
+ T::into_shared(b)
+ }
+}
+
impl<T: RefCounted> Drop for ARef<T> {
fn drop(&mut self) {
// SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 8ef01393352b..a9b72709d0d3 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -11,7 +11,7 @@
};
use pin_init::{PinInit, Wrapper, Zeroable};
-pub use crate::owned::{Ownable, Owned};
+pub use crate::owned::{Ownable, OwnableRefCounted, Owned};
pub use crate::sync::aref::{ARef, AlwaysRefCounted, RefCounted};
--
2.51.2
^ permalink raw reply related
* [PATCH v13 3/4] rust: Add missing SAFETY documentation for `ARef` example
From: Oliver Mangold @ 2025-11-17 10:08 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Benno Lossin, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
Serge Hallyn, Asahi Lina
Cc: rust-for-linux, linux-kernel, linux-block, dri-devel,
linux-fsdevel, linux-mm, linux-pm, linux-pci,
linux-security-module, Oliver Mangold
In-Reply-To: <20251117-unique-ref-v13-0-b5b243df1250@pm.me>
SAFETY comment in rustdoc example was just 'TODO'. Fixed.
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/sync/aref.rs | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index 4226119d5ac9..937dcf6ed5de 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -129,12 +129,14 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
/// # Examples
///
/// ```
- /// use core::ptr::NonNull;
- /// use kernel::sync::aref::{ARef, RefCounted};
+ /// # use core::ptr::NonNull;
+ /// # use kernel::sync::aref::{ARef, RefCounted};
///
/// struct Empty {}
///
- /// # // SAFETY: TODO.
+ /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references and
+ /// // never frees the underlying object. Thus we can act as having a refcount on the object
+ /// // that we pass to the newly created `ARef`.
/// unsafe impl RefCounted for Empty {
/// fn inc_ref(&self) {}
/// unsafe fn dec_ref(_obj: NonNull<Self>) {}
@@ -142,7 +144,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
///
/// let mut data = Empty {};
/// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
- /// # // SAFETY: TODO.
+ /// // SAFETY: We keep `data` around longer than the `ARef`.
/// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
/// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
///
--
2.51.2
^ permalink raw reply related
* [PATCH v13 2/4] rust: `AlwaysRefCounted` is renamed to `RefCounted`.
From: Oliver Mangold @ 2025-11-17 10:07 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Benno Lossin, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
Serge Hallyn, Asahi Lina
Cc: rust-for-linux, linux-kernel, linux-block, dri-devel,
linux-fsdevel, linux-mm, linux-pm, linux-pci,
linux-security-module, Oliver Mangold
In-Reply-To: <20251117-unique-ref-v13-0-b5b243df1250@pm.me>
`AlwaysRefCounted` will become a marker trait to indicate that it is
allowed to obtain an `ARef<T>` from a `&T`, which cannot be allowed for
types which are also Ownable.
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Suggested-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/auxiliary.rs | 7 +++++-
rust/kernel/block/mq/request.rs | 15 +++++++------
rust/kernel/cred.rs | 13 ++++++++++--
rust/kernel/device.rs | 13 ++++++++----
rust/kernel/device/property.rs | 7 +++++-
rust/kernel/drm/device.rs | 10 ++++++---
rust/kernel/drm/gem/mod.rs | 10 ++++++---
rust/kernel/fs/file.rs | 16 ++++++++++----
rust/kernel/mm.rs | 15 +++++++++----
rust/kernel/mm/mmput_async.rs | 9 ++++++--
rust/kernel/opp.rs | 10 ++++++---
rust/kernel/owned.rs | 2 +-
rust/kernel/pci.rs | 10 ++++++---
rust/kernel/pid_namespace.rs | 12 +++++++++--
rust/kernel/platform.rs | 7 +++++-
rust/kernel/sync/aref.rs | 47 ++++++++++++++++++++++++++---------------
rust/kernel/task.rs | 10 ++++++---
rust/kernel/types.rs | 2 +-
18 files changed, 154 insertions(+), 61 deletions(-)
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 7a3b0b9c418e..7f5b16053c11 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -10,6 +10,7 @@
driver,
error::{from_result, to_result, Result},
prelude::*,
+ sync::aref::{AlwaysRefCounted, RefCounted},
types::Opaque,
ThisModule,
};
@@ -239,7 +240,7 @@ extern "C" fn release(dev: *mut bindings::device) {
kernel::impl_device_context_into_aref!(Device);
// SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
+unsafe impl RefCounted for Device {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
unsafe { bindings::get_device(self.as_ref().as_raw()) };
@@ -258,6 +259,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
+// `&Device`.
+unsafe impl AlwaysRefCounted for Device {}
+
impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
fn as_ref(&self) -> &device::Device<Ctx> {
// SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index c5f1f6b1ccfb..b6165f96b4ce 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -8,7 +8,7 @@
bindings,
block::mq::Operations,
error::Result,
- sync::{atomic::Relaxed, Refcount},
+ sync::{aref::RefCounted, atomic::Relaxed, Refcount},
types::{ARef, AlwaysRefCounted, Opaque},
};
use core::{marker::PhantomData, ptr::NonNull};
@@ -225,11 +225,10 @@ unsafe impl<T: Operations> Send for Request<T> {}
// mutate `self` are internally synchronized`
unsafe impl<T: Operations> Sync for Request<T> {}
-// SAFETY: All instances of `Request<T>` are reference counted. This
-// implementation of `AlwaysRefCounted` ensure that increments to the ref count
-// keeps the object alive in memory at least until a matching reference count
-// decrement is executed.
-unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
+// SAFETY: All instances of `Request<T>` are reference counted. This implementation of `RefCounted`
+// ensure that increments to the ref count keeps the object alive in memory at least until a
+// matching reference count decrement is executed.
+unsafe impl<T: Operations> RefCounted for Request<T> {
fn inc_ref(&self) {
self.wrapper_ref().refcount().inc();
}
@@ -251,3 +250,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
}
}
}
+
+// SAFETY: We currently do not implement `Ownable`, thus it is okay to obtain an `ARef<Request>`
+// from a `&Request` (but this will change in the future).
+unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}
diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
index ffa156b9df37..20ef0144094b 100644
--- a/rust/kernel/cred.rs
+++ b/rust/kernel/cred.rs
@@ -8,7 +8,12 @@
//!
//! Reference: <https://www.kernel.org/doc/html/latest/security/credentials.html>
-use crate::{bindings, sync::aref::AlwaysRefCounted, task::Kuid, types::Opaque};
+use crate::{
+ bindings,
+ sync::aref::RefCounted,
+ task::Kuid,
+ types::{AlwaysRefCounted, Opaque},
+};
/// Wraps the kernel's `struct cred`.
///
@@ -76,7 +81,7 @@ pub fn euid(&self) -> Kuid {
}
// SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
-unsafe impl AlwaysRefCounted for Credential {
+unsafe impl RefCounted for Credential {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -90,3 +95,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
unsafe { bindings::put_cred(obj.cast().as_ptr()) };
}
}
+
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Credential>` from a
+// `&Credential`.
+unsafe impl AlwaysRefCounted for Credential {}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index a849b7dde2fd..a69ee32997c1 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -5,9 +5,10 @@
//! C header: [`include/linux/device.h`](srctree/include/linux/device.h)
use crate::{
- bindings, fmt,
- sync::aref::ARef,
- types::{ForeignOwnable, Opaque},
+ bindings,
+ fmt,
+ sync::aref::RefCounted,
+ types::{ARef, AlwaysRefCounted, ForeignOwnable, Opaque},
};
use core::{marker::PhantomData, ptr};
@@ -407,7 +408,7 @@ pub fn fwnode(&self) -> Option<&property::FwNode> {
kernel::impl_device_context_into_aref!(Device);
// SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
+unsafe impl RefCounted for Device {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
unsafe { bindings::get_device(self.as_raw()) };
@@ -419,6 +420,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
+// `&Device`.
+unsafe impl AlwaysRefCounted for Device {}
+
// SAFETY: As by the type invariant `Device` can be sent to any thread.
unsafe impl Send for Device {}
diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
index 3a332a8c53a9..a8bb824ad0ec 100644
--- a/rust/kernel/device/property.rs
+++ b/rust/kernel/device/property.rs
@@ -14,6 +14,7 @@
fmt,
prelude::*,
str::{CStr, CString},
+ sync::aref::{AlwaysRefCounted, RefCounted},
types::{ARef, Opaque},
};
@@ -359,7 +360,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
}
// SAFETY: Instances of `FwNode` are always reference-counted.
-unsafe impl crate::types::AlwaysRefCounted for FwNode {
+unsafe impl RefCounted for FwNode {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the
// refcount is non-zero.
@@ -373,6 +374,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<FwNode>` from a
+// `&FwNode`.
+unsafe impl AlwaysRefCounted for FwNode {}
+
enum Node<'a> {
Borrowed(&'a FwNode),
Owned(ARef<FwNode>),
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 3ce8f62a0056..38ce7f389ed0 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -11,8 +11,8 @@
error::from_err_ptr,
error::Result,
prelude::*,
- sync::aref::{ARef, AlwaysRefCounted},
- types::Opaque,
+ sync::aref::{AlwaysRefCounted, RefCounted},
+ types::{ARef, Opaque},
};
use core::{alloc::Layout, mem, ops::Deref, ptr, ptr::NonNull};
@@ -198,7 +198,7 @@ fn deref(&self) -> &Self::Target {
// SAFETY: DRM device objects are always reference counted and the get/put functions
// satisfy the requirements.
-unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
+unsafe impl<T: drm::Driver> RefCounted for Device<T> {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
unsafe { bindings::drm_dev_get(self.as_raw()) };
@@ -213,6 +213,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
+// `&Device`.
+unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {}
+
impl<T: drm::Driver> AsRef<device::Device> for Device<T> {
fn as_ref(&self) -> &device::Device {
// SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 30c853988b94..4afd36e49205 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -10,8 +10,8 @@
drm::driver::{AllocImpl, AllocOps},
error::{to_result, Result},
prelude::*,
- sync::aref::{ARef, AlwaysRefCounted},
- types::Opaque,
+ sync::aref::RefCounted,
+ types::{ARef, AlwaysRefCounted, Opaque},
};
use core::{ops::Deref, ptr::NonNull};
@@ -56,7 +56,7 @@ pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted {
}
// SAFETY: All gem objects are refcounted.
-unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T {
+unsafe impl<T: IntoGEMObject> RefCounted for T {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
unsafe { bindings::drm_gem_object_get(self.as_raw()) };
@@ -75,6 +75,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<T>` from a
+// `&T`.
+unsafe impl<T: IntoGEMObject> crate::types::AlwaysRefCounted for T {}
+
extern "C" fn open_callback<T: DriverObject>(
raw_obj: *mut bindings::drm_gem_object,
raw_file: *mut bindings::drm_file,
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index cd6987850332..86309ca5dad0 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -12,8 +12,8 @@
cred::Credential,
error::{code::*, to_result, Error, Result},
fmt,
- sync::aref::{ARef, AlwaysRefCounted},
- types::{NotThreadSafe, Opaque},
+ sync::aref::RefCounted,
+ types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
};
use core::ptr;
@@ -192,7 +192,7 @@ unsafe impl Sync for File {}
// SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
// makes `ARef<File>` own a normal refcount.
-unsafe impl AlwaysRefCounted for File {
+unsafe impl RefCounted for File {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -207,6 +207,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<File>` from a
+// `&File`.
+unsafe impl AlwaysRefCounted for File {}
+
/// Wraps the kernel's `struct file`. Not thread safe.
///
/// This type represents a file that is not known to be safe to transfer across thread boundaries.
@@ -228,7 +232,7 @@ pub struct LocalFile {
// SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
// makes `ARef<LocalFile>` own a normal refcount.
-unsafe impl AlwaysRefCounted for LocalFile {
+unsafe impl RefCounted for LocalFile {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -244,6 +248,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<LocalFile>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<LocalFile>` from a
+// `&LocalFile`.
+unsafe impl AlwaysRefCounted for LocalFile {}
+
impl LocalFile {
/// Constructs a new `struct file` wrapper from a file descriptor.
///
diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs
index 4764d7b68f2a..dd9e3969e720 100644
--- a/rust/kernel/mm.rs
+++ b/rust/kernel/mm.rs
@@ -13,8 +13,8 @@
use crate::{
bindings,
- sync::aref::{ARef, AlwaysRefCounted},
- types::{NotThreadSafe, Opaque},
+ sync::aref::RefCounted,
+ types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
};
use core::{ops::Deref, ptr::NonNull};
@@ -55,7 +55,7 @@ unsafe impl Send for Mm {}
unsafe impl Sync for Mm {}
// SAFETY: By the type invariants, this type is always refcounted.
-unsafe impl AlwaysRefCounted for Mm {
+unsafe impl RefCounted for Mm {
#[inline]
fn inc_ref(&self) {
// SAFETY: The pointer is valid since self is a reference.
@@ -69,6 +69,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Mm>` from a `&Mm`.
+unsafe impl AlwaysRefCounted for Mm {}
+
/// A wrapper for the kernel's `struct mm_struct`.
///
/// This type is like [`Mm`], but with non-zero `mm_users`. It can only be used when `mm_users` can
@@ -91,7 +94,7 @@ unsafe impl Send for MmWithUser {}
unsafe impl Sync for MmWithUser {}
// SAFETY: By the type invariants, this type is always refcounted.
-unsafe impl AlwaysRefCounted for MmWithUser {
+unsafe impl RefCounted for MmWithUser {
#[inline]
fn inc_ref(&self) {
// SAFETY: The pointer is valid since self is a reference.
@@ -105,6 +108,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUser>` from a
+// `&MmWithUser`.
+unsafe impl AlwaysRefCounted for MmWithUser {}
+
// Make all `Mm` methods available on `MmWithUser`.
impl Deref for MmWithUser {
type Target = Mm;
diff --git a/rust/kernel/mm/mmput_async.rs b/rust/kernel/mm/mmput_async.rs
index b8d2f051225c..aba4ce675c86 100644
--- a/rust/kernel/mm/mmput_async.rs
+++ b/rust/kernel/mm/mmput_async.rs
@@ -10,7 +10,8 @@
use crate::{
bindings,
mm::MmWithUser,
- sync::aref::{ARef, AlwaysRefCounted},
+ sync::aref::RefCounted,
+ types::{ARef, AlwaysRefCounted},
};
use core::{ops::Deref, ptr::NonNull};
@@ -34,7 +35,7 @@ unsafe impl Send for MmWithUserAsync {}
unsafe impl Sync for MmWithUserAsync {}
// SAFETY: By the type invariants, this type is always refcounted.
-unsafe impl AlwaysRefCounted for MmWithUserAsync {
+unsafe impl RefCounted for MmWithUserAsync {
#[inline]
fn inc_ref(&self) {
// SAFETY: The pointer is valid since self is a reference.
@@ -48,6 +49,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUserAsync>`
+// from a `&MmWithUserAsync`.
+unsafe impl AlwaysRefCounted for MmWithUserAsync {}
+
// Make all `MmWithUser` methods available on `MmWithUserAsync`.
impl Deref for MmWithUserAsync {
type Target = MmWithUser;
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index 2c763fa9276d..77d1cc89c412 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -16,8 +16,8 @@
ffi::c_ulong,
prelude::*,
str::CString,
- sync::aref::{ARef, AlwaysRefCounted},
- types::Opaque,
+ sync::aref::RefCounted,
+ types::{ARef, AlwaysRefCounted, Opaque},
};
#[cfg(CONFIG_CPU_FREQ)]
@@ -1037,7 +1037,7 @@ unsafe impl Send for OPP {}
unsafe impl Sync for OPP {}
/// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
-unsafe impl AlwaysRefCounted for OPP {
+unsafe impl RefCounted for OPP {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
unsafe { bindings::dev_pm_opp_get(self.0.get()) };
@@ -1049,6 +1049,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<OPP>` from an
+// `&OPP`.
+unsafe impl AlwaysRefCounted for OPP {}
+
impl OPP {
/// Creates an owned reference to a [`OPP`] from a valid pointer.
///
diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
index a2cdd2cb8a10..a26747cbc13b 100644
--- a/rust/kernel/owned.rs
+++ b/rust/kernel/owned.rs
@@ -21,7 +21,7 @@
///
/// Note: The underlying object is not required to provide internal reference counting, because it
/// represents a unique, owned reference. If reference counting (on the Rust side) is required,
-/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
+/// [`RefCounted`](crate::types::RefCounted) should be implemented.
///
/// # Safety
///
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 7fcc5f6022c1..9ac70823fb4d 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -13,8 +13,8 @@
io::{Io, IoRaw},
irq::{self, IrqRequest},
str::CStr,
- sync::aref::ARef,
- types::Opaque,
+ sync::aref::{AlwaysRefCounted, RefCounted},
+ types::{ARef, Opaque},
ThisModule,
};
use core::{
@@ -601,7 +601,7 @@ pub fn set_master(&self) {
impl crate::dma::Device for Device<device::Core> {}
// SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
+unsafe impl RefCounted for Device {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
unsafe { bindings::pci_dev_get(self.as_raw()) };
@@ -613,6 +613,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
+// `&Device`.
+unsafe impl AlwaysRefCounted for Device {}
+
impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
fn as_ref(&self) -> &device::Device<Ctx> {
// SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
index 979a9718f153..4f6a94540e33 100644
--- a/rust/kernel/pid_namespace.rs
+++ b/rust/kernel/pid_namespace.rs
@@ -7,7 +7,11 @@
//! C header: [`include/linux/pid_namespace.h`](srctree/include/linux/pid_namespace.h) and
//! [`include/linux/pid.h`](srctree/include/linux/pid.h)
-use crate::{bindings, sync::aref::AlwaysRefCounted, types::Opaque};
+use crate::{
+ bindings,
+ sync::aref::RefCounted,
+ types::{AlwaysRefCounted, Opaque},
+};
use core::ptr;
/// Wraps the kernel's `struct pid_namespace`. Thread safe.
@@ -41,7 +45,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
}
// SAFETY: Instances of `PidNamespace` are always reference-counted.
-unsafe impl AlwaysRefCounted for PidNamespace {
+unsafe impl RefCounted for PidNamespace {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -55,6 +59,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<PidNamespace>` from
+// a `&PidNamespace`.
+unsafe impl AlwaysRefCounted for PidNamespace {}
+
// SAFETY:
// - `PidNamespace::dec_ref` can be called from any thread.
// - It is okay to send ownership of `PidNamespace` across thread boundaries.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 7205fe3416d3..bf2aa496b377 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -13,6 +13,7 @@
irq::{self, IrqRequest},
of,
prelude::*,
+ sync::aref::{AlwaysRefCounted, RefCounted},
types::Opaque,
ThisModule,
};
@@ -468,7 +469,7 @@ pub fn optional_irq_by_name(&self, name: &CStr) -> Result<IrqRequest<'_>> {
impl crate::dma::Device for Device<device::Core> {}
// SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
+unsafe impl RefCounted for Device {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
unsafe { bindings::get_device(self.as_ref().as_raw()) };
@@ -480,6 +481,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
+// `&Device`.
+unsafe impl AlwaysRefCounted for Device {}
+
impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
fn as_ref(&self) -> &device::Device<Ctx> {
// SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index e175aefe8615..4226119d5ac9 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -19,11 +19,9 @@
use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull};
-/// Types that are _always_ reference counted.
+/// Types that are internally reference counted.
///
/// It allows such types to define their own custom ref increment and decrement functions.
-/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference
-/// [`ARef<T>`].
///
/// This is usually implemented by wrappers to existing structures on the C side of the code. For
/// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
@@ -40,9 +38,8 @@
/// at least until matching decrements are performed.
///
/// Implementers must also ensure that all instances are reference-counted. (Otherwise they
-/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object
-/// alive.)
-pub unsafe trait AlwaysRefCounted {
+/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
+pub unsafe trait RefCounted {
/// Increments the reference count on the object.
fn inc_ref(&self);
@@ -55,11 +52,27 @@ pub unsafe trait AlwaysRefCounted {
/// Callers must ensure that there was a previous matching increment to the reference count,
/// and that the object is no longer used after its reference count is decremented (as it may
/// result in the object being freed), unless the caller owns another increment on the refcount
- /// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls
- /// [`AlwaysRefCounted::dec_ref`] once).
+ /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls [`RefCounted::dec_ref`] once).
unsafe fn dec_ref(obj: NonNull<Self>);
}
+/// Always reference-counted type.
+///
+/// It allows to derive a counted reference [`ARef<T>`] from a `&T`.
+///
+/// This provides some convenience, but it allows "escaping" borrow checks on `&T`. As it
+/// complicates attempts to ensure that a reference to T is unique, it is optional to provide for
+/// [`RefCounted`] types. See *Safety* below.
+///
+/// # Safety
+///
+/// Implementers must ensure that no safety invariants are violated by upgrading an `&T` to an
+/// [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`crate::types::Ownable`]
+/// cannot be implemented for the same type, as this would allow to violate the uniqueness guarantee
+/// of [`crate::types::Owned<T>`] by derefencing it into an `&T` and obtaining an [`ARef`] from
+/// that.
+pub unsafe trait AlwaysRefCounted: RefCounted {}
+
/// An owned reference to an always-reference-counted object.
///
/// The object's reference count is automatically decremented when an instance of [`ARef`] is
@@ -70,7 +83,7 @@ pub unsafe trait AlwaysRefCounted {
///
/// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In
/// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
-pub struct ARef<T: AlwaysRefCounted> {
+pub struct ARef<T: RefCounted> {
ptr: NonNull<T>,
_p: PhantomData<T>,
}
@@ -79,16 +92,16 @@ pub struct ARef<T: AlwaysRefCounted> {
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
// `T` to be `Send` because any thread that has an `ARef<T>` may ultimately access `T` using a
// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
-unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
+unsafe impl<T: RefCounted + Sync + Send> Send for ARef<T> {}
// SAFETY: It is safe to send `&ARef<T>` to another thread when the underlying `T` is `Sync`
// because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
// it needs `T` to be `Send` because any thread that has a `&ARef<T>` may clone it and get an
// `ARef<T>` on that thread, so the thread may ultimately access `T` using a mutable reference, for
// example, when the reference count reaches zero and `T` is dropped.
-unsafe impl<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
+unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
-impl<T: AlwaysRefCounted> ARef<T> {
+impl<T: RefCounted> ARef<T> {
/// Creates a new instance of [`ARef`].
///
/// It takes over an increment of the reference count on the underlying object.
@@ -117,12 +130,12 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
///
/// ```
/// use core::ptr::NonNull;
- /// use kernel::sync::aref::{ARef, AlwaysRefCounted};
+ /// use kernel::sync::aref::{ARef, RefCounted};
///
/// struct Empty {}
///
/// # // SAFETY: TODO.
- /// unsafe impl AlwaysRefCounted for Empty {
+ /// unsafe impl RefCounted for Empty {
/// fn inc_ref(&self) {}
/// unsafe fn dec_ref(_obj: NonNull<Self>) {}
/// }
@@ -140,7 +153,7 @@ pub fn into_raw(me: Self) -> NonNull<T> {
}
}
-impl<T: AlwaysRefCounted> Clone for ARef<T> {
+impl<T: RefCounted> Clone for ARef<T> {
fn clone(&self) -> Self {
self.inc_ref();
// SAFETY: We just incremented the refcount above.
@@ -148,7 +161,7 @@ fn clone(&self) -> Self {
}
}
-impl<T: AlwaysRefCounted> Deref for ARef<T> {
+impl<T: RefCounted> Deref for ARef<T> {
type Target = T;
fn deref(&self) -> &Self::Target {
@@ -165,7 +178,7 @@ fn from(b: &T) -> Self {
}
}
-impl<T: AlwaysRefCounted> Drop for ARef<T> {
+impl<T: RefCounted> Drop for ARef<T> {
fn drop(&mut self) {
// SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
// decrement.
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 49fad6de0674..0a6e38d98456 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -9,8 +9,8 @@
ffi::{c_int, c_long, c_uint},
mm::MmWithUser,
pid_namespace::PidNamespace,
- sync::aref::ARef,
- types::{NotThreadSafe, Opaque},
+ sync::aref::{AlwaysRefCounted, RefCounted},
+ types::{ARef, NotThreadSafe, Opaque},
};
use core::{
cmp::{Eq, PartialEq},
@@ -348,7 +348,7 @@ pub fn active_pid_ns(&self) -> Option<&PidNamespace> {
}
// SAFETY: The type invariants guarantee that `Task` is always refcounted.
-unsafe impl crate::sync::aref::AlwaysRefCounted for Task {
+unsafe impl RefCounted for Task {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -362,6 +362,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Task>` from a
+// `&Task`.
+unsafe impl AlwaysRefCounted for Task {}
+
impl Kuid {
/// Get the current euid.
#[inline]
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 7bc07c38cd6c..8ef01393352b 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -13,7 +13,7 @@
pub use crate::owned::{Ownable, Owned};
-pub use crate::sync::aref::{ARef, AlwaysRefCounted};
+pub use crate::sync::aref::{ARef, AlwaysRefCounted, RefCounted};
/// Used to transfer ownership to and from foreign (non-Rust) languages.
///
--
2.51.2
^ permalink raw reply related
* [PATCH v13 1/4] rust: types: Add Ownable/Owned types
From: Oliver Mangold @ 2025-11-17 10:07 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Benno Lossin, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
Serge Hallyn, Asahi Lina
Cc: rust-for-linux, linux-kernel, linux-block, dri-devel,
linux-fsdevel, linux-mm, linux-pm, linux-pci,
linux-security-module, Oliver Mangold
In-Reply-To: <20251117-unique-ref-v13-0-b5b243df1250@pm.me>
From: Asahi Lina <lina+kernel@asahilina.net>
By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
(typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
`AlwaysRefCounted`, this mechanism expects the reference to be unique
within Rust, and does not allow cloning.
Conceptually, this is similar to a `KBox<T>`, except that it delegates
resource management to the `T` instead of using a generic allocator.
[ om:
- Split code into separate file and `pub use` it from types.rs.
- Make from_raw() and into_raw() public.
- Remove OwnableMut, and make DerefMut dependent on Unpin instead.
- Usage example/doctest for Ownable/Owned.
- Fixes to documentation and commit message.
]
Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
Co-developed-by: Oliver Mangold <oliver.mangold@pm.me>
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
---
rust/kernel/lib.rs | 1 +
rust/kernel/owned.rs | 195 +++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/sync/aref.rs | 5 ++
rust/kernel/types.rs | 2 +
4 files changed, 203 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3dd7bebe7888..e0ee04330dd0 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -112,6 +112,7 @@
pub mod of;
#[cfg(CONFIG_PM_OPP)]
pub mod opp;
+pub mod owned;
pub mod page;
#[cfg(CONFIG_PCI)]
pub mod pci;
diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
new file mode 100644
index 000000000000..a2cdd2cb8a10
--- /dev/null
+++ b/rust/kernel/owned.rs
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Unique owned pointer types for objects with custom drop logic.
+//!
+//! These pointer types are useful for C-allocated objects which by API-contract
+//! are owned by Rust, but need to be freed through the C API.
+
+use core::{
+ mem::ManuallyDrop,
+ ops::{Deref, DerefMut},
+ pin::Pin,
+ ptr::NonNull,
+};
+
+/// Type allocated and destroyed on the C side, but owned by Rust.
+///
+/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type. This
+/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather
+/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is
+/// executed when the owned reference [`Owned<Self>`] pointing to the object is dropped.
+///
+/// Note: The underlying object is not required to provide internal reference counting, because it
+/// represents a unique, owned reference. If reference counting (on the Rust side) is required,
+/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
+///
+/// # Safety
+///
+/// Implementers must ensure that the [`release()`](Self::release) function frees the underlying
+/// object in the correct way for a valid, owned object of this type.
+///
+/// # Examples
+///
+/// A minimal example implementation of [`Ownable`] and its usage with [`Owned`] looks like this:
+///
+/// ```
+/// # #![expect(clippy::disallowed_names)]
+/// # use core::cell::Cell;
+/// # use core::ptr::NonNull;
+/// # use kernel::sync::global_lock;
+/// # use kernel::alloc::{flags, kbox::KBox, AllocError};
+/// # use kernel::types::{Owned, Ownable};
+///
+/// // Let's count the allocations to see if freeing works.
+/// kernel::sync::global_lock! {
+/// // SAFETY: we call `init()` right below, before doing anything else.
+/// unsafe(uninit) static FOO_ALLOC_COUNT: Mutex<usize> = 0;
+/// }
+/// // SAFETY: We call `init()` only once, here.
+/// unsafe { FOO_ALLOC_COUNT.init() };
+///
+/// struct Foo {
+/// }
+///
+/// impl Foo {
+/// fn new() -> Result<Owned<Self>, AllocError> {
+/// // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is
+/// // not actually a C-allocated object.
+/// let result = KBox::new(
+/// Foo {},
+/// flags::GFP_KERNEL,
+/// )?;
+/// let result = NonNull::new(KBox::into_raw(result))
+/// .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
+/// // Count new allocation
+/// *FOO_ALLOC_COUNT.lock() += 1;
+/// // SAFETY: We just allocated the `Self`, thus it is valid and there cannot be any other
+/// // Rust references. Calling `into_raw()` makes us responsible for ownership and we won't
+/// // use the raw pointer anymore. Thus we can transfer ownership to the `Owned`.
+/// Ok(unsafe { Owned::from_raw(result) })
+/// }
+/// }
+///
+/// // SAFETY: What out `release()` function does is safe of any valid `Self`.
+/// unsafe impl Ownable for Foo {
+/// unsafe fn release(this: NonNull<Self>) {
+/// // The `Foo` will be dropped when `KBox` goes out of scope.
+/// // SAFETY: The [`KBox<Self>`] is still alive. We can pass ownership to the [`KBox`], as
+/// // by requirement on calling this function, the `Self` will no longer be used by the
+/// // caller.
+/// unsafe { KBox::from_raw(this.as_ptr()) };
+/// // Count released allocation
+/// *FOO_ALLOC_COUNT.lock() -= 1;
+/// }
+/// }
+///
+/// {
+/// let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
+/// assert!(*FOO_ALLOC_COUNT.lock() == 1);
+/// }
+/// // `foo` is out of scope now, so we expect no live allocations.
+/// assert!(*FOO_ALLOC_COUNT.lock() == 0);
+/// ```
+pub unsafe trait Ownable {
+ /// Releases the object.
+ ///
+ /// # Safety
+ ///
+ /// Callers must ensure that:
+ /// - `this` points to a valid `Self`.
+ /// - `*this` is no longer used after this call.
+ unsafe fn release(this: NonNull<Self>);
+}
+
+/// An owned reference to an owned `T`.
+///
+/// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
+/// dropped.
+///
+/// # Invariants
+///
+/// - The [`Owned<T>`] has exclusive access to the instance of `T`.
+/// - The instance of `T` will stay alive at least as long as the [`Owned<T>`] is alive.
+pub struct Owned<T: Ownable> {
+ ptr: NonNull<T>,
+}
+
+// SAFETY: It is safe to send an [`Owned<T>`] to another thread when the underlying `T` is [`Send`],
+// because of the ownership invariant. Sending an [`Owned<T>`] is equivalent to sending the `T`.
+unsafe impl<T: Ownable + Send> Send for Owned<T> {}
+
+// SAFETY: It is safe to send [`&Owned<T>`] to another thread when the underlying `T` is [`Sync`],
+// because of the ownership invariant. Sending an [`&Owned<T>`] is equivalent to sending the `&T`.
+unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
+
+impl<T: Ownable> Owned<T> {
+ /// Creates a new instance of [`Owned`].
+ ///
+ /// It takes over ownership of the underlying object.
+ ///
+ /// # Safety
+ ///
+ /// Callers must ensure that:
+ /// - `ptr` points to a valid instance of `T`.
+ /// - Ownership of the underlying `T` can be transferred to the `Self<T>` (i.e. operations
+ /// which require ownership will be safe).
+ /// - No other Rust references to the underlying object exist. This implies that the underlying
+ /// object is not accessed through `ptr` anymore after the function call (at least until the
+ /// the `Self<T>` is dropped.
+ /// - The C code follows the usual shared reference requirements. That is, the kernel will never
+ /// mutate or free the underlying object (excluding interior mutability that follows the usual
+ /// rules) while Rust owns it.
+ /// - In case `T` implements [`Unpin`] the previous requirement is extended from shared to
+ /// mutable reference requirements. That is, the kernel will not mutate or free the underlying
+ /// object and is okay with it being modified by Rust code.
+ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
+ Self {
+ ptr,
+ }
+ }
+
+ /// Consumes the [`Owned`], returning a raw pointer.
+ ///
+ /// This function does not actually relinquish ownership of the object. After calling this
+ /// function, the caller is responsible for ownership previously managed
+ /// by the [`Owned`].
+ pub fn into_raw(me: Self) -> NonNull<T> {
+ ManuallyDrop::new(me).ptr
+ }
+
+ /// Get a pinned mutable reference to the data owned by this `Owned<T>`.
+ pub fn get_pin_mut(&mut self) -> Pin<&mut T> {
+ // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
+ // return a mutable reference to it.
+ let unpinned = unsafe { self.ptr.as_mut() };
+
+ // SAFETY: We never hand out unpinned mutable references to the data in
+ // `Self`, unless the contained type is `Unpin`.
+ unsafe { Pin::new_unchecked(unpinned) }
+ }
+}
+
+impl<T: Ownable> Deref for Owned<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: The type invariants guarantee that the object is valid.
+ unsafe { self.ptr.as_ref() }
+ }
+}
+
+impl<T: Ownable + Unpin> DerefMut for Owned<T> {
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
+ // return a mutable reference to it.
+ unsafe { self.ptr.as_mut() }
+ }
+}
+
+impl<T: Ownable> Drop for Owned<T> {
+ fn drop(&mut self) {
+ // SAFETY: The type invariants guarantee that the `Owned` owns the object we're about to
+ // release.
+ unsafe { T::release(self.ptr) };
+ }
+}
diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index 0d24a0432015..e175aefe8615 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -29,6 +29,11 @@
/// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
/// instances of a type.
///
+/// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
+/// internal reference count and provides only shared references. If unique references are required
+/// [`Ownable`](crate::types::Ownable) should be implemented which allows types to be wrapped in an
+/// [`Owned<Self>`](crate::types::Owned).
+///
/// # Safety
///
/// Implementers must ensure that increments to the reference count keep the object alive in memory
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index dc0a02f5c3cf..7bc07c38cd6c 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -11,6 +11,8 @@
};
use pin_init::{PinInit, Wrapper, Zeroable};
+pub use crate::owned::{Ownable, Owned};
+
pub use crate::sync::aref::{ARef, AlwaysRefCounted};
/// Used to transfer ownership to and from foreign (non-Rust) languages.
--
2.51.2
^ permalink raw reply related
* [PATCH v13 0/4] New trait OwnableRefCounted for ARef<->Owned conversion.
From: Oliver Mangold @ 2025-11-17 10:07 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Benno Lossin, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
Serge Hallyn, Asahi Lina
Cc: rust-for-linux, linux-kernel, linux-block, dri-devel,
linux-fsdevel, linux-mm, linux-pm, linux-pci,
linux-security-module, Oliver Mangold
This allows to convert between ARef<T> and Owned<T> by
implementing the new trait OwnedRefCounted.
This way we will have a shared/unique reference counting scheme
for types with built-in refcounts in analogy to Arc/UniqueArc.
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
Changes in v13:
- Rebase onto v6.18-rc1 (Andreas's work).
- Documentation and style fixes contributed by Andreas
- Link to v12: https://lore.kernel.org/r/20251001-unique-ref-v12-0-fa5c31f0c0c4@pm.me
Changes in v12:
-
- Rebase onto v6.17-rc1 (Andreas's work).
- moved kernel/types/ownable.rs to kernel/owned.rs
- Drop OwnableMut, make DerefMut depend on Unpin instead. I understood
ML discussion as that being okay, but probably needs further scrunity.
- Lots of more documentation changes suggested by reviewers.
- Usage example for Ownable/Owned.
- Link to v11: https://lore.kernel.org/r/20250618-unique-ref-v11-0-49eadcdc0aa6@pm.me
Changes in v11:
- Rework of documentation. I tried to honor all requests for changes "in
spirit" plus some clearifications and corrections of my own.
- Dropping `SimpleOwnedRefCounted` by request from Alice, as it creates a
potentially problematic blanket implementation (which a derive macro that
could be created later would not have).
- Dropping Miguel's "kbuild: provide `RUSTC_HAS_DO_NOT_RECOMMEND` symbol"
patch, as it is not needed anymore after dropping `SimpleOwnedRefCounted`.
(I can add it again, if it is considered useful anyway).
- Link to v10: https://lore.kernel.org/r/20250502-unique-ref-v10-0-25de64c0307f@pm.me
Changes in v10:
- Moved kernel/ownable.rs to kernel/types/ownable.rs
- Fixes in documentation / comments as suggested by Andreas Hindborg
- Added Reviewed-by comment for Andreas Hindborg
- Fix rustfmt of pid_namespace.rs
- Link to v9: https://lore.kernel.org/r/20250325-unique-ref-v9-0-e91618c1de26@pm.me
Changes in v9:
- Rebase onto v6.14-rc7
- Move Ownable/OwnedRefCounted/Ownable, etc., into separate module
- Documentation fixes to Ownable/OwnableMut/OwnableRefCounted
- Add missing SAFETY documentation to ARef example
- Link to v8: https://lore.kernel.org/r/20250313-unique-ref-v8-0-3082ffc67a31@pm.me
Changes in v8:
- Fix Co-developed-by and Suggested-by tags as suggested by Miguel and Boqun
- Some small documentation fixes in Owned/Ownable patch
- removing redundant trait constraint on DerefMut for Owned as suggested by Boqun Feng
- make SimpleOwnedRefCounted no longer implement RefCounted as suggested by Boqun Feng
- documentation for RefCounted as suggested by Boqun Feng
- Link to v7: https://lore.kernel.org/r/20250310-unique-ref-v7-0-4caddb78aa05@pm.me
Changes in v7:
- Squash patch to make Owned::from_raw/into_raw public into parent
- Added Signed-off-by to other people's commits
- Link to v6: https://lore.kernel.org/r/20250310-unique-ref-v6-0-1ff53558617e@pm.me
Changes in v6:
- Changed comments/formatting as suggested by Miguel Ojeda
- Included and used new config flag RUSTC_HAS_DO_NOT_RECOMMEND,
thus no changes to types.rs will be needed when the attribute
becomes available.
- Fixed commit message for Owned patch.
- Link to v5: https://lore.kernel.org/r/20250307-unique-ref-v5-0-bffeb633277e@pm.me
Changes in v5:
- Rebase the whole thing on top of the Ownable/Owned traits by Asahi Lina.
- Rename AlwaysRefCounted to RefCounted and make AlwaysRefCounted a
marker trait instead to allow to obtain an ARef<T> from an &T,
which (as Alice pointed out) is unsound when combined with UniqueRef/Owned.
- Change the Trait design and naming to implement this feature,
UniqueRef/UniqueRefCounted is dropped in favor of Ownable/Owned and
OwnableRefCounted is used to provide the functions to convert
between Owned and ARef.
- Link to v4: https://lore.kernel.org/r/20250305-unique-ref-v4-1-a8fdef7b1c2c@pm.me
Changes in v4:
- Just a minor change in naming by request from Andreas Hindborg,
try_shared_to_unique() -> try_from_shared(),
unique_to_shared() -> into_shared(),
which is more in line with standard Rust naming conventions.
- Link to v3: https://lore.kernel.org/r/Z8Wuud2UQX6Yukyr@mango
---
Asahi Lina (1):
rust: types: Add Ownable/Owned types
Oliver Mangold (3):
rust: `AlwaysRefCounted` is renamed to `RefCounted`.
rust: Add missing SAFETY documentation for `ARef` example
rust: Add `OwnableRefCounted`
rust/kernel/auxiliary.rs | 7 +-
rust/kernel/block/mq/request.rs | 15 +-
rust/kernel/cred.rs | 13 +-
rust/kernel/device.rs | 13 +-
rust/kernel/device/property.rs | 7 +-
rust/kernel/drm/device.rs | 10 +-
rust/kernel/drm/gem/mod.rs | 10 +-
rust/kernel/fs/file.rs | 16 +-
rust/kernel/lib.rs | 1 +
rust/kernel/mm.rs | 15 +-
rust/kernel/mm/mmput_async.rs | 9 +-
rust/kernel/opp.rs | 10 +-
rust/kernel/owned.rs | 317 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/pci.rs | 10 +-
rust/kernel/pid_namespace.rs | 12 +-
rust/kernel/platform.rs | 7 +-
rust/kernel/sync/aref.rs | 69 ++++++---
rust/kernel/task.rs | 10 +-
rust/kernel/types.rs | 4 +-
19 files changed, 492 insertions(+), 63 deletions(-)
---
base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
change-id: 20250305-unique-ref-29fcd675f9e9
Best regards,
--
Oliver Mangold <oliver.mangold@pm.me>
^ 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