* [RFC PATCH 3/5] samples/landlock: Add support for LANDLOCK_ACCESS_FS_CONNECT_UNIX
From: Günther Noack @ 2026-01-01 13:41 UTC (permalink / raw)
To: Mickaël Salaün, Paul Moore
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Günther Noack
In-Reply-To: <20260101134102.25938-1-gnoack3000@gmail.com>
Add unix(7) connect() support to the Landlock sample tool.
The "connect UNIX" right is grouped with the read-write rights in the
sample tool. Rationale: In the general case, any operations are
possible through a Unix domain socket, including data-mutating
operations.
Cc: Justin Suess <utilityemal77@gmail.com>
Cc: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
samples/landlock/sandboxer.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208b..b24ef317d1ea9 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -295,11 +295,12 @@ static bool check_ruleset_scope(const char *const env_var,
LANDLOCK_ACCESS_FS_MAKE_SYM | \
LANDLOCK_ACCESS_FS_REFER | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CONNECT_UNIX)
/* clang-format on */
-#define LANDLOCK_ABI_LAST 7
+#define LANDLOCK_ABI_LAST 8
#define XSTR(s) #s
#define STR(s) XSTR(s)
@@ -444,6 +445,12 @@ int main(const int argc, char *const argv[], char *const *const envp)
"provided by ABI version %d (instead of %d).\n",
LANDLOCK_ABI_LAST, abi);
__attribute__((fallthrough));
+ case 7:
+ /* Removes LANDLOCK_ACCESS_FS_CONNECT_UNIX for ABI < 8 */
+ ruleset_attr.handled_access_fs &=
+ ~LANDLOCK_ACCESS_FS_CONNECT_UNIX;
+
+ __attribute__((fallthrough));
case LANDLOCK_ABI_LAST:
break;
default:
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 2/5] landlock: Control connections to pathname UNIX sockets by path
From: Günther Noack @ 2026-01-01 13:40 UTC (permalink / raw)
To: Mickaël Salaün, Paul Moore
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Günther Noack
In-Reply-To: <20260101134102.25938-1-gnoack3000@gmail.com>
* Add the LANDLOCK_ACCESS_FS_CONNECT_UNIX right.
* Hook into the path lookup in unix_find_bsd() in af_unix.c,
using a LSM hook. Decide based on the new access right.
* Increment the Landlock ABI version.
* Minor test adaptions to keep the tests working.
Cc: Justin Suess <utilityemal77@gmail.com>
Cc: Mickaël Salaün <mic@digikod.net>
Suggested-by: Jann Horn <jannh@google.com>
Link: https://github.com/landlock-lsm/linux/issues/36
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
include/uapi/linux/landlock.h | 3 +++
security/landlock/access.h | 2 +-
security/landlock/audit.c | 1 +
security/landlock/fs.c | 9 ++++++++-
security/landlock/limits.h | 2 +-
security/landlock/syscalls.c | 2 +-
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/fs_test.c | 5 +++--
8 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f030adc462ee7..189a6e3bbac05 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -216,6 +216,8 @@ struct landlock_net_port_attr {
* :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with
* ``O_TRUNC``. This access right is available since the third version of the
* Landlock ABI.
+ * - %LANDLOCK_ACCESS_FS_CONNECT_UNIX: Connect to named :manpage:`unix(7)`
+ * socket files.
*
* Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
* with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
@@ -321,6 +323,7 @@ struct landlock_net_port_attr {
#define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
#define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15)
+#define LANDLOCK_ACCESS_FS_CONNECT_UNIX (1ULL << 16)
/* clang-format on */
/**
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 7961c6630a2d7..c7784922be3ca 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -34,7 +34,7 @@
LANDLOCK_ACCESS_FS_IOCTL_DEV)
/* clang-format on */
-typedef u16 access_mask_t;
+typedef u32 access_mask_t;
/* Makes sure all filesystem access rights can be stored. */
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index e899995f1fd59..1a937cbdca7af 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
[BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
+ [BIT_INDEX(LANDLOCK_ACCESS_FS_CONNECT_UNIX)] = "fs.connect_unix",
};
static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 8205673c8b1c4..e8cecbd7f2490 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -314,7 +314,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CONNECT_UNIX)
/* clang-format on */
/*
@@ -1588,6 +1589,11 @@ static int hook_path_truncate(const struct path *const path)
return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
}
+static int hook_unix_path_connect(const struct path *const path)
+{
+ return current_check_access_path(path, LANDLOCK_ACCESS_FS_CONNECT_UNIX);
+}
+
/* File hooks */
/**
@@ -1872,6 +1878,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
LSM_HOOK_INIT(path_unlink, hook_path_unlink),
LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
LSM_HOOK_INIT(path_truncate, hook_path_truncate),
+ LSM_HOOK_INIT(unix_path_connect, hook_unix_path_connect),
LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
LSM_HOOK_INIT(file_open, hook_file_open),
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 65b5ff0516747..5928d538d8115 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -19,7 +19,7 @@
#define LANDLOCK_MAX_NUM_LAYERS 16
#define LANDLOCK_MAX_NUM_RULES U32_MAX
-#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV
+#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_CONNECT_UNIX
#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 0116e9f93ffe3..66fd196be85a8 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -161,7 +161,7 @@ static const struct file_operations ruleset_fops = {
* Documentation/userspace-api/landlock.rst should be updated to reflect the
* UAPI change.
*/
-const int landlock_abi_version = 7;
+const int landlock_abi_version = 8;
/**
* sys_landlock_create_ruleset - Create a new ruleset
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 7b69002239d7e..f4b1a275d8d96 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -76,7 +76,7 @@ TEST(abi_version)
const struct landlock_ruleset_attr ruleset_attr = {
.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
};
- ASSERT_EQ(7, landlock_create_ruleset(NULL, 0,
+ ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
LANDLOCK_CREATE_RULESET_VERSION));
ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 16503f2e6a481..74e975c5e9847 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -575,9 +575,10 @@ TEST_F_FORK(layout1, inval)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CONNECT_UNIX)
-#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV
+#define ACCESS_LAST LANDLOCK_ACCESS_FS_CONNECT_UNIX
#define ACCESS_ALL ( \
ACCESS_FILE | \
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 1/5] landlock/selftests: add a missing close(srv_fd) call
From: Günther Noack @ 2026-01-01 13:40 UTC (permalink / raw)
To: Mickaël Salaün, Paul Moore
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Günther Noack
In-Reply-To: <20260101134102.25938-1-gnoack3000@gmail.com>
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
tools/testing/selftests/landlock/fs_test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 37a5a3df712ec..16503f2e6a481 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -4400,6 +4400,7 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
EXPECT_EQ(0, test_fionread_ioctl(cli_fd));
ASSERT_EQ(0, close(cli_fd));
+ ASSERT_EQ(0, close(srv_fd));
}
/* clang-format off */
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 0/5] landlock: Pathname-based UNIX connect() control
From: Günther Noack @ 2026-01-01 13:40 UTC (permalink / raw)
To: Mickaël Salaün, Paul Moore
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Günther Noack
Happy New Year!
This patch set introduces a file-system-based Landlock restriction
mechanism for connecting to Unix sockets.
## Motivation
Currently, landlocked processes can connect() to named UNIX sockets
through the BSD socket API described in unix(7), by invoking socket(2)
followed by connect(2) with a suitable struct sockname_un holding the
socket's filename. This can come as a surprise for users (e.g. in
[1]) and it can be used to escape a sandbox when a Unix service offers
command execution (some scenarios were listed by Tingmao Wang in [2]).
These patches are built on Justin Suess's patch which adds the LSM
hook:
https://lore.kernel.org/all/20251231213314.2979118-1-utilityemal77@gmail.com/
I am keeping this tagged as "RFC" since Justin's patch is RFC as well
so far.
The original feature request is at [4].
## Alternatives and Related Work
### Alternative: Use existing LSM hooks
The existing hooks security_unix_stream_connect(),
security_unix_may_send() and security_socket_connect() do not give
access to the resolved file system path.
Resolving the file system path again within Landlock would in my
understanding produce a TOCTOU race, so making the decision based on
the struct sockaddr_un contents is not an option.
It is tempting to use the struct path that the listening socket is
bound to, which can be acquired through the existing hooks.
Unfortunately, the listening socket may have been bound from within a
different namespace, and it is therefore a path that can not actually
be referenced by the sandboxed program at the time of constructing the
Landlock policy. (More details are on the Github issue at [6]).
### Related work: Scope Control for Pathname Unix Sockets
The motivation for this patch is the same as in Tingmao Wang's patch
set for "scoped" control for pathname Unix sockets [2], originally
proposed in the Github feature request [5].
In my reply to this patch set [3], I have discussed the differences
between these two approaches. On the related discussions on Github
[4] and [5], there was consensus that the scope-based control is
complimentary to the file system based control, but does not replace
it. Mickael's opening remark on [5] says:
> This scoping would be complementary to #36 which would mainly be
> about allowing a sandboxed process to connect to a more privileged
> service (identified with a path).
## Credits
The feature was originally suggested by Jann Horn in [7].
Tingmao Wang and Demi Marie Obenour have taken the initiative to
revive this discussion again in [1], [4] and [5] and Tingmao Wang has
sent the patch set for the scoped access control for pathname Unix
sockets [2].
Justin Suess has sent the patch for the LSM hook in [8].
Ryan Sullivan has started on an initial implementation and has brought
up relevant discussion points on the Github issue at [4] that lead to
the current approach.
[1] https://lore.kernel.org/landlock/515ff0f4-2ab3-46de-8d1e-5c66a93c6ede@gmail.com/
[2] Tingmao Wang's "Implemnet scope control for pathname Unix sockets"
https://lore.kernel.org/all/cover.1767115163.git.m@maowtm.org/
[3] https://lore.kernel.org/all/20251230.bcae69888454@gnoack.org/
[4] Github issue for FS-based control for named Unix sockets:
https://github.com/landlock-lsm/linux/issues/36
[5] Github issue for scope-based restriction of named Unix sockets:
https://github.com/landlock-lsm/linux/issues/51
[6] https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
[7] https://lore.kernel.org/linux-security-module/CAG48ez3NvVnonOqKH4oRwRqbSOLO0p9djBqgvxVwn6gtGQBPcw@mail.gmail.com/
[8] Patch for the LSM hook:
https://lore.kernel.org/linux-security-module/CAG48ez3NvVnonOqKH4oRwRqbSOLO0p9djBqgvxVwn6gtGQBPcw@mail.gmail.com/
Günther Noack (5):
landlock/selftests: add a missing close(srv_fd) call
landlock: Control connections to pathname UNIX sockets by path
samples/landlock: Add support for LANDLOCK_ACCESS_FS_CONNECT_UNIX
landlock/selftests: test LANDLOCK_ACCESS_FS_CONNECT_UNIX
landlock: Document LANDLOCK_ACCESS_FS_UNIX_CONNECT
Documentation/userspace-api/landlock.rst | 14 ++-
include/uapi/linux/landlock.h | 3 +
samples/landlock/sandboxer.c | 11 ++-
security/landlock/access.h | 2 +-
security/landlock/audit.c | 1 +
security/landlock/fs.c | 9 +-
security/landlock/limits.h | 2 +-
security/landlock/syscalls.c | 2 +-
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/fs_test.c | 94 ++++++++++++++++----
10 files changed, 113 insertions(+), 27 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [RFC PATCH 1/1] lsm: Add hook unix_path_connect
From: Günther Noack @ 2026-01-01 12:13 UTC (permalink / raw)
To: Justin Suess
Cc: Paul Moore, James Morris, Serge E . Hallyn, Kuniyuki Iwashima,
Simon Horman, Mickaël Salaün, Günther Noack,
linux-security-module, Tingmao Wang, netdev
In-Reply-To: <20251231213314.2979118-2-utilityemal77@gmail.com>
On Wed, Dec 31, 2025 at 04:33:14PM -0500, Justin Suess wrote:
> Adds an LSM hook unix_path_connect.
>
> This hook is called to check the path of a named unix socket before a
> connection is initiated.
>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> Cc: Günther Noack <gnoack3000@gmail.com>
> ---
> include/linux/lsm_hook_defs.h | 1 +
> include/linux/security.h | 6 ++++++
> net/unix/af_unix.c | 8 ++++++++
> security/security.c | 16 ++++++++++++++++
> 4 files changed, 31 insertions(+)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..a42d1aaf3b8a 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -318,6 +318,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
> #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>
> #ifdef CONFIG_SECURITY_NETWORK
> +LSM_HOOK(int, 0, unix_path_connect, const struct path *path)
You are placing this guarded by CONFIG_SECURITY_NETWORK, but there is
also CONFIG_SECURITY_PATH. Should it be guarded by both?
> LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
> struct sock *newsk)
> LSM_HOOK(int, 0, unix_may_send, struct socket *sock, struct socket *other)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..ab66f22f7e5a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1638,6 +1638,7 @@ static inline int security_watch_key(struct key *key)
>
> #ifdef CONFIG_SECURITY_NETWORK
>
> +int security_unix_path_connect(const struct path *path);
> int security_netlink_send(struct sock *sk, struct sk_buff *skb);
> int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
> int security_unix_may_send(struct socket *sock, struct socket *other);
> @@ -1699,6 +1700,11 @@ static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb)
> return 0;
> }
>
> +static inline int security_unix_path_connect(const struct path *path)
> +{
> + return 0;
> +}
> +
> static inline int security_unix_stream_connect(struct sock *sock,
> struct sock *other,
> struct sock *newsk)
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 55cdebfa0da0..af1a6083a69b 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1226,6 +1226,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> if (!S_ISSOCK(inode->i_mode))
> goto path_put;
>
> + /*
> + * We call the hook because we know that the inode is a socket
> + * and we hold a valid reference to it via the path.
> + */
> + err = security_unix_path_connect(&path);
> + if (err)
> + goto path_put;
In this place, the hook call is done also for the coredump socket.
The coredump socket is a system-wide setting, and it feels weird to me
that unprivileged processes should be able to inhibit that connection?
> +
> sk = unix_find_socket_byinode(inode);
> if (!sk)
> goto path_put;
> diff --git a/security/security.c b/security/security.c
> index 31a688650601..17af5d0ddf28 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4047,6 +4047,22 @@ int security_unix_stream_connect(struct sock *sock, struct sock *other,
> }
> EXPORT_SYMBOL(security_unix_stream_connect);
>
> +/*
> + * security_unix_path_connect() - Check if a named AF_UNIX socket can connect
> + * @path: Path of the socket being connected to
^
mega-nit: lowercase for consistency
> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket. This is necessary because it was not possible to check the
> + * VFS inode of the target socket before the connection is made.
I'd drop the last sentence; the defense why this is necessary can go
in the commit message, and once we have a call-site for the hook,
someone browsing the kernel code can look up what it is used for.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_path_connect(const struct path *path)
> +{
> + return call_int_hook(unix_path_connect, path);
> +}
> +EXPORT_SYMBOL(security_unix_path_connect);
> +
> /**
> * security_unix_may_send() - Check if AF_UNIX socket can send datagrams
> * @sock: originating sock
> --
> 2.51.0
>
^ permalink raw reply
* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Günther Noack @ 2026-01-01 11:56 UTC (permalink / raw)
To: Justin Suess, Paul Moore
Cc: James Morris, Serge E . Hallyn, Kuniyuki Iwashima, Simon Horman,
Mickaël Salaün, Günther Noack,
linux-security-module, Tingmao Wang, netdev
In-Reply-To: <20251231213314.2979118-1-utilityemal77@gmail.com>
Thank you for sending this, Justin!
Paul: Could you please have a look at this new LSM hook? -- it extends
the LSM interface and it is an approach that I have suggested in [1].
On Wed, Dec 31, 2025 at 04:33:13PM -0500, Justin Suess wrote:
> Hi,
>
> This patch introduces a new LSM hook unix_path_connect.
>
> The idea for this patch and the hook came from Günther Noack, who
> is cc'd. Much credit to him for the idea and discussion.
>
> This patch is based on the lsm next branch.
>
> Motivation
> ---
>
> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> identifying object from a policy perspective is the path passed to
> connect(2). However, this operation currently restricts LSMs that rely
> on VFS-based mediation, because the pathname resolved during connect()
> is not preserved in a form visible to existing hooks before connection
> establishment. As a result, LSMs such as Landlock cannot currently
> restrict connections to named UNIX domain sockets by their VFS path.
>
> This gap has been discussed previously (e.g. in the context of Landlock's
> path-based access controls). [1] [2]
+1
The use case here is that Landlock should be able to restrict
connect() to named Unix sockets and control this based on the natural
identifier for named Unix sockets -- the path of the socket file.
This feature is a useful and necessary addition to Landlock. The
discussion that Tingmao Wang linked to on her patch also shows that
users are caught by surprise when they find that connecting to UNIX
sockets is not restrictable [2]. Her patch set [3] lists some ways in
which this can be a problem.
I understand that adding LSM hooks might be controversial, but I think
that the alternatives to the new LSM hook are both worse:
* The patch set from Tingmao Wang at [3] is not restricting Unix
sockets based on path, but on Landlock policy scope (domain). This
is useful, but only complementary to a path-based restriction. If
be build both features at some point, they'd potentially have
surprising interactions that make the UAPI more confusing. (I've
written more about this at [4])
* We can not use the existing security_socket_connect() hook for this,
because the resolved struct path has already been discarded by the
time when security_socket_connect() is called, and looking up the
struct path again would create a TOCTOU race condition.
The hook is called from the function unix_find_bsd() in the AF_UNIX
implementation, which looks up the struct path and keeps it
transiently in order to find the associated listening-side struct
sock.
Please let us know what you think!
Thanks!
–Günther
[1] https://github.com/landlock-lsm/linux/issues/36#issuecomment-3669080619
[2] https://spectrum-os.org/lists/archives/spectrum-devel/00256266-26db-40cf-8f5b-f7c7064084c2@gmail.com/
[3] https://lore.kernel.org/all/cover.1767115163.git.m@maowtm.org/
[4] https://lore.kernel.org/all/20251230.bcae69888454@gnoack.org/
^ permalink raw reply
* [syzbot ci] Re: lsm: Add hook unix_path_connect
From: syzbot ci @ 2026-01-01 9:46 UTC (permalink / raw)
To: gnoack3000, gnoack, horms, jmorris, kuniyu, linux-security-module,
m, mic, netdev, paul, serge, utilityemal77
Cc: syzbot, syzkaller-bugs
In-Reply-To: <20251231213314.2979118-1-utilityemal77@gmail.com>
syzbot ci has tested the following series
[v1] lsm: Add hook unix_path_connect
https://lore.kernel.org/all/20251231213314.2979118-1-utilityemal77@gmail.com
* [RFC PATCH 1/1] lsm: Add hook unix_path_connect
and found the following issues:
* KASAN: null-ptr-deref Read in unix_dgram_connect
* general protection fault in apparmor_socket_sock_rcv_skb
* general protection fault in unix_stream_connect
Full report is available here:
https://ci.syzbot.org/series/c288b2d0-af95-47d8-b359-79ff653da27b
***
KASAN: null-ptr-deref Read in unix_dgram_connect
tree: net-next
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base: dbf8fe85a16a33d6b6bd01f2bc606fc017771465
arch: amd64
compiler: Debian clang version 21.1.8 (++20251202083448+f68f64eb8130-1~exp1~20251202083504.46), Debian LLD 21.1.8
config: https://ci.syzbot.org/builds/9f532f02-856f-4157-a897-26c37e30c537/config
C repro: https://ci.syzbot.org/findings/d6706e7e-97bc-45af-910c-fb20c328ac02/c_repro
syz repro: https://ci.syzbot.org/findings/d6706e7e-97bc-45af-910c-fb20c328ac02/syz_repro
==================================================================
BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:68 [inline]
BUG: KASAN: null-ptr-deref in _test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
BUG: KASAN: null-ptr-deref in sock_flag include/net/sock.h:1048 [inline]
BUG: KASAN: null-ptr-deref in unix_dgram_connect+0x356/0xc20 net/unix/af_unix.c:1550
Read of size 8 at addr 0000000000000060 by task syz.0.17/5977
CPU: 0 UID: 0 PID: 5977 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
kasan_report+0x117/0x150 mm/kasan/report.c:595
check_region_inline mm/kasan/generic.c:-1 [inline]
kasan_check_range+0x264/0x2c0 mm/kasan/generic.c:200
instrument_atomic_read include/linux/instrumented.h:68 [inline]
_test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
sock_flag include/net/sock.h:1048 [inline]
unix_dgram_connect+0x356/0xc20 net/unix/af_unix.c:1550
__sys_connect_file net/socket.c:2089 [inline]
__sys_connect+0x312/0x450 net/socket.c:2108
__do_sys_connect net/socket.c:2114 [inline]
__se_sys_connect net/socket.c:2111 [inline]
__x64_sys_connect+0x7a/0x90 net/socket.c:2111
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xe2/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f78e4f9acb9
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc8b576858 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 00007f78e5205fa0 RCX: 00007f78e4f9acb9
RDX: 000000000000006e RSI: 0000200000000280 RDI: 0000000000000005
RBP: 00007f78e5008bf7 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f78e5205fac R14: 00007f78e5205fa0 R15: 00007f78e5205fa0
</TASK>
==================================================================
***
general protection fault in apparmor_socket_sock_rcv_skb
tree: net-next
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base: dbf8fe85a16a33d6b6bd01f2bc606fc017771465
arch: amd64
compiler: Debian clang version 21.1.8 (++20251202083448+f68f64eb8130-1~exp1~20251202083504.46), Debian LLD 21.1.8
config: https://ci.syzbot.org/builds/9f532f02-856f-4157-a897-26c37e30c537/config
C repro: https://ci.syzbot.org/findings/7e600b6d-75f6-48a2-a2a0-c69a3aadaa9b/c_repro
syz repro: https://ci.syzbot.org/findings/7e600b6d-75f6-48a2-a2a0-c69a3aadaa9b/syz_repro
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000096: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00000000000004b0-0x00000000000004b7]
CPU: 0 UID: 0 PID: 5987 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:aa_sock security/apparmor/include/net.h:57 [inline]
RIP: 0010:apparmor_socket_sock_rcv_skb+0x3a/0x350 security/apparmor/lsm.c:1513
Code: ec 10 49 89 f7 48 89 fb 49 bd 00 00 00 00 00 fc ff df e8 99 57 6f fd 48 89 5c 24 08 48 81 c3 b0 04 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 17 54 d6 fd 44 8b 25 20 70 74 09
RSP: 0018:ffffc90004237530 EFLAGS: 00010206
RAX: 0000000000000096 RBX: 00000000000004b0 RCX: ffff88816872d7c0
RDX: 0000000000000000 RSI: ffff888110f4e600 RDI: 0000000000000000
RBP: ffffc900042376d0 R08: ffffffff82447acc R09: ffffffff8e341b20
R10: dffffc0000000000 R11: ffffed102388124c R12: ffff888110f4e67e
R13: dffffc0000000000 R14: 0000000000000000 R15: ffff888110f4e600
FS: 00007f36295926c0(0000) GS:ffff88818e40e000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3629591ff8 CR3: 0000000113938000 CR4: 00000000000006f0
Call Trace:
<TASK>
security_sock_rcv_skb+0x8f/0x270 security/security.c:4333
sk_filter_trim_cap+0x19b/0xd90 net/core/filter.c:156
sk_filter include/linux/filter.h:1102 [inline]
unix_dgram_sendmsg+0x7bc/0x17b0 net/unix/af_unix.c:2173
sock_sendmsg_nosec net/socket.c:727 [inline]
__sock_sendmsg+0x21c/0x270 net/socket.c:742
____sys_sendmsg+0x500/0x810 net/socket.c:2592
___sys_sendmsg+0x2a5/0x360 net/socket.c:2646
__sys_sendmmsg+0x27c/0x4e0 net/socket.c:2735
__do_sys_sendmmsg net/socket.c:2762 [inline]
__se_sys_sendmmsg net/socket.c:2759 [inline]
__x64_sys_sendmmsg+0xa0/0xc0 net/socket.c:2759
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xe2/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f362879acb9
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f3629592028 EFLAGS: 00000246 ORIG_RAX: 0000000000000133
RAX: ffffffffffffffda RBX: 00007f3628a06090 RCX: 00007f362879acb9
RDX: 0000000000000002 RSI: 0000200000000ec0 RDI: 0000000000000006
RBP: 00007f3628808bf7 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f3628a06128 R14: 00007f3628a06090 R15: 00007ffcf5635f88
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:aa_sock security/apparmor/include/net.h:57 [inline]
RIP: 0010:apparmor_socket_sock_rcv_skb+0x3a/0x350 security/apparmor/lsm.c:1513
Code: ec 10 49 89 f7 48 89 fb 49 bd 00 00 00 00 00 fc ff df e8 99 57 6f fd 48 89 5c 24 08 48 81 c3 b0 04 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 17 54 d6 fd 44 8b 25 20 70 74 09
RSP: 0018:ffffc90004237530 EFLAGS: 00010206
RAX: 0000000000000096 RBX: 00000000000004b0 RCX: ffff88816872d7c0
RDX: 0000000000000000 RSI: ffff888110f4e600 RDI: 0000000000000000
RBP: ffffc900042376d0 R08: ffffffff82447acc R09: ffffffff8e341b20
R10: dffffc0000000000 R11: ffffed102388124c R12: ffff888110f4e67e
R13: dffffc0000000000 R14: 0000000000000000 R15: ffff888110f4e600
FS: 00007f36295926c0(0000) GS:ffff88818e40e000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3629591ff8 CR3: 0000000113938000 CR4: 00000000000006f0
----------------
Code disassembly (best guess), 2 bytes skipped:
0: 49 89 f7 mov %rsi,%r15
3: 48 89 fb mov %rdi,%rbx
6: 49 bd 00 00 00 00 00 movabs $0xdffffc0000000000,%r13
d: fc ff df
10: e8 99 57 6f fd call 0xfd6f57ae
15: 48 89 5c 24 08 mov %rbx,0x8(%rsp)
1a: 48 81 c3 b0 04 00 00 add $0x4b0,%rbx
21: 48 89 d8 mov %rbx,%rax
24: 48 c1 e8 03 shr $0x3,%rax
* 28: 42 80 3c 28 00 cmpb $0x0,(%rax,%r13,1) <-- trapping instruction
2d: 74 08 je 0x37
2f: 48 89 df mov %rbx,%rdi
32: e8 17 54 d6 fd call 0xfdd6544e
37: 44 8b 25 20 70 74 09 mov 0x9747020(%rip),%r12d # 0x974705e
***
general protection fault in unix_stream_connect
tree: net-next
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base: dbf8fe85a16a33d6b6bd01f2bc606fc017771465
arch: amd64
compiler: Debian clang version 21.1.8 (++20251202083448+f68f64eb8130-1~exp1~20251202083504.46), Debian LLD 21.1.8
config: https://ci.syzbot.org/builds/9f532f02-856f-4157-a897-26c37e30c537/config
C repro: https://ci.syzbot.org/findings/b05720e7-cc44-4796-b729-e2f0c0b9e015/c_repro
syz repro: https://ci.syzbot.org/findings/b05720e7-cc44-4796-b729-e2f0c0b9e015/syz_repro
Oops: general protection fault, probably for non-canonical address 0xdffffc00000000dc: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00000000000006e0-0x00000000000006e7]
CPU: 1 UID: 0 PID: 6000 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:kasan_byte_accessible+0x12/0x30 mm/kasan/generic.c:210
Code: 79 ff ff ff 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 48 c1 ef 03 48 b8 00 00 00 00 00 fc ff df <0f> b6 04 07 3c 08 0f 92 c0 e9 40 68 4e 09 cc 66 66 66 66 66 66 2e
RSP: 0018:ffffc90003f47c00 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffffffff8b7776ee RCX: 0000000080000002
RDX: 0000000000000000 RSI: ffffffff8b7776ee RDI: 00000000000000dc
RBP: ffffffff8a18a569 R08: 0000000000000001 R09: 0000000000000000
R10: dffffc0000000000 R11: ffffed1037900903 R12: 0000000000000000
R13: 00000000000006e0 R14: 00000000000006e0 R15: 0000000000000001
FS: 00007fe5e870a6c0(0000) GS:ffff8882a9a0e000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe5e77e8400 CR3: 0000000114c7e000 CR4: 00000000000006f0
Call Trace:
<TASK>
__kasan_check_byte+0x12/0x40 mm/kasan/common.c:573
kasan_check_byte include/linux/kasan.h:402 [inline]
lock_acquire+0x84/0x330 kernel/locking/lockdep.c:5842
__raw_spin_lock include/linux/spinlock_api_smp.h:133 [inline]
_raw_spin_lock+0x2e/0x40 kernel/locking/spinlock.c:154
spin_lock include/linux/spinlock.h:351 [inline]
unix_stream_connect+0x469/0x1010 net/unix/af_unix.c:1692
__sys_connect_file net/socket.c:2089 [inline]
__sys_connect+0x312/0x450 net/socket.c:2108
__do_sys_connect net/socket.c:2114 [inline]
__se_sys_connect net/socket.c:2111 [inline]
__x64_sys_connect+0x7a/0x90 net/socket.c:2111
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xe2/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe5e779acb9
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fe5e870a028 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 00007fe5e7a05fa0 RCX: 00007fe5e779acb9
RDX: 000000000000006e RSI: 0000200000000000 RDI: 0000000000000005
RBP: 00007fe5e7808bf7 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fe5e7a06038 R14: 00007fe5e7a05fa0 R15: 00007ffcf198af58
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:kasan_byte_accessible+0x12/0x30 mm/kasan/generic.c:210
Code: 79 ff ff ff 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 48 c1 ef 03 48 b8 00 00 00 00 00 fc ff df <0f> b6 04 07 3c 08 0f 92 c0 e9 40 68 4e 09 cc 66 66 66 66 66 66 2e
RSP: 0018:ffffc90003f47c00 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffffffff8b7776ee RCX: 0000000080000002
RDX: 0000000000000000 RSI: ffffffff8b7776ee RDI: 00000000000000dc
RBP: ffffffff8a18a569 R08: 0000000000000001 R09: 0000000000000000
R10: dffffc0000000000 R11: ffffed1037900903 R12: 0000000000000000
R13: 00000000000006e0 R14: 00000000000006e0 R15: 0000000000000001
FS: 00007fe5e870a6c0(0000) GS:ffff8882a9a0e000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe5e77e8400 CR3: 0000000114c7e000 CR4: 00000000000006f0
----------------
Code disassembly (best guess), 4 bytes skipped:
0: 0f 1f 40 00 nopl 0x0(%rax)
4: 90 nop
5: 90 nop
6: 90 nop
7: 90 nop
8: 90 nop
9: 90 nop
a: 90 nop
b: 90 nop
c: 90 nop
d: 90 nop
e: 90 nop
f: 90 nop
10: 90 nop
11: 90 nop
12: 90 nop
13: 90 nop
14: 0f 1f 40 d6 nopl -0x2a(%rax)
18: 48 c1 ef 03 shr $0x3,%rdi
1c: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
23: fc ff df
* 26: 0f b6 04 07 movzbl (%rdi,%rax,1),%eax <-- trapping instruction
2a: 3c 08 cmp $0x8,%al
2c: 0f 92 c0 setb %al
2f: e9 40 68 4e 09 jmp 0x94e6874
34: cc int3
35: 66 data16
36: 66 data16
37: 66 data16
38: 66 data16
39: 66 data16
3a: 66 data16
3b: 2e cs
***
If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
Tested-by: syzbot@syzkaller.appspotmail.com
---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Justin Suess @ 2025-12-31 23:14 UTC (permalink / raw)
To: gnoack3000
Cc: ivanov.mikhail1, konstantin.meskhidze, linux-security-module, m,
matthieu, mic, samasth.norway.ananda, utilityemal77
In-Reply-To: <20251230103917.10549-7-gnoack3000@gmail.com>
On 12/30/25 05:39, Günther Noack wrote:
> The layer masks data structure tracks the requested but unfulfilled
> access rights during an operations security check. It stores one bit
> for each combination of access right and layer index. If the bit is
> set, that access right is not granted (yet) in the given layer and we
> have to traverse the path further upwards to grant it.
>
> Previously, the layer masks were stored as arrays mapping from access
> right indices to layer_mask_t. The layer_mask_t value then indicates
> all layers in which the given access right is still (tentatively)
> denied.
>
> This patch introduces struct layer_access_masks instead: This struct
> contains an array with the access_mask_t of each (tentatively) denied
> access right in that layer.
>
> The hypothesis of this patch is that this simplifies the code enough
> so that the resulting code will run faster:
>
> * We can use bitwise operations in multiple places where we previously
> looped over bits individually with macros. (Should require less
> branch speculation)
>
> * Code is ~160 lines smaller.
>
> Other noteworthy changes:
>
> * Clarify deny_mask_t and the code assembling it.
> * Document what that value looks like
> * Make writing and reading functions specific to file system rules.
> (It only worked for FS rules before as well, but going all the way
> simplifies the code logic more.)
> * In no_more_access(), call a new helper function may_refer(), which
> only solves the asymmetric case. Previously, the code interleaved
> the checks for the two symmetric cases in RENAME_EXCHANGE. It feels
> that the code is clearer when renames without RENAME_EXCHANGE are
> more obviously the normal case.
I was able to reproduce your benchmark, with slightly slower but very
close results.
Definite improvement.
I was also doing it from QEMU, so there might be some confounding variables.
Great job.
>
>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> security/landlock/access.h | 10 +-
> security/landlock/audit.c | 155 ++++++----------
> security/landlock/audit.h | 3 +-
> security/landlock/domain.c | 120 +++----------
> security/landlock/domain.h | 6 +-
> security/landlock/fs.c | 350 ++++++++++++++++--------------------
> security/landlock/net.c | 10 +-
> security/landlock/ruleset.c | 78 +++-----
> security/landlock/ruleset.h | 18 +-
> 9 files changed, 290 insertions(+), 460 deletions(-)
>
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d7..aa0efa36a37db 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -61,14 +61,14 @@ union access_masks_all {
> static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
> sizeof(typeof_member(union access_masks_all, all)));
>
> -typedef u16 layer_mask_t;
> -
> -/* Makes sure all layers can be checked. */
> -static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
> -
> /*
> * Tracks domains responsible of a denied access. This is required to avoid
> * storing in each object the full layer_masks[] required by update_request().
> + *
> + * Each nibble represents the layer index of the newest layer which denied a
> + * certain access right. For file system access rights, the upper four bits are
> + * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
> + * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
> */
> typedef u8 deny_masks_t;
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index c52d079cdb77b..650bd7f5cb6be 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -182,36 +182,18 @@ static void test_get_hierarchy(struct kunit *const test)
>
> static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> access_mask_t *const access_request,
> - const layer_mask_t (*const layer_masks)[],
> - const size_t layer_masks_size)
> + const struct layer_access_masks *masks)
> {
> - const unsigned long access_req = *access_request;
> - unsigned long access_bit;
> - access_mask_t missing = 0;
> - long youngest_layer = -1;
> -
> - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> - const access_mask_t mask = (*layer_masks)[access_bit];
> - long layer;
> -
> - if (!mask)
> - continue;
> -
> - /* __fls(1) == 0 */
> - layer = __fls(mask);
> - if (layer > youngest_layer) {
> - youngest_layer = layer;
> - missing = BIT(access_bit);
> - } else if (layer == youngest_layer) {
> - missing |= BIT(access_bit);
> + for (int i = LANDLOCK_MAX_NUM_LAYERS - 1; i >= 0; i--) {
> + if (masks->access[i] & *access_request) {
> + *access_request &= masks->access[i];
> + return i;
> }
> }
>
> - *access_request = missing;
> - if (youngest_layer == -1)
> - return domain->num_layers - 1;
> -
> - return youngest_layer;
> + /* Not found - fall back to default values */
> + *access_request = 0;
> + return domain->num_layers - 1;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> @@ -221,94 +203,82 @@ static void test_get_denied_layer(struct kunit *const test)
> const struct landlock_ruleset dom = {
> .num_layers = 5,
> };
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
> + const struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_READ_DIR,
> + .access[1] = LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_READ_DIR,
> + .access[2] = LANDLOCK_ACCESS_FS_REMOVE_DIR,
> };
> access_mask_t access;
>
> access = LANDLOCK_ACCESS_FS_EXECUTE;
> - KUNIT_EXPECT_EQ(test, 0,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 0, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
>
> access = LANDLOCK_ACCESS_FS_READ_FILE;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
>
> access = LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access,
> LANDLOCK_ACCESS_FS_READ_FILE |
> LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_WRITE_FILE;
> - KUNIT_EXPECT_EQ(test, 4,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 4, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, 0);
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
>
> -static size_t
> -get_layer_from_deny_masks(access_mask_t *const access_request,
> - const access_mask_t all_existing_optional_access,
> - const deny_masks_t deny_masks)
> +/*
> + * get_layer_from_fs_deny_masks - get the layer which denied the access request
> + *
> + * As a side effect, stores the denied access rights from that layer(!) in
> + * *access_request.
> + */
> +static size_t get_layer_from_fs_deny_masks(access_mask_t *const access_request,
> + const deny_masks_t deny_masks)
> {
> - const unsigned long access_opt = all_existing_optional_access;
> - const unsigned long access_req = *access_request;
> - access_mask_t missing = 0;
> + const access_mask_t access_req = *access_request;
> size_t youngest_layer = 0;
> - size_t access_index = 0;
> - unsigned long access_bit;
> + access_mask_t missing = 0;
>
> - /* This will require change with new object types. */
> - WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
> + WARN_ON_ONCE((access_req | _LANDLOCK_ACCESS_FS_OPTIONAL) !=
> + _LANDLOCK_ACCESS_FS_OPTIONAL);
>
> - for_each_set_bit(access_bit, &access_opt,
> - BITS_PER_TYPE(access_mask_t)) {
> - if (access_req & BIT(access_bit)) {
> - const size_t layer =
> - (deny_masks >> (access_index * 4)) &
> - (LANDLOCK_MAX_NUM_LAYERS - 1);
> + if (access_req & LANDLOCK_ACCESS_FS_TRUNCATE) {
> + size_t layer = deny_masks & 0x0f;
>
> - if (layer > youngest_layer) {
> - youngest_layer = layer;
> - missing = BIT(access_bit);
> - } else if (layer == youngest_layer) {
> - missing |= BIT(access_bit);
> - }
> - }
> - access_index++;
> + missing |= LANDLOCK_ACCESS_FS_TRUNCATE;
> + youngest_layer = max(youngest_layer, layer);
> }
> + if (access_req & LANDLOCK_ACCESS_FS_IOCTL_DEV) {
> + size_t layer = (deny_masks & 0xf0) >> 4;
>
> + if (layer > youngest_layer)
> + missing = 0;
> +
> + missing |= LANDLOCK_ACCESS_FS_IOCTL_DEV;
> + youngest_layer = max(youngest_layer, layer);
> + }
> *access_request = missing;
> return youngest_layer;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
>
> -static void test_get_layer_from_deny_masks(struct kunit *const test)
> +static void test_get_layer_from_fs_deny_masks(struct kunit *const test)
> {
> deny_masks_t deny_mask;
> access_mask_t access;
> @@ -318,16 +288,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE;
> KUNIT_EXPECT_EQ(test, 0,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
> KUNIT_EXPECT_EQ(test, 2,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
>
> /* truncate:15 ioctl_dev:15 */
> @@ -335,16 +301,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE;
> KUNIT_EXPECT_EQ(test, 15,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
> KUNIT_EXPECT_EQ(test, 15,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access,
> LANDLOCK_ACCESS_FS_TRUNCATE |
> LANDLOCK_ACCESS_FS_IOCTL_DEV);
> @@ -361,18 +323,15 @@ static bool is_valid_request(const struct landlock_request *const request)
> return false;
>
> if (request->access) {
> - if (WARN_ON_ONCE(!(!!request->layer_masks ^
> + if (WARN_ON_ONCE(!(!!request->masks ^
> !!request->all_existing_optional_access)))
> return false;
> } else {
> - if (WARN_ON_ONCE(request->layer_masks ||
> + if (WARN_ON_ONCE(request->masks ||
> request->all_existing_optional_access))
> return false;
> }
>
> - if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
> - return false;
> -
> if (request->deny_masks) {
> if (WARN_ON_ONCE(!request->all_existing_optional_access))
> return false;
> @@ -405,14 +364,12 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> missing = request->access;
> if (missing) {
> /* Gets the nearest domain that denies the request. */
> - if (request->layer_masks) {
> + if (request->masks) {
> youngest_layer = get_denied_layer(
> - subject->domain, &missing, request->layer_masks,
> - request->layer_masks_size);
> + subject->domain, &missing, request->masks);
> } else {
> - youngest_layer = get_layer_from_deny_masks(
> - &missing, request->all_existing_optional_access,
> - request->deny_masks);
> + youngest_layer = get_layer_from_fs_deny_masks(
> + &missing, request->deny_masks);
> }
> youngest_denied =
> get_hierarchy(subject->domain, youngest_layer);
> @@ -507,7 +464,7 @@ static struct kunit_case test_cases[] = {
> /* clang-format off */
> KUNIT_CASE(test_get_hierarchy),
> KUNIT_CASE(test_get_denied_layer),
> - KUNIT_CASE(test_get_layer_from_deny_masks),
> + KUNIT_CASE(test_get_layer_from_fs_deny_masks),
> {}
> /* clang-format on */
> };
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 92428b7fc4d80..104472060ef5e 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -43,8 +43,7 @@ struct landlock_request {
> access_mask_t access;
>
> /* Required fields for requests with layer masks. */
> - const layer_mask_t (*layer_masks)[];
> - size_t layer_masks_size;
> + const struct layer_access_masks *masks;
>
> /* Required fields for requests with deny masks. */
> const access_mask_t all_existing_optional_access;
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index a647b68e8d060..e8e4ae5d075fe 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -23,6 +23,7 @@
> #include "common.h"
> #include "domain.h"
> #include "id.h"
> +#include "limits.h"
>
> #ifdef CONFIG_AUDIT
>
> @@ -133,111 +134,47 @@ int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy)
> return 0;
> }
>
> -static deny_masks_t
> -get_layer_deny_mask(const access_mask_t all_existing_optional_access,
> - const unsigned long access_bit, const size_t layer)
> -{
> - unsigned long access_weight;
> -
> - /* This may require change with new object types. */
> - WARN_ON_ONCE(all_existing_optional_access !=
> - _LANDLOCK_ACCESS_FS_OPTIONAL);
> -
> - if (WARN_ON_ONCE(layer >= LANDLOCK_MAX_NUM_LAYERS))
> - return 0;
> -
> - access_weight = hweight_long(all_existing_optional_access &
> - GENMASK(access_bit, 0));
> - if (WARN_ON_ONCE(access_weight < 1))
> - return 0;
> -
> - return layer
> - << ((access_weight - 1) * HWEIGHT(LANDLOCK_MAX_NUM_LAYERS - 1));
> -}
> -
> -#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> -
> -static void test_get_layer_deny_mask(struct kunit *const test)
> -{
> - const unsigned long truncate = BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE);
> - const unsigned long ioctl_dev = BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV);
> -
> - KUNIT_EXPECT_EQ(test, 0,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - truncate, 0));
> - KUNIT_EXPECT_EQ(test, 0x3,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - truncate, 3));
> -
> - KUNIT_EXPECT_EQ(test, 0,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - ioctl_dev, 0));
> - KUNIT_EXPECT_EQ(test, 0xf0,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - ioctl_dev, 15));
> -}
> -
> -#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> -
> deny_masks_t
> -landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> - const access_mask_t optional_access,
> - const layer_mask_t (*const layer_masks)[],
> - const size_t layer_masks_size)
> +landlock_get_fs_deny_masks(const access_mask_t optional_access,
> + const struct layer_access_masks *layer_masks)
> {
> - const unsigned long access_opt = optional_access;
> - unsigned long access_bit;
> - deny_masks_t deny_masks = 0;
> + u8 truncate_layer = 0;
> + u8 ioctl_dev_layer = 0;
>
> - /* This may require change with new object types. */
> - WARN_ON_ONCE(access_opt !=
> - (optional_access & all_existing_optional_access));
> -
> - if (WARN_ON_ONCE(!layer_masks))
> - return 0;
> -
> - if (WARN_ON_ONCE(!access_opt))
> - return 0;
> -
> - for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> - const layer_mask_t mask = (*layer_masks)[access_bit];
> -
> - if (!mask)
> - continue;
> -
> - /* __fls(1) == 0 */
> - deny_masks |= get_layer_deny_mask(all_existing_optional_access,
> - access_bit, __fls(mask));
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
> + if (layer_masks->access[i] & optional_access &
> + LANDLOCK_ACCESS_FS_TRUNCATE)
> + truncate_layer = i;
> + if (layer_masks->access[i] & optional_access &
> + LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + ioctl_dev_layer = i;
> }
> - return deny_masks;
> + return ((ioctl_dev_layer << 4) & 0xf0) | (truncate_layer & 0x0f);
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
>
> -static void test_landlock_get_deny_masks(struct kunit *const test)
> +static void test_landlock_get_fs_deny_masks(struct kunit *const test)
> {
> - const layer_mask_t layers1[BITS_PER_TYPE(access_mask_t)] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> - BIT_ULL(9),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = BIT_ULL(1),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = BIT_ULL(2) |
> - BIT_ULL(0),
> + const struct layer_access_masks layers1 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .access[1] = LANDLOCK_ACCESS_FS_TRUNCATE,
> + .access[2] = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .access[9] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
>
> KUNIT_EXPECT_EQ(test, 0x1,
> - landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - LANDLOCK_ACCESS_FS_TRUNCATE,
> - &layers1, ARRAY_SIZE(layers1)));
> + landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE,
> + &layers1));
> KUNIT_EXPECT_EQ(test, 0x20,
> - landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - LANDLOCK_ACCESS_FS_IOCTL_DEV,
> - &layers1, ARRAY_SIZE(layers1)));
> + landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + &layers1));
> KUNIT_EXPECT_EQ(
> test, 0x21,
> - landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - LANDLOCK_ACCESS_FS_TRUNCATE |
> - LANDLOCK_ACCESS_FS_IOCTL_DEV,
> - &layers1, ARRAY_SIZE(layers1)));
> + landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE |
> + LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + &layers1));
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> @@ -246,8 +183,7 @@ static void test_landlock_get_deny_masks(struct kunit *const test)
>
> static struct kunit_case test_cases[] = {
> /* clang-format off */
> - KUNIT_CASE(test_get_layer_deny_mask),
> - KUNIT_CASE(test_landlock_get_deny_masks),
> + KUNIT_CASE(test_landlock_get_fs_deny_masks),
> {}
> /* clang-format on */
> };
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 7fb70b25f85a1..39600acb63897 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -120,10 +120,8 @@ struct landlock_hierarchy {
> #ifdef CONFIG_AUDIT
>
> deny_masks_t
> -landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> - const access_mask_t optional_access,
> - const layer_mask_t (*const layer_masks)[],
> - size_t layer_masks_size);
> +landlock_get_fs_deny_masks(const access_mask_t optional_access,
> + const struct layer_access_masks *layer_masks);
>
> int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index b4ce03bef4b8e..1e765d22d8d49 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -407,57 +407,55 @@ static bool access_mask_subset(access_mask_t a, access_mask_t b)
> return (a | b) == b;
> }
>
> +/*
> + * Returns true iff the child file with the given src_child access rights under
> + * src_parent would result in having the same or fewer access rights if it were
> + * moved under new_parent.
> + */
> +static bool may_refer(const struct layer_access_masks *const src_parent,
> + const struct layer_access_masks *const src_child,
> + const struct layer_access_masks *const new_parent,
> + const bool child_is_dir)
> +{
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
> + access_mask_t child_access = src_parent->access[i] &
> + src_child->access[i];
> + access_mask_t parent_access = new_parent->access[i];
> +
> + if (!child_is_dir) {
> + child_access &= ACCESS_FILE;
> + parent_access &= ACCESS_FILE;
> + }
> +
> + if (!access_mask_subset(child_access, parent_access))
> + return false;
> + }
> + return true;
> +}
> +
> /*
> * Check that a destination file hierarchy has more restrictions than a source
> * file hierarchy. This is only used for link and rename actions.
> *
> - * @layer_masks_child2: Optional child masks.
> + * Returns: true if child1 may be moved from parent1 to parent2 without
> + * increasing its access rights. If child2 is set, an additional condition is
> + * that child2 may be used from parent2 to parent1 without increasing its access
> + * rights.
> */
> -static bool no_more_access(
> - const layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> - const layer_mask_t (*const layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS],
> - const bool child1_is_directory,
> - const layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> - const layer_mask_t (*const layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS],
> - const bool child2_is_directory)
> +static bool no_more_access(const struct layer_access_masks *const parent1,
> + const struct layer_access_masks *const child1,
> + const bool child1_is_dir,
> + const struct layer_access_masks *const parent2,
> + const struct layer_access_masks *const child2,
> + const bool child2_is_dir)
> {
> - unsigned long access_bit;
> + if (!may_refer(parent1, child1, parent2, child1_is_dir))
> + return false;
>
> - for (access_bit = 0; access_bit < ARRAY_SIZE(*layer_masks_parent2);
> - access_bit++) {
> - /* Ignores accesses that only make sense for directories. */
> - const bool is_file_access =
> - !!(BIT_ULL(access_bit) & ACCESS_FILE);
> + if (!child2)
> + return true;
>
> - if (child1_is_directory || is_file_access) {
> - /*
> - * Checks if the destination restrictions are a
> - * superset of the source ones (i.e. inherited access
> - * rights without child exceptions):
> - * restrictions(parent2) >= restrictions(child1)
> - */
> - if ((((*layer_masks_parent1)[access_bit] &
> - (*layer_masks_child1)[access_bit]) |
> - (*layer_masks_parent2)[access_bit]) !=
> - (*layer_masks_parent2)[access_bit])
> - return false;
> - }
> -
> - if (!layer_masks_child2)
> - continue;
> - if (child2_is_directory || is_file_access) {
> - /*
> - * Checks inverted restrictions for RENAME_EXCHANGE:
> - * restrictions(parent1) >= restrictions(child2)
> - */
> - if ((((*layer_masks_parent2)[access_bit] &
> - (*layer_masks_child2)[access_bit]) |
> - (*layer_masks_parent1)[access_bit]) !=
> - (*layer_masks_parent1)[access_bit])
> - return false;
> - }
> - }
> - return true;
> + return may_refer(parent2, child2, parent1, child2_is_dir);
> }
>
> #define NMA_TRUE(...) KUNIT_EXPECT_TRUE(test, no_more_access(__VA_ARGS__))
> @@ -467,25 +465,25 @@ static bool no_more_access(
>
> static void test_no_more_access(struct kunit *const test)
> {
> - const layer_mask_t rx0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT_ULL(0),
> + const struct layer_access_masks rx0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - const layer_mask_t mx0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = BIT_ULL(0),
> + const struct layer_access_masks mx0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_MAKE_REG,
> };
> - const layer_mask_t x0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> + const struct layer_access_masks x0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t x1[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(1),
> + const struct layer_access_masks x1 = {
> + .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t x01[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> - BIT_ULL(1),
> + const struct layer_access_masks x01 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> + .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t allows_all[LANDLOCK_NUM_ACCESS_FS] = {};
> + const struct layer_access_masks allows_all = {};
>
> /* Checks without restriction. */
> NMA_TRUE(&x0, &allows_all, false, &allows_all, NULL, false);
> @@ -573,31 +571,30 @@ static void test_no_more_access(struct kunit *const test)
> #undef NMA_TRUE
> #undef NMA_FALSE
>
> -static bool is_layer_masks_allowed(
> - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool is_layer_masks_allowed(const struct layer_access_masks *masks)
> {
> - return !memchr_inv(layer_masks, 0, sizeof(*layer_masks));
> + return !memchr_inv(&masks->access, 0, sizeof(masks->access));
> }
>
> /*
> - * Removes @layer_masks accesses that are not requested.
> + * Removes @masks accesses that are not requested.
> *
> * Returns true if the request is allowed, false otherwise.
> */
> -static bool
> -scope_to_request(const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool scope_to_request(const access_mask_t access_request,
> + struct layer_access_masks *masks)
> {
> - const unsigned long access_req = access_request;
> - unsigned long access_bit;
> + bool saw_unfulfilled_access = false;
Kind of confusing variable name with a double negative.
Maybe accesses_met? (and invert the return condition)
>
>
> - if (WARN_ON_ONCE(!layer_masks))
> + if (WARN_ON_ONCE(!masks))
> return true;
>
> - for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> - (*layer_masks)[access_bit] = 0;
> -
> - return is_layer_masks_allowed(layer_masks);
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
> + masks->access[i] &= access_request;
> + if (masks->access[i])
> + saw_unfulfilled_access = true;
> + }
> + return !saw_unfulfilled_access;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> @@ -605,48 +602,41 @@ scope_to_request(const access_mask_t access_request,
> static void test_scope_to_request_with_exec_none(struct kunit *const test)
> {
> /* Allows everything. */
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks masks = {};
>
> /* Checks and scopes with execute. */
> - KUNIT_EXPECT_TRUE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
> - &layer_masks));
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
> + KUNIT_EXPECT_TRUE(test,
> + scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE, &masks));
> + KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
> }
>
> static void test_scope_to_request_with_exec_some(struct kunit *const test)
> {
> /* Denies execute and write. */
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
> + struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> + .access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
> };
>
> /* Checks and scopes with execute. */
> KUNIT_EXPECT_FALSE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
> - &layer_masks));
> - KUNIT_EXPECT_EQ(test, BIT_ULL(0),
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
> + &masks));
> + KUNIT_EXPECT_EQ(test, LANDLOCK_ACCESS_FS_EXECUTE, masks.access[0]);
> + KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
> }
>
> static void test_scope_to_request_without_access(struct kunit *const test)
> {
> /* Denies execute and write. */
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
> + struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> + .access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
> };
>
> /* Checks and scopes without access request. */
> - KUNIT_EXPECT_TRUE(test, scope_to_request(0, &layer_masks));
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
> + KUNIT_EXPECT_TRUE(test, scope_to_request(0, &masks));
> + KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
> + KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> @@ -655,20 +645,16 @@ static void test_scope_to_request_without_access(struct kunit *const test)
> * Returns true if there is at least one access right different than
> * LANDLOCK_ACCESS_FS_REFER.
> */
> -static bool
> -is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
> - const access_mask_t access_request)
> +static bool is_eacces(const struct layer_access_masks *masks,
> + const access_mask_t access_request)
> {
> - unsigned long access_bit;
> - /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
> - const unsigned long access_check = access_request &
> - ~LANDLOCK_ACCESS_FS_REFER;
> -
> - if (!layer_masks)
> + if (!masks)
> return false;
>
> - for_each_set_bit(access_bit, &access_check, ARRAY_SIZE(*layer_masks)) {
> - if ((*layer_masks)[access_bit])
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
> + /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
> + if (masks->access[i] & access_request &
> + ~LANDLOCK_ACCESS_FS_REFER)
> return true;
> }
> return false;
> @@ -681,37 +667,37 @@ is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
>
> static void test_is_eacces_with_none(struct kunit *const test)
> {
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + const struct layer_access_masks masks = {};
>
> - IE_FALSE(&layer_masks, 0);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> + IE_FALSE(&masks, 0);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> }
>
> static void test_is_eacces_with_refer(struct kunit *const test)
> {
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = BIT_ULL(0),
> + const struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_REFER,
> };
>
> - IE_FALSE(&layer_masks, 0);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> + IE_FALSE(&masks, 0);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> }
>
> static void test_is_eacces_with_write(struct kunit *const test)
> {
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(0),
> + const struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_WRITE_FILE,
> };
>
> - IE_FALSE(&layer_masks, 0);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
> + IE_FALSE(&masks, 0);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
>
> - IE_TRUE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> + IE_TRUE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> @@ -761,26 +747,25 @@ static void test_is_eacces_with_write(struct kunit *const test)
> * - true if the access request is granted;
> * - false otherwise.
> */
> -static bool is_access_to_paths_allowed(
> - const struct landlock_ruleset *const domain,
> - const struct path *const path,
> - const access_mask_t access_request_parent1,
> - layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> - struct landlock_request *const log_request_parent1,
> - struct dentry *const dentry_child1,
> - const access_mask_t access_request_parent2,
> - layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> - struct landlock_request *const log_request_parent2,
> - struct dentry *const dentry_child2)
> +static bool
> +is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
> + const struct path *const path,
> + const access_mask_t access_request_parent1,
> + struct layer_access_masks *layer_masks_parent1,
> + struct landlock_request *const log_request_parent1,
> + struct dentry *const dentry_child1,
> + const access_mask_t access_request_parent2,
> + struct layer_access_masks *layer_masks_parent2,
> + struct landlock_request *const log_request_parent2,
> + struct dentry *const dentry_child2)
> {
> bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
> child1_is_directory = true, child2_is_directory = true;
> struct path walker_path;
> access_mask_t access_masked_parent1, access_masked_parent2;
> - layer_mask_t _layer_masks_child1[LANDLOCK_NUM_ACCESS_FS],
> - _layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
> - layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
> - (*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
> + struct layer_access_masks _layer_masks_child1, _layer_masks_child2;
> + struct layer_access_masks *layer_masks_child1 = NULL,
> + *layer_masks_child2 = NULL;
>
> if (!access_request_parent1 && !access_request_parent2)
> return true;
> @@ -820,22 +805,20 @@ static bool is_access_to_paths_allowed(
> }
>
> if (unlikely(dentry_child1)) {
> - landlock_unmask_layers(
> - find_rule(domain, dentry_child1),
> - landlock_init_layer_masks(
> - domain, LANDLOCK_MASK_ACCESS_FS,
> - &_layer_masks_child1, LANDLOCK_KEY_INODE),
> - &_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1));
> + if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
> + &_layer_masks_child1,
> + LANDLOCK_KEY_INODE))
> + landlock_unmask_layers(find_rule(domain, dentry_child1),
> + &_layer_masks_child1);
> layer_masks_child1 = &_layer_masks_child1;
> child1_is_directory = d_is_dir(dentry_child1);
> }
> if (unlikely(dentry_child2)) {
> - landlock_unmask_layers(
> - find_rule(domain, dentry_child2),
> - landlock_init_layer_masks(
> - domain, LANDLOCK_MASK_ACCESS_FS,
> - &_layer_masks_child2, LANDLOCK_KEY_INODE),
> - &_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2));
> + if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
> + &_layer_masks_child2,
> + LANDLOCK_KEY_INODE))
> + landlock_unmask_layers(find_rule(domain, dentry_child2),
> + &_layer_masks_child2);
> layer_masks_child2 = &_layer_masks_child2;
> child2_is_directory = d_is_dir(dentry_child2);
> }
> @@ -890,16 +873,12 @@ static bool is_access_to_paths_allowed(
> }
>
> rule = find_rule(domain, walker_path.dentry);
> - allowed_parent1 = allowed_parent1 ||
> - landlock_unmask_layers(
> - rule, access_masked_parent1,
> - layer_masks_parent1,
> - ARRAY_SIZE(*layer_masks_parent1));
> - allowed_parent2 = allowed_parent2 ||
> - landlock_unmask_layers(
> - rule, access_masked_parent2,
> - layer_masks_parent2,
> - ARRAY_SIZE(*layer_masks_parent2));
> + allowed_parent1 =
> + allowed_parent1 ||
> + landlock_unmask_layers(rule, layer_masks_parent1);
> + allowed_parent2 =
> + allowed_parent2 ||
> + landlock_unmask_layers(rule, layer_masks_parent2);
>
> /* Stops when a rule from each layer grants access. */
> if (allowed_parent1 && allowed_parent2)
> @@ -953,9 +932,7 @@ static bool is_access_to_paths_allowed(
> log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent1->audit.u.path = *path;
> log_request_parent1->access = access_masked_parent1;
> - log_request_parent1->layer_masks = layer_masks_parent1;
> - log_request_parent1->layer_masks_size =
> - ARRAY_SIZE(*layer_masks_parent1);
> + log_request_parent1->masks = layer_masks_parent1;
> }
>
> if (!allowed_parent2) {
> @@ -963,9 +940,7 @@ static bool is_access_to_paths_allowed(
> log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent2->audit.u.path = *path;
> log_request_parent2->access = access_masked_parent2;
> - log_request_parent2->layer_masks = layer_masks_parent2;
> - log_request_parent2->layer_masks_size =
> - ARRAY_SIZE(*layer_masks_parent2);
> + log_request_parent2->masks = layer_masks_parent2;
> }
> return allowed_parent1 && allowed_parent2;
> }
> @@ -978,7 +953,7 @@ static int current_check_access_path(const struct path *const path,
> };
> const struct landlock_cred_security *const subject =
> landlock_get_applicable_subject(current_cred(), masks, NULL);
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks layer_masks;
> struct landlock_request request = {};
>
> if (!subject)
> @@ -1053,10 +1028,10 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
> * - true if all the domain access rights are allowed for @dir;
> * - false if the walk reached @mnt_root.
> */
> -static bool collect_domain_accesses(
> - const struct landlock_ruleset *const domain,
> - const struct dentry *const mnt_root, struct dentry *dir,
> - layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS])
> +static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> + const struct dentry *const mnt_root,
> + struct dentry *dir,
> + struct layer_access_masks *layer_masks_dom)
> {
> unsigned long access_dom;
> bool ret = false;
> @@ -1075,9 +1050,8 @@ static bool collect_domain_accesses(
> struct dentry *parent_dentry;
>
> /* Gets all layers allowing all domain accesses. */
> - if (landlock_unmask_layers(find_rule(domain, dir), access_dom,
> - layer_masks_dom,
> - ARRAY_SIZE(*layer_masks_dom))) {
> + if (landlock_unmask_layers(find_rule(domain, dir),
> + layer_masks_dom)) {
> /*
> * Stops when all handled accesses are allowed by at
> * least one rule in each layer.
> @@ -1165,8 +1139,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> access_mask_t access_request_parent1, access_request_parent2;
> struct path mnt_dir;
> struct dentry *old_parent;
> - layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
> - layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks layer_masks_parent1 = {},
> + layer_masks_parent2 = {};
> struct landlock_request request1 = {}, request2 = {};
>
> if (!subject)
> @@ -1323,7 +1297,8 @@ static void hook_sb_delete(struct super_block *const sb)
> * second call to iput() for the same Landlock object. Also
> * checks I_NEW because such inode cannot be tied to an object.
> */
> - if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) {
> + if (inode_state_read(inode) &
> + (I_FREEING | I_WILL_FREE | I_NEW)) {
> spin_unlock(&inode->i_lock);
> continue;
> }
> @@ -1641,7 +1616,7 @@ static bool is_device(const struct file *const file)
>
> static int hook_file_open(struct file *const file)
> {
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks layer_masks = {};
> access_mask_t open_access_request, full_access_request, allowed_access,
> optional_access;
> const struct landlock_cred_security *const subject =
> @@ -1676,20 +1651,14 @@ static int hook_file_open(struct file *const file)
> &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
> allowed_access = full_access_request;
> } else {
> - unsigned long access_bit;
> - const unsigned long access_req = full_access_request;
> -
> /*
> * Calculate the actual allowed access rights from layer_masks.
> - * Add each access right to allowed_access which has not been
> - * vetoed by any layer.
> + * Remove the access rights from the full access request which
> + * are still unfulfilled in any of the layers.
> */
> - allowed_access = 0;
> - for_each_set_bit(access_bit, &access_req,
> - ARRAY_SIZE(layer_masks)) {
> - if (!layer_masks[access_bit])
> - allowed_access |= BIT_ULL(access_bit);
> - }
> + allowed_access = full_access_request;
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++)
> + allowed_access &= ~layer_masks.access[i];
> }
>
> /*
> @@ -1700,9 +1669,8 @@ static int hook_file_open(struct file *const file)
> */
> landlock_file(file)->allowed_access = allowed_access;
> #ifdef CONFIG_AUDIT
> - landlock_file(file)->deny_masks = landlock_get_deny_masks(
> - _LANDLOCK_ACCESS_FS_OPTIONAL, optional_access, &layer_masks,
> - ARRAY_SIZE(layer_masks));
> + landlock_file(file)->deny_masks =
> + landlock_get_fs_deny_masks(optional_access, &layer_masks);
> #endif /* CONFIG_AUDIT */
>
> if (access_mask_subset(open_access_request, allowed_access))
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index 1f3915a90a808..2a5456f4f017e 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -47,7 +47,7 @@ static int current_check_access_socket(struct socket *const sock,
> access_mask_t access_request)
> {
> __be16 port;
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
> + struct layer_access_masks layer_masks = {};
> const struct landlock_rule *rule;
> struct landlock_id id = {
> .type = LANDLOCK_KEY_NET_PORT,
> @@ -178,8 +178,9 @@ static int current_check_access_socket(struct socket *const sock,
> access_request = landlock_init_layer_masks(subject->domain,
> access_request, &layer_masks,
> LANDLOCK_KEY_NET_PORT);
> - if (landlock_unmask_layers(rule, access_request, &layer_masks,
> - ARRAY_SIZE(layer_masks)))
> + if (!access_request)
> + return 0;
> + if (landlock_unmask_layers(rule, &layer_masks))
> return 0;
>
> audit_net.family = address->sa_family;
> @@ -189,8 +190,7 @@ static int current_check_access_socket(struct socket *const sock,
> .audit.type = LSM_AUDIT_DATA_NET,
> .audit.u.net = &audit_net,
> .access = access_request,
> - .layer_masks = &layer_masks,
> - .layer_masks_size = ARRAY_SIZE(layer_masks),
> + .masks = &layer_masks,
> });
> return -EACCES;
> }
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index dfcdc19ea2683..d20e28d38e9c9 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -622,49 +622,24 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
> * request are empty).
> */
> bool landlock_unmask_layers(const struct landlock_rule *const rule,
> - const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> - const size_t masks_array_size)
> + struct layer_access_masks *masks)
> {
> - size_t layer_level;
> -
> - if (!access_request || !layer_masks)
> + if (!masks)
> return true;
> if (!rule)
> return false;
>
> - /*
> - * An access is granted if, for each policy layer, at least one rule
> - * encountered on the pathwalk grants the requested access,
> - * regardless of its position in the layer stack. We must then check
> - * the remaining layers for each inode, from the first added layer to
> - * the last one. When there is multiple requested accesses, for each
> - * policy layer, the full set of requested accesses may not be granted
> - * by only one rule, but by the union (binary OR) of multiple rules.
> - * E.g. /a/b <execute> + /a <read> => /a/b <execute + read>
> - */
> - for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
> - const struct landlock_layer *const layer =
> - &rule->layers[layer_level];
> - const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
> - const unsigned long access_req = access_request;
> - unsigned long access_bit;
> - bool is_empty;
> + for (int i = 0; i < rule->num_layers; i++) {
> + const struct landlock_layer *l = &rule->layers[i];
Nit: using l for a variable makes it a little harder to read and
confused me for a second.
Maybe this_layer?
>
>
> - /*
> - * Records in @layer_masks which layer grants access to each requested
> - * access: bit cleared if the related layer grants access.
> - */
> - is_empty = true;
> - for_each_set_bit(access_bit, &access_req, masks_array_size) {
> - if (layer->access & BIT_ULL(access_bit))
> - (*layer_masks)[access_bit] &= ~layer_bit;
> - is_empty = is_empty && !(*layer_masks)[access_bit];
> - }
> - if (is_empty)
> - return true;
> + masks->access[l->level - 1] &= ~l->access;
> }
> - return false;
> +
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
> + if (masks->access[i])
> + return false;
> + }
> + return true;
> }
>
> typedef access_mask_t
> @@ -679,8 +654,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
> *
> * @domain: The domain that defines the current restrictions.
> * @access_request: The requested access rights to check.
> - * @layer_masks: It must contain %LANDLOCK_NUM_ACCESS_FS or
> - * %LANDLOCK_NUM_ACCESS_NET elements according to @key_type.
> + * @masks: Layer access masks to populate.
> * @key_type: The key type to switch between access masks of different types.
> *
> * Returns: An access mask where each access right bit is set which is handled
> @@ -689,23 +663,20 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
> access_mask_t
> landlock_init_layer_masks(const struct landlock_ruleset *const domain,
> const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> + struct layer_access_masks *masks,
> const enum landlock_key_type key_type)
> {
> access_mask_t handled_accesses = 0;
> - size_t layer_level, num_access;
> get_access_mask_t *get_access_mask;
>
> switch (key_type) {
> case LANDLOCK_KEY_INODE:
> get_access_mask = landlock_get_fs_access_mask;
> - num_access = LANDLOCK_NUM_ACCESS_FS;
> break;
>
> #if IS_ENABLED(CONFIG_INET)
> case LANDLOCK_KEY_NET_PORT:
> get_access_mask = landlock_get_net_access_mask;
> - num_access = LANDLOCK_NUM_ACCESS_NET;
> break;
> #endif /* IS_ENABLED(CONFIG_INET) */
>
> @@ -714,27 +685,18 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
> return 0;
> }
>
> - memset(layer_masks, 0,
> - array_size(sizeof((*layer_masks)[0]), num_access));
> -
> /* An empty access request can happen because of O_WRONLY | O_RDWR. */
> if (!access_request)
> return 0;
>
> - /* Saves all handled accesses per layer. */
> - for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
> - const unsigned long access_req = access_request;
> - const access_mask_t access_mask =
> - get_access_mask(domain, layer_level);
> - unsigned long access_bit;
> + for (int i = 0; i < domain->num_layers; i++) {
> + const access_mask_t handled = get_access_mask(domain, i);
>
> - for_each_set_bit(access_bit, &access_req, num_access) {
> - if (BIT_ULL(access_bit) & access_mask) {
> - (*layer_masks)[access_bit] |=
> - BIT_ULL(layer_level);
> - handled_accesses |= BIT_ULL(access_bit);
> - }
> - }
> + masks->access[i] = access_request & handled;
> + handled_accesses |= masks->access[i];
> }
> + for (int i = domain->num_layers; i < LANDLOCK_MAX_NUM_LAYERS; i++)
> + masks->access[i] = 0;
> +
> return handled_accesses;
> }
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 1a78cba662b24..f7b80b18c2a70 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -301,15 +301,25 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
> return ruleset->access_masks[layer_level].scope;
> }
>
> +/**
> + * struct layer_accesses - A boolean matrix of layers and access rights
> + *
> + * This has a bit for each combination of layer numbers and access rights.
> + * During access checks, it is used to represent the access rights for each
> + * layer which still need to be fulfilled. When all bits are 0, the access
> + * request is considered to be fulfilled.
> + */
> +struct layer_access_masks {
> + access_mask_t access[LANDLOCK_MAX_NUM_LAYERS];
> +};
Nit: Probably best to format this with the doc comment style used elsewhere.
>
> +
> bool landlock_unmask_layers(const struct landlock_rule *const rule,
> - const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> - const size_t masks_array_size);
> + struct layer_access_masks *masks);
>
> access_mask_t
> landlock_init_layer_masks(const struct landlock_ruleset *const domain,
> const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> + struct layer_access_masks *masks,
> const enum landlock_key_type key_type);
>
> #endif /* _SECURITY_LANDLOCK_RULESET_H */
^ permalink raw reply
* [RFC PATCH 1/1] lsm: Add hook unix_path_connect
From: Justin Suess @ 2025-12-31 21:33 UTC (permalink / raw)
To: Paul Moore, James Morris, Serge E . Hallyn, Kuniyuki Iwashima
Cc: Simon Horman, Mickaël Salaün, Günther Noack,
linux-security-module, Tingmao Wang, netdev, Justin Suess,
Günther Noack
In-Reply-To: <20251231213314.2979118-1-utilityemal77@gmail.com>
Adds an LSM hook unix_path_connect.
This hook is called to check the path of a named unix socket before a
connection is initiated.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
Cc: Günther Noack <gnoack3000@gmail.com>
---
include/linux/lsm_hook_defs.h | 1 +
include/linux/security.h | 6 ++++++
net/unix/af_unix.c | 8 ++++++++
security/security.c | 16 ++++++++++++++++
4 files changed, 31 insertions(+)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..a42d1aaf3b8a 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -318,6 +318,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
#endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
#ifdef CONFIG_SECURITY_NETWORK
+LSM_HOOK(int, 0, unix_path_connect, const struct path *path)
LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
struct sock *newsk)
LSM_HOOK(int, 0, unix_may_send, struct socket *sock, struct socket *other)
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..ab66f22f7e5a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1638,6 +1638,7 @@ static inline int security_watch_key(struct key *key)
#ifdef CONFIG_SECURITY_NETWORK
+int security_unix_path_connect(const struct path *path);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
int security_unix_may_send(struct socket *sock, struct socket *other);
@@ -1699,6 +1700,11 @@ static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb)
return 0;
}
+static inline int security_unix_path_connect(const struct path *path)
+{
+ return 0;
+}
+
static inline int security_unix_stream_connect(struct sock *sock,
struct sock *other,
struct sock *newsk)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 55cdebfa0da0..af1a6083a69b 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1226,6 +1226,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
if (!S_ISSOCK(inode->i_mode))
goto path_put;
+ /*
+ * We call the hook because we know that the inode is a socket
+ * and we hold a valid reference to it via the path.
+ */
+ err = security_unix_path_connect(&path);
+ if (err)
+ goto path_put;
+
sk = unix_find_socket_byinode(inode);
if (!sk)
goto path_put;
diff --git a/security/security.c b/security/security.c
index 31a688650601..17af5d0ddf28 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4047,6 +4047,22 @@ int security_unix_stream_connect(struct sock *sock, struct sock *other,
}
EXPORT_SYMBOL(security_unix_stream_connect);
+/*
+ * security_unix_path_connect() - Check if a named AF_UNIX socket can connect
+ * @path: Path of the socket being connected to
+ *
+ * This hook is called to check permissions before connecting to a named
+ * AF_UNIX socket. This is necessary because it was not possible to check the
+ * VFS inode of the target socket before the connection is made.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_unix_path_connect(const struct path *path)
+{
+ return call_int_hook(unix_path_connect, path);
+}
+EXPORT_SYMBOL(security_unix_path_connect);
+
/**
* security_unix_may_send() - Check if AF_UNIX socket can send datagrams
* @sock: originating sock
--
2.51.0
^ permalink raw reply related
* [RFC PATCH 0/1] lsm: Add hook unix_path_connect
From: Justin Suess @ 2025-12-31 21:33 UTC (permalink / raw)
To: Paul Moore, James Morris, Serge E . Hallyn, Kuniyuki Iwashima
Cc: Simon Horman, Mickaël Salaün, Günther Noack,
linux-security-module, Tingmao Wang, netdev, Justin Suess
Hi,
This patch introduces a new LSM hook unix_path_connect.
The idea for this patch and the hook came from Günther Noack, who
is cc'd. Much credit to him for the idea and discussion.
This patch is based on the lsm next branch.
Motivation
---
For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
identifying object from a policy perspective is the path passed to
connect(2). However, this operation currently restricts LSMs that rely
on VFS-based mediation, because the pathname resolved during connect()
is not preserved in a form visible to existing hooks before connection
establishment. As a result, LSMs such as Landlock cannot currently
restrict connections to named UNIX domain sockets by their VFS path.
This gap has been discussed previously (e.g. in the context of Landlock's
path-based access controls). [1] [2]
I've cc'd the netdev folks as well on this, as the placement of this hook is
important and in a core unix socket function.
Design Choices
---
The hook is called in net/unix/af_unix.c in the function unix_find_bsd().
The hook takes a single parameter, a const struct path* to the named unix
socket to which the connection is being established.
The hook takes place after normal permissions checks, and after the
inode is determined to be a socket. It however, takes place before
the socket is actually connected to.
If the hook returns non-zero it will do a put on the path, and return.
References
---
[1]: https://github.com/landlock-lsm/linux/issues/36#issue-2354007438
[2]: https://lore.kernel.org/linux-security-module/cover.1767115163.git.m@maowtm.org/
Kind Regards,
Justin Suess
Justin Suess (1):
lsm: Add hook unix_path_connect
include/linux/lsm_hook_defs.h | 1 +
include/linux/security.h | 6 ++++++
net/unix/af_unix.c | 8 ++++++++
security/security.c | 16 ++++++++++++++++
4 files changed, 31 insertions(+)
base-commit: 1c0860d4415d52f3ad1c8e0a15c1272869278a06
--
2.51.0
^ permalink raw reply
* Re: [PATCH] smack: /smack/doi must be > 0
From: Casey Schaufler @ 2025-12-31 20:52 UTC (permalink / raw)
To: Konstantin Andreev; +Cc: linux-security-module, Casey Schaufler
In-Reply-To: <20250930121602.138337-1-andreev@swemel.ru>
On 9/30/2025 5:16 AM, Konstantin Andreev wrote:
> /smack/doi allows writing and keeping negative doi values.
> Correct values are 0 < doi <= (max 32-bit positive integer)
>
> (2008-02-04, Casey Schaufler)
> Fixes: e114e473771c ("Smack: Simplified Mandatory Access Control Kernel")
>
> Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
Added to smack-next for 6.20. Thank you.
> ---
> The patch applies on top of smack/next, commit 6ddd169d0288 ("smack: fix
> kernel-doc warnings for smk_import_valid_label()")
>
> security/smack/smackfs.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index b1e5e62f5cbd..316c2ea401e8 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -141,7 +141,7 @@ struct smack_parsed_rule {
> int smk_access2;
> };
>
> -static int smk_cipso_doi_value = SMACK_CIPSO_DOI_DEFAULT;
> +static u32 smk_cipso_doi_value = SMACK_CIPSO_DOI_DEFAULT;
>
> /*
> * Values for parsing cipso rules
> @@ -1562,7 +1562,7 @@ static ssize_t smk_read_doi(struct file *filp, char __user *buf,
> if (*ppos != 0)
> return 0;
>
> - sprintf(temp, "%d", smk_cipso_doi_value);
> + sprintf(temp, "%lu", (unsigned long)smk_cipso_doi_value);
> rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
>
> return rc;
> @@ -1581,7 +1581,7 @@ static ssize_t smk_write_doi(struct file *file, const char __user *buf,
> size_t count, loff_t *ppos)
> {
> char temp[80];
> - int i;
> + unsigned long u;
>
> if (!smack_privileged(CAP_MAC_ADMIN))
> return -EPERM;
> @@ -1594,10 +1594,12 @@ static ssize_t smk_write_doi(struct file *file, const char __user *buf,
>
> temp[count] = '\0';
>
> - if (sscanf(temp, "%d", &i) != 1)
> + if (kstrtoul(temp, 10, &u))
> return -EINVAL;
>
> - smk_cipso_doi_value = i;
> + if (u == CIPSO_V4_DOI_UNKNOWN || u > U32_MAX)
> + return -EINVAL;
> + smk_cipso_doi_value = u;
>
> smk_cipso_doi();
>
^ permalink raw reply
* Re: [PATCH] security: smack: fix indentation in smack_access.c
From: Casey Schaufler @ 2025-12-31 20:50 UTC (permalink / raw)
To: Taimoor Zaeem, paul, jmorris, serge
Cc: linux-security-module, Casey Schaufler
In-Reply-To: <20251009112055.142440-1-taimoorzaeem@gmail.com>
On 10/9/2025 4:20 AM, Taimoor Zaeem wrote:
> Replace spaces in code indent with tab character.
>
> Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
Added to smack-next for 6.20. Thank you.
> ---
> security/smack/smack_access.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index 2e4a0cb22782..3cc0e00ca1ae 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -392,7 +392,7 @@ void smack_log(char *subject_label, char *object_label, int request,
> }
> #else /* #ifdef CONFIG_AUDIT */
> void smack_log(char *subject_label, char *object_label, int request,
> - int result, struct smk_audit_info *ad)
> + int result, struct smk_audit_info *ad)
> {
> }
> #endif
^ permalink raw reply
* Re: [PATCH] smack: /smack/doi: accept previously used values
From: Casey Schaufler @ 2025-12-31 20:52 UTC (permalink / raw)
To: Konstantin Andreev; +Cc: linux-security-module, Casey Schaufler
In-Reply-To: <20250930123153.138644-1-andreev@swemel.ru>
On 9/30/2025 5:31 AM, Konstantin Andreev wrote:
> Writing to /smack/doi a value that has ever been
> written there in the past disables networking for
> non-ambient labels.
> E.g.
>
> # cat /smack/doi
> 3
> # netlabelctl -p cipso list
> Configured CIPSO mappings (1)
> DOI value : 3
> mapping type : PASS_THROUGH
> # netlabelctl -p map list
> Configured NetLabel domain mappings (3)
> domain: "_" (IPv4)
> protocol: UNLABELED
> domain: DEFAULT (IPv4)
> protocol: CIPSO, DOI = 3
> domain: DEFAULT (IPv6)
> protocol: UNLABELED
>
> # cat /smack/ambient
> _
> # cat /proc/$$/attr/smack/current
> _
> # ping -c1 10.1.95.12
> 64 bytes from 10.1.95.12: icmp_seq=1 ttl=64 time=0.964 ms
> # echo foo >/proc/$$/attr/smack/current
> # ping -c1 10.1.95.12
> 64 bytes from 10.1.95.12: icmp_seq=1 ttl=64 time=0.956 ms
> unknown option 86
>
> # echo 4 >/smack/doi
> # echo 3 >/smack/doi
> !> [ 214.050395] smk_cipso_doi:691 cipso add rc = -17
> # echo 3 >/smack/doi
> !> [ 249.402261] smk_cipso_doi:678 remove rc = -2
> !> [ 249.402261] smk_cipso_doi:691 cipso add rc = -17
>
> # ping -c1 10.1.95.12
> !!> ping: 10.1.95.12: Address family for hostname not supported
>
> # echo _ >/proc/$$/attr/smack/current
> # ping -c1 10.1.95.12
> 64 bytes from 10.1.95.12: icmp_seq=1 ttl=64 time=0.617 ms
>
> This happens because Smack keeps decommissioned DOIs,
> fails to re-add them, and consequently refuses to add
> the “default” domain map:
>
> # netlabelctl -p cipso list
> Configured CIPSO mappings (2)
> DOI value : 3
> mapping type : PASS_THROUGH
> DOI value : 4
> mapping type : PASS_THROUGH
> # netlabelctl -p map list
> Configured NetLabel domain mappings (2)
> domain: "_" (IPv4)
> protocol: UNLABELED
> !> (no ipv4 map for default domain here)
> domain: DEFAULT (IPv6)
> protocol: UNLABELED
>
> Fix by clearing decommissioned DOI definitions and
> serializing concurrent DOI updates with a new lock.
>
> Also:
> - allow /smack/doi to live unconfigured, since
> adding a map (netlbl_cfg_cipsov4_map_add) may fail.
> CIPSO_V4_DOI_UNKNOWN(0) indicates the unconfigured DOI
> - add new DOI before removing the old default map,
> so the old map remains if the add fails
>
> (2008-02-04, Casey Schaufler)
> Fixes: e114e473771c ("Smack: Simplified Mandatory Access Control Kernel")
>
> Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
Added to smack-next for 6.20. Thank you.
> ---
> The patch is verified to work on top of "smack: /smack/doi must be > 0" patch.
> Link: https://lore.kernel.org/linux-security-module/20250930121602.138337-1-andreev@swemel.ru
>
> security/smack/smackfs.c | 71 +++++++++++++++++++++++++---------------
> 1 file changed, 45 insertions(+), 26 deletions(-)
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 316c2ea401e8..d27d9140dda2 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -70,6 +70,7 @@ enum smk_inos {
> static DEFINE_MUTEX(smack_cipso_lock);
> static DEFINE_MUTEX(smack_ambient_lock);
> static DEFINE_MUTEX(smk_net4addr_lock);
> +static DEFINE_MUTEX(smk_cipso_doi_lock);
> #if IS_ENABLED(CONFIG_IPV6)
> static DEFINE_MUTEX(smk_net6addr_lock);
> #endif /* CONFIG_IPV6 */
> @@ -141,7 +142,7 @@ struct smack_parsed_rule {
> int smk_access2;
> };
>
> -static u32 smk_cipso_doi_value = SMACK_CIPSO_DOI_DEFAULT;
> +static u32 smk_cipso_doi_value = CIPSO_V4_DOI_UNKNOWN;
>
> /*
> * Values for parsing cipso rules
> @@ -663,43 +664,60 @@ static const struct file_operations smk_load_ops = {
> };
>
> /**
> - * smk_cipso_doi - initialize the CIPSO domain
> + * smk_cipso_doi - set netlabel maps
> + * @ndoi: new value for our CIPSO DOI
> + * @gfp_flags: kmalloc allocation context
> */
> -static void smk_cipso_doi(void)
> +static int
> +smk_cipso_doi(u32 ndoi, gfp_t gfp_flags)
> {
> - int rc;
> + int rc = 0;
> struct cipso_v4_doi *doip;
> struct netlbl_audit nai;
>
> + mutex_lock(&smk_cipso_doi_lock);
> +
> + if (smk_cipso_doi_value == ndoi)
> + goto clr_doi_lock;
> +
> smk_netlabel_audit_set(&nai);
>
> - rc = netlbl_cfg_map_del(NULL, PF_INET, NULL, NULL, &nai);
> - if (rc != 0)
> - printk(KERN_WARNING "%s:%d remove rc = %d\n",
> - __func__, __LINE__, rc);
> -
> - doip = kmalloc(sizeof(struct cipso_v4_doi), GFP_KERNEL | __GFP_NOFAIL);
> + doip = kmalloc(sizeof(struct cipso_v4_doi), gfp_flags);
> + if (!doip) {
> + rc = -ENOMEM;
> + goto clr_doi_lock;
> + }
> doip->map.std = NULL;
> - doip->doi = smk_cipso_doi_value;
> + doip->doi = ndoi;
> doip->type = CIPSO_V4_MAP_PASS;
> doip->tags[0] = CIPSO_V4_TAG_RBITMAP;
> for (rc = 1; rc < CIPSO_V4_TAG_MAXCNT; rc++)
> doip->tags[rc] = CIPSO_V4_TAG_INVALID;
>
> rc = netlbl_cfg_cipsov4_add(doip, &nai);
> - if (rc != 0) {
> - printk(KERN_WARNING "%s:%d cipso add rc = %d\n",
> - __func__, __LINE__, rc);
> + if (rc) {
> kfree(doip);
> - return;
> + goto clr_doi_lock;
> }
> - rc = netlbl_cfg_cipsov4_map_add(doip->doi, NULL, NULL, NULL, &nai);
> - if (rc != 0) {
> - printk(KERN_WARNING "%s:%d map add rc = %d\n",
> - __func__, __LINE__, rc);
> - netlbl_cfg_cipsov4_del(doip->doi, &nai);
> - return;
> +
> + if (smk_cipso_doi_value != CIPSO_V4_DOI_UNKNOWN) {
> + rc = netlbl_cfg_map_del(NULL, PF_INET, NULL, NULL, &nai);
> + if (rc && rc != -ENOENT)
> + goto clr_ndoi_def;
> +
> + netlbl_cfg_cipsov4_del(smk_cipso_doi_value, &nai);
> }
> +
> + rc = netlbl_cfg_cipsov4_map_add(ndoi, NULL, NULL, NULL, &nai);
> + if (rc) {
> + smk_cipso_doi_value = CIPSO_V4_DOI_UNKNOWN; // no default map
> +clr_ndoi_def: netlbl_cfg_cipsov4_del(ndoi, &nai);
> + } else
> + smk_cipso_doi_value = ndoi;
> +
> +clr_doi_lock:
> + mutex_unlock(&smk_cipso_doi_lock);
> + return rc;
> }
>
> /**
> @@ -1599,11 +1617,8 @@ static ssize_t smk_write_doi(struct file *file, const char __user *buf,
>
> if (u == CIPSO_V4_DOI_UNKNOWN || u > U32_MAX)
> return -EINVAL;
> - smk_cipso_doi_value = u;
>
> - smk_cipso_doi();
> -
> - return count;
> + return smk_cipso_doi(u, GFP_KERNEL) ? : count;
> }
>
> static const struct file_operations smk_doi_ops = {
> @@ -2984,6 +2999,7 @@ static int __init init_smk_fs(void)
> {
> int err;
> int rc;
> + struct netlbl_audit nai;
>
> if (smack_enabled == 0)
> return 0;
> @@ -3002,7 +3018,10 @@ static int __init init_smk_fs(void)
> }
> }
>
> - smk_cipso_doi();
> + smk_netlabel_audit_set(&nai);
> + (void) netlbl_cfg_map_del(NULL, PF_INET, NULL, NULL, &nai);
> + (void) smk_cipso_doi(SMACK_CIPSO_DOI_DEFAULT,
> + GFP_KERNEL | __GFP_NOFAIL);
> smk_unlbl_ambient(NULL);
>
> rc = smack_populate_secattr(&smack_known_floor);
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Demi Marie Obenour @ 2025-12-31 16:54 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: Mickaël Salaün, Günther Noack, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <20251230.bcae69888454@gnoack.org>
[-- Attachment #1.1.1: Type: text/plain, Size: 7681 bytes --]
On 12/30/25 18:16, Günther Noack wrote:
> Hello!
>
> Thanks for sending this patch!
>
> On Tue, Dec 30, 2025 at 05:20:18PM +0000, Tingmao Wang wrote:
>> Changes in v2:
>> Fix grammar in doc, rebased on mic/next, and extracted common code from
>> hook_unix_stream_connect and hook_unix_may_send into a separate
>> function.
>>
>> The rest is the same as the v1 cover letter:
>>
>> This patch series extend the existing abstract Unix socket scoping to
>> pathname (i.e. normal file-based) sockets as well, by adding a new scope
>> bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET that works the same as
>> LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET, except that restricts pathname Unix
>> sockets. This means that a sandboxed process with this scope enabled will
>> not be able to connect to Unix sockets created outside the sandbox via the
>> filesystem.
>>
>> There is a future plan [1] for allowing specific sockets based on FS
>> hierarchy, but this series is only determining access based on domain
>> parent-child relationship. There is currently no way to allow specific
>> (outside the Landlock domain) Unix sockets, and none of the existing
>> Landlock filesystem controls apply to socket connect().
>>
>> With this series, we can now properly protect against things like the the
>> following while only relying on Landlock:
>>
>> (running under tmux)
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb ~# LL_FS_RO=/ LL_FS_RW= ./sandboxer bash
>> Executing the sandboxed command...
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
>> cat: /tmp/hi: No such file or directory
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# tmux new-window 'echo hi > /tmp/hi'
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
>> hi
>>
>> The above but with Unix socket scoping enabled (both pathname and abstract
>> sockets) - the sandboxed shell can now no longer talk to tmux due to the
>> socket being created from outside the Landlock sandbox:
>>
>> (running under tmux)
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb ~# LL_FS_RO=/ LL_FS_RW= LL_SCOPED=u:a ./sandboxer bash
>> Executing the sandboxed command...
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
>> cat: /tmp/hi: No such file or directory
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# tmux new-window 'echo hi > /tmp/hi'
>> error connecting to /tmp/tmux-0/default (Operation not permitted)
>> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
>> cat: /tmp/hi: No such file or directory
>>
>> Tmux is just one example. In a standard systemd session, `systemd-run
>> --user` can also be used (--user will run the command in the user's
>> session, without requiring any root privileges), and likely a lot more if
>> running in a desktop environment with many popular applications. This
>> change therefore makes it possible to create sandboxes without relying on
>> additional mechanisms like seccomp to protect against such issues.
>>
>> These kind of issues was originally discussed on here (I took the idea for
>> systemd-run from Demi):
>> https://spectrum-os.org/lists/archives/spectrum-devel/00256266-26db-40cf-8f5b-f7c7064084c2@gmail.com/
>
> What is unclear to me from the examples and the description is: Why is
> the boundary between allowed and denied connection targets drawn at
> the border of the Landlock domain (the "scope") and why don't we solve
> this with the file-system-based approach described in [1]?
>
> **Do we have existing use cases where a service is both offered and
> connected to all from within the same Landlock domain, and where the
> process enforcing the policy does not control the child process enough
> so that it would be possible to allow-list it with a
> LANDLOCK_ACCESS_FS_CONNECT_UNIX rule?**
>
> If we do not have such a use case, it seems that the planned FS-based
> control mechanism from [1] would do the same job? Long term, we might
> be better off if we only have only one control -- as we have discussed
> in [2], having two of these might mean that they interact in
> unconventional and possibly confusing ways.
I agree with this.
> Apart from that, there are some other weaker hints that make me
> slightly critical of this patch set:
>
> * We discussed the idea that a FS-based path_beneath rule would act
> implicitly also as an exception for
> LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET, in [2] - one possible way to
> interpret this is that the gravity of the system's logic pulls us
> back towards a FS-based control, and we would have to swim less
> against the stream if we integrated the Unix connect() control in
> that way?
I agree with this as well. Having FS-based controls apply consistently
to all types of inodes is what I would expect.
> * I am struggling to convince myself that we can tell users to
> restrict LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET as a default, as we are
> currently doing it with the other "scoped" and file system controls.
> (The scoped signals are OK because killing out-of-domain processes
> is clearly bad. The scoped abstract sockets are usually OK because
> most processes do not need that feature.)
>
> But there are legitimate Unix services that are still needed by
> unprivileged processes and which are designed to be safe to use.
> For instance, systemd exposes a user database lookup service over
> varlink [3], which can be accessed from arbitrary glibc programs
> through a NSS module. Using this is incompatible with
> LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET as long as we do not have the
> FS-based control and the surprising implicit permission through a
> path_beneath rule as discussed in [2].
>
> (Another example is the X11 socket, to which the same reasoning
> should apply, I think, and which is also used by a large number of
> programs.)
I think making FS-based controls on by default is the least surprising
option for users. It's what I suspect most programs intend, and it
would stop a lot of unexpected sandbox escapes.
Without FS-based controls, scoping is also necessary to prevent
sandbox escapes. In my opinion, it's better to block access to
unprivileged services than to allow connecting to any service.
X11 is a trivial sandbox escape by taking over the user's GUI.
A sandboxed program accessing it indicates a misdesign. The NSS module
is not needed by programs that do not do user database resolution,
which I suspect includes almost all sandboxed programs.
> I agree that the bug [1] has been asleep for a bit too long, and we
> should probably pick this up soon. As we have not heard back from
> Ryan after our last inquiry, IMHO I think it would be fair to take it
> over.
I definitely support this!
> Apologies for the difficult feedback - I do not mean to get in the way
> of your enthusiasm here, but I would like to make sure that we don't
> implement this as a stop-gap measure just because the other bug [1]
> seemed more difficult and/or stuck in a github issue interaction.
> Let's rather unblock that bug, if that is the case. :)
>
> As usual, I fully acknowledge that I might well be wrong and might
> have missed some of the underlying reasons, in which case I will
> happily be corrected and change my mind. :)
>
> [1] https://github.com/landlock-lsm/linux/issues/36
> [2] https://github.com/landlock-lsm/linux/issues/36#issuecomment-3699749541
> [3] https://systemd.io/USER_GROUP_API/
>
> Have a good start into the new year!
> –Günther
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [BUG REPORT] memory leak in prepare creds triggered by Netlink/fremovexattr (v6.12.62 , found C repro for Invalid bugs)
From: Paul Moore @ 2025-12-31 14:47 UTC (permalink / raw)
To: wangqing
Cc: yangyuhang0619, jmorris, linux-kernel, linux-security-module,
serge, syzkaller-bugs
In-Reply-To: <20251231031117.37793-1-wq513132@alibaba-inc.com>
On Tue, Dec 30, 2025 at 10:11 PM wangqing <wangqing7171@gmail.com> wrote:
> On Thu, 25 Dec 2025 at 19:12, yuhang hang <yangyuhang0619@gmail.com> wrote:
> > BUG: memory leak
> > unreferenced object 0xffff888023928000 (size 184):
> > comm "syz-executor", pid 10631, jiffies 4294970296
> > hex dump (first 32 bytes):
> > 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > backtrace (crc 1adddbfd):
> > kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline]
> > slab_post_alloc_hook mm/slub.c:4152 [inline]
> > slab_alloc_node mm/slub.c:4197 [inline]
> > kmem_cache_alloc_noprof+0x29a/0x320 mm/slub.c:4204
> > prepare_creds+0x2e/0x760 kernel/cred.c:212
> > copy_creds+0xa7/0xa50 kernel/cred.c:312
> > copy_process+0xf7d/0x8b20 kernel/fork.c:2262
> > kernel_clone+0xeb/0x900 kernel/fork.c:2810
> > __do_sys_clone+0xcf/0x120 kernel/fork.c:2953
> > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > do_syscall_64+0xcb/0x220 arch/x86/entry/common.c:83
> > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >
> > BUG: memory leak
> > unreferenced object 0xffff88802067d920 (size 16):
> > comm "syz-executor", pid 10631, jiffies 4294970296
> > hex dump (first 16 bytes):
> > 00 00 00 00 00 00 00 00 00 3d 08 1b 80 88 ff ff .........=......
> > backtrace (crc 8e8e0e90):
> > kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline]
> > slab_post_alloc_hook mm/slub.c:4152 [inline]
> > slab_alloc_node mm/slub.c:4197 [inline]
> > __do_kmalloc_node mm/slub.c:4331 [inline]
> > __kmalloc_noprof+0x331/0x460 mm/slub.c:4344
> > kmalloc_noprof include/linux/slab.h:882 [inline]
> > kzalloc_noprof include/linux/slab.h:1014 [inline]
> > lsm_blob_alloc security/security.c:685 [inline]
> > lsm_blob_alloc security/security.c:678 [inline]
> > lsm_cred_alloc security/security.c:702 [inline]
> > security_prepare_creds+0x294/0x320 security/security.c:3240
> > prepare_creds+0x54e/0x760 kernel/cred.c:242
> > copy_creds+0xa7/0xa50 kernel/cred.c:312
> > copy_process+0xf7d/0x8b20 kernel/fork.c:2262
> > kernel_clone+0xeb/0x900 kernel/fork.c:2810
> > __do_sys_clone+0xcf/0x120 kernel/fork.c:2953
> > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > do_syscall_64+0xcb/0x220 arch/x86/entry/common.c:83
> > entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Hi,
>
> I've analyzed the kmemleak output and the copy_process() code path.
> The allocation happens in prepare_creds(), but all error paths after
> copy_creds() correctly call exit_creds() to release the credential.
>
> This is likely a false positive caused by RCU delayed freeing of
> struct cred. The object is queued via call_rcu() and not immediately
> freed, so kmemleak may report it as "unreferenced" before the grace
> period completes.
>
> To verify, please:
> 1. Run `echo scan > /sys/kernel/debug/kmemleak`
> 2. Wait 2-3 seconds
> 3. Run `echo scan > /sys/kernel/debug/kmemleak` again
> 4. Check if the report disappears.
>
> If it persists across multiple scans, then it might be a real leak.
Thanks for looking into this, if you are able to confirm that this is
a real leak, please let us know.
--
paul-moore.com
^ permalink raw reply
* Re: [BUG REPORT] memory leak in prepare creds triggered by Netlink/fremovexattr (v6.12.62 , found C repro for Invalid bugs)
From: wangqing @ 2025-12-31 3:11 UTC (permalink / raw)
To: yangyuhang0619
Cc: jmorris, linux-kernel, linux-security-module, paul, serge,
syzkaller-bugs
In-Reply-To: <CACVn-YmCOUQVQd1FYZhcMSQAK5gsvzA7+k5oiocN23nKRMKBMA@mail.gmail.com>
On Thu, 25 Dec 2025 at 19:12, yuhang hang <yangyuhang0619@gmail.com> wrote:
> BUG: memory leak
> unreferenced object 0xffff888023928000 (size 184):
> comm "syz-executor", pid 10631, jiffies 4294970296
> hex dump (first 32 bytes):
> 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc 1adddbfd):
> kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline]
> slab_post_alloc_hook mm/slub.c:4152 [inline]
> slab_alloc_node mm/slub.c:4197 [inline]
> kmem_cache_alloc_noprof+0x29a/0x320 mm/slub.c:4204
> prepare_creds+0x2e/0x760 kernel/cred.c:212
> copy_creds+0xa7/0xa50 kernel/cred.c:312
> copy_process+0xf7d/0x8b20 kernel/fork.c:2262
> kernel_clone+0xeb/0x900 kernel/fork.c:2810
> __do_sys_clone+0xcf/0x120 kernel/fork.c:2953
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xcb/0x220 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> BUG: memory leak
> unreferenced object 0xffff88802067d920 (size 16):
> comm "syz-executor", pid 10631, jiffies 4294970296
> hex dump (first 16 bytes):
> 00 00 00 00 00 00 00 00 00 3d 08 1b 80 88 ff ff .........=......
> backtrace (crc 8e8e0e90):
> kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline]
> slab_post_alloc_hook mm/slub.c:4152 [inline]
> slab_alloc_node mm/slub.c:4197 [inline]
> __do_kmalloc_node mm/slub.c:4331 [inline]
> __kmalloc_noprof+0x331/0x460 mm/slub.c:4344
> kmalloc_noprof include/linux/slab.h:882 [inline]
> kzalloc_noprof include/linux/slab.h:1014 [inline]
> lsm_blob_alloc security/security.c:685 [inline]
> lsm_blob_alloc security/security.c:678 [inline]
> lsm_cred_alloc security/security.c:702 [inline]
> security_prepare_creds+0x294/0x320 security/security.c:3240
> prepare_creds+0x54e/0x760 kernel/cred.c:242
> copy_creds+0xa7/0xa50 kernel/cred.c:312
> copy_process+0xf7d/0x8b20 kernel/fork.c:2262
> kernel_clone+0xeb/0x900 kernel/fork.c:2810
> __do_sys_clone+0xcf/0x120 kernel/fork.c:2953
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xcb/0x220 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
Hi,
I've analyzed the kmemleak output and the copy_process() code path.
The allocation happens in prepare_creds(), but all error paths after
copy_creds() correctly call exit_creds() to release the credential.
This is likely a false positive caused by RCU delayed freeing of
struct cred. The object is queued via call_rcu() and not immediately
freed, so kmemleak may report it as "unreferenced" before the grace
period completes.
To verify, please:
1. Run `echo scan > /sys/kernel/debug/kmemleak`
2. Wait 2-3 seconds
3. Run `echo scan > /sys/kernel/debug/kmemleak` again
4. Check if the report disappears.
If it persists across multiple scans, then it might be a real leak.
--
Best regards,
Qing Wang
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Günther Noack @ 2025-12-30 23:16 UTC (permalink / raw)
To: Tingmao Wang
Cc: Mickaël Salaün, Günther Noack, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi, Justin Suess,
linux-security-module
In-Reply-To: <cover.1767115163.git.m@maowtm.org>
Hello!
Thanks for sending this patch!
On Tue, Dec 30, 2025 at 05:20:18PM +0000, Tingmao Wang wrote:
> Changes in v2:
> Fix grammar in doc, rebased on mic/next, and extracted common code from
> hook_unix_stream_connect and hook_unix_may_send into a separate
> function.
>
> The rest is the same as the v1 cover letter:
>
> This patch series extend the existing abstract Unix socket scoping to
> pathname (i.e. normal file-based) sockets as well, by adding a new scope
> bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET that works the same as
> LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET, except that restricts pathname Unix
> sockets. This means that a sandboxed process with this scope enabled will
> not be able to connect to Unix sockets created outside the sandbox via the
> filesystem.
>
> There is a future plan [1] for allowing specific sockets based on FS
> hierarchy, but this series is only determining access based on domain
> parent-child relationship. There is currently no way to allow specific
> (outside the Landlock domain) Unix sockets, and none of the existing
> Landlock filesystem controls apply to socket connect().
>
> With this series, we can now properly protect against things like the the
> following while only relying on Landlock:
>
> (running under tmux)
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb ~# LL_FS_RO=/ LL_FS_RW= ./sandboxer bash
> Executing the sandboxed command...
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
> cat: /tmp/hi: No such file or directory
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# tmux new-window 'echo hi > /tmp/hi'
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
> hi
>
> The above but with Unix socket scoping enabled (both pathname and abstract
> sockets) - the sandboxed shell can now no longer talk to tmux due to the
> socket being created from outside the Landlock sandbox:
>
> (running under tmux)
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb ~# LL_FS_RO=/ LL_FS_RW= LL_SCOPED=u:a ./sandboxer bash
> Executing the sandboxed command...
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
> cat: /tmp/hi: No such file or directory
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# tmux new-window 'echo hi > /tmp/hi'
> error connecting to /tmp/tmux-0/default (Operation not permitted)
> root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
> cat: /tmp/hi: No such file or directory
>
> Tmux is just one example. In a standard systemd session, `systemd-run
> --user` can also be used (--user will run the command in the user's
> session, without requiring any root privileges), and likely a lot more if
> running in a desktop environment with many popular applications. This
> change therefore makes it possible to create sandboxes without relying on
> additional mechanisms like seccomp to protect against such issues.
>
> These kind of issues was originally discussed on here (I took the idea for
> systemd-run from Demi):
> https://spectrum-os.org/lists/archives/spectrum-devel/00256266-26db-40cf-8f5b-f7c7064084c2@gmail.com/
What is unclear to me from the examples and the description is: Why is
the boundary between allowed and denied connection targets drawn at
the border of the Landlock domain (the "scope") and why don't we solve
this with the file-system-based approach described in [1]?
**Do we have existing use cases where a service is both offered and
connected to all from within the same Landlock domain, and where the
process enforcing the policy does not control the child process enough
so that it would be possible to allow-list it with a
LANDLOCK_ACCESS_FS_CONNECT_UNIX rule?**
If we do not have such a use case, it seems that the planned FS-based
control mechanism from [1] would do the same job? Long term, we might
be better off if we only have only one control -- as we have discussed
in [2], having two of these might mean that they interact in
unconventional and possibly confusing ways.
Apart from that, there are some other weaker hints that make me
slightly critical of this patch set:
* We discussed the idea that a FS-based path_beneath rule would act
implicitly also as an exception for
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET, in [2] - one possible way to
interpret this is that the gravity of the system's logic pulls us
back towards a FS-based control, and we would have to swim less
against the stream if we integrated the Unix connect() control in
that way?
* I am struggling to convince myself that we can tell users to
restrict LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET as a default, as we are
currently doing it with the other "scoped" and file system controls.
(The scoped signals are OK because killing out-of-domain processes
is clearly bad. The scoped abstract sockets are usually OK because
most processes do not need that feature.)
But there are legitimate Unix services that are still needed by
unprivileged processes and which are designed to be safe to use.
For instance, systemd exposes a user database lookup service over
varlink [3], which can be accessed from arbitrary glibc programs
through a NSS module. Using this is incompatible with
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET as long as we do not have the
FS-based control and the surprising implicit permission through a
path_beneath rule as discussed in [2].
(Another example is the X11 socket, to which the same reasoning
should apply, I think, and which is also used by a large number of
programs.)
I agree that the bug [1] has been asleep for a bit too long, and we
should probably pick this up soon. As we have not heard back from
Ryan after our last inquiry, IMHO I think it would be fair to take it
over.
Apologies for the difficult feedback - I do not mean to get in the way
of your enthusiasm here, but I would like to make sure that we don't
implement this as a stop-gap measure just because the other bug [1]
seemed more difficult and/or stuck in a github issue interaction.
Let's rather unblock that bug, if that is the case. :)
As usual, I fully acknowledge that I might well be wrong and might
have missed some of the underlying reasons, in which case I will
happily be corrected and change my mind. :)
[1] https://github.com/landlock-lsm/linux/issues/36
[2] https://github.com/landlock-lsm/linux/issues/36#issuecomment-3699749541
[3] https://systemd.io/USER_GROUP_API/
Have a good start into the new year!
–Günther
^ permalink raw reply
* [PATCH v2 6/6] selftests/landlock: Add pathname socket variants for more tests
From: Tingmao Wang @ 2025-12-30 17:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <cover.1767115163.git.m@maowtm.org>
While this produces a lot of change, it does allow us to "simultaneously"
test both abstract and pathname UNIX sockets with reletively little code
duplication, since they are really similar.
Tests touched: scoped_vs_unscoped, outside_socket,
various_address_sockets, datagram_sockets, self_connect.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
.../selftests/landlock/scoped_unix_test.c | 599 ++++++++++++------
1 file changed, 395 insertions(+), 204 deletions(-)
diff --git a/tools/testing/selftests/landlock/scoped_unix_test.c b/tools/testing/selftests/landlock/scoped_unix_test.c
index 669418c97509..6d1541f77dbe 100644
--- a/tools/testing/selftests/landlock/scoped_unix_test.c
+++ b/tools/testing/selftests/landlock/scoped_unix_test.c
@@ -536,8 +536,12 @@ TEST_F(scoped_audit, connect_to_child)
FIXTURE(scoped_vs_unscoped)
{
- struct service_fixture parent_stream_address, parent_dgram_address,
- child_stream_address, child_dgram_address;
+ struct service_fixture parent_stream_address_abstract,
+ parent_dgram_address_abstract, child_stream_address_abstract,
+ child_dgram_address_abstract;
+ struct service_fixture parent_stream_address_pathname,
+ parent_dgram_address_pathname, child_stream_address_pathname,
+ child_dgram_address_pathname;
};
#include "scoped_multiple_domain_variants.h"
@@ -546,35 +550,75 @@ FIXTURE_SETUP(scoped_vs_unscoped)
{
drop_caps(_metadata);
- memset(&self->parent_stream_address, 0,
- sizeof(self->parent_stream_address));
- set_unix_address(&self->parent_stream_address, 0, true);
- memset(&self->parent_dgram_address, 0,
- sizeof(self->parent_dgram_address));
- set_unix_address(&self->parent_dgram_address, 1, true);
- memset(&self->child_stream_address, 0,
- sizeof(self->child_stream_address));
- set_unix_address(&self->child_stream_address, 2, true);
- memset(&self->child_dgram_address, 0,
- sizeof(self->child_dgram_address));
- set_unix_address(&self->child_dgram_address, 3, true);
+ ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
+
+ /* Abstract addresses. */
+ memset(&self->parent_stream_address_abstract, 0,
+ sizeof(self->parent_stream_address_abstract));
+ set_unix_address(&self->parent_stream_address_abstract, 0, true);
+ memset(&self->parent_dgram_address_abstract, 0,
+ sizeof(self->parent_dgram_address_abstract));
+ set_unix_address(&self->parent_dgram_address_abstract, 1, true);
+ memset(&self->child_stream_address_abstract, 0,
+ sizeof(self->child_stream_address_abstract));
+ set_unix_address(&self->child_stream_address_abstract, 2, true);
+ memset(&self->child_dgram_address_abstract, 0,
+ sizeof(self->child_dgram_address_abstract));
+ set_unix_address(&self->child_dgram_address_abstract, 3, true);
+
+ /* Pathname addresses. */
+ memset(&self->parent_stream_address_pathname, 0,
+ sizeof(self->parent_stream_address_pathname));
+ set_unix_address(&self->parent_stream_address_pathname, 4, false);
+ memset(&self->parent_dgram_address_pathname, 0,
+ sizeof(self->parent_dgram_address_pathname));
+ set_unix_address(&self->parent_dgram_address_pathname, 5, false);
+ memset(&self->child_stream_address_pathname, 0,
+ sizeof(self->child_stream_address_pathname));
+ set_unix_address(&self->child_stream_address_pathname, 6, false);
+ memset(&self->child_dgram_address_pathname, 0,
+ sizeof(self->child_dgram_address_pathname));
+ set_unix_address(&self->child_dgram_address_pathname, 7, false);
}
FIXTURE_TEARDOWN(scoped_vs_unscoped)
{
+ EXPECT_EQ(0, remove_path(self->parent_stream_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, remove_path(self->parent_dgram_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, remove_path(self->child_stream_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, remove_path(self->child_dgram_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
}
/*
* Test unix_stream_connect and unix_may_send for parent, child and
* grand child processes when they can have scoped or non-scoped domains.
*/
-TEST_F(scoped_vs_unscoped, unix_scoping)
+static void test_scoped_vs_unscoped(
+ struct __test_metadata *const _metadata,
+ FIXTURE_DATA(scoped_vs_unscoped) * self,
+ const FIXTURE_VARIANT(scoped_vs_unscoped) * variant,
+ const bool abstract)
{
pid_t child;
int status;
bool can_connect_to_parent, can_connect_to_child;
int pipe_parent[2];
int stream_server_parent, dgram_server_parent;
+ const __u16 scope = abstract ? LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
+ const struct service_fixture *parent_stream_address =
+ abstract ? &self->parent_stream_address_abstract :
+ &self->parent_stream_address_pathname;
+ const struct service_fixture *parent_dgram_address =
+ abstract ? &self->parent_dgram_address_abstract :
+ &self->parent_dgram_address_pathname;
+ const struct service_fixture *child_stream_address =
+ abstract ? &self->child_stream_address_abstract :
+ &self->child_stream_address_pathname;
+ const struct service_fixture *child_dgram_address =
+ abstract ? &self->child_dgram_address_abstract :
+ &self->child_dgram_address_pathname;
can_connect_to_child = (variant->domain_grand_child != SCOPE_SANDBOX);
can_connect_to_parent = (can_connect_to_child &&
@@ -585,8 +629,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
if (variant->domain_all == OTHER_SANDBOX)
create_fs_domain(_metadata);
else if (variant->domain_all == SCOPE_SANDBOX)
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
child = fork();
ASSERT_LE(0, child);
@@ -600,8 +643,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
if (variant->domain_children == OTHER_SANDBOX)
create_fs_domain(_metadata);
else if (variant->domain_children == SCOPE_SANDBOX)
- create_scoped_domain(
- _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
grand_child = fork();
ASSERT_LE(0, grand_child);
@@ -616,9 +658,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
if (variant->domain_grand_child == OTHER_SANDBOX)
create_fs_domain(_metadata);
else if (variant->domain_grand_child == SCOPE_SANDBOX)
- create_scoped_domain(
- _metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
stream_client = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_client);
@@ -626,15 +666,13 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
ASSERT_LE(0, dgram_client);
ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
- stream_err = connect(
- stream_client,
- &self->child_stream_address.unix_addr,
- self->child_stream_address.unix_addr_len);
+ stream_err = connect(stream_client,
+ &child_stream_address->unix_addr,
+ child_stream_address->unix_addr_len);
stream_errno = errno;
- dgram_err = connect(
- dgram_client,
- &self->child_dgram_address.unix_addr,
- self->child_dgram_address.unix_addr_len);
+ dgram_err = connect(dgram_client,
+ &child_dgram_address->unix_addr,
+ child_dgram_address->unix_addr_len);
dgram_errno = errno;
if (can_connect_to_child) {
EXPECT_EQ(0, stream_err);
@@ -653,14 +691,12 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
ASSERT_EQ(1, read(pipe_parent[0], &buf, 1));
stream_err = connect(
- stream_client,
- &self->parent_stream_address.unix_addr,
- self->parent_stream_address.unix_addr_len);
+ stream_client, &parent_stream_address->unix_addr,
+ parent_stream_address->unix_addr_len);
stream_errno = errno;
dgram_err = connect(
- dgram_client,
- &self->parent_dgram_address.unix_addr,
- self->parent_dgram_address.unix_addr_len);
+ dgram_client, &parent_dgram_address->unix_addr,
+ parent_dgram_address->unix_addr_len);
dgram_errno = errno;
if (can_connect_to_parent) {
EXPECT_EQ(0, stream_err);
@@ -681,8 +717,7 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
if (variant->domain_child == OTHER_SANDBOX)
create_fs_domain(_metadata);
else if (variant->domain_child == SCOPE_SANDBOX)
- create_scoped_domain(
- _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
stream_server_child = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_server_child);
@@ -690,11 +725,11 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
ASSERT_LE(0, dgram_server_child);
ASSERT_EQ(0, bind(stream_server_child,
- &self->child_stream_address.unix_addr,
- self->child_stream_address.unix_addr_len));
- ASSERT_EQ(0, bind(dgram_server_child,
- &self->child_dgram_address.unix_addr,
- self->child_dgram_address.unix_addr_len));
+ &child_stream_address->unix_addr,
+ child_stream_address->unix_addr_len));
+ ASSERT_EQ(0,
+ bind(dgram_server_child, &child_dgram_address->unix_addr,
+ child_dgram_address->unix_addr_len));
ASSERT_EQ(0, listen(stream_server_child, backlog));
ASSERT_EQ(1, write(pipe_child[1], ".", 1));
@@ -708,19 +743,16 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
if (variant->domain_parent == OTHER_SANDBOX)
create_fs_domain(_metadata);
else if (variant->domain_parent == SCOPE_SANDBOX)
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
stream_server_parent = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_server_parent);
dgram_server_parent = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_server_parent);
- ASSERT_EQ(0, bind(stream_server_parent,
- &self->parent_stream_address.unix_addr,
- self->parent_stream_address.unix_addr_len));
- ASSERT_EQ(0, bind(dgram_server_parent,
- &self->parent_dgram_address.unix_addr,
- self->parent_dgram_address.unix_addr_len));
+ ASSERT_EQ(0, bind(stream_server_parent, &parent_stream_address->unix_addr,
+ parent_stream_address->unix_addr_len));
+ ASSERT_EQ(0, bind(dgram_server_parent, &parent_dgram_address->unix_addr,
+ parent_dgram_address->unix_addr_len));
ASSERT_EQ(0, listen(stream_server_parent, backlog));
@@ -734,57 +766,119 @@ TEST_F(scoped_vs_unscoped, unix_scoping)
_metadata->exit_code = KSFT_FAIL;
}
+TEST_F(scoped_vs_unscoped, unix_scoping_abstract)
+{
+ test_scoped_vs_unscoped(_metadata, self, variant, true);
+}
+
+TEST_F(scoped_vs_unscoped, unix_scoping_pathname)
+{
+ test_scoped_vs_unscoped(_metadata, self, variant, false);
+}
+
FIXTURE(outside_socket)
{
- struct service_fixture address, transit_address;
+ struct service_fixture address_abstract, transit_address_abstract;
+ struct service_fixture address_pathname, transit_address_pathname;
};
FIXTURE_VARIANT(outside_socket)
{
const bool child_socket;
const int type;
+ const bool abstract;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(outside_socket, abstract_allow_dgram_child) {
+ /* clang-format on */
+ .child_socket = true,
+ .type = SOCK_DGRAM,
+ .abstract = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(outside_socket, abstract_deny_dgram_server) {
+ /* clang-format on */
+ .child_socket = false,
+ .type = SOCK_DGRAM,
+ .abstract = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(outside_socket, abstract_allow_stream_child) {
+ /* clang-format on */
+ .child_socket = true,
+ .type = SOCK_STREAM,
+ .abstract = true,
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(outside_socket, allow_dgram_child) {
+FIXTURE_VARIANT_ADD(outside_socket, abstract_deny_stream_server) {
+ /* clang-format on */
+ .child_socket = false,
+ .type = SOCK_STREAM,
+ .abstract = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(outside_socket, pathname_allow_dgram_child) {
/* clang-format on */
.child_socket = true,
.type = SOCK_DGRAM,
+ .abstract = false,
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(outside_socket, deny_dgram_server) {
+FIXTURE_VARIANT_ADD(outside_socket, pathname_deny_dgram_server) {
/* clang-format on */
.child_socket = false,
.type = SOCK_DGRAM,
+ .abstract = false,
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(outside_socket, allow_stream_child) {
+FIXTURE_VARIANT_ADD(outside_socket, pathname_allow_stream_child) {
/* clang-format on */
.child_socket = true,
.type = SOCK_STREAM,
+ .abstract = false,
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(outside_socket, deny_stream_server) {
+FIXTURE_VARIANT_ADD(outside_socket, pathname_deny_stream_server) {
/* clang-format on */
.child_socket = false,
.type = SOCK_STREAM,
+ .abstract = false,
};
FIXTURE_SETUP(outside_socket)
{
drop_caps(_metadata);
- memset(&self->transit_address, 0, sizeof(self->transit_address));
- set_unix_address(&self->transit_address, 0, true);
- memset(&self->address, 0, sizeof(self->address));
- set_unix_address(&self->address, 1, true);
+ ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
+
+ /* Abstract addresses. */
+ memset(&self->transit_address_abstract, 0,
+ sizeof(self->transit_address_abstract));
+ set_unix_address(&self->transit_address_abstract, 0, true);
+ memset(&self->address_abstract, 0, sizeof(self->address_abstract));
+ set_unix_address(&self->address_abstract, 1, true);
+
+ /* Pathname addresses. */
+ memset(&self->transit_address_pathname, 0,
+ sizeof(self->transit_address_pathname));
+ set_unix_address(&self->transit_address_pathname, 2, false);
+ memset(&self->address_pathname, 0, sizeof(self->address_pathname));
+ set_unix_address(&self->address_pathname, 3, false);
}
FIXTURE_TEARDOWN(outside_socket)
{
+ EXPECT_EQ(0, remove_path(self->transit_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, remove_path(self->address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
}
/*
@@ -798,6 +892,15 @@ TEST_F(outside_socket, socket_with_different_domain)
int pipe_child[2], pipe_parent[2];
char buf_parent;
int server_socket;
+ const __u16 scope = variant->abstract ?
+ LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
+ const struct service_fixture *transit_address =
+ variant->abstract ? &self->transit_address_abstract :
+ &self->transit_address_pathname;
+ const struct service_fixture *address =
+ variant->abstract ? &self->address_abstract :
+ &self->address_pathname;
ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
@@ -812,8 +915,7 @@ TEST_F(outside_socket, socket_with_different_domain)
EXPECT_EQ(0, close(pipe_child[0]));
/* Client always has a domain. */
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
if (variant->child_socket) {
int data_socket, passed_socket, stream_server;
@@ -823,8 +925,8 @@ TEST_F(outside_socket, socket_with_different_domain)
stream_server = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_server);
ASSERT_EQ(0, bind(stream_server,
- &self->transit_address.unix_addr,
- self->transit_address.unix_addr_len));
+ &transit_address->unix_addr,
+ transit_address->unix_addr_len));
ASSERT_EQ(0, listen(stream_server, backlog));
ASSERT_EQ(1, write(pipe_child[1], ".", 1));
data_socket = accept(stream_server, NULL, NULL);
@@ -839,8 +941,8 @@ TEST_F(outside_socket, socket_with_different_domain)
/* Waits for parent signal for connection. */
ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
- err = connect(client_socket, &self->address.unix_addr,
- self->address.unix_addr_len);
+ err = connect(client_socket, &address->unix_addr,
+ address->unix_addr_len);
if (variant->child_socket) {
EXPECT_EQ(0, err);
} else {
@@ -859,9 +961,8 @@ TEST_F(outside_socket, socket_with_different_domain)
ASSERT_LE(0, client_child);
ASSERT_EQ(1, read(pipe_child[0], &buf_parent, 1));
- ASSERT_EQ(0, connect(client_child,
- &self->transit_address.unix_addr,
- self->transit_address.unix_addr_len));
+ ASSERT_EQ(0, connect(client_child, &transit_address->unix_addr,
+ transit_address->unix_addr_len));
server_socket = recv_fd(client_child);
EXPECT_EQ(0, close(client_child));
} else {
@@ -870,10 +971,10 @@ TEST_F(outside_socket, socket_with_different_domain)
ASSERT_LE(0, server_socket);
/* Server always has a domain. */
- create_scoped_domain(_metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
- ASSERT_EQ(0, bind(server_socket, &self->address.unix_addr,
- self->address.unix_addr_len));
+ ASSERT_EQ(0,
+ bind(server_socket, &address->unix_addr, address->unix_addr_len));
if (variant->type == SOCK_STREAM)
ASSERT_EQ(0, listen(server_socket, backlog));
@@ -888,52 +989,85 @@ TEST_F(outside_socket, socket_with_different_domain)
_metadata->exit_code = KSFT_FAIL;
}
-static const char stream_path[] = TMP_DIR "/stream.sock";
-static const char dgram_path[] = TMP_DIR "/dgram.sock";
-
/* clang-format off */
-FIXTURE(various_address_sockets) {};
+FIXTURE(various_address_sockets) {
+ struct service_fixture stream_pathname_addr, dgram_pathname_addr;
+ struct service_fixture stream_abstract_addr, dgram_abstract_addr;
+};
/* clang-format on */
-FIXTURE_VARIANT(various_address_sockets)
-{
- const int domain;
+/*
+ * Test all 4 combinations of abstract and pathname socket scope bits,
+ * plus a case with no Landlock domain at all.
+ */
+/* clang-format off */
+FIXTURE_VARIANT(various_address_sockets) {
+ /* clang-format on */
+ const __u16 scope_bits;
+ const bool no_sandbox;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(various_address_sockets, scope_abstract) {
+ /* clang-format on */
+ .scope_bits = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(various_address_sockets, scope_pathname) {
+ /* clang-format on */
+ .scope_bits = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(various_address_sockets, pathname_socket_scoped_domain) {
+FIXTURE_VARIANT_ADD(various_address_sockets, scope_both) {
/* clang-format on */
- .domain = SCOPE_SANDBOX,
+ .scope_bits = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(various_address_sockets, pathname_socket_other_domain) {
+FIXTURE_VARIANT_ADD(various_address_sockets, scope_none) {
/* clang-format on */
- .domain = OTHER_SANDBOX,
+ .scope_bits = 0,
};
/* clang-format off */
-FIXTURE_VARIANT_ADD(various_address_sockets, pathname_socket_no_domain) {
+FIXTURE_VARIANT_ADD(various_address_sockets, no_domain) {
/* clang-format on */
- .domain = NO_SANDBOX,
+ .no_sandbox = true,
};
FIXTURE_SETUP(various_address_sockets)
{
drop_caps(_metadata);
- umask(0077);
- ASSERT_EQ(0, mkdir(TMP_DIR, 0700));
+ ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
+
+ memset(&self->stream_pathname_addr, 0, sizeof(self->stream_pathname_addr));
+ set_unix_address(&self->stream_pathname_addr, 0, false);
+ memset(&self->dgram_pathname_addr, 0, sizeof(self->dgram_pathname_addr));
+ set_unix_address(&self->dgram_pathname_addr, 1, false);
+
+ memset(&self->stream_abstract_addr, 0, sizeof(self->stream_abstract_addr));
+ set_unix_address(&self->stream_abstract_addr, 2, true);
+ memset(&self->dgram_abstract_addr, 0, sizeof(self->dgram_abstract_addr));
+ set_unix_address(&self->dgram_abstract_addr, 3, true);
}
FIXTURE_TEARDOWN(various_address_sockets)
{
- EXPECT_EQ(0, unlink(stream_path));
- EXPECT_EQ(0, unlink(dgram_path));
- EXPECT_EQ(0, rmdir(TMP_DIR));
+ EXPECT_EQ(0, remove_path(self->stream_pathname_addr.unix_addr.sun_path));
+ EXPECT_EQ(0, remove_path(self->dgram_pathname_addr.unix_addr.sun_path));
+ EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
}
-TEST_F(various_address_sockets, scoped_pathname_sockets)
+/*
+ * Test interaction of various scope flags (controlled by variant->domain)
+ * with pathname and abstract sockets when connecting from a sandboxed
+ * child.
+ */
+TEST_F(various_address_sockets, scoped_sockets)
{
pid_t child;
int status;
@@ -942,25 +1076,10 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
int unnamed_sockets[2];
int stream_pathname_socket, dgram_pathname_socket,
stream_abstract_socket, dgram_abstract_socket, data_socket;
- struct service_fixture stream_abstract_addr, dgram_abstract_addr;
- struct sockaddr_un stream_pathname_addr = {
- .sun_family = AF_UNIX,
- };
- struct sockaddr_un dgram_pathname_addr = {
- .sun_family = AF_UNIX,
- };
-
- /* Pathname address. */
- snprintf(stream_pathname_addr.sun_path,
- sizeof(stream_pathname_addr.sun_path), "%s", stream_path);
- snprintf(dgram_pathname_addr.sun_path,
- sizeof(dgram_pathname_addr.sun_path), "%s", dgram_path);
-
- /* Abstract address. */
- memset(&stream_abstract_addr, 0, sizeof(stream_abstract_addr));
- set_unix_address(&stream_abstract_addr, 0, true);
- memset(&dgram_abstract_addr, 0, sizeof(dgram_abstract_addr));
- set_unix_address(&dgram_abstract_addr, 1, true);
+ bool pathname_restricted =
+ (variant->scope_bits & LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET);
+ bool abstract_restricted =
+ (variant->scope_bits & LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
/* Unnamed address for datagram socket. */
ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_DGRAM, 0, unnamed_sockets));
@@ -975,82 +1094,103 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
EXPECT_EQ(0, close(pipe_parent[1]));
EXPECT_EQ(0, close(unnamed_sockets[1]));
- if (variant->domain == SCOPE_SANDBOX)
- create_scoped_domain(
- _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
- else if (variant->domain == OTHER_SANDBOX)
+ /* Create domain based on variant. */
+ if (variant->scope_bits)
+ create_scoped_domain(_metadata, variant->scope_bits);
+ else if (!variant->no_sandbox)
create_fs_domain(_metadata);
/* Waits for parent to listen. */
ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
EXPECT_EQ(0, close(pipe_parent[0]));
- /* Checks that we can send data through a datagram socket. */
+ /* Checks that we can send data through a unnamed socket. */
ASSERT_EQ(1, write(unnamed_sockets[0], "a", 1));
EXPECT_EQ(0, close(unnamed_sockets[0]));
/* Connects with pathname sockets. */
stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_pathname_socket);
- ASSERT_EQ(0,
- connect(stream_pathname_socket, &stream_pathname_addr,
- sizeof(stream_pathname_addr)));
- ASSERT_EQ(1, write(stream_pathname_socket, "b", 1));
+ err = connect(stream_pathname_socket,
+ &self->stream_pathname_addr.unix_addr,
+ self->stream_pathname_addr.unix_addr_len);
+ if (pathname_restricted) {
+ EXPECT_EQ(-1, err);
+ EXPECT_EQ(EPERM, errno);
+ } else {
+ EXPECT_EQ(0, err);
+ ASSERT_EQ(1, write(stream_pathname_socket, "b", 1));
+ }
EXPECT_EQ(0, close(stream_pathname_socket));
- /* Sends without connection. */
+ /* Sends without connection (pathname). */
dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_pathname_socket);
err = sendto(dgram_pathname_socket, "c", 1, 0,
- &dgram_pathname_addr, sizeof(dgram_pathname_addr));
- EXPECT_EQ(1, err);
+ &self->dgram_pathname_addr.unix_addr,
+ self->dgram_pathname_addr.unix_addr_len);
+ if (pathname_restricted) {
+ EXPECT_EQ(-1, err);
+ EXPECT_EQ(EPERM, errno);
+ } else {
+ EXPECT_EQ(1, err);
+ }
+
+ /* Sends with connection (pathname). */
+ err = connect(dgram_pathname_socket,
+ &self->dgram_pathname_addr.unix_addr,
+ self->dgram_pathname_addr.unix_addr_len);
+ if (pathname_restricted) {
+ EXPECT_EQ(-1, err);
+ EXPECT_EQ(EPERM, errno);
+ } else {
+ EXPECT_EQ(0, err);
+ ASSERT_EQ(1, write(dgram_pathname_socket, "d", 1));
+ }
- /* Sends with connection. */
- ASSERT_EQ(0,
- connect(dgram_pathname_socket, &dgram_pathname_addr,
- sizeof(dgram_pathname_addr)));
- ASSERT_EQ(1, write(dgram_pathname_socket, "d", 1));
EXPECT_EQ(0, close(dgram_pathname_socket));
/* Connects with abstract sockets. */
stream_abstract_socket = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_abstract_socket);
err = connect(stream_abstract_socket,
- &stream_abstract_addr.unix_addr,
- stream_abstract_addr.unix_addr_len);
- if (variant->domain == SCOPE_SANDBOX) {
+ &self->stream_abstract_addr.unix_addr,
+ self->stream_abstract_addr.unix_addr_len);
+ if (abstract_restricted) {
EXPECT_EQ(-1, err);
EXPECT_EQ(EPERM, errno);
} else {
EXPECT_EQ(0, err);
ASSERT_EQ(1, write(stream_abstract_socket, "e", 1));
}
+
EXPECT_EQ(0, close(stream_abstract_socket));
- /* Sends without connection. */
+ /* Sends without connection (abstract). */
dgram_abstract_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_abstract_socket);
err = sendto(dgram_abstract_socket, "f", 1, 0,
- &dgram_abstract_addr.unix_addr,
- dgram_abstract_addr.unix_addr_len);
- if (variant->domain == SCOPE_SANDBOX) {
+ &self->dgram_abstract_addr.unix_addr,
+ self->dgram_abstract_addr.unix_addr_len);
+ if (abstract_restricted) {
EXPECT_EQ(-1, err);
EXPECT_EQ(EPERM, errno);
} else {
EXPECT_EQ(1, err);
}
- /* Sends with connection. */
+ /* Sends with connection (abstract). */
err = connect(dgram_abstract_socket,
- &dgram_abstract_addr.unix_addr,
- dgram_abstract_addr.unix_addr_len);
- if (variant->domain == SCOPE_SANDBOX) {
+ &self->dgram_abstract_addr.unix_addr,
+ self->dgram_abstract_addr.unix_addr_len);
+ if (abstract_restricted) {
EXPECT_EQ(-1, err);
EXPECT_EQ(EPERM, errno);
} else {
EXPECT_EQ(0, err);
ASSERT_EQ(1, write(dgram_abstract_socket, "g", 1));
}
+
EXPECT_EQ(0, close(dgram_abstract_socket));
_exit(_metadata->exit_code);
@@ -1062,27 +1202,30 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
/* Sets up pathname servers. */
stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_pathname_socket);
- ASSERT_EQ(0, bind(stream_pathname_socket, &stream_pathname_addr,
- sizeof(stream_pathname_addr)));
+ ASSERT_EQ(0, bind(stream_pathname_socket,
+ &self->stream_pathname_addr.unix_addr,
+ self->stream_pathname_addr.unix_addr_len));
ASSERT_EQ(0, listen(stream_pathname_socket, backlog));
dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_pathname_socket);
- ASSERT_EQ(0, bind(dgram_pathname_socket, &dgram_pathname_addr,
- sizeof(dgram_pathname_addr)));
+ ASSERT_EQ(0, bind(dgram_pathname_socket,
+ &self->dgram_pathname_addr.unix_addr,
+ self->dgram_pathname_addr.unix_addr_len));
/* Sets up abstract servers. */
stream_abstract_socket = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_abstract_socket);
- ASSERT_EQ(0,
- bind(stream_abstract_socket, &stream_abstract_addr.unix_addr,
- stream_abstract_addr.unix_addr_len));
+ ASSERT_EQ(0, bind(stream_abstract_socket,
+ &self->stream_abstract_addr.unix_addr,
+ self->stream_abstract_addr.unix_addr_len));
+ ASSERT_EQ(0, listen(stream_abstract_socket, backlog));
dgram_abstract_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_abstract_socket);
- ASSERT_EQ(0, bind(dgram_abstract_socket, &dgram_abstract_addr.unix_addr,
- dgram_abstract_addr.unix_addr_len));
- ASSERT_EQ(0, listen(stream_abstract_socket, backlog));
+ ASSERT_EQ(0, bind(dgram_abstract_socket,
+ &self->dgram_abstract_addr.unix_addr,
+ self->dgram_abstract_addr.unix_addr_len));
ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
EXPECT_EQ(0, close(pipe_parent[1]));
@@ -1092,24 +1235,31 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
ASSERT_EQ('a', buf_parent);
EXPECT_LE(0, close(unnamed_sockets[1]));
- /* Reads from pathname sockets. */
- data_socket = accept(stream_pathname_socket, NULL, NULL);
- ASSERT_LE(0, data_socket);
- ASSERT_EQ(1, read(data_socket, &buf_parent, sizeof(buf_parent)));
- ASSERT_EQ('b', buf_parent);
- EXPECT_EQ(0, close(data_socket));
- EXPECT_EQ(0, close(stream_pathname_socket));
+ if (!pathname_restricted) {
+ /*
+ * Reads from pathname sockets if we expect child to be able to
+ * send.
+ */
+ data_socket = accept(stream_pathname_socket, NULL, NULL);
+ ASSERT_LE(0, data_socket);
+ ASSERT_EQ(1,
+ read(data_socket, &buf_parent, sizeof(buf_parent)));
+ ASSERT_EQ('b', buf_parent);
+ EXPECT_EQ(0, close(data_socket));
- ASSERT_EQ(1,
- read(dgram_pathname_socket, &buf_parent, sizeof(buf_parent)));
- ASSERT_EQ('c', buf_parent);
- ASSERT_EQ(1,
- read(dgram_pathname_socket, &buf_parent, sizeof(buf_parent)));
- ASSERT_EQ('d', buf_parent);
- EXPECT_EQ(0, close(dgram_pathname_socket));
+ ASSERT_EQ(1, read(dgram_pathname_socket, &buf_parent,
+ sizeof(buf_parent)));
+ ASSERT_EQ('c', buf_parent);
+ ASSERT_EQ(1, read(dgram_pathname_socket, &buf_parent,
+ sizeof(buf_parent)));
+ ASSERT_EQ('d', buf_parent);
+ }
- if (variant->domain != SCOPE_SANDBOX) {
- /* Reads from abstract sockets if allowed to send. */
+ if (!abstract_restricted) {
+ /*
+ * Reads from abstract sockets if we expect child to be able to
+ * send.
+ */
data_socket = accept(stream_abstract_socket, NULL, NULL);
ASSERT_LE(0, data_socket);
ASSERT_EQ(1,
@@ -1125,30 +1275,73 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
ASSERT_EQ('g', buf_parent);
}
- /* Waits for all abstract socket tests. */
+ /* Waits for child to complete, and only close the socket afterwards. */
ASSERT_EQ(child, waitpid(child, &status, 0));
EXPECT_EQ(0, close(stream_abstract_socket));
EXPECT_EQ(0, close(dgram_abstract_socket));
+ EXPECT_EQ(0, close(stream_pathname_socket));
+ EXPECT_EQ(0, close(dgram_pathname_socket));
if (WIFSIGNALED(status) || !WIFEXITED(status) ||
WEXITSTATUS(status) != EXIT_SUCCESS)
_metadata->exit_code = KSFT_FAIL;
}
-TEST(datagram_sockets)
+/* Fixture for datagram_sockets and self_connect tests */
+FIXTURE(socket_type_test)
{
struct service_fixture connected_addr, non_connected_addr;
+};
+
+FIXTURE_VARIANT(socket_type_test)
+{
+ const bool abstract;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(socket_type_test, abstract) {
+ /* clang-format on */
+ .abstract = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(socket_type_test, pathname) {
+ /* clang-format on */
+ .abstract = false,
+};
+
+FIXTURE_SETUP(socket_type_test)
+{
+ drop_caps(_metadata);
+
+ if (!variant->abstract)
+ ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
+
+ memset(&self->connected_addr, 0, sizeof(self->connected_addr));
+ set_unix_address(&self->connected_addr, 0, variant->abstract);
+ memset(&self->non_connected_addr, 0, sizeof(self->non_connected_addr));
+ set_unix_address(&self->non_connected_addr, 1, variant->abstract);
+}
+
+FIXTURE_TEARDOWN(socket_type_test)
+{
+ if (!variant->abstract) {
+ EXPECT_EQ(0, remove_path(self->connected_addr.unix_addr.sun_path));
+ EXPECT_EQ(0, remove_path(self->non_connected_addr.unix_addr.sun_path));
+ EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
+ }
+}
+
+TEST_F(socket_type_test, datagram_sockets)
+{
int server_conn_socket, server_unconn_socket;
int pipe_parent[2], pipe_child[2];
int status;
char buf;
pid_t child;
-
- drop_caps(_metadata);
- memset(&connected_addr, 0, sizeof(connected_addr));
- set_unix_address(&connected_addr, 0, true);
- memset(&non_connected_addr, 0, sizeof(non_connected_addr));
- set_unix_address(&non_connected_addr, 1, true);
+ const __u16 scope = variant->abstract ?
+ LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
@@ -1169,8 +1362,9 @@ TEST(datagram_sockets)
/* Waits for parent to listen. */
ASSERT_EQ(1, read(pipe_parent[0], &buf, 1));
ASSERT_EQ(0,
- connect(client_conn_socket, &connected_addr.unix_addr,
- connected_addr.unix_addr_len));
+ connect(client_conn_socket,
+ &self->connected_addr.unix_addr,
+ self->connected_addr.unix_addr_len));
/*
* Both connected and non-connected sockets can send data when
@@ -1178,13 +1372,12 @@ TEST(datagram_sockets)
*/
ASSERT_EQ(1, send(client_conn_socket, ".", 1, 0));
ASSERT_EQ(1, sendto(client_unconn_socket, ".", 1, 0,
- &non_connected_addr.unix_addr,
- non_connected_addr.unix_addr_len));
+ &self->non_connected_addr.unix_addr,
+ self->non_connected_addr.unix_addr_len));
ASSERT_EQ(1, write(pipe_child[1], ".", 1));
/* Scopes the domain. */
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
/*
* Connected socket sends data to the receiver, but the
@@ -1192,8 +1385,8 @@ TEST(datagram_sockets)
*/
ASSERT_EQ(1, send(client_conn_socket, ".", 1, 0));
ASSERT_EQ(-1, sendto(client_unconn_socket, ".", 1, 0,
- &non_connected_addr.unix_addr,
- non_connected_addr.unix_addr_len));
+ &self->non_connected_addr.unix_addr,
+ self->non_connected_addr.unix_addr_len));
ASSERT_EQ(EPERM, errno);
ASSERT_EQ(1, write(pipe_child[1], ".", 1));
@@ -1210,10 +1403,11 @@ TEST(datagram_sockets)
ASSERT_LE(0, server_conn_socket);
ASSERT_LE(0, server_unconn_socket);
- ASSERT_EQ(0, bind(server_conn_socket, &connected_addr.unix_addr,
- connected_addr.unix_addr_len));
- ASSERT_EQ(0, bind(server_unconn_socket, &non_connected_addr.unix_addr,
- non_connected_addr.unix_addr_len));
+ ASSERT_EQ(0, bind(server_conn_socket, &self->connected_addr.unix_addr,
+ self->connected_addr.unix_addr_len));
+ ASSERT_EQ(0, bind(server_unconn_socket,
+ &self->non_connected_addr.unix_addr,
+ self->non_connected_addr.unix_addr_len));
ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
/* Waits for child to test. */
@@ -1238,52 +1432,49 @@ TEST(datagram_sockets)
_metadata->exit_code = KSFT_FAIL;
}
-TEST(self_connect)
+TEST_F(socket_type_test, self_connect)
{
- struct service_fixture connected_addr, non_connected_addr;
int connected_socket, non_connected_socket, status;
pid_t child;
-
- drop_caps(_metadata);
- memset(&connected_addr, 0, sizeof(connected_addr));
- set_unix_address(&connected_addr, 0, true);
- memset(&non_connected_addr, 0, sizeof(non_connected_addr));
- set_unix_address(&non_connected_addr, 1, true);
+ const __u16 scope = variant->abstract ?
+ LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
connected_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
non_connected_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, connected_socket);
ASSERT_LE(0, non_connected_socket);
- ASSERT_EQ(0, bind(connected_socket, &connected_addr.unix_addr,
- connected_addr.unix_addr_len));
- ASSERT_EQ(0, bind(non_connected_socket, &non_connected_addr.unix_addr,
- non_connected_addr.unix_addr_len));
+ ASSERT_EQ(0, bind(connected_socket, &self->connected_addr.unix_addr,
+ self->connected_addr.unix_addr_len));
+ ASSERT_EQ(0, bind(non_connected_socket,
+ &self->non_connected_addr.unix_addr,
+ self->non_connected_addr.unix_addr_len));
child = fork();
ASSERT_LE(0, child);
if (child == 0) {
/* Child's domain is scoped. */
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
/*
* The child inherits the sockets, and cannot connect or
* send data to them.
*/
ASSERT_EQ(-1,
- connect(connected_socket, &connected_addr.unix_addr,
- connected_addr.unix_addr_len));
+ connect(connected_socket,
+ &self->connected_addr.unix_addr,
+ self->connected_addr.unix_addr_len));
ASSERT_EQ(EPERM, errno);
ASSERT_EQ(-1, sendto(connected_socket, ".", 1, 0,
- &connected_addr.unix_addr,
- connected_addr.unix_addr_len));
+ &self->connected_addr.unix_addr,
+ self->connected_addr.unix_addr_len));
ASSERT_EQ(EPERM, errno);
ASSERT_EQ(-1, sendto(non_connected_socket, ".", 1, 0,
- &non_connected_addr.unix_addr,
- non_connected_addr.unix_addr_len));
+ &self->non_connected_addr.unix_addr,
+ self->non_connected_addr.unix_addr_len));
ASSERT_EQ(EPERM, errno);
EXPECT_EQ(0, close(connected_socket));
--
2.52.0
^ permalink raw reply related
* [PATCH v2 5/6] selftests/landlock: Repurpose scoped_abstract_unix_test.c for pathname sockets too.
From: Tingmao Wang @ 2025-12-30 17:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <cover.1767115163.git.m@maowtm.org>
Since there is very little difference between abstract and pathname
sockets in terms of testing of the scoped access checks (the only
difference is in which scope bit control which form of socket), it makes
sense to reuse the existing test for both type of sockets. Therefore, we
rename scoped_abstract_unix_test.c to scoped_unix_test.c and extend the
scoped_domains test to test pathname (i.e. non-abstract) sockets too.
Since we can't change the variant data of scoped_domains (as it is defined
in the shared .h file), we do this by extracting the actual test code into
a function, and call it from different test cases.
Also extend scoped_audit (this time we can use variants) to test both
abstract and pathname sockets. For pathname sockets, audit_log_lsm_data
will produce path="..." (or hex if path contains control characters) with
absolute paths from the dentry, so we need to construct the escaped regex
for the real path like in fs_test.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
...bstract_unix_test.c => scoped_unix_test.c} | 256 ++++++++++++++----
1 file changed, 206 insertions(+), 50 deletions(-)
rename tools/testing/selftests/landlock/{scoped_abstract_unix_test.c => scoped_unix_test.c} (81%)
diff --git a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c b/tools/testing/selftests/landlock/scoped_unix_test.c
similarity index 81%
rename from tools/testing/selftests/landlock/scoped_abstract_unix_test.c
rename to tools/testing/selftests/landlock/scoped_unix_test.c
index 4a790e2d387d..669418c97509 100644
--- a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
+++ b/tools/testing/selftests/landlock/scoped_unix_test.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Landlock tests - Abstract UNIX socket
+ * Landlock tests - Scoped access checks for UNIX socket (abstract and
+ * pathname)
*
* Copyright © 2024 Tahera Fahimi <fahimitahera@gmail.com>
*/
@@ -19,6 +20,7 @@
#include <sys/un.h>
#include <sys/wait.h>
#include <unistd.h>
+#include <stdlib.h>
#include "audit.h"
#include "common.h"
@@ -47,7 +49,8 @@ static void create_fs_domain(struct __test_metadata *const _metadata)
FIXTURE(scoped_domains)
{
- struct service_fixture stream_address, dgram_address;
+ struct service_fixture stream_address_abstract, dgram_address_abstract,
+ stream_address_pathname, dgram_address_pathname;
};
#include "scoped_base_variants.h"
@@ -56,27 +59,62 @@ FIXTURE_SETUP(scoped_domains)
{
drop_caps(_metadata);
- memset(&self->stream_address, 0, sizeof(self->stream_address));
- memset(&self->dgram_address, 0, sizeof(self->dgram_address));
- set_unix_address(&self->stream_address, 0, true);
- set_unix_address(&self->dgram_address, 1, true);
+ ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
+
+ memset(&self->stream_address_abstract, 0,
+ sizeof(self->stream_address_abstract));
+ memset(&self->dgram_address_abstract, 0,
+ sizeof(self->dgram_address_abstract));
+ memset(&self->stream_address_pathname, 0,
+ sizeof(self->stream_address_pathname));
+ memset(&self->dgram_address_pathname, 0,
+ sizeof(self->dgram_address_pathname));
+ set_unix_address(&self->stream_address_abstract, 0, true);
+ set_unix_address(&self->dgram_address_abstract, 1, true);
+ set_unix_address(&self->stream_address_pathname, 0, false);
+ set_unix_address(&self->dgram_address_pathname, 1, false);
+}
+
+/* Remove @path if it exists */
+int remove_path(const char *path)
+{
+ if (unlink(path) == -1) {
+ if (errno != ENOENT)
+ return -errno;
+ }
+ return 0;
}
FIXTURE_TEARDOWN(scoped_domains)
{
+ EXPECT_EQ(0, remove_path(self->stream_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, remove_path(self->dgram_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
}
/*
* Test unix_stream_connect() and unix_may_send() for a child connecting to its
* parent, when they have scoped domain or no domain.
*/
-TEST_F(scoped_domains, connect_to_parent)
+static void test_connect_to_parent(struct __test_metadata *const _metadata,
+ FIXTURE_DATA(scoped_domains) * self,
+ const FIXTURE_VARIANT(scoped_domains) *
+ variant,
+ const bool abstract)
{
pid_t child;
bool can_connect_to_parent;
int status;
int pipe_parent[2];
int stream_server, dgram_server;
+ const __u16 scope = abstract ? LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
+ const struct service_fixture *stream_address =
+ abstract ? &self->stream_address_abstract :
+ &self->stream_address_pathname;
+ const struct service_fixture *dgram_address =
+ abstract ? &self->dgram_address_abstract :
+ &self->dgram_address_pathname;
/*
* can_connect_to_parent is true if a child process can connect to its
@@ -87,8 +125,7 @@ TEST_F(scoped_domains, connect_to_parent)
ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
if (variant->domain_both) {
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
if (!__test_passed(_metadata))
return;
}
@@ -102,8 +139,7 @@ TEST_F(scoped_domains, connect_to_parent)
EXPECT_EQ(0, close(pipe_parent[1]));
if (variant->domain_child)
- create_scoped_domain(
- _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
stream_client = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_client);
@@ -113,8 +149,8 @@ TEST_F(scoped_domains, connect_to_parent)
/* Waits for the server. */
ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
- err = connect(stream_client, &self->stream_address.unix_addr,
- self->stream_address.unix_addr_len);
+ err = connect(stream_client, &stream_address->unix_addr,
+ stream_address->unix_addr_len);
if (can_connect_to_parent) {
EXPECT_EQ(0, err);
} else {
@@ -123,8 +159,8 @@ TEST_F(scoped_domains, connect_to_parent)
}
EXPECT_EQ(0, close(stream_client));
- err = connect(dgram_client, &self->dgram_address.unix_addr,
- self->dgram_address.unix_addr_len);
+ err = connect(dgram_client, &dgram_address->unix_addr,
+ dgram_address->unix_addr_len);
if (can_connect_to_parent) {
EXPECT_EQ(0, err);
} else {
@@ -137,17 +173,16 @@ TEST_F(scoped_domains, connect_to_parent)
}
EXPECT_EQ(0, close(pipe_parent[0]));
if (variant->domain_parent)
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
stream_server = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_server);
dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_server);
- ASSERT_EQ(0, bind(stream_server, &self->stream_address.unix_addr,
- self->stream_address.unix_addr_len));
- ASSERT_EQ(0, bind(dgram_server, &self->dgram_address.unix_addr,
- self->dgram_address.unix_addr_len));
+ ASSERT_EQ(0, bind(stream_server, &stream_address->unix_addr,
+ stream_address->unix_addr_len));
+ ASSERT_EQ(0, bind(dgram_server, &dgram_address->unix_addr,
+ dgram_address->unix_addr_len));
ASSERT_EQ(0, listen(stream_server, backlog));
/* Signals to child that the parent is listening. */
@@ -166,7 +201,11 @@ TEST_F(scoped_domains, connect_to_parent)
* Test unix_stream_connect() and unix_may_send() for a parent connecting to
* its child, when they have scoped domain or no domain.
*/
-TEST_F(scoped_domains, connect_to_child)
+static void test_connect_to_child(struct __test_metadata *const _metadata,
+ FIXTURE_DATA(scoped_domains) * self,
+ const FIXTURE_VARIANT(scoped_domains) *
+ variant,
+ const bool abstract)
{
pid_t child;
bool can_connect_to_child;
@@ -174,6 +213,14 @@ TEST_F(scoped_domains, connect_to_child)
int pipe_child[2], pipe_parent[2];
char buf;
int stream_client, dgram_client;
+ const __u16 scope = abstract ? LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET :
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
+ const struct service_fixture *stream_address =
+ abstract ? &self->stream_address_abstract :
+ &self->stream_address_pathname;
+ const struct service_fixture *dgram_address =
+ abstract ? &self->dgram_address_abstract :
+ &self->dgram_address_pathname;
/*
* can_connect_to_child is true if a parent process can connect to its
@@ -185,8 +232,7 @@ TEST_F(scoped_domains, connect_to_child)
ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
if (variant->domain_both) {
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
if (!__test_passed(_metadata))
return;
}
@@ -199,8 +245,7 @@ TEST_F(scoped_domains, connect_to_child)
EXPECT_EQ(0, close(pipe_parent[1]));
EXPECT_EQ(0, close(pipe_child[0]));
if (variant->domain_child)
- create_scoped_domain(
- _metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
/* Waits for the parent to be in a domain, if any. */
ASSERT_EQ(1, read(pipe_parent[0], &buf, 1));
@@ -209,11 +254,10 @@ TEST_F(scoped_domains, connect_to_child)
ASSERT_LE(0, stream_server);
dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_server);
- ASSERT_EQ(0,
- bind(stream_server, &self->stream_address.unix_addr,
- self->stream_address.unix_addr_len));
- ASSERT_EQ(0, bind(dgram_server, &self->dgram_address.unix_addr,
- self->dgram_address.unix_addr_len));
+ ASSERT_EQ(0, bind(stream_server, &stream_address->unix_addr,
+ stream_address->unix_addr_len));
+ ASSERT_EQ(0, bind(dgram_server, &dgram_address->unix_addr,
+ dgram_address->unix_addr_len));
ASSERT_EQ(0, listen(stream_server, backlog));
/* Signals to the parent that child is listening. */
@@ -230,8 +274,7 @@ TEST_F(scoped_domains, connect_to_child)
EXPECT_EQ(0, close(pipe_parent[0]));
if (variant->domain_parent)
- create_scoped_domain(_metadata,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata, scope);
/* Signals that the parent is in a domain, if any. */
ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
@@ -243,11 +286,11 @@ TEST_F(scoped_domains, connect_to_child)
/* Waits for the child to listen */
ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
- err_stream = connect(stream_client, &self->stream_address.unix_addr,
- self->stream_address.unix_addr_len);
+ err_stream = connect(stream_client, &stream_address->unix_addr,
+ stream_address->unix_addr_len);
errno_stream = errno;
- err_dgram = connect(dgram_client, &self->dgram_address.unix_addr,
- self->dgram_address.unix_addr_len);
+ err_dgram = connect(dgram_client, &dgram_address->unix_addr,
+ dgram_address->unix_addr_len);
errno_dgram = errno;
if (can_connect_to_child) {
EXPECT_EQ(0, err_stream);
@@ -268,19 +311,79 @@ TEST_F(scoped_domains, connect_to_child)
_metadata->exit_code = KSFT_FAIL;
}
+/*
+ * Test unix_stream_connect() and unix_may_send() for a child connecting to its
+ * parent, when they have scoped domain or no domain.
+ */
+TEST_F(scoped_domains, abstract_connect_to_parent)
+{
+ test_connect_to_parent(_metadata, self, variant, true);
+}
+
+/*
+ * Test unix_stream_connect() and unix_may_send() for a parent connecting to
+ * its child, when they have scoped domain or no domain.
+ */
+TEST_F(scoped_domains, abstract_connect_to_child)
+{
+ test_connect_to_child(_metadata, self, variant, true);
+}
+
+/*
+ * Test unix_stream_connect() and unix_may_send() for a child connecting to its
+ * parent with pathname sockets.
+ */
+TEST_F(scoped_domains, pathname_connect_to_parent)
+{
+ test_connect_to_parent(_metadata, self, variant, false);
+}
+
+/*
+ * Test unix_stream_connect() and unix_may_send() for a parent connecting to
+ * its child with pathname sockets.
+ */
+TEST_F(scoped_domains, pathname_connect_to_child)
+{
+ test_connect_to_child(_metadata, self, variant, false);
+}
+
FIXTURE(scoped_audit)
{
- struct service_fixture dgram_address;
+ struct service_fixture dgram_address_abstract, dgram_address_pathname;
struct audit_filter audit_filter;
int audit_fd;
};
+FIXTURE_VARIANT(scoped_audit)
+{
+ const bool abstract_socket;
+};
+
+// clang-format off
+FIXTURE_VARIANT_ADD(scoped_audit, abstract_socket)
+{
+ // clang-format on
+ .abstract_socket = true,
+};
+
+// clang-format off
+FIXTURE_VARIANT_ADD(scoped_audit, pathname_socket)
+{
+ // clang-format on
+ .abstract_socket = false,
+};
+
FIXTURE_SETUP(scoped_audit)
{
disable_caps(_metadata);
- memset(&self->dgram_address, 0, sizeof(self->dgram_address));
- set_unix_address(&self->dgram_address, 1, true);
+ ASSERT_EQ(0, mkdir(PATHNAME_UNIX_SOCK_DIR, 0700));
+ memset(&self->dgram_address_abstract, 0,
+ sizeof(self->dgram_address_abstract));
+ memset(&self->dgram_address_pathname, 0,
+ sizeof(self->dgram_address_pathname));
+ set_unix_address(&self->dgram_address_abstract, 1, true);
+ set_unix_address(&self->dgram_address_pathname, 1, false);
set_cap(_metadata, CAP_AUDIT_CONTROL);
self->audit_fd = audit_init_with_exe_filter(&self->audit_filter);
@@ -291,6 +394,8 @@ FIXTURE_SETUP(scoped_audit)
FIXTURE_TEARDOWN_PARENT(scoped_audit)
{
EXPECT_EQ(0, audit_cleanup(-1, NULL));
+ EXPECT_EQ(0, remove_path(self->dgram_address_pathname.unix_addr.sun_path));
+ EXPECT_EQ(0, rmdir(PATHNAME_UNIX_SOCK_DIR));
}
/* python -c 'print(b"\0selftests-landlock-abstract-unix-".hex().upper())' */
@@ -308,6 +413,12 @@ TEST_F(scoped_audit, connect_to_child)
char buf;
int dgram_client;
struct audit_records records;
+ struct service_fixture *const dgram_address =
+ variant->abstract_socket ? &self->dgram_address_abstract :
+ &self->dgram_address_pathname;
+ size_t log_match_remaining = 500;
+ char log_match[log_match_remaining];
+ char *log_match_cursor = log_match;
/* Makes sure there is no superfluous logged records. */
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
@@ -330,8 +441,8 @@ TEST_F(scoped_audit, connect_to_child)
dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_server);
- ASSERT_EQ(0, bind(dgram_server, &self->dgram_address.unix_addr,
- self->dgram_address.unix_addr_len));
+ ASSERT_EQ(0, bind(dgram_server, &dgram_address->unix_addr,
+ dgram_address->unix_addr_len));
/* Signals to the parent that child is listening. */
ASSERT_EQ(1, write(pipe_child[1], ".", 1));
@@ -345,7 +456,9 @@ TEST_F(scoped_audit, connect_to_child)
EXPECT_EQ(0, close(pipe_child[1]));
EXPECT_EQ(0, close(pipe_parent[0]));
- create_scoped_domain(_metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ create_scoped_domain(_metadata,
+ LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET);
/* Signals that the parent is in a domain, if any. */
ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
@@ -355,19 +468,62 @@ TEST_F(scoped_audit, connect_to_child)
/* Waits for the child to listen */
ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
- err_dgram = connect(dgram_client, &self->dgram_address.unix_addr,
- self->dgram_address.unix_addr_len);
+ err_dgram = connect(dgram_client, &dgram_address->unix_addr,
+ dgram_address->unix_addr_len);
EXPECT_EQ(-1, err_dgram);
EXPECT_EQ(EPERM, errno);
- EXPECT_EQ(
- 0,
- audit_match_record(
- self->audit_fd, AUDIT_LANDLOCK_ACCESS,
+ if (variant->abstract_socket) {
+ log_match_cursor = stpncpy(
+ log_match,
REGEX_LANDLOCK_PREFIX
" blockers=scope\\.abstract_unix_socket path=" ABSTRACT_SOCKET_PATH_PREFIX
"[0-9A-F]\\+$",
- NULL));
+ log_match_remaining);
+ log_match_remaining =
+ sizeof(log_match) - (log_match_cursor - log_match);
+ ASSERT_NE(0, log_match_remaining);
+ } else {
+ /*
+ * It is assumed that absolute_path does not contain control
+ * characters nor spaces, see audit_string_contains_control().
+ */
+ char *absolute_path =
+ realpath(dgram_address->unix_addr.sun_path, NULL);
+
+ EXPECT_NE(NULL, absolute_path)
+ {
+ TH_LOG("realpath() failed: %s", strerror(errno));
+ return;
+ }
+
+ log_match_cursor =
+ stpncpy(log_match,
+ REGEX_LANDLOCK_PREFIX
+ " blockers=scope\\.pathname_unix_socket path=\"",
+ log_match_remaining);
+ log_match_remaining =
+ sizeof(log_match) - (log_match_cursor - log_match);
+ ASSERT_NE(0, log_match_remaining);
+ log_match_cursor = regex_escape(absolute_path, log_match_cursor,
+ log_match_remaining);
+ free(absolute_path);
+ if (log_match_cursor < 0) {
+ TH_LOG("regex_escape() failed (buffer too small)");
+ return;
+ }
+ log_match_remaining =
+ sizeof(log_match) - (log_match_cursor - log_match);
+ ASSERT_NE(0, log_match_remaining);
+ log_match_cursor =
+ stpncpy(log_match_cursor, "\"$", log_match_remaining);
+ log_match_remaining =
+ sizeof(log_match) - (log_match_cursor - log_match);
+ ASSERT_NE(0, log_match_remaining);
+ }
+
+ EXPECT_EQ(0, audit_match_record(self->audit_fd, AUDIT_LANDLOCK_ACCESS,
+ log_match, NULL));
ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
EXPECT_EQ(0, close(dgram_client));
--
2.52.0
^ permalink raw reply related
* [PATCH v2 4/6] selftests/landlock: Support pathname socket path in set_unix_address
From: Tingmao Wang @ 2025-12-30 17:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <cover.1767115163.git.m@maowtm.org>
To prepare for extending the socket tests to do non-abstract sockets too,
extend set_unix_address() to also be able to populate a non-abstract
socket path under TMP_DIR. Also use snprintf for good measure.
This also changes existing callers to pass true for the abstract argument.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
tools/testing/selftests/landlock/common.h | 33 +++++++++++++++----
tools/testing/selftests/landlock/net_test.c | 2 +-
.../landlock/scoped_abstract_unix_test.c | 30 ++++++++---------
.../selftests/landlock/scoped_signal_test.c | 2 +-
4 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
index 90551650299c..c55c11434e27 100644
--- a/tools/testing/selftests/landlock/common.h
+++ b/tools/testing/selftests/landlock/common.h
@@ -241,13 +241,34 @@ struct service_fixture {
};
};
+#define PATHNAME_UNIX_SOCK_DIR TMP_DIR
+
+/**
+ * set_unix_address - Set up srv->unix_addr and srv->unix_addr_len.
+ * @srv: Service fixture containing the socket address to initialize
+ * @index: Index to include in socket names
+ * @abstract: If true, creates an abstract socket address (sun_path[0] ==
+ * '\0') with the given name. If false, creates a pathname socket
+ * address with the given path.
+ */
static void __maybe_unused set_unix_address(struct service_fixture *const srv,
- const unsigned short index)
+ const unsigned short index,
+ const bool abstract)
{
srv->unix_addr.sun_family = AF_UNIX;
- sprintf(srv->unix_addr.sun_path,
- "_selftests-landlock-abstract-unix-tid%d-index%d", sys_gettid(),
- index);
- srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
- srv->unix_addr.sun_path[0] = '\0';
+ if (abstract) {
+ snprintf(srv->unix_addr.sun_path,
+ sizeof(srv->unix_addr.sun_path),
+ "_selftests-landlock-abstract-unix-tid%d-index%d",
+ sys_gettid(), index);
+ srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
+ srv->unix_addr.sun_path[0] = '\0';
+ } else {
+ snprintf(srv->unix_addr.sun_path,
+ sizeof(srv->unix_addr.sun_path),
+ PATHNAME_UNIX_SOCK_DIR
+ "/pathname-unix-tid%d-index%d.sock",
+ sys_gettid(), index);
+ srv->unix_addr_len = sizeof(srv->unix_addr);
+ }
}
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index b34b139b3f89..fd3fe51ce92f 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -69,7 +69,7 @@ static int set_service(struct service_fixture *const srv,
return 0;
case AF_UNIX:
- set_unix_address(srv, index);
+ set_unix_address(srv, index, true);
return 0;
}
return 1;
diff --git a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
index 72f97648d4a7..4a790e2d387d 100644
--- a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
+++ b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
@@ -58,8 +58,8 @@ FIXTURE_SETUP(scoped_domains)
memset(&self->stream_address, 0, sizeof(self->stream_address));
memset(&self->dgram_address, 0, sizeof(self->dgram_address));
- set_unix_address(&self->stream_address, 0);
- set_unix_address(&self->dgram_address, 1);
+ set_unix_address(&self->stream_address, 0, true);
+ set_unix_address(&self->dgram_address, 1, true);
}
FIXTURE_TEARDOWN(scoped_domains)
@@ -280,7 +280,7 @@ FIXTURE_SETUP(scoped_audit)
disable_caps(_metadata);
memset(&self->dgram_address, 0, sizeof(self->dgram_address));
- set_unix_address(&self->dgram_address, 1);
+ set_unix_address(&self->dgram_address, 1, true);
set_cap(_metadata, CAP_AUDIT_CONTROL);
self->audit_fd = audit_init_with_exe_filter(&self->audit_filter);
@@ -392,16 +392,16 @@ FIXTURE_SETUP(scoped_vs_unscoped)
memset(&self->parent_stream_address, 0,
sizeof(self->parent_stream_address));
- set_unix_address(&self->parent_stream_address, 0);
+ set_unix_address(&self->parent_stream_address, 0, true);
memset(&self->parent_dgram_address, 0,
sizeof(self->parent_dgram_address));
- set_unix_address(&self->parent_dgram_address, 1);
+ set_unix_address(&self->parent_dgram_address, 1, true);
memset(&self->child_stream_address, 0,
sizeof(self->child_stream_address));
- set_unix_address(&self->child_stream_address, 2);
+ set_unix_address(&self->child_stream_address, 2, true);
memset(&self->child_dgram_address, 0,
sizeof(self->child_dgram_address));
- set_unix_address(&self->child_dgram_address, 3);
+ set_unix_address(&self->child_dgram_address, 3, true);
}
FIXTURE_TEARDOWN(scoped_vs_unscoped)
@@ -622,9 +622,9 @@ FIXTURE_SETUP(outside_socket)
drop_caps(_metadata);
memset(&self->transit_address, 0, sizeof(self->transit_address));
- set_unix_address(&self->transit_address, 0);
+ set_unix_address(&self->transit_address, 0, true);
memset(&self->address, 0, sizeof(self->address));
- set_unix_address(&self->address, 1);
+ set_unix_address(&self->address, 1, true);
}
FIXTURE_TEARDOWN(outside_socket)
@@ -802,9 +802,9 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
/* Abstract address. */
memset(&stream_abstract_addr, 0, sizeof(stream_abstract_addr));
- set_unix_address(&stream_abstract_addr, 0);
+ set_unix_address(&stream_abstract_addr, 0, true);
memset(&dgram_abstract_addr, 0, sizeof(dgram_abstract_addr));
- set_unix_address(&dgram_abstract_addr, 1);
+ set_unix_address(&dgram_abstract_addr, 1, true);
/* Unnamed address for datagram socket. */
ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_DGRAM, 0, unnamed_sockets));
@@ -990,9 +990,9 @@ TEST(datagram_sockets)
drop_caps(_metadata);
memset(&connected_addr, 0, sizeof(connected_addr));
- set_unix_address(&connected_addr, 0);
+ set_unix_address(&connected_addr, 0, true);
memset(&non_connected_addr, 0, sizeof(non_connected_addr));
- set_unix_address(&non_connected_addr, 1);
+ set_unix_address(&non_connected_addr, 1, true);
ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
@@ -1090,9 +1090,9 @@ TEST(self_connect)
drop_caps(_metadata);
memset(&connected_addr, 0, sizeof(connected_addr));
- set_unix_address(&connected_addr, 0);
+ set_unix_address(&connected_addr, 0, true);
memset(&non_connected_addr, 0, sizeof(non_connected_addr));
- set_unix_address(&non_connected_addr, 1);
+ set_unix_address(&non_connected_addr, 1, true);
connected_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
non_connected_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
index d8bf33417619..8d1e1dc89c43 100644
--- a/tools/testing/selftests/landlock/scoped_signal_test.c
+++ b/tools/testing/selftests/landlock/scoped_signal_test.c
@@ -463,7 +463,7 @@ TEST_F(fown, sigurg_socket)
pid_t child;
memset(&server_address, 0, sizeof(server_address));
- set_unix_address(&server_address, 0);
+ set_unix_address(&server_address, 0, true);
ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
--
2.52.0
^ permalink raw reply related
* [PATCH v2 3/6] samples/landlock: Support LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Tingmao Wang @ 2025-12-30 17:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <cover.1767115163.git.m@maowtm.org>
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
I've decided to use "u" as the character to control this scope bit since
it stands for (normal) Unix sockets. Imo using "p" or "n" would make it less
clear / memorable. Open to suggestions.
Also, open to suggestion whether socket scoping (pathname and abstract)
should be enabled by default, if LL_SCOPED is not set. This would break
backward compatibility, but maybe we shouldn't guarentee backward
compatibility of this sandboxer in the first place, and almost all cases
of Landlock usage would want socket scoping.
samples/landlock/sandboxer.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208..2de14e1c787d 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -234,14 +234,16 @@ static bool check_ruleset_scope(const char *const env_var,
bool error = false;
bool abstract_scoping = false;
bool signal_scoping = false;
+ bool named_scoping = false;
/* Scoping is not supported by Landlock ABI */
if (!(ruleset_attr->scoped &
- (LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET | LANDLOCK_SCOPE_SIGNAL)))
+ (LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET | LANDLOCK_SCOPE_SIGNAL |
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET)))
goto out_unset;
env_type_scope = getenv(env_var);
- /* Scoping is not supported by the user */
+ /* Scoping is not requested by the user */
if (!env_type_scope || strcmp("", env_type_scope) == 0)
goto out_unset;
@@ -254,6 +256,9 @@ static bool check_ruleset_scope(const char *const env_var,
} else if (strcmp("s", ipc_scoping_name) == 0 &&
!signal_scoping) {
signal_scoping = true;
+ } else if (strcmp("u", ipc_scoping_name) == 0 &&
+ !named_scoping) {
+ named_scoping = true;
} else {
fprintf(stderr, "Unknown or duplicate scope \"%s\"\n",
ipc_scoping_name);
@@ -270,6 +275,8 @@ static bool check_ruleset_scope(const char *const env_var,
ruleset_attr->scoped &= ~LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
if (!signal_scoping)
ruleset_attr->scoped &= ~LANDLOCK_SCOPE_SIGNAL;
+ if (!named_scoping)
+ ruleset_attr->scoped &= ~LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
unsetenv(env_var);
return error;
@@ -299,7 +306,7 @@ static bool check_ruleset_scope(const char *const env_var,
/* clang-format on */
-#define LANDLOCK_ABI_LAST 7
+#define LANDLOCK_ABI_LAST 8
#define XSTR(s) #s
#define STR(s) XSTR(s)
@@ -325,6 +332,7 @@ static const char help[] =
"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
" - \"a\" to restrict opening abstract unix sockets\n"
" - \"s\" to restrict sending signals\n"
+ " - \"u\" to restrict opening pathname (non-abstract) unix sockets\n"
"\n"
"A sandboxer should not log denied access requests to avoid spamming logs, "
"but to test audit we can set " ENV_FORCE_LOG_NAME "=1\n"
@@ -334,7 +342,7 @@ static const char help[] =
ENV_FS_RW_NAME "=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
ENV_TCP_BIND_NAME "=\"9418\" "
ENV_TCP_CONNECT_NAME "=\"80:443\" "
- ENV_SCOPED_NAME "=\"a:s\" "
+ ENV_SCOPED_NAME "=\"a:s:u\" "
"%1$s bash -i\n"
"\n"
"This sandboxer can use Landlock features up to ABI version "
@@ -356,7 +364,8 @@ int main(const int argc, char *const argv[], char *const *const envp)
.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
.scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
- LANDLOCK_SCOPE_SIGNAL,
+ LANDLOCK_SCOPE_SIGNAL |
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
};
int supported_restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
int set_restrict_flags = 0;
@@ -436,6 +445,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
/* Removes LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON for ABI < 7 */
supported_restrict_flags &=
~LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
+ __attribute__((fallthrough));
+ case 7:
+ /* Removes LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET for ABI < 8 */
+ ruleset_attr.scoped &= ~LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
/* Must be printed for any ABI < LANDLOCK_ABI_LAST. */
fprintf(stderr,
--
2.52.0
^ permalink raw reply related
* [PATCH v2 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Tingmao Wang @ 2025-12-30 17:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <cover.1767115163.git.m@maowtm.org>
Extend the existing abstract UNIX socket scoping to pathname sockets as
well. Basically all of the logic is reused between the two types, just
that pathname sockets scoping are controlled by another bit, and has its
own audit request type (since the current one is named
"abstract_unix_socket").
Closes: https://github.com/landlock-lsm/linux/issues/51
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes in v2:
- Factor out common code in hook_unix_stream_connect and
hook_unix_may_send into check_socket_access(), and inline
is_abstract_socket().
security/landlock/audit.c | 4 ++
security/landlock/audit.h | 1 +
security/landlock/task.c | 109 ++++++++++++++++++++++----------------
3 files changed, 67 insertions(+), 47 deletions(-)
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index e899995f1fd5..0626cc553ab0 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -75,6 +75,10 @@ get_blocker(const enum landlock_request_type type,
WARN_ON_ONCE(access_bit != -1);
return "scope.abstract_unix_socket";
+ case LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET:
+ WARN_ON_ONCE(access_bit != -1);
+ return "scope.pathname_unix_socket";
+
case LANDLOCK_REQUEST_SCOPE_SIGNAL:
WARN_ON_ONCE(access_bit != -1);
return "scope.signal";
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d8..1c9ce8588102 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -21,6 +21,7 @@ enum landlock_request_type {
LANDLOCK_REQUEST_NET_ACCESS,
LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
LANDLOCK_REQUEST_SCOPE_SIGNAL,
+ LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET,
};
/*
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 833bc0cfe5c9..10dc356baf6f 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -232,35 +232,81 @@ static bool domain_is_scoped(const struct landlock_ruleset *const client,
return false;
}
+/**
+ * sock_is_scoped - Check if socket connect or send should be restricted
+ * based on scope controls.
+ *
+ * @other: The server socket.
+ * @domain: The client domain.
+ * @scope: The relevant scope bit to check (i.e. pathname or abstract).
+ *
+ * Returns: True if connect should be restricted, false otherwise.
+ */
static bool sock_is_scoped(struct sock *const other,
- const struct landlock_ruleset *const domain)
+ const struct landlock_ruleset *const domain,
+ access_mask_t scope)
{
const struct landlock_ruleset *dom_other;
/* The credentials will not change. */
lockdep_assert_held(&unix_sk(other)->lock);
dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
- return domain_is_scoped(domain, dom_other,
- LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ return domain_is_scoped(domain, dom_other, scope);
}
-static bool is_abstract_socket(struct sock *const sock)
+/* Allow us to quickly test if the current domain scopes any form of socket */
+static const struct access_masks unix_scope = {
+ .scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
+};
+
+/*
+ * UNIX sockets can have three types of addresses: pathname (a filesystem path),
+ * unnamed (not bound to an address), and abstract (sun_path[0] is '\0').
+ * Unnamed sockets include those created with socketpair() and unbound sockets.
+ * We do not restrict unnamed sockets since they have no address to identify.
+ */
+static int
+check_socket_access(struct sock *const other,
+ const struct landlock_cred_security *const subject,
+ const size_t handle_layer)
{
- struct unix_address *addr = unix_sk(sock)->addr;
+ const struct unix_address *addr = unix_sk(other)->addr;
+ access_mask_t scope;
+ enum landlock_request_type request_type;
+ /* Unnamed sockets are not restricted. */
if (!addr)
- return false;
+ return 0;
+ /*
+ * Abstract and pathname Unix sockets have separate scope and audit
+ * request type.
+ */
if (addr->len >= offsetof(struct sockaddr_un, sun_path) + 1 &&
- addr->name->sun_path[0] == '\0')
- return true;
+ addr->name->sun_path[0] == '\0') {
+ scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
+ request_type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET;
+ } else {
+ scope = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
+ request_type = LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET;
+ }
- return false;
-}
+ if (!sock_is_scoped(other, subject->domain, scope))
+ return 0;
-static const struct access_masks unix_scope = {
- .scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET,
-};
+ landlock_log_denial(subject, &(struct landlock_request) {
+ .type = request_type,
+ .audit = {
+ .type = LSM_AUDIT_DATA_NET,
+ .u.net = &(struct lsm_network_audit) {
+ .sk = other,
+ },
+ },
+ .layer_plus_one = handle_layer + 1,
+ });
+ return -EPERM;
+}
static int hook_unix_stream_connect(struct sock *const sock,
struct sock *const other,
@@ -275,23 +321,7 @@ static int hook_unix_stream_connect(struct sock *const sock,
if (!subject)
return 0;
- if (!is_abstract_socket(other))
- return 0;
-
- if (!sock_is_scoped(other, subject->domain))
- return 0;
-
- landlock_log_denial(subject, &(struct landlock_request) {
- .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
- .audit = {
- .type = LSM_AUDIT_DATA_NET,
- .u.net = &(struct lsm_network_audit) {
- .sk = other,
- },
- },
- .layer_plus_one = handle_layer + 1,
- });
- return -EPERM;
+ return check_socket_access(other, subject, handle_layer);
}
static int hook_unix_may_send(struct socket *const sock,
@@ -302,6 +332,7 @@ static int hook_unix_may_send(struct socket *const sock,
landlock_get_applicable_subject(current_cred(), unix_scope,
&handle_layer);
+ /* Quick return for non-landlocked tasks. */
if (!subject)
return 0;
@@ -312,23 +343,7 @@ static int hook_unix_may_send(struct socket *const sock,
if (unix_peer(sock->sk) == other->sk)
return 0;
- if (!is_abstract_socket(other->sk))
- return 0;
-
- if (!sock_is_scoped(other->sk, subject->domain))
- return 0;
-
- landlock_log_denial(subject, &(struct landlock_request) {
- .type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
- .audit = {
- .type = LSM_AUDIT_DATA_NET,
- .u.net = &(struct lsm_network_audit) {
- .sk = other->sk,
- },
- },
- .layer_plus_one = handle_layer + 1,
- });
- return -EPERM;
+ return check_socket_access(other->sk, subject, handle_layer);
}
static const struct access_masks signal_scope = {
--
2.52.0
^ permalink raw reply related
* [PATCH v2 1/6] landlock: Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET scope bit to uAPI
From: Tingmao Wang @ 2025-12-30 17:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <cover.1767115163.git.m@maowtm.org>
Add the new scope bit to the uAPI header, add documentation, and bump ABI
version to 8.
This documentation edit specifically calls out the security implications of
not restricting sockets.
Fix some minor cosmetic issue in landlock.h around the changed lines as
well.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes in v2:
- Fix grammar
Note that in the code block in "Defining and enforcing a security policy"
the switch case currently jumps from 5 to 7. This should be fixed by
https://lore.kernel.org/all/20251216210248.4150777-1-samasth.norway.ananda@oracle.com/
Documentation/userspace-api/landlock.rst | 37 ++++++++++++++++---
include/uapi/linux/landlock.h | 8 +++-
security/landlock/limits.h | 2 +-
security/landlock/syscalls.c | 2 +-
tools/testing/selftests/landlock/base_test.c | 2 +-
.../testing/selftests/landlock/scoped_test.c | 2 +-
6 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 1d0c2c15c22e..5620a2be1091 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -83,7 +83,8 @@ to be explicit about the denied-by-default access rights.
LANDLOCK_ACCESS_NET_CONNECT_TCP,
.scoped =
LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
- LANDLOCK_SCOPE_SIGNAL,
+ LANDLOCK_SCOPE_SIGNAL |
+ LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET,
};
Because we may not know which kernel version an application will be executed
@@ -127,6 +128,10 @@ version, and only use the available subset of access rights:
/* Removes LANDLOCK_SCOPE_* for ABI < 6 */
ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
LANDLOCK_SCOPE_SIGNAL);
+ __attribute__((fallthrough));
+ case 7:
+ /* Removes LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET for ABI < 8 */
+ ruleset_attr.scoped &= ~LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
}
This enables the creation of an inclusive ruleset that will contain our rules.
@@ -328,10 +333,15 @@ The operations which can be scoped are:
This limits the sending of signals to target processes which run within the
same or a nested Landlock domain.
-``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET``
- This limits the set of abstract :manpage:`unix(7)` sockets to which we can
- :manpage:`connect(2)` to socket addresses which were created by a process in
- the same or a nested Landlock domain.
+``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET`` and ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET``
+ This limits the set of :manpage:`unix(7)` sockets to which we can
+ :manpage:`connect(2)` to socket addresses which were created by a
+ process in the same or a nested Landlock domain.
+ ``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET`` applies to abstract sockets,
+ and ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` applies to pathname
+ sockets. Even though pathname sockets are represented in the
+ filesystem, Landlock filesystem rules do not currently control access
+ to them.
A :manpage:`sendto(2)` on a non-connected datagram socket is treated as if
it were doing an implicit :manpage:`connect(2)` and will be blocked if the
@@ -604,6 +614,23 @@ Landlock audit events with the ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``,
sys_landlock_restrict_self(). See Documentation/admin-guide/LSM/landlock.rst
for more details on audit.
+Pathname UNIX socket (ABI < 8)
+------------------------------
+
+Starting with the Landlock ABI version 8, it is possible to restrict
+connections to a pathname (non-abstract) :manpage:`unix(7)` socket by
+setting ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` to the ``scoped`` ruleset
+attribute. This works the same way as the abstract socket scoping.
+
+This allows sandboxing applications using only Landlock to protect against
+bypasses relying on connecting to Unix sockets of other services running
+under the same user. These services typically assume that any process
+capable of connecting to a local Unix socket, or connecting with the
+expected user credentials, is trusted. Without this protection, sandbox
+escapes may be possible, especially when running in a standard desktop
+environment, such as by using systemd-run, or sockets exposed by other
+common applications.
+
.. _kernel_support:
Kernel support
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f030adc462ee..590c6d4171a0 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -364,10 +364,14 @@ struct landlock_net_port_attr {
* related Landlock domain (e.g., a parent domain or a non-sandboxed process).
* - %LANDLOCK_SCOPE_SIGNAL: Restrict a sandboxed process from sending a signal
* to another process outside the domain.
+ * - %LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET: Restrict a sandboxed process from
+ * connecting to a pathname UNIX socket created by a process outside the
+ * related Landlock domain.
*/
/* clang-format off */
#define LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET (1ULL << 0)
-#define LANDLOCK_SCOPE_SIGNAL (1ULL << 1)
-/* clang-format on*/
+#define LANDLOCK_SCOPE_SIGNAL (1ULL << 1)
+#define LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET (1ULL << 2)
+/* clang-format on */
#endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 65b5ff051674..d653e14dba10 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -27,7 +27,7 @@
#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
#define LANDLOCK_NUM_ACCESS_NET __const_hweight64(LANDLOCK_MASK_ACCESS_NET)
-#define LANDLOCK_LAST_SCOPE LANDLOCK_SCOPE_SIGNAL
+#define LANDLOCK_LAST_SCOPE LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
#define LANDLOCK_MASK_SCOPE ((LANDLOCK_LAST_SCOPE << 1) - 1)
#define LANDLOCK_NUM_SCOPE __const_hweight64(LANDLOCK_MASK_SCOPE)
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 0116e9f93ffe..66fd196be85a 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -161,7 +161,7 @@ static const struct file_operations ruleset_fops = {
* Documentation/userspace-api/landlock.rst should be updated to reflect the
* UAPI change.
*/
-const int landlock_abi_version = 7;
+const int landlock_abi_version = 8;
/**
* sys_landlock_create_ruleset - Create a new ruleset
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 7b69002239d7..f4b1a275d8d9 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -76,7 +76,7 @@ TEST(abi_version)
const struct landlock_ruleset_attr ruleset_attr = {
.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
};
- ASSERT_EQ(7, landlock_create_ruleset(NULL, 0,
+ ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
LANDLOCK_CREATE_RULESET_VERSION));
ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/scoped_test.c b/tools/testing/selftests/landlock/scoped_test.c
index b90f76ed0d9c..7f83512a328d 100644
--- a/tools/testing/selftests/landlock/scoped_test.c
+++ b/tools/testing/selftests/landlock/scoped_test.c
@@ -12,7 +12,7 @@
#include "common.h"
-#define ACCESS_LAST LANDLOCK_SCOPE_SIGNAL
+#define ACCESS_LAST LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
TEST(ruleset_with_unknown_scope)
{
--
2.52.0
^ permalink raw reply related
* [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Tingmao Wang @ 2025-12-30 17:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Demi Marie Obenour, Alyssa Ross,
Jann Horn, Tahera Fahimi, Justin Suess, linux-security-module
Changes in v2:
Fix grammar in doc, rebased on mic/next, and extracted common code from
hook_unix_stream_connect and hook_unix_may_send into a separate
function.
The rest is the same as the v1 cover letter:
This patch series extend the existing abstract Unix socket scoping to
pathname (i.e. normal file-based) sockets as well, by adding a new scope
bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET that works the same as
LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET, except that restricts pathname Unix
sockets. This means that a sandboxed process with this scope enabled will
not be able to connect to Unix sockets created outside the sandbox via the
filesystem.
There is a future plan [1] for allowing specific sockets based on FS
hierarchy, but this series is only determining access based on domain
parent-child relationship. There is currently no way to allow specific
(outside the Landlock domain) Unix sockets, and none of the existing
Landlock filesystem controls apply to socket connect().
With this series, we can now properly protect against things like the the
following while only relying on Landlock:
(running under tmux)
root@6-19-0-rc1-dev-00023-g68f0b276cbeb ~# LL_FS_RO=/ LL_FS_RW= ./sandboxer bash
Executing the sandboxed command...
root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
cat: /tmp/hi: No such file or directory
root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# tmux new-window 'echo hi > /tmp/hi'
root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
hi
The above but with Unix socket scoping enabled (both pathname and abstract
sockets) - the sandboxed shell can now no longer talk to tmux due to the
socket being created from outside the Landlock sandbox:
(running under tmux)
root@6-19-0-rc1-dev-00023-g68f0b276cbeb ~# LL_FS_RO=/ LL_FS_RW= LL_SCOPED=u:a ./sandboxer bash
Executing the sandboxed command...
root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
cat: /tmp/hi: No such file or directory
root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# tmux new-window 'echo hi > /tmp/hi'
error connecting to /tmp/tmux-0/default (Operation not permitted)
root@6-19-0-rc1-dev-00023-g68f0b276cbeb:/# cat /tmp/hi
cat: /tmp/hi: No such file or directory
Tmux is just one example. In a standard systemd session, `systemd-run
--user` can also be used (--user will run the command in the user's
session, without requiring any root privileges), and likely a lot more if
running in a desktop environment with many popular applications. This
change therefore makes it possible to create sandboxes without relying on
additional mechanisms like seccomp to protect against such issues.
These kind of issues was originally discussed on here (I took the idea for
systemd-run from Demi):
https://spectrum-os.org/lists/archives/spectrum-devel/00256266-26db-40cf-8f5b-f7c7064084c2@gmail.com/
Demo with socat + sandboxer:
Outside:
socat unix-listen:/foo.sock,fork -
Sandbox with pathname socket scope bit:
root@6-19-0-rc1-dev-00023-g0994a10d6512 ~# LL_FS_RW=/ LL_FS_RO= LL_SCOPED=u /sandboxer socat -d2 unix:/foo.sock -
Executing the sandboxed command...
2025/12/27 20:28:54 socat[1227] E UNIX-CLIENT: /foo.sock: Operation not permitted
2025/12/27 20:28:54 socat[1227] N exit(1)
Sandbox without pathname socket scope bit:
root@6-19-0-rc1-dev-00023-g0994a10d6512 ~# LL_FS_RW=/ LL_FS_RO= LL_SCOPED= /sandboxer socat -d2 unix:/foo.sock -
Executing the sandboxed command...
2025/12/27 20:29:22 socat[1250] N successfully connected from local address AF=1 "(7\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xB0\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xAE\xC3\xAE\xAE\xAE\xAE"
...
Sandbox with only abstract socket scope bit:
root@6-19-0-rc1-dev-00023-g0994a10d6512 ~# LL_FS_RW=/ LL_FS_RO= LL_SCOPED=a /sandboxer socat -d2 unix:/foo.sock -
Executing the sandboxed command...
2025/12/27 20:29:26 socat[1259] N successfully connected from local address AF=1 "\0\0\0\0\0\0\0\0\0"
...
Sendmsg/recvmsg - outside:
socat unix-recvfrom:/datagram.sock -
Sandbox with pathname socket scope bit:
root@6-19-0-rc1-dev-00023-g0994a10d6512 ~# LL_FS_RW=/ LL_FS_RO= LL_SCOPED=u /sandboxer socat -d2 unix-sendto:/datagram.sock -
Executing the sandboxed command...
...
2025/12/27 20:33:04 socat[1446] N starting data transfer loop with FDs [5,5] and [0,1]
123
2025/12/27 20:33:05 socat[1446] E sendto(5, 0x55d260d8f000, 4, 0, AF=1 "/datagram.sock", 16): Operation not permitted
2025/12/27 20:33:05 socat[1446] N exit(1)
[1]: https://github.com/landlock-lsm/linux/issues/36
Closes: https://github.com/landlock-lsm/linux/issues/51
Tingmao Wang (6):
landlock: Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET scope bit to uAPI
landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
samples/landlock: Support LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
selftests/landlock: Support pathname socket path in set_unix_address
selftests/landlock: Repurpose scoped_abstract_unix_test.c for pathname
sockets too.
selftests/landlock: Add pathname socket variants for more tests
Documentation/userspace-api/landlock.rst | 37 +-
include/uapi/linux/landlock.h | 8 +-
samples/landlock/sandboxer.c | 23 +-
security/landlock/audit.c | 4 +
security/landlock/audit.h | 1 +
security/landlock/limits.h | 2 +-
security/landlock/syscalls.c | 2 +-
security/landlock/task.c | 109 ++-
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/common.h | 33 +-
tools/testing/selftests/landlock/net_test.c | 2 +-
.../selftests/landlock/scoped_signal_test.c | 2 +-
.../testing/selftests/landlock/scoped_test.c | 2 +-
...bstract_unix_test.c => scoped_unix_test.c} | 855 ++++++++++++------
14 files changed, 757 insertions(+), 325 deletions(-)
rename tools/testing/selftests/landlock/{scoped_abstract_unix_test.c => scoped_unix_test.c} (51%)
base-commit: ef4536f15224418b327a7b5d5cae07dab042760f
--
2.52.0
^ permalink raw reply
* Re: [PATCH 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Tingmao Wang @ 2025-12-30 15:56 UTC (permalink / raw)
To: Justin Suess, Mickaël Salaün
Cc: demiobenour, fahimitahera, gnoack, hi, jannh,
linux-security-module
In-Reply-To: <09f721d0-2644-44ba-b2e8-93626a17fa55@maowtm.org>
On 12/30/25 15:52, Tingmao Wang wrote:
> On 12/28/25 16:37, Justin Suess wrote:
>> On 12/28/25 07:45, Tingmao Wang wrote:
>>> [...]
>>> diff --git a/security/landlock/task.c b/security/landlock/task.c
>>> index 6dfcc1860d6e..9fbb0ada440b 100644
>>> --- a/security/landlock/task.c
>>> +++ b/security/landlock/task.c
>>> @@ -233,57 +233,84 @@ static bool domain_is_scoped(const struct landlock_ruleset *const client,
>>> return false;
>>> }
>>>
>>> +/**
>>> + * sock_is_scoped - Check if socket connect or send should be restricted
>>> + * based on scope controls.
>>> + *
>>> + * @other: The server socket.
>>> + * @domain: The client domain.
>>> + * @scope: The relevant scope bit to check (i.e. pathname or abstract).
>>> + *
>>> + * Returns: True if connect should be restricted, false otherwise.
>>> + */
>>> static bool sock_is_scoped(struct sock *const other,
>>> - const struct landlock_ruleset *const domain)
>>> + const struct landlock_ruleset *const domain,
>>> + access_mask_t scope)
>>> {
>>> const struct landlock_ruleset *dom_other;
>>>
>>> /* The credentials will not change. */
>>> lockdep_assert_held(&unix_sk(other)->lock);
>>> dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
>>> - return domain_is_scoped(domain, dom_other,
>>> - LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
>>> + return domain_is_scoped(domain, dom_other, scope);
>>> }
>>>
>>> -static bool is_abstract_socket(struct sock *const sock)
>>> +static bool sock_addr_is_abstract(const struct unix_address *const addr)
>>
>> Nit: From the name sock_addr_is_abstract, it's unclear without reading
>> the parameter that this function only works with unix sockets, when
>> socket is an overloaded term that can refer to other kinds of sockets
>> (e.g tcp/udp/raw).
>>
>> Maybe is_unix_sock_addr_abstract? or unix_sock_addr_is_abstract?
>
> I guess sock_addr_is_abstract is indeed a bit of a weird name, but it
> helps that this function is static to this file and also very short.
> Maybe is_unix_addr_abstract?
>
(nevermind I didn't read Mickaël's reply before sending this, if we can
get rid of this extra function then the name doesn't matter)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox