* Re: [RFC bpf-next] lsm: bpf: Remove lsm_prop_bpf
From: Paul Moore @ 2025-10-27 21:13 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-security-module, jmorris, serge, casey, kpsingh,
mattbobrowski, ast, daniel, andrii, john.johansen
In-Reply-To: <20251025001022.1707437-1-song@kernel.org>
On Fri, Oct 24, 2025 at 8:10 PM Song Liu <song@kernel.org> wrote:
>
> lsm_prop_bpf is not used in any code. Remove it.
>
> Signed-off-by: Song Liu <song@kernel.org>
>
> ---
>
> Or did I miss any user of it?
> ---
> include/linux/lsm/bpf.h | 16 ----------------
> include/linux/security.h | 2 --
> 2 files changed, 18 deletions(-)
> delete mode 100644 include/linux/lsm/bpf.h
You probably didn't miss any direct reference to lsm_prop_bpf, but the
data type you really should look for when deciding on this is
lsm_prop. There are a number of LSM hooks that operate on a lsm_prop
struct instead of secid tokens, and without a lsm_prop_bpf
struct/field in the lsm_prop struct a BPF LSM will be limited compared
to other LSMs. Perhaps that limitation is okay, but it is something
that should be discussed; I see you've added KP to the To/CC line, I
would want to see an ACK from him before I merge anything removing
lsm_prop_bpf.
I haven't checked to see if the LSM hooks associated with a lsm_prop
struct are currently allowed for a BPF LSM, but I would expect a patch
removing the lsm_prop_bpf struct/field to also disable those LSM hooks
for BPF LSM use.
--
paul-moore.com
^ permalink raw reply
* Re: [GIT PULL] Block fixes for 6.18-rc3
From: Paul Moore @ 2025-10-27 20:24 UTC (permalink / raw)
To: Linus Torvalds
Cc: Serge E. Hallyn, Jens Axboe, Serge Hallyn, Christian Brauner,
linux-block@vger.kernel.org, LSM List
In-Reply-To: <CAHk-=wjE5t6=eO90iXqEsw6yMGfE8Y6=THP0dqXUJHvNQ7=gMg@mail.gmail.com>
On Sun, Oct 26, 2025 at 6:57 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sun, 26 Oct 2025 at 14:10, Serge E. Hallyn <serge@hallyn.com> wrote:
> >
> > The keychains are all NULL and won't be allocated (by init) without
> > copying a new cred, right?
>
> Right. As mentioned, 'struct init_cred' really should be 'const' -
> it's not *technically* really constant, because the reference counting
> casts away the const, but refs are designed to be copy-on-write apart
> from the reference counting.
>
> So whenever you change it, that's when you are supposed to always copy
> things. So that prepare_kernel_cred() thing exists for a good reason.
>
> But the pattern here in nbd (and the other three usage cases I found)
> is really just "use the kernel creds as-is".
>
> They don't even need any reference counting as long as they can just
> rely on the cred staying around for the duration of the use - which
> obviously is the case for init_cred.
>
> > Now, in theory, some LSM *could* come by and try to merge
> > current's info with init's, but that would probably be misguided.
> >
> > So this does seem like it should work.
>
> Yeah, I can't see how any LSM could possibly do anything about
> init_cred - it really ends up being the source of all other creds. You
> can't really validly mess with it or deny it anything.
This came in just as I was logging off for the weekend and I've been
kicking it around in my head and I can't think of any *good* LSM
related reason why this should be a problem, however I do have a
somewhat generic concern about potential future issues caused by
someone choosing the wrong access pattern and causing an odd bug. In
theory, a const attribute should catch a lot of that before it starts,
but that assumes we don't have some casting somewhere doing odd
things.
If we care about this, and I'm not sure we do, or rather I'm not sure
how much I care about this, we could create a new cred instance, say
'kernel_cred', that is purely for things like nbd where no changes are
expected and it can be accessed directly. This would limit the
direct-access pattern to just kernel_cred, making code
inspection/review easier and leaving the door open for WARN_ON-esque
assertions in things like prepare_creds() and similar*.
* This reminds me that we need to talk some more with the keys folks
and see if we can get rid of the ugliness that is
key_change_session_keyring()/security_transfer_creds(). Jann had some
patches for that, but if I recall correctly there was a concern about
backwards compatibility.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] KEYS: fix compilation warnings in the dump_options() function
From: Jarkko Sakkinen @ 2025-10-27 20:04 UTC (permalink / raw)
To: Ye Bin
Cc: a.fatoum, kernel, James.Bottomley, zohar, dhowells, paul, jmorris,
serge, linux-integrity, keyrings, linux-security-module, yebin10
In-Reply-To: <20251024061153.61470-1-yebin@huaweicloud.com>
On Fri, Oct 24, 2025 at 02:11:53PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> There's issue as follows:
> security/keys/trusted-keys/trusted_caam.c: In function ‘dump_options’:
> security/keys/trusted-keys/trusted_caam.c:37:20: note: the ABI of passing struct with a flexible array member has changed in GCC 4.4
> 37 | static inline void dump_options(struct caam_pkey_info pkey_info)
> | ^~~~~~~~~~~~
>
> To solve the above problem, pass 'struct caam_pkey_info*' type parameter
> to the dump_options() function.
>
> Fixes: 9eb25ca6c973 ("KEYS: trusted: caam based protected key")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> security/keys/trusted-keys/trusted_caam.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> index 090099d1b04d..dd7a69bcf6a3 100644
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -29,12 +29,12 @@ static const match_table_t key_tokens = {
> };
>
> #ifdef CAAM_DEBUG
> -static inline void dump_options(struct caam_pkey_info pkey_info)
> +static inline void dump_options(struct caam_pkey_info *pkey_info)
> {
> - pr_info("key encryption algo %d\n", pkey_info.key_enc_algo);
> + pr_info("key encryption algo %d\n", pkey_info->key_enc_algo);
> }
> #else
> -static inline void dump_options(struct caam_pkey_info pkey_info)
> +static inline void dump_options(struct caam_pkey_info *pkey_info)
> {
> }
> #endif
Please fix the broken design while at it:
1. Remove the ad-hoc compilation flag (i.e., CAAM_DEBUG).
2. Substitute pr_info calls with pr_debug calls.
Then you can turn then on and off either dynamically, or alternatively
from the kernel command-line [1].
[1] https://www.kernel.org/doc/html/latest/admin-guide/dynamic-debug-howto.html
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 2/2] keys: Remove unnecessary local variable from proc_keys_show
From: Jarkko Sakkinen @ 2025-10-27 19:54 UTC (permalink / raw)
To: Thorsten Blum
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251023143231.2086-4-thorsten.blum@linux.dev>
On Thu, Oct 23, 2025 at 04:32:33PM +0200, Thorsten Blum wrote:
> The local variable 'rc' is only used to temporary store the result of
> calling key_task_permission(). Use the result directly and remove the
> local variable.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/proc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/security/keys/proc.c b/security/keys/proc.c
> index 4f4e2c1824f1..39af57af2aad 100644
> --- a/security/keys/proc.c
> +++ b/security/keys/proc.c
> @@ -160,7 +160,6 @@ static int proc_keys_show(struct seq_file *m, void *v)
> char xbuf[16];
> short state;
> u64 timo;
> - int rc;
>
> struct keyring_search_context ctx = {
> .index_key = key->index_key,
> @@ -188,8 +187,7 @@ static int proc_keys_show(struct seq_file *m, void *v)
> }
>
> /* check whether the current task is allowed to view the key */
> - rc = key_task_permission(key_ref, ctx.cred, KEY_NEED_VIEW);
> - if (rc < 0)
> + if (key_task_permission(key_ref, ctx.cred, KEY_NEED_VIEW) < 0)
> return 0;
I absolutely dislike combining return value check to the call.
The old version documents that we ignore the return value, which
is convoluted in the new version.
>
> now = ktime_get_real_seconds();
> --
> 2.51.0
>
BR, Jarkko
^ permalink raw reply
* [RFC PATCH v1 3/3] landlock: Fix TCP handling of short AF_UNSPEC addresses
From: Matthieu Buffet @ 2025-10-27 19:07 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
Konstantin Meskhidze, Matthieu Buffet
In-Reply-To: <20251027190726.626244-1-matthieu@buffet.re>
current_check_access_socket() treats AF_UNSPEC addresses as
AF_INET ones, and only later adds special case handling to
allow connect(AF_UNSPEC), and on IPv4 sockets
bind(AF_UNSPEC+INADDR_ANY).
This would be fine except AF_UNSPEC addresses can be as
short as a bare AF_UNSPEC sa_family_t field, and nothing
more. The AF_INET code path incorrectly enforces a length of
sizeof(struct sockaddr_in) instead.
Move AF_UNSPEC edge case handling up inside the switch-case,
before the address is (potentially incorrectly) treated as
AF_INET.
Fixes: fff69fb03dde ("landlock: Support network rules with TCP bind and connect")
Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
security/landlock/net.c | 118 +++++++++++++++++++++++-----------------
1 file changed, 67 insertions(+), 51 deletions(-)
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 1f3915a90a80..e6367e30e5b0 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -71,6 +71,61 @@ static int current_check_access_socket(struct socket *const sock,
switch (address->sa_family) {
case AF_UNSPEC:
+ if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
+ /*
+ * Connecting to an address with AF_UNSPEC dissolves
+ * the TCP association, which have the same effect as
+ * closing the connection while retaining the socket
+ * object (i.e., the file descriptor). As for dropping
+ * privileges, closing connections is always allowed.
+ *
+ * For a TCP access control system, this request is
+ * legitimate. Let the network stack handle potential
+ * inconsistencies and return -EINVAL if needed.
+ */
+ return 0;
+ } else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
+ /*
+ * Binding to an AF_UNSPEC address is treated
+ * differently by IPv4 and IPv6 sockets. The socket's
+ * family may change under our feet due to
+ * setsockopt(IPV6_ADDRFORM), but that's ok: we either
+ * reject entirely or require
+ * %LANDLOCK_ACCESS_NET_BIND_TCP for the given port, so
+ * it cannot be used to bypass the policy.
+ *
+ * IPv4 sockets map AF_UNSPEC to AF_INET for
+ * retrocompatibility for bind accesses, only if the
+ * address is INADDR_ANY (cf. __inet_bind). IPv6
+ * sockets always reject it.
+ *
+ * Checking the address is required to not wrongfully
+ * return -EACCES instead of -EAFNOSUPPORT or -EINVAL.
+ * We could return 0 and let the network stack handle
+ * these checks, but it is safer to return a proper
+ * error and test consistency thanks to kselftest.
+ */
+ if (sock->sk->__sk_common.skc_family == AF_INET) {
+ const struct sockaddr_in *const sockaddr =
+ (struct sockaddr_in *)address;
+
+ if (addrlen < sizeof(struct sockaddr_in))
+ return -EINVAL;
+
+ if (sockaddr->sin_addr.s_addr !=
+ htonl(INADDR_ANY))
+ return -EAFNOSUPPORT;
+ } else {
+ if (addrlen < SIN6_LEN_RFC2133)
+ return -EINVAL;
+ else
+ return -EAFNOSUPPORT;
+ }
+ } else {
+ WARN_ON_ONCE(1);
+ }
+ /* Only for bind(AF_UNSPEC+INADDR_ANY) on IPv4 socket. */
+ fallthrough;
case AF_INET: {
const struct sockaddr_in *addr4;
@@ -119,57 +174,18 @@ static int current_check_access_socket(struct socket *const sock,
return 0;
}
- /* Specific AF_UNSPEC handling. */
- if (address->sa_family == AF_UNSPEC) {
- /*
- * Connecting to an address with AF_UNSPEC dissolves the TCP
- * association, which have the same effect as closing the
- * connection while retaining the socket object (i.e., the file
- * descriptor). As for dropping privileges, closing
- * connections is always allowed.
- *
- * For a TCP access control system, this request is legitimate.
- * Let the network stack handle potential inconsistencies and
- * return -EINVAL if needed.
- */
- if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP)
- return 0;
-
- /*
- * For compatibility reason, accept AF_UNSPEC for bind
- * accesses (mapped to AF_INET) only if the address is
- * INADDR_ANY (cf. __inet_bind). Checking the address is
- * required to not wrongfully return -EACCES instead of
- * -EAFNOSUPPORT.
- *
- * We could return 0 and let the network stack handle these
- * checks, but it is safer to return a proper error and test
- * consistency thanks to kselftest.
- */
- if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
- /* addrlen has already been checked for AF_UNSPEC. */
- const struct sockaddr_in *const sockaddr =
- (struct sockaddr_in *)address;
-
- if (sock->sk->__sk_common.skc_family != AF_INET)
- return -EINVAL;
-
- if (sockaddr->sin_addr.s_addr != htonl(INADDR_ANY))
- return -EAFNOSUPPORT;
- }
- } else {
- /*
- * Checks sa_family consistency to not wrongfully return
- * -EACCES instead of -EINVAL. Valid sa_family changes are
- * only (from AF_INET or AF_INET6) to AF_UNSPEC.
- *
- * We could return 0 and let the network stack handle this
- * check, but it is safer to return a proper error and test
- * consistency thanks to kselftest.
- */
- if (address->sa_family != sock->sk->__sk_common.skc_family)
- return -EINVAL;
- }
+ /*
+ * Checks sa_family consistency to not wrongfully return
+ * -EACCES instead of -EINVAL. Valid sa_family changes are
+ * only (from AF_INET or AF_INET6) to AF_UNSPEC.
+ *
+ * We could return 0 and let the network stack handle this
+ * check, but it is safer to return a proper error and test
+ * consistency thanks to kselftest.
+ */
+ if (address->sa_family != sock->sk->__sk_common.skc_family &&
+ address->sa_family != AF_UNSPEC)
+ return -EINVAL;
id.key.data = (__force uintptr_t)port;
BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
--
2.47.2
^ permalink raw reply related
* [RFC PATCH v1 2/3] selftests/landlock: Add missing connect(minimal AF_UNSPEC) test
From: Matthieu Buffet @ 2025-10-27 19:07 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
Konstantin Meskhidze, Matthieu Buffet
In-Reply-To: <20251027190726.626244-1-matthieu@buffet.re>
connect_variant(unspec_any0) is called twice. Both calls end
up in connect_variant_addrlen() with an address length of
get_addrlen(minimal=false).
However, the connect() syscall and its variants (e.g.
iouring/compat) accept much shorter addresses of 4 bytes
and that behaviour was not tested.
Replace one of these calls with one using a minimal address
length (just a bare sa_family=AF_UNSPEC field with no actual
address). Also add a call using a truncated address for good
measure.
Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
tools/testing/selftests/landlock/net_test.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 3bbc0508420b..b34b139b3f89 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -906,7 +906,19 @@ TEST_F(protocol, connect_unspec)
EXPECT_EQ(0, close(ruleset_fd));
}
- ret = connect_variant(connect_fd, &self->unspec_any0);
+ /* Try to re-disconnect with a truncated address struct. */
+ EXPECT_EQ(-EINVAL,
+ connect_variant_addrlen(
+ connect_fd, &self->unspec_any0,
+ get_addrlen(&self->unspec_any0, true) - 1));
+
+ /*
+ * Re-disconnect, with a minimal sockaddr struct (just a
+ * bare af_family=AF_UNSPEC field).
+ */
+ ret = connect_variant_addrlen(connect_fd, &self->unspec_any0,
+ get_addrlen(&self->unspec_any0,
+ true));
if (self->srv0.protocol.domain == AF_UNIX &&
self->srv0.protocol.type == SOCK_STREAM) {
EXPECT_EQ(-EINVAL, ret);
--
2.47.2
^ permalink raw reply related
* [RFC PATCH v1 1/3] selftests/landlock: Fix TCP bind(AF_UNSPEC) test case
From: Matthieu Buffet @ 2025-10-27 19:07 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
Konstantin Meskhidze, Matthieu Buffet
In-Reply-To: <20251027190726.626244-1-matthieu@buffet.re>
The nominal error code for bind(AF_UNSPEC) on an IPv6 socket
is -EAFNOSUPPORT, not -EINVAL. -EINVAL is only returned when
the supplied address struct is too short, which happens to be
the case in current selftests because they treat AF_UNSPEC
like IPv4 sockets do: as an alias for AF_INET (which is a
16-byte struct instead of the 24 bytes required by IPv6
sockets).
Make the union large enough for any address (by adding struct
sockaddr_storage to the union), and make AF_UNSPEC addresses
large enough for any family.
Test for -EAFNOSUPPORT instead, and add a dedicated test case
for truncated inputs with -EINVAL.
Fixes: a549d055a22e ("selftests/landlock: Add network tests")
Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
tools/testing/selftests/landlock/common.h | 1 +
tools/testing/selftests/landlock/net_test.c | 16 +++++++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
index 88a3c78f5d98..9a295b5b90e3 100644
--- a/tools/testing/selftests/landlock/common.h
+++ b/tools/testing/selftests/landlock/common.h
@@ -241,6 +241,7 @@ struct service_fixture {
struct sockaddr_un unix_addr;
socklen_t unix_addr_len;
};
+ struct sockaddr_storage _largest;
};
};
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 2a45208551e6..3bbc0508420b 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -121,6 +121,10 @@ static socklen_t get_addrlen(const struct service_fixture *const srv,
{
switch (srv->protocol.domain) {
case AF_UNSPEC:
+ if (minimal)
+ return sizeof(sa_family_t);
+ return sizeof(struct sockaddr_storage);
+
case AF_INET:
return sizeof(srv->ipv4_addr);
@@ -758,6 +762,11 @@ TEST_F(protocol, bind_unspec)
bind_fd = socket_variant(&self->srv0);
ASSERT_LE(0, bind_fd);
+ /* Tries to bind with too small addrlen. */
+ EXPECT_EQ(-EINVAL, bind_variant_addrlen(
+ bind_fd, &self->unspec_any0,
+ get_addrlen(&self->unspec_any0, true) - 1));
+
/* Allowed bind on AF_UNSPEC/INADDR_ANY. */
ret = bind_variant(bind_fd, &self->unspec_any0);
if (variant->prot.domain == AF_INET) {
@@ -766,6 +775,8 @@ TEST_F(protocol, bind_unspec)
TH_LOG("Failed to bind to unspec/any socket: %s",
strerror(errno));
}
+ } else if (variant->prot.domain == AF_INET6) {
+ EXPECT_EQ(-EAFNOSUPPORT, ret);
} else {
EXPECT_EQ(-EINVAL, ret);
}
@@ -792,6 +803,8 @@ TEST_F(protocol, bind_unspec)
} else {
EXPECT_EQ(0, ret);
}
+ } else if (variant->prot.domain == AF_INET6) {
+ EXPECT_EQ(-EAFNOSUPPORT, ret);
} else {
EXPECT_EQ(-EINVAL, ret);
}
@@ -801,7 +814,8 @@ TEST_F(protocol, bind_unspec)
bind_fd = socket_variant(&self->srv0);
ASSERT_LE(0, bind_fd);
ret = bind_variant(bind_fd, &self->unspec_srv0);
- if (variant->prot.domain == AF_INET) {
+ if (variant->prot.domain == AF_INET ||
+ variant->prot.domain == AF_INET6) {
EXPECT_EQ(-EAFNOSUPPORT, ret);
} else {
EXPECT_EQ(-EINVAL, ret)
--
2.47.2
^ permalink raw reply related
* [RFC PATCH v1 0/3] Fix TCP short AF_UNSPEC handling
From: Matthieu Buffet @ 2025-10-27 19:07 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
Konstantin Meskhidze, Matthieu Buffet
Hi Mickaël, Günther, Konstantin, Mikhail,
Here's a small fix for userland behaviour+error consistency in TCP, in
the edge case where userland uses an AF_UNSPEC address consisting of
just a bare sa_family_t.
First two commits fix/add test cases (which then fail), and third commit
patches current_check_access_socket() to make these tests pass. There's
more than one way to patch it, I just chose this because it should also
make it easier to merge UDP support (soon if this goes well).
Feedback welcome!
Matthieu Buffet (3):
selftests/landlock: Fix TCP bind(AF_UNSPEC) test case
selftests/landlock: Add missing connect(minimal AF_UNSPEC) test
landlock: Fix TCP handling of short AF_UNSPEC addresses
security/landlock/net.c | 118 +++++++++++---------
tools/testing/selftests/landlock/common.h | 1 +
tools/testing/selftests/landlock/net_test.c | 30 ++++-
3 files changed, 96 insertions(+), 53 deletions(-)
base-commit: 6dde339a3df80a57ac3d780d8cfc14d9262e2acd
--
2.47.2
^ permalink raw reply
* Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Peter Zijlstra @ 2025-10-27 12:06 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Eric W. Biederman,
Adrian Reber, Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Cyrill Gorcunov, Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE9EF78827D73D3D7212F7E432A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
On Thu, Aug 21, 2025 at 07:34:58PM +0200, Bernd Edlinger wrote:
> The solution is to detect this situation and allow
> ptrace_attach to continue by temporarily releasing the
> cred_guard_mutex, while de_thread() is still waiting for
> traced zombies to be eventually released by the tracer.
> In the case of the thread group leader we only have to wait
> for the thread to become a zombie, which may also need
> co-operation from the tracer due to PTRACE_O_TRACEEXIT.
>
> When a tracer wants to ptrace_attach a task that already
> is in execve, we simply retry the ptrace_may_access
> check while temporarily installing the new credentials
> and dumpability which are about to be used after execve
> completes. If the ptrace_attach happens on a thread that
> is a sibling-thread of the thread doing execve, it is
> sufficient to check against the old credentials, as this
> thread will be waited for, before the new credentials are
> installed.
>
> Other threads die quickly since the cred_guard_mutex is
> released, but a deadly signal is already pending. In case
> the mutex_lock_killable misses the signal, the non-zero
> current->signal->exec_bprm makes sure they release the
> mutex immediately and return with -ERESTARTNOINTR.
> diff --git a/fs/exec.c b/fs/exec.c
> index 2a1e5e4042a1..31c6ceaa5f69 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -905,11 +905,13 @@ static int exec_mmap(struct mm_struct *mm)
> return 0;
> }
>
> -static int de_thread(struct task_struct *tsk)
> +static int de_thread(struct task_struct *tsk, struct linux_binprm *bprm)
> {
> struct signal_struct *sig = tsk->signal;
> struct sighand_struct *oldsighand = tsk->sighand;
> spinlock_t *lock = &oldsighand->siglock;
> + struct task_struct *t;
> + bool unsafe_execve_in_progress = false;
>
> if (thread_group_empty(tsk))
> goto no_thread_group;
> @@ -932,6 +934,19 @@ static int de_thread(struct task_struct *tsk)
> if (!thread_group_leader(tsk))
> sig->notify_count--;
>
> + for_other_threads(tsk, t) {
> + if (unlikely(t->ptrace)
> + && (t != tsk->group_leader || !t->exit_state))
&& goes at the end of the previous line
> + unsafe_execve_in_progress = true;
> + }
> +
> + if (unlikely(unsafe_execve_in_progress)) {
> + spin_unlock_irq(lock);
> + sig->exec_bprm = bprm;
> + mutex_unlock(&sig->cred_guard_mutex);
> + spin_lock_irq(lock);
I'm not clear why we need to drop and re-acquire siglock here.
And I would like a very large comment here explaining why it is safe to
drop cred_guard_mutex here.
> + }
> +
> while (sig->notify_count) {
> __set_current_state(TASK_KILLABLE);
> spin_unlock_irq(lock);
> @@ -1021,6 +1036,11 @@ static int de_thread(struct task_struct *tsk)
> release_task(leader);
> }
>
> + if (unlikely(unsafe_execve_in_progress)) {
> + mutex_lock(&sig->cred_guard_mutex);
> + sig->exec_bprm = NULL;
> + }
> +
> sig->group_exec_task = NULL;
> sig->notify_count = 0;
>
> @@ -1032,6 +1052,11 @@ static int de_thread(struct task_struct *tsk)
> return 0;
>
> killed:
> + if (unlikely(unsafe_execve_in_progress)) {
> + mutex_lock(&sig->cred_guard_mutex);
> + sig->exec_bprm = NULL;
> + }
> +
> /* protects against exit_notify() and __exit_signal() */
> read_lock(&tasklist_lock);
> sig->group_exec_task = NULL;
> @@ -1114,13 +1139,31 @@ int begin_new_exec(struct linux_binprm * bprm)
> */
> trace_sched_prepare_exec(current, bprm);
>
> + /* If the binary is not readable then enforce mm->dumpable=0 */
> + would_dump(bprm, bprm->file);
> + if (bprm->have_execfd)
> + would_dump(bprm, bprm->executable);
> +
> + /*
> + * Figure out dumpability. Note that this checking only of current
> + * is wrong, but userspace depends on it. This should be testing
> + * bprm->secureexec instead.
> + */
> + if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> + is_dumpability_changed(current_cred(), bprm->cred) ||
> + !(uid_eq(current_euid(), current_uid()) &&
> + gid_eq(current_egid(), current_gid())))
> + set_dumpable(bprm->mm, suid_dumpable);
> + else
> + set_dumpable(bprm->mm, SUID_DUMP_USER);
> +
I feel like moving this dumpable stuff around could be a separate patch.
Which can explain how that is correct and why it is needed and all that.
> /*
> * Ensure all future errors are fatal.
> */
> bprm->point_of_no_return = true;
>
> /* Make this the only thread in the thread group */
> - retval = de_thread(me);
> + retval = de_thread(me, bprm);
> if (retval)
> goto out;
> /* see the comment in check_unsafe_exec() */
> @@ -1144,11 +1187,6 @@ int begin_new_exec(struct linux_binprm * bprm)
> if (retval)
> goto out;
>
> - /* If the binary is not readable then enforce mm->dumpable=0 */
> - would_dump(bprm, bprm->file);
> - if (bprm->have_execfd)
> - would_dump(bprm, bprm->executable);
> -
> /*
> * Release all of the old mmap stuff
> */
> @@ -1210,18 +1248,6 @@ int begin_new_exec(struct linux_binprm * bprm)
>
> me->sas_ss_sp = me->sas_ss_size = 0;
>
> - /*
> - * Figure out dumpability. Note that this checking only of current
> - * is wrong, but userspace depends on it. This should be testing
> - * bprm->secureexec instead.
> - */
> - if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> - !(uid_eq(current_euid(), current_uid()) &&
> - gid_eq(current_egid(), current_gid())))
> - set_dumpable(current->mm, suid_dumpable);
> - else
> - set_dumpable(current->mm, SUID_DUMP_USER);
> -
> perf_event_exec();
>
> /*
> @@ -1361,6 +1387,11 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
> if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> return -ERESTARTNOINTR;
>
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + return -ERESTARTNOINTR;
> + }
#1
> +
> bprm->cred = prepare_exec_creds();
> if (likely(bprm->cred))
> return 0;
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 62d35631ba8c..e5bcf812cee0 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2838,6 +2838,12 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
> if (rv < 0)
> goto out_free;
>
Comment explaining why this needs checking goes here.
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + rv = -ERESTARTNOINTR;
> + goto out_free;
> + }
> +
> rv = security_setprocattr(PROC_I(inode)->op.lsmid,
> file->f_path.dentry->d_name.name, page,
> count);
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index a102a10f833f..fb0361911489 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -153,6 +153,7 @@ extern const struct cred *get_task_cred(struct task_struct *);
> extern struct cred *cred_alloc_blank(void);
> extern struct cred *prepare_creds(void);
> extern struct cred *prepare_exec_creds(void);
> +extern bool is_dumpability_changed(const struct cred *, const struct cred *);
> extern int commit_creds(struct cred *);
> extern void abort_creds(struct cred *);
> extern struct cred *prepare_kernel_cred(struct task_struct *);
> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
> index 1ef1edbaaf79..3c47d8b55863 100644
> --- a/include/linux/sched/signal.h
> +++ b/include/linux/sched/signal.h
> @@ -237,9 +237,27 @@ struct signal_struct {
> struct mm_struct *oom_mm; /* recorded mm when the thread group got
> * killed by the oom killer */
>
> + struct linux_binprm *exec_bprm; /* Used to check ptrace_may_access
> + * against new credentials while
> + * de_thread is waiting for other
> + * traced threads to terminate.
> + * Set while de_thread is executing.
> + * The cred_guard_mutex is released
> + * after de_thread() has called
> + * zap_other_threads(), therefore
> + * a fatal signal is guaranteed to be
> + * already pending in the unlikely
> + * event, that
> + * current->signal->exec_bprm happens
> + * to be non-zero after the
> + * cred_guard_mutex was acquired.
> + */
> +
> struct mutex cred_guard_mutex; /* guard against foreign influences on
> * credential calculations
> * (notably. ptrace)
> + * Held while execve runs, except when
> + * a sibling thread is being traced.
> * Deprecated do not use in new code.
> * Use exec_update_lock instead.
> */
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 9676965c0981..0b2822c762df 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -375,6 +375,30 @@ static bool cred_cap_issubset(const struct cred *set, const struct cred *subset)
> return false;
> }
>
> +/**
> + * is_dumpability_changed - Will changing creds affect dumpability?
> + * @old: The old credentials.
> + * @new: The new credentials.
> + *
> + * If the @new credentials have no elevated privileges compared to the
> + * @old credentials, the task may remain dumpable. Otherwise we have
> + * to mark the task as undumpable to avoid information leaks from higher
> + * to lower privilege domains.
> + *
> + * Return: True if the task will become undumpable.
> + */
> +bool is_dumpability_changed(const struct cred *old, const struct cred *new)
> +{
> + if (!uid_eq(old->euid, new->euid) ||
> + !gid_eq(old->egid, new->egid) ||
> + !uid_eq(old->fsuid, new->fsuid) ||
> + !gid_eq(old->fsgid, new->fsgid) ||
> + !cred_cap_issubset(old, new))
> + return true;
> +
> + return false;
> +}
> +
> /**
> * commit_creds - Install new credentials upon the current task
> * @new: The credentials to be assigned
> @@ -403,11 +427,7 @@ int commit_creds(struct cred *new)
> get_cred(new); /* we will require a ref for the subj creds too */
>
> /* dumpability changes */
> - if (!uid_eq(old->euid, new->euid) ||
> - !gid_eq(old->egid, new->egid) ||
> - !uid_eq(old->fsuid, new->fsuid) ||
> - !gid_eq(old->fsgid, new->fsgid) ||
> - !cred_cap_issubset(old, new)) {
> + if (is_dumpability_changed(old, new)) {
> if (task->mm)
> set_dumpable(task->mm, suid_dumpable);
> task->pdeath_signal = 0;
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 75a84efad40f..230298817dbf 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -20,6 +20,7 @@
> #include <linux/pagemap.h>
> #include <linux/ptrace.h>
> #include <linux/security.h>
> +#include <linux/binfmts.h>
> #include <linux/signal.h>
> #include <linux/uio.h>
> #include <linux/audit.h>
> @@ -453,6 +454,28 @@ static int ptrace_attach(struct task_struct *task, long request,
> return retval;
> }
>
> + if (unlikely(task == task->signal->group_exec_task)) {
> + retval = down_write_killable(&task->signal->exec_update_lock);
> + if (retval)
> + return retval;
This could be written like:
ACQUIRE(rwsem_write_kill, guard)(&task->signal->exec_update_lock);
retval = ACQUIRE_ERR(rwsem_write_kill, guard);
if (retval)
return retval;
> +
> + scoped_guard (task_lock, task) {
> + struct linux_binprm *bprm = task->signal->exec_bprm;
> + const struct cred __rcu *old_cred = task->real_cred;
> + struct mm_struct *old_mm = task->mm;
> +
> + rcu_assign_pointer(task->real_cred, bprm->cred);
> + task->mm = bprm->mm;
> + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> + rcu_assign_pointer(task->real_cred, old_cred);
> + task->mm = old_mm;
> + }
> +
> + up_write(&task->signal->exec_update_lock);
And then this goes away ^
> + if (retval)
> + return retval;
> + }
> +
> scoped_guard (write_lock_irq, &tasklist_lock) {
> if (unlikely(task->exit_state))
> return -EPERM;
> @@ -488,6 +511,14 @@ static int ptrace_traceme(void)
> {
> int ret = -EPERM;
>
This needs comments.
> + if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> + return -ERESTARTNOINTR;
> +
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + return -ERESTARTNOINTR;
> + }
#2
> +
> write_lock_irq(&tasklist_lock);
> /* Are we already being traced? */
> if (!current->ptrace) {
> @@ -503,6 +534,7 @@ static int ptrace_traceme(void)
> }
> }
> write_unlock_irq(&tasklist_lock);
> + mutex_unlock(¤t->signal->cred_guard_mutex);
>
> return ret;
> }
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 41aa761c7738..d61fc275235a 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -1994,9 +1994,15 @@ static long seccomp_set_mode_filter(unsigned int flags,
> * Make sure we cannot change seccomp or nnp state via TSYNC
> * while another thread is in the middle of calling exec.
> */
> - if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
> - mutex_lock_killable(¤t->signal->cred_guard_mutex))
> - goto out_put_fd;
> + if (flags & SECCOMP_FILTER_FLAG_TSYNC) {
> + if (mutex_lock_killable(¤t->signal->cred_guard_mutex))
> + goto out_put_fd;
> +
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + goto out_put_fd;
> + }
#3, and after typing this same pattern 3 times, you didn't think it
needed a helper function ?
> + }
>
> spin_lock_irq(¤t->sighand->siglock);
>
^ permalink raw reply
* Re: [RFC bpf-next] lsm: bpf: Remove lsm_prop_bpf
From: Matt Bobrowski @ 2025-10-27 9:40 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-security-module, paul, jmorris, serge, casey, kpsingh,
ast, daniel, andrii, john.johansen
In-Reply-To: <20251025001022.1707437-1-song@kernel.org>
On Fri, Oct 24, 2025 at 05:10:22PM -0700, Song Liu wrote:
> lsm_prop_bpf is not used in any code. Remove it.
>
> Signed-off-by: Song Liu <song@kernel.org>
Reviewed-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
>
> Or did I miss any user of it?
No, it doesn't look like you have, and I don't see how struct lsm_prop
could be used from the context of the BPF LSM ATM anyway. Thanks for
cleaning this up.
> ---
> include/linux/lsm/bpf.h | 16 ----------------
> include/linux/security.h | 2 --
> 2 files changed, 18 deletions(-)
> delete mode 100644 include/linux/lsm/bpf.h
>
> diff --git a/include/linux/lsm/bpf.h b/include/linux/lsm/bpf.h
> deleted file mode 100644
> index 8106e206fcef..000000000000
> --- a/include/linux/lsm/bpf.h
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * Linux Security Module interface to other subsystems.
> - * BPF may present a single u32 value.
> - */
> -#ifndef __LINUX_LSM_BPF_H
> -#define __LINUX_LSM_BPF_H
> -#include <linux/types.h>
> -
> -struct lsm_prop_bpf {
> -#ifdef CONFIG_BPF_LSM
> - u32 secid;
> -#endif
> -};
> -
> -#endif /* ! __LINUX_LSM_BPF_H */
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 92ac3f27b973..b6ace332576f 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -37,7 +37,6 @@
> #include <linux/lsm/selinux.h>
> #include <linux/lsm/smack.h>
> #include <linux/lsm/apparmor.h>
> -#include <linux/lsm/bpf.h>
>
> struct linux_binprm;
> struct cred;
> @@ -163,7 +162,6 @@ struct lsm_prop {
> struct lsm_prop_selinux selinux;
> struct lsm_prop_smack smack;
> struct lsm_prop_apparmor apparmor;
> - struct lsm_prop_bpf bpf;
> };
>
> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Bernd Edlinger @ 2025-10-27 6:26 UTC (permalink / raw)
To: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Eric W. Biederman,
Adrian Reber, Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Alexey Dobriyan,
Jens Axboe, Paul Moore, Elena Reshetova, David Windsor,
Mateusz Guzik, YueHaibing, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, tiozhang, Penglei Jiang, Lorenzo Stoakes,
Adrian Ratiu, Ingo Molnar, Peter Zijlstra (Intel),
Cyrill Gorcunov, Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE9EF78827D73D3D7212F7E432A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
Hi all,
This is a friendly ping, just a gentle reminder since this series has been around a while.
FYI the patch still applies cleanly to current kernel sources, compiles correctly and
tests are still passed.
Thanks
Bernd.
On 8/21/25 19:34, Bernd Edlinger wrote:
> This introduces signal->exec_bprm, which is used to
> fix the case when at least one of the sibling threads
> is traced, and therefore the trace process may dead-lock
> in ptrace_attach, but de_thread will need to wait for the
> tracer to continue execution.
>
> The problem happens when a tracer tries to ptrace_attach
> to a multi-threaded process, that does an execve in one of
> the threads at the same time, without doing that in a forked
> sub-process. That means: There is a race condition, when one
> or more of the threads are already ptraced, but the thread
> that invoked the execve is not yet traced. Now in this
> case the execve locks the cred_guard_mutex and waits for
> de_thread to complete. But that waits for the traced
> sibling threads to exit, and those have to wait for the
> tracer to receive the exit signal, but the tracer cannot
> call wait right now, because it is waiting for the ptrace
> call to complete, and this never does not happen.
> The traced process and the tracer are now in a deadlock
> situation, and can only be killed by a fatal signal.
>
> The solution is to detect this situation and allow
> ptrace_attach to continue by temporarily releasing the
> cred_guard_mutex, while de_thread() is still waiting for
> traced zombies to be eventually released by the tracer.
> In the case of the thread group leader we only have to wait
> for the thread to become a zombie, which may also need
> co-operation from the tracer due to PTRACE_O_TRACEEXIT.
>
> When a tracer wants to ptrace_attach a task that already
> is in execve, we simply retry the ptrace_may_access
> check while temporarily installing the new credentials
> and dumpability which are about to be used after execve
> completes. If the ptrace_attach happens on a thread that
> is a sibling-thread of the thread doing execve, it is
> sufficient to check against the old credentials, as this
> thread will be waited for, before the new credentials are
> installed.
>
> Other threads die quickly since the cred_guard_mutex is
> released, but a deadly signal is already pending. In case
> the mutex_lock_killable misses the signal, the non-zero
> current->signal->exec_bprm makes sure they release the
> mutex immediately and return with -ERESTARTNOINTR.
>
> This means there is no API change, unlike the previous
> version of this patch which was discussed here:
>
> https://lore.kernel.org/lkml/b6537ae6-31b1-5c50-f32b-8b8332ace882@hotmail.de/
>
> See tools/testing/selftests/ptrace/vmaccess.c
> for a test case that gets fixed by this change.
>
> Note that since the test case was originally designed to
> test the ptrace_attach returning an error in this situation,
> the test expectation needed to be adjusted, to allow the
> API to succeed at the first attempt.
>
> Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
> ---
> fs/exec.c | 69 ++++++++---
> fs/proc/base.c | 6 +
> include/linux/cred.h | 1 +
> include/linux/sched/signal.h | 18 +++
> kernel/cred.c | 30 ++++-
> kernel/ptrace.c | 32 +++++
> kernel/seccomp.c | 12 +-
> tools/testing/selftests/ptrace/vmaccess.c | 135 ++++++++++++++++++++--
> 8 files changed, 266 insertions(+), 37 deletions(-)
>
> v10: Changes to previous version, make the PTRACE_ATTACH
> return -EAGAIN, instead of execve return -ERESTARTSYS.
> Added some lessions learned to the description.
>
> v11: Check old and new credentials in PTRACE_ATTACH again without
> changing the API.
>
> Note: I got actually one response from an automatic checker to the v11 patch,
>
> https://lore.kernel.org/lkml/202107121344.wu68hEPF-lkp@intel.com/
>
> which is complaining about:
>
>>>>> kernel/ptrace.c:425:26: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cred const *old_cred @@ got struct cred const [noderef] __rcu *real_cred @@
>
> 417 struct linux_binprm *bprm = task->signal->exec_bprm;
> 418 const struct cred *old_cred;
> 419 struct mm_struct *old_mm;
> 420
> 421 retval = down_write_killable(&task->signal->exec_update_lock);
> 422 if (retval)
> 423 goto unlock_creds;
> 424 task_lock(task);
> > 425 old_cred = task->real_cred;
>
> v12: Essentially identical to v11.
>
> - Fixed a minor merge conflict in linux v5.17, and fixed the
> above mentioned nit by adding __rcu to the declaration.
>
> - re-tested the patch with all linux versions from v5.11 to v6.6
>
> v10 was an alternative approach which did imply an API change.
> But I would prefer to avoid such an API change.
>
> The difficult part is getting the right dumpability flags assigned
> before de_thread starts, hope you like this version.
> If not, the v10 is of course also acceptable.
>
> v13: Fixed duplicated Return section in function header of
> is_dumpability_changed which was reported by the kernel test robot
>
> v14: rebased to v6.7, refreshed and retested.
> And added a more detailed description of the actual bug.
>
> v15: rebased to v6.8-rc1, addressed some review comments.
> Split the test case vmaccess into vmaccess1 and vmaccess2
> to improve overall test coverage.
>
> v16: rebased to 6.17-rc2, fixed some minor merge conflicts.
>
> v17: avoid use of task->in_execve in ptrace_attach.
>
>
> Thanks
> Bernd.
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 2a1e5e4042a1..31c6ceaa5f69 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -905,11 +905,13 @@ static int exec_mmap(struct mm_struct *mm)
> return 0;
> }
>
> -static int de_thread(struct task_struct *tsk)
> +static int de_thread(struct task_struct *tsk, struct linux_binprm *bprm)
> {
> struct signal_struct *sig = tsk->signal;
> struct sighand_struct *oldsighand = tsk->sighand;
> spinlock_t *lock = &oldsighand->siglock;
> + struct task_struct *t;
> + bool unsafe_execve_in_progress = false;
>
> if (thread_group_empty(tsk))
> goto no_thread_group;
> @@ -932,6 +934,19 @@ static int de_thread(struct task_struct *tsk)
> if (!thread_group_leader(tsk))
> sig->notify_count--;
>
> + for_other_threads(tsk, t) {
> + if (unlikely(t->ptrace)
> + && (t != tsk->group_leader || !t->exit_state))
> + unsafe_execve_in_progress = true;
> + }
> +
> + if (unlikely(unsafe_execve_in_progress)) {
> + spin_unlock_irq(lock);
> + sig->exec_bprm = bprm;
> + mutex_unlock(&sig->cred_guard_mutex);
> + spin_lock_irq(lock);
> + }
> +
> while (sig->notify_count) {
> __set_current_state(TASK_KILLABLE);
> spin_unlock_irq(lock);
> @@ -1021,6 +1036,11 @@ static int de_thread(struct task_struct *tsk)
> release_task(leader);
> }
>
> + if (unlikely(unsafe_execve_in_progress)) {
> + mutex_lock(&sig->cred_guard_mutex);
> + sig->exec_bprm = NULL;
> + }
> +
> sig->group_exec_task = NULL;
> sig->notify_count = 0;
>
> @@ -1032,6 +1052,11 @@ static int de_thread(struct task_struct *tsk)
> return 0;
>
> killed:
> + if (unlikely(unsafe_execve_in_progress)) {
> + mutex_lock(&sig->cred_guard_mutex);
> + sig->exec_bprm = NULL;
> + }
> +
> /* protects against exit_notify() and __exit_signal() */
> read_lock(&tasklist_lock);
> sig->group_exec_task = NULL;
> @@ -1114,13 +1139,31 @@ int begin_new_exec(struct linux_binprm * bprm)
> */
> trace_sched_prepare_exec(current, bprm);
>
> + /* If the binary is not readable then enforce mm->dumpable=0 */
> + would_dump(bprm, bprm->file);
> + if (bprm->have_execfd)
> + would_dump(bprm, bprm->executable);
> +
> + /*
> + * Figure out dumpability. Note that this checking only of current
> + * is wrong, but userspace depends on it. This should be testing
> + * bprm->secureexec instead.
> + */
> + if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> + is_dumpability_changed(current_cred(), bprm->cred) ||
> + !(uid_eq(current_euid(), current_uid()) &&
> + gid_eq(current_egid(), current_gid())))
> + set_dumpable(bprm->mm, suid_dumpable);
> + else
> + set_dumpable(bprm->mm, SUID_DUMP_USER);
> +
> /*
> * Ensure all future errors are fatal.
> */
> bprm->point_of_no_return = true;
>
> /* Make this the only thread in the thread group */
> - retval = de_thread(me);
> + retval = de_thread(me, bprm);
> if (retval)
> goto out;
> /* see the comment in check_unsafe_exec() */
> @@ -1144,11 +1187,6 @@ int begin_new_exec(struct linux_binprm * bprm)
> if (retval)
> goto out;
>
> - /* If the binary is not readable then enforce mm->dumpable=0 */
> - would_dump(bprm, bprm->file);
> - if (bprm->have_execfd)
> - would_dump(bprm, bprm->executable);
> -
> /*
> * Release all of the old mmap stuff
> */
> @@ -1210,18 +1248,6 @@ int begin_new_exec(struct linux_binprm * bprm)
>
> me->sas_ss_sp = me->sas_ss_size = 0;
>
> - /*
> - * Figure out dumpability. Note that this checking only of current
> - * is wrong, but userspace depends on it. This should be testing
> - * bprm->secureexec instead.
> - */
> - if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> - !(uid_eq(current_euid(), current_uid()) &&
> - gid_eq(current_egid(), current_gid())))
> - set_dumpable(current->mm, suid_dumpable);
> - else
> - set_dumpable(current->mm, SUID_DUMP_USER);
> -
> perf_event_exec();
>
> /*
> @@ -1361,6 +1387,11 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
> if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> return -ERESTARTNOINTR;
>
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + return -ERESTARTNOINTR;
> + }
> +
> bprm->cred = prepare_exec_creds();
> if (likely(bprm->cred))
> return 0;
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 62d35631ba8c..e5bcf812cee0 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2838,6 +2838,12 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
> if (rv < 0)
> goto out_free;
>
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + rv = -ERESTARTNOINTR;
> + goto out_free;
> + }
> +
> rv = security_setprocattr(PROC_I(inode)->op.lsmid,
> file->f_path.dentry->d_name.name, page,
> count);
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index a102a10f833f..fb0361911489 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -153,6 +153,7 @@ extern const struct cred *get_task_cred(struct task_struct *);
> extern struct cred *cred_alloc_blank(void);
> extern struct cred *prepare_creds(void);
> extern struct cred *prepare_exec_creds(void);
> +extern bool is_dumpability_changed(const struct cred *, const struct cred *);
> extern int commit_creds(struct cred *);
> extern void abort_creds(struct cred *);
> extern struct cred *prepare_kernel_cred(struct task_struct *);
> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
> index 1ef1edbaaf79..3c47d8b55863 100644
> --- a/include/linux/sched/signal.h
> +++ b/include/linux/sched/signal.h
> @@ -237,9 +237,27 @@ struct signal_struct {
> struct mm_struct *oom_mm; /* recorded mm when the thread group got
> * killed by the oom killer */
>
> + struct linux_binprm *exec_bprm; /* Used to check ptrace_may_access
> + * against new credentials while
> + * de_thread is waiting for other
> + * traced threads to terminate.
> + * Set while de_thread is executing.
> + * The cred_guard_mutex is released
> + * after de_thread() has called
> + * zap_other_threads(), therefore
> + * a fatal signal is guaranteed to be
> + * already pending in the unlikely
> + * event, that
> + * current->signal->exec_bprm happens
> + * to be non-zero after the
> + * cred_guard_mutex was acquired.
> + */
> +
> struct mutex cred_guard_mutex; /* guard against foreign influences on
> * credential calculations
> * (notably. ptrace)
> + * Held while execve runs, except when
> + * a sibling thread is being traced.
> * Deprecated do not use in new code.
> * Use exec_update_lock instead.
> */
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 9676965c0981..0b2822c762df 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -375,6 +375,30 @@ static bool cred_cap_issubset(const struct cred *set, const struct cred *subset)
> return false;
> }
>
> +/**
> + * is_dumpability_changed - Will changing creds affect dumpability?
> + * @old: The old credentials.
> + * @new: The new credentials.
> + *
> + * If the @new credentials have no elevated privileges compared to the
> + * @old credentials, the task may remain dumpable. Otherwise we have
> + * to mark the task as undumpable to avoid information leaks from higher
> + * to lower privilege domains.
> + *
> + * Return: True if the task will become undumpable.
> + */
> +bool is_dumpability_changed(const struct cred *old, const struct cred *new)
> +{
> + if (!uid_eq(old->euid, new->euid) ||
> + !gid_eq(old->egid, new->egid) ||
> + !uid_eq(old->fsuid, new->fsuid) ||
> + !gid_eq(old->fsgid, new->fsgid) ||
> + !cred_cap_issubset(old, new))
> + return true;
> +
> + return false;
> +}
> +
> /**
> * commit_creds - Install new credentials upon the current task
> * @new: The credentials to be assigned
> @@ -403,11 +427,7 @@ int commit_creds(struct cred *new)
> get_cred(new); /* we will require a ref for the subj creds too */
>
> /* dumpability changes */
> - if (!uid_eq(old->euid, new->euid) ||
> - !gid_eq(old->egid, new->egid) ||
> - !uid_eq(old->fsuid, new->fsuid) ||
> - !gid_eq(old->fsgid, new->fsgid) ||
> - !cred_cap_issubset(old, new)) {
> + if (is_dumpability_changed(old, new)) {
> if (task->mm)
> set_dumpable(task->mm, suid_dumpable);
> task->pdeath_signal = 0;
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 75a84efad40f..230298817dbf 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -20,6 +20,7 @@
> #include <linux/pagemap.h>
> #include <linux/ptrace.h>
> #include <linux/security.h>
> +#include <linux/binfmts.h>
> #include <linux/signal.h>
> #include <linux/uio.h>
> #include <linux/audit.h>
> @@ -453,6 +454,28 @@ static int ptrace_attach(struct task_struct *task, long request,
> return retval;
> }
>
> + if (unlikely(task == task->signal->group_exec_task)) {
> + retval = down_write_killable(&task->signal->exec_update_lock);
> + if (retval)
> + return retval;
> +
> + scoped_guard (task_lock, task) {
> + struct linux_binprm *bprm = task->signal->exec_bprm;
> + const struct cred __rcu *old_cred = task->real_cred;
> + struct mm_struct *old_mm = task->mm;
> +
> + rcu_assign_pointer(task->real_cred, bprm->cred);
> + task->mm = bprm->mm;
> + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> + rcu_assign_pointer(task->real_cred, old_cred);
> + task->mm = old_mm;
> + }
> +
> + up_write(&task->signal->exec_update_lock);
> + if (retval)
> + return retval;
> + }
> +
> scoped_guard (write_lock_irq, &tasklist_lock) {
> if (unlikely(task->exit_state))
> return -EPERM;
> @@ -488,6 +511,14 @@ static int ptrace_traceme(void)
> {
> int ret = -EPERM;
>
> + if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> + return -ERESTARTNOINTR;
> +
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + return -ERESTARTNOINTR;
> + }
> +
> write_lock_irq(&tasklist_lock);
> /* Are we already being traced? */
> if (!current->ptrace) {
> @@ -503,6 +534,7 @@ static int ptrace_traceme(void)
> }
> }
> write_unlock_irq(&tasklist_lock);
> + mutex_unlock(¤t->signal->cred_guard_mutex);
>
> return ret;
> }
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 41aa761c7738..d61fc275235a 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -1994,9 +1994,15 @@ static long seccomp_set_mode_filter(unsigned int flags,
> * Make sure we cannot change seccomp or nnp state via TSYNC
> * while another thread is in the middle of calling exec.
> */
> - if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
> - mutex_lock_killable(¤t->signal->cred_guard_mutex))
> - goto out_put_fd;
> + if (flags & SECCOMP_FILTER_FLAG_TSYNC) {
> + if (mutex_lock_killable(¤t->signal->cred_guard_mutex))
> + goto out_put_fd;
> +
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + goto out_put_fd;
> + }
> + }
>
> spin_lock_irq(¤t->sighand->siglock);
>
> diff --git a/tools/testing/selftests/ptrace/vmaccess.c b/tools/testing/selftests/ptrace/vmaccess.c
> index 4db327b44586..5d4a65eb5a8d 100644
> --- a/tools/testing/selftests/ptrace/vmaccess.c
> +++ b/tools/testing/selftests/ptrace/vmaccess.c
> @@ -14,6 +14,7 @@
> #include <signal.h>
> #include <unistd.h>
> #include <sys/ptrace.h>
> +#include <sys/syscall.h>
>
> static void *thread(void *arg)
> {
> @@ -23,7 +24,7 @@ static void *thread(void *arg)
>
> TEST(vmaccess)
> {
> - int f, pid = fork();
> + int s, f, pid = fork();
> char mm[64];
>
> if (!pid) {
> @@ -31,19 +32,42 @@ TEST(vmaccess)
>
> pthread_create(&pt, NULL, thread, NULL);
> pthread_join(pt, NULL);
> - execlp("true", "true", NULL);
> + execlp("false", "false", NULL);
> + return;
> }
>
> sleep(1);
> sprintf(mm, "/proc/%d/mem", pid);
> + /* deadlock did happen here */
> f = open(mm, O_RDONLY);
> ASSERT_GE(f, 0);
> close(f);
> - f = kill(pid, SIGCONT);
> - ASSERT_EQ(f, 0);
> + f = waitpid(-1, &s, WNOHANG);
> + ASSERT_NE(f, -1);
> + ASSERT_NE(f, 0);
> + ASSERT_NE(f, pid);
> + ASSERT_EQ(WIFEXITED(s), 1);
> + ASSERT_EQ(WEXITSTATUS(s), 0);
> + f = waitpid(-1, &s, 0);
> + ASSERT_EQ(f, pid);
> + ASSERT_EQ(WIFEXITED(s), 1);
> + ASSERT_EQ(WEXITSTATUS(s), 1);
> + f = waitpid(-1, NULL, 0);
> + ASSERT_EQ(f, -1);
> + ASSERT_EQ(errno, ECHILD);
> }
>
> -TEST(attach)
> +/*
> + * Same test as previous, except that
> + * we try to ptrace the group leader,
> + * which is about to call execve,
> + * when the other thread is already ptraced.
> + * This exercises the code in de_thread
> + * where it is waiting inside the
> + * while (sig->notify_count) {
> + * loop.
> + */
> +TEST(attach1)
> {
> int s, k, pid = fork();
>
> @@ -52,19 +76,76 @@ TEST(attach)
>
> pthread_create(&pt, NULL, thread, NULL);
> pthread_join(pt, NULL);
> - execlp("sleep", "sleep", "2", NULL);
> + execlp("false", "false", NULL);
> + return;
> }
>
> sleep(1);
> + /* deadlock may happen here */
> k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
> - ASSERT_EQ(errno, EAGAIN);
> - ASSERT_EQ(k, -1);
> + ASSERT_EQ(k, 0);
> k = waitpid(-1, &s, WNOHANG);
> ASSERT_NE(k, -1);
> ASSERT_NE(k, 0);
> ASSERT_NE(k, pid);
> ASSERT_EQ(WIFEXITED(s), 1);
> ASSERT_EQ(WEXITSTATUS(s), 0);
> + k = waitpid(-1, &s, 0);
> + ASSERT_EQ(k, pid);
> + ASSERT_EQ(WIFSTOPPED(s), 1);
> + ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
> + k = waitpid(-1, &s, WNOHANG);
> + ASSERT_EQ(k, 0);
> + k = ptrace(PTRACE_CONT, pid, 0L, 0L);
> + ASSERT_EQ(k, 0);
> + k = waitpid(-1, &s, 0);
> + ASSERT_EQ(k, pid);
> + ASSERT_EQ(WIFSTOPPED(s), 1);
> + ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
> + k = waitpid(-1, &s, WNOHANG);
> + ASSERT_EQ(k, 0);
> + k = ptrace(PTRACE_CONT, pid, 0L, 0L);
> + ASSERT_EQ(k, 0);
> + k = waitpid(-1, &s, 0);
> + ASSERT_EQ(k, pid);
> + ASSERT_EQ(WIFEXITED(s), 1);
> + ASSERT_EQ(WEXITSTATUS(s), 1);
> + k = waitpid(-1, NULL, 0);
> + ASSERT_EQ(k, -1);
> + ASSERT_EQ(errno, ECHILD);
> +}
> +
> +/*
> + * Same test as previous, except that
> + * the group leader is ptraced first,
> + * but this time with PTRACE_O_TRACEEXIT,
> + * and the thread that does execve is
> + * not yet ptraced. This exercises the
> + * code block in de_thread where the
> + * if (!thread_group_leader(tsk)) {
> + * is executed and enters a wait state.
> + */
> +static long thread2_tid;
> +static void *thread2(void *arg)
> +{
> + thread2_tid = syscall(__NR_gettid);
> + sleep(2);
> + execlp("false", "false", NULL);
> + return NULL;
> +}
> +
> +TEST(attach2)
> +{
> + int s, k, pid = fork();
> +
> + if (!pid) {
> + pthread_t pt;
> +
> + pthread_create(&pt, NULL, thread2, NULL);
> + pthread_join(pt, NULL);
> + return;
> + }
> +
> sleep(1);
> k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
> ASSERT_EQ(k, 0);
> @@ -72,12 +153,46 @@ TEST(attach)
> ASSERT_EQ(k, pid);
> ASSERT_EQ(WIFSTOPPED(s), 1);
> ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
> - k = ptrace(PTRACE_DETACH, pid, 0L, 0L);
> + k = ptrace(PTRACE_SETOPTIONS, pid, 0L, PTRACE_O_TRACEEXIT);
> + ASSERT_EQ(k, 0);
> + thread2_tid = ptrace(PTRACE_PEEKDATA, pid, &thread2_tid, 0L);
> + ASSERT_NE(thread2_tid, -1);
> + ASSERT_NE(thread2_tid, 0);
> + ASSERT_NE(thread2_tid, pid);
> + k = waitpid(-1, &s, WNOHANG);
> + ASSERT_EQ(k, 0);
> + sleep(2);
> + /* deadlock may happen here */
> + k = ptrace(PTRACE_ATTACH, thread2_tid, 0L, 0L);
> + ASSERT_EQ(k, 0);
> + k = waitpid(-1, &s, WNOHANG);
> + ASSERT_EQ(k, pid);
> + ASSERT_EQ(WIFSTOPPED(s), 1);
> + ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
> + k = waitpid(-1, &s, WNOHANG);
> + ASSERT_EQ(k, 0);
> + k = ptrace(PTRACE_CONT, pid, 0L, 0L);
> + ASSERT_EQ(k, 0);
> + k = waitpid(-1, &s, 0);
> + ASSERT_EQ(k, pid);
> + ASSERT_EQ(WIFSTOPPED(s), 1);
> + ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
> + k = waitpid(-1, &s, WNOHANG);
> + ASSERT_EQ(k, 0);
> + k = ptrace(PTRACE_CONT, pid, 0L, 0L);
> + ASSERT_EQ(k, 0);
> + k = waitpid(-1, &s, 0);
> + ASSERT_EQ(k, pid);
> + ASSERT_EQ(WIFSTOPPED(s), 1);
> + ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
> + k = waitpid(-1, &s, WNOHANG);
> + ASSERT_EQ(k, 0);
> + k = ptrace(PTRACE_CONT, pid, 0L, 0L);
> ASSERT_EQ(k, 0);
> k = waitpid(-1, &s, 0);
> ASSERT_EQ(k, pid);
> ASSERT_EQ(WIFEXITED(s), 1);
> - ASSERT_EQ(WEXITSTATUS(s), 0);
> + ASSERT_EQ(WEXITSTATUS(s), 1);
> k = waitpid(-1, NULL, 0);
> ASSERT_EQ(k, -1);
> ASSERT_EQ(errno, ECHILD);
^ permalink raw reply
* [PATCH] selftests/landlock: Fix makefile header list
From: Matthieu Buffet @ 2025-10-27 1:14 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Matthieu Buffet
Make all headers part of make's dependencies computations.
Otherwise, updating audit.h, common.h, scoped_base_variants.h,
scoped_common.h, scoped_multiple_domain_variants.h, or wrappers.h,
re-running make and running selftests could lead to testing stale headers.
Fixes: 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")
Fixes: fefcf0f7cf47 ("selftests/landlock: Test abstract UNIX socket scoping")
Fixes: 5147779d5e1b ("selftests/landlock: Add wrappers.h")
Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
I was troubleshooting why some auditing selftests kept failing at random
(recvfrom() sometimes returning EAGAIN in audit_recv()). I did not
troubleshoot it yet, but I did find this first step while trying to edit
audit.h in vain:
tools/testing/selftests/landlock/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
index a3f449914bf9..044b83bde16e 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -4,7 +4,7 @@
CFLAGS += -Wall -O2 $(KHDR_INCLUDES)
-LOCAL_HDRS += common.h
+LOCAL_HDRS += $(wildcard *.h)
src_test := $(wildcard *_test.c)
base-commit: 6dde339a3df80a57ac3d780d8cfc14d9262e2acd
--
2.47.2
^ permalink raw reply related
* Re: [GIT PULL] Block fixes for 6.18-rc3
From: Linus Torvalds @ 2025-10-26 22:57 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Jens Axboe, Paul Moore, Serge Hallyn, Christian Brauner,
linux-block@vger.kernel.org, LSM List
In-Reply-To: <aP6OJTTWPQBkll56@mail.hallyn.com>
On Sun, 26 Oct 2025 at 14:10, Serge E. Hallyn <serge@hallyn.com> wrote:
>
> The keychains are all NULL and won't be allocated (by init) without
> copying a new cred, right?
Right. As mentioned, 'struct init_cred' really should be 'const' -
it's not *technically* really constant, because the reference counting
casts away the const, but refs are designed to be copy-on-write apart
from the reference counting.
So whenever you change it, that's when you are supposed to always copy
things. So that prepare_kernel_cred() thing exists for a good reason.
But the pattern here in nbd (and the other three usage cases I found)
is really just "use the kernel creds as-is".
They don't even need any reference counting as long as they can just
rely on the cred staying around for the duration of the use - which
obviously is the case for init_cred.
> Now, in theory, some LSM *could* come by and try to merge
> current's info with init's, but that would probably be misguided.
>
> So this does seem like it should work.
Yeah, I can't see how any LSM could possibly do anything about
init_cred - it really ends up being the source of all other creds. You
can't really validly mess with it or deny it anything.
Linus
^ permalink raw reply
* Re: [GIT PULL] Block fixes for 6.18-rc3
From: Serge E. Hallyn @ 2025-10-26 21:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jens Axboe, Paul Moore, Serge Hallyn, Christian Brauner,
linux-block@vger.kernel.org, LSM List
In-Reply-To: <CAHk-=wgZ9x+yxUB9sjete2s9KBiHnPm2+rcwiWNXhx-rpcKxcw@mail.gmail.com>
On Fri, Oct 24, 2025 at 01:31:11PM -0700, Linus Torvalds wrote:
> [ Adding LSM people. Also Christian, because he did the cred refcount
> cleanup with override_creds() and friends last year, and I'm
> suggesting taking that one step further ]
>
> On Fri, 24 Oct 2025 at 06:58, Jens Axboe <axboe@kernel.dk> wrote:
> >
> > Ondrej Mosnacek (1):
> > nbd: override creds to kernel when calling sock_{send,recv}msg()
>
> I've pulled this, but looking at the patch, I note that more than half
> the patch - 75% to be exact - is just boilerplate for "I need to
> allocate the kernel cred and deal with error handling there".
>
> It literally has three lines of new actual useful code (two statements
> and one local variable declaration), and then nine lines of the "setup
> dance".
>
> Which isn't wrong, but when the infrastructure boilerplate is three
> times more than the actual code, it makes me think we should maybe
> just get rid of the
>
> my_kernel_cred = prepare_kernel_cred(&init_task);
>
> pattern for this use-case, and just let people use "init_cred"
> directly for things like this.
>
> Because that's essentially what that prepare_kernel_cred() thing
> returns, except it allocates a new copy of said thing, so now you have
> error handling and you have to free it after-the-fact.
>
> And I'm not seeing that the extra error handling and freeing dance
> actually buys us anything at all.
>
> Now, some *other* users actually go on to change the creds: they want
> that prepare_kernel_cred() dance because they then actually do
> something else like using their own keyring or whatever (eg the NFS
> idmap code or some other filesystem stuff).
>
> So it's not like prepare_kernel_cred() is wrong, but in this kind of
> case where people just go "I'm a driver with hardware access, I want
> to do something with kernel privileges not user privileges", it
> actually seems counterproductive to have extra code just to complicate
> things.
>
> Now, my gut feel is that if we just let people use 'init_cred'
> directly, we should also make sure that it's always exposed as a
> 'const struct cred' , but wouldn't that be a whole lot simpler and
> more straightforward?
>
> This is *not* the only use case of that.
>
> We now have at least four use-cases of this "raw kernel cred" pattern:
> core-dumping over unix domain socket, nbd, firmware loading and SCSI
> target all do this exact thing as far as I can tell.
>
> So they all just want that bare kernel cred, and this interface then
> forces it to do extra work instead of just doing
>
> old_cred = override_creds(&init_cred);
> ...
> revert_creds(old_cred);
>
> and it ends up being extra code for allocating and freeing that copy
> of a cred that we already *had* and could just have used directly.
>
> I did just check that making 'init_cred' be const
>
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -28 +28 @@ extern struct nsproxy init_nsproxy;
> -extern struct cred init_cred;
> +extern const struct cred init_cred;
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -44 +44 @@ static struct group_info init_groups = { .usage =
> REFCOUNT_INIT(2) };
> -struct cred init_cred = {
> +const struct cred init_cred = {
>
> seems to build just fine and would seem to be the right thing to do
> even if we *don't* expect people to use it. And override_creds() is
> perfectly happy with a
>
> Maybe there's some reason for that extra work that I'm not seeing and
> thinking of? But it all smells like make-believe work to me that
The keychains are all NULL and won't be allocated (by init) without
copying a new cred, right? And it seems like smack, selinux, and
apparmor at least each set the security field to a copy of the
daemon's. Now, in theory, some LSM *could* come by and try to merge
current's info with init's, but that would probably be misguided.
So this does seem like it should work.
> probably has a historical reason for it, but doesn't seem to make a
> lot of sense any more.
>
> Hmm?
>
> Linus
^ permalink raw reply
* Re: [PATCH v3 2/8] landlock: Add API support and docs for the quiet flags
From: Tingmao Wang @ 2025-10-26 21:01 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <2794ddec86d8f24bdfe8a4b3adae18a5f0c41494.1761511023.git.m@maowtm.org>
On 10/26/25 20:44, Tingmao Wang wrote:
> As for kselftests, for now we just change add_rule_checks_ordering to use
> a different invalid flag. I will add tests for the quiet flag in a later
> version.
This message is out of date. Will remove in v4. (Also will remember to
put these sort of things under ---)
^ permalink raw reply
* Re: [PATCH v2 6/6] Implement quiet for optional accesses
From: Tingmao Wang @ 2025-10-26 20:50 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <20251015.sohxe1NohFei@digikod.net>
On 10/15/25 20:09, Mickaël Salaün wrote:
> This extra patch makes the review easier, but it should be squashed into
> the others if possible.
Done in v3.
>
> On Sun, Oct 05, 2025 at 06:55:29PM +0100, Tingmao Wang wrote:
>> [...]
>> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
>> index 4ba44fb1dccb..f183124755a4 100644
>> --- a/security/landlock/audit.c
>> +++ b/security/landlock/audit.c
>> @@ -273,7 +273,7 @@ static void test_get_denied_layer(struct kunit *const 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)
>> + const deny_masks_t deny_masks, u8 quiet_optional_accesses, bool *quiet)
>> {
>> const unsigned long access_opt = all_existing_optional_access;
>> const unsigned long access_req = *access_request;
>> @@ -285,6 +285,7 @@ get_layer_from_deny_masks(access_mask_t *const access_request,
>> /* This will require change with new object types. */
>> WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
>>
>> + *quiet = false;
>> for_each_set_bit(access_bit, &access_opt,
>> BITS_PER_TYPE(access_mask_t)) {
>> if (access_req & BIT(access_bit)) {
>> @@ -298,6 +299,11 @@ get_layer_from_deny_masks(access_mask_t *const access_request,
>> } else if (layer == youngest_layer) {
>> missing |= BIT(access_bit);
>> }
>> +
>> + /* Make sure we set *quiet even if this is the first layer */
>
> Missing final dot.
>
>> + if (layer >= youngest_layer)
>> + *quiet = !!(quiet_optional_accesses &
>> + BIT(access_index));
>
> This code is good but a bit confusing at first, especially without more
> context than this patch provides, where we don't see the relation
> between layer and youngest_layer. Anyway, please extend the comment to
> say that quiet is always overridden for the youngest layer.
TBH I didn't really like it either. I've moved this around a bit in the
latest version - can you check if that is clearer?
^ permalink raw reply
* Re: [PATCH v2 3/6] landlock/audit: Check for quiet flag in landlock_log_denial
From: Tingmao Wang @ 2025-10-26 20:48 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <20251015.Iengoh1eeT0c@digikod.net>
On 10/15/25 20:09, Mickaël Salaün wrote:
> Just use "landlock: " as subject prefix.
>
> On Sun, Oct 05, 2025 at 06:55:26PM +0100, Tingmao Wang wrote:
>> [...]
>> @@ -436,6 +456,52 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
>> if (!audit_enabled)
>> return;
>>
>> + /*
>> + * Checks if the object is marked quiet by the layer that denied the
>> + * request. If it's a different layer that marked it as quiet, but
>> + * that layer is not the one that denied the request, we should still
>> + * audit log the denial.
>> + */
>> + quiet_flag_on_rule = !!(rule_flags.quiet_masks & BIT(youngest_layer));
>> +
>> + if (quiet_flag_on_rule) {
>> + /*
>> + * This is not a scope request, since rule_flags is not zero. We
>> + * now check if the denied requests are all covered by the layer's
>> + * quiet access bits.
>> + */
>> + quiet_mask = pick_access_mask_for_req_type(
>> + request->type, youngest_denied->quiet_masks);
>> + quiet_applicable_to_access = (quiet_mask & missing) == missing;
>> +
>> + if (quiet_applicable_to_access)
>> + return;
>> + } else {
>> + quiet_mask = youngest_denied->quiet_masks.scope;
>> + switch (request->type) {
>> + case LANDLOCK_REQUEST_SCOPE_SIGNAL:
>> + quiet_applicable_to_access =
>> + !!(quiet_mask & LANDLOCK_SCOPE_SIGNAL);
>> + break;
>> + case LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET:
>> + quiet_applicable_to_access =
>> + !!(quiet_mask &
>> + LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
>> + break;
>> + /*
>> + * Leave LANDLOCK_REQUEST_PTRACE and
>> + * LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY unhandled for now - they are
>> + * never quiet
>> + */
>
> This also covers the case where the object is not quiet.
>
>> + default:
>> + break;
>> + }
I went with adding a comment above the line
quiet_mask = youngest_denied->quiet_masks.scope;
^ permalink raw reply
* [PATCH v3 8/8] selftests/landlock: add tests for quiet flag with net rules
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
Tests that:
- Quiet flag works on network rules
- Quiet flag applied to unrelated ports has no effect
- Denied access not in quiet_access_net is still logged
This is not as thorough as the fs tests, but given the shared logic it
should be sufficient. There is also no "optional" access for network
rules.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- New patch
tools/testing/selftests/landlock/net_test.c | 121 ++++++++++++++++++--
1 file changed, 111 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 2a45208551e6..0fd0ec85faff 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -1871,21 +1871,22 @@ TEST_F(port_specific, bind_connect_1023)
static int matches_log_tcp(const int audit_fd, const char *const blockers,
const char *const dir_addr, const char *const addr,
- const char *const dir_port)
+ const char *const dir_port, const __u16 port)
{
static const char log_template[] = REGEX_LANDLOCK_PREFIX
- " blockers=%s %s=%s %s=1024$";
+ " blockers=%s %s=%s %s=%u$";
/*
* Max strlen(blockers): 16
* Max strlen(dir_addr): 5
* Max strlen(addr): 12
* Max strlen(dir_port): 4
+ * Max strlen(%d port): 5
*/
- char log_match[sizeof(log_template) + 37];
+ char log_match[sizeof(log_template) + 42];
int log_match_len;
log_match_len = snprintf(log_match, sizeof(log_match), log_template,
- blockers, dir_addr, addr, dir_port);
+ blockers, dir_addr, addr, dir_port, port);
if (log_match_len > sizeof(log_match))
return -E2BIG;
@@ -1895,7 +1896,8 @@ static int matches_log_tcp(const int audit_fd, const char *const blockers,
FIXTURE(audit)
{
- struct service_fixture srv0;
+ /* srv1 has a rule with no access but quiet bit set, srv0 does not. */
+ struct service_fixture srv0, srv1;
struct audit_filter audit_filter;
int audit_fd;
};
@@ -1929,6 +1931,7 @@ FIXTURE_VARIANT_ADD(audit, ipv6) {
FIXTURE_SETUP(audit)
{
ASSERT_EQ(0, set_service(&self->srv0, variant->prot, 0));
+ ASSERT_EQ(0, set_service(&self->srv1, variant->prot, 1));
setup_loopback(_metadata);
set_cap(_metadata, CAP_AUDIT_CONTROL);
@@ -1949,6 +1952,12 @@ TEST_F(audit, bind)
const struct landlock_ruleset_attr ruleset_attr = {
.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .quiet_access_net = LANDLOCK_ACCESS_NET_CONNECT_TCP |
+ LANDLOCK_ACCESS_NET_BIND_TCP,
+ };
+ const struct landlock_net_port_attr quiet_rule = {
+ .allowed_access = 0,
+ .port = self->srv1.port,
};
struct audit_records records;
int ruleset_fd, sock_fd;
@@ -1956,6 +1965,8 @@ TEST_F(audit, bind)
ruleset_fd =
landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
ASSERT_LE(0, ruleset_fd);
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+ &quiet_rule, LANDLOCK_ADD_RULE_QUIET));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
@@ -1963,11 +1974,21 @@ TEST_F(audit, bind)
ASSERT_LE(0, sock_fd);
EXPECT_EQ(-EACCES, bind_variant(sock_fd, &self->srv0));
EXPECT_EQ(0, matches_log_tcp(self->audit_fd, "net\\.bind_tcp", "saddr",
- variant->addr, "src"));
+ variant->addr, "src", self->srv0.port));
+ /* No other logs expected. */
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+
+ EXPECT_EQ(0, close(sock_fd));
+
+ sock_fd = socket_variant(&self->srv1);
+ ASSERT_LE(0, sock_fd);
+ EXPECT_EQ(-EACCES, bind_variant(sock_fd, &self->srv1));
+
+ /* No log expected due to quiet rule. */
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
EXPECT_EQ(0, records.access);
- EXPECT_EQ(1, records.domain);
EXPECT_EQ(0, close(sock_fd));
}
@@ -1977,6 +1998,12 @@ TEST_F(audit, connect)
const struct landlock_ruleset_attr ruleset_attr = {
.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .quiet_access_net = LANDLOCK_ACCESS_NET_CONNECT_TCP |
+ LANDLOCK_ACCESS_NET_BIND_TCP,
+ };
+ const struct landlock_net_port_attr quiet_rule = {
+ .allowed_access = 0,
+ .port = self->srv1.port,
};
struct audit_records records;
int ruleset_fd, sock_fd;
@@ -1984,18 +2011,92 @@ TEST_F(audit, connect)
ruleset_fd =
landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
ASSERT_LE(0, ruleset_fd);
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+ &quiet_rule, LANDLOCK_ADD_RULE_QUIET));
enforce_ruleset(_metadata, ruleset_fd);
EXPECT_EQ(0, close(ruleset_fd));
sock_fd = socket_variant(&self->srv0);
ASSERT_LE(0, sock_fd);
EXPECT_EQ(-EACCES, connect_variant(sock_fd, &self->srv0));
- EXPECT_EQ(0, matches_log_tcp(self->audit_fd, "net\\.connect_tcp",
- "daddr", variant->addr, "dest"));
+ EXPECT_EQ(0,
+ matches_log_tcp(self->audit_fd, "net\\.connect_tcp", "daddr",
+ variant->addr, "dest", self->srv0.port));
+
+ /* No other logs expected. */
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+
+ EXPECT_EQ(0, close(sock_fd));
+
+ sock_fd = socket_variant(&self->srv1);
+ ASSERT_LE(0, sock_fd);
+ EXPECT_EQ(-EACCES, connect_variant(sock_fd, &self->srv1));
+
+ /* Quieted - no logs expected. */
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+
+ EXPECT_EQ(0, close(sock_fd));
+}
+
+/* Quieting bind access has no effect on connect. */
+TEST_F(audit, connect_quiet_bind)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .quiet_access_net = LANDLOCK_ACCESS_NET_BIND_TCP,
+ };
+ const struct landlock_ruleset_attr ruleset_attr_2 = {
+ .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
+ LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .quiet_access_net = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ };
+ const struct landlock_net_port_attr quiet_rule = {
+ .allowed_access = 0,
+ .port = self->srv1.port,
+ };
+ struct audit_records records;
+ int ruleset_fd, sock_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+ &quiet_rule, LANDLOCK_ADD_RULE_QUIET));
+ enforce_ruleset(_metadata, ruleset_fd);
+ EXPECT_EQ(0, close(ruleset_fd));
+
+ sock_fd = socket_variant(&self->srv1);
+ ASSERT_LE(0, sock_fd);
+ EXPECT_EQ(-EACCES, connect_variant(sock_fd, &self->srv1));
+ EXPECT_EQ(0,
+ matches_log_tcp(self->audit_fd, "net\\.connect_tcp", "daddr",
+ variant->addr, "dest", self->srv1.port));
+
+ /* No other logs expected. */
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+
+ EXPECT_EQ(0, close(sock_fd));
+
+ /* New layer that also denies connect but has the correct quiet bit. */
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr_2, sizeof(ruleset_attr_2), 0);
+ ASSERT_LE(0, ruleset_fd);
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+ &quiet_rule, LANDLOCK_ADD_RULE_QUIET));
+ enforce_ruleset(_metadata, ruleset_fd);
+ EXPECT_EQ(0, close(ruleset_fd));
+
+ sock_fd = socket_variant(&self->srv1);
+ ASSERT_LE(0, sock_fd);
+ EXPECT_EQ(-EACCES, connect_variant(sock_fd, &self->srv1));
+ /* Quieted - no logs expected. */
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
EXPECT_EQ(0, records.access);
- EXPECT_EQ(1, records.domain);
EXPECT_EQ(0, close(sock_fd));
}
--
2.51.1
^ permalink raw reply related
* [PATCH v3 7/8] selftests/landlock: add tests for quiet flag with fs rules
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
Test various interactions of the quiet flag with filesystem rules:
- Non-optional access (tested with open and rename).
- Optional access (tested with truncate and ioctl).
- Behaviour around mounts matches with normal Landlock rules.
- Behaviour around disconnected directories matches with normal Landlock
rules (test expected behaviour of 9a868cdbe66a ("landlock: Fix handling of
disconnected directories") applied to the collected quiet flag).
- Multiple layers works as expected.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- New patch
tools/testing/selftests/landlock/fs_test.c | 2236 +++++++++++++++++++-
1 file changed, 2225 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 943b6e2ac53d..d4819ff44230 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -718,11 +718,15 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
static void add_path_beneath(struct __test_metadata *const _metadata,
const int ruleset_fd, const __u64 allowed_access,
- const char *const path)
+ const char *const path, bool quiet)
{
struct landlock_path_beneath_attr path_beneath = {
.allowed_access = allowed_access,
};
+ __u32 flags = 0;
+
+ if (quiet)
+ flags |= LANDLOCK_ADD_RULE_QUIET;
path_beneath.parent_fd = open(path, O_PATH | O_CLOEXEC);
ASSERT_LE(0, path_beneath.parent_fd)
@@ -731,7 +735,7 @@ static void add_path_beneath(struct __test_metadata *const _metadata,
strerror(errno));
}
ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
- &path_beneath, 0))
+ &path_beneath, flags))
{
TH_LOG("Failed to update the ruleset with \"%s\": %s", path,
strerror(errno));
@@ -786,7 +790,7 @@ static int create_ruleset(struct __test_metadata *const _metadata,
continue;
add_path_beneath(_metadata, ruleset_fd, rules[i].access,
- rules[i].path);
+ rules[i].path, false);
}
return ruleset_fd;
}
@@ -1364,7 +1368,7 @@ TEST_F_FORK(layout1, inherit_subset)
* ANDed with the previous ones.
*/
add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
- dir_s1d2);
+ dir_s1d2, false);
/*
* According to ruleset_fd, dir_s1d2 should now have the
* LANDLOCK_ACCESS_FS_READ_FILE and LANDLOCK_ACCESS_FS_WRITE_FILE
@@ -1396,7 +1400,7 @@ TEST_F_FORK(layout1, inherit_subset)
* Try to get more privileges by adding new access rights to the parent
* directory: dir_s1d1.
*/
- add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, false);
enforce_ruleset(_metadata, ruleset_fd);
/* Same tests and results as above. */
@@ -1419,7 +1423,7 @@ TEST_F_FORK(layout1, inherit_subset)
* that there was no rule tied to it before.
*/
add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
- dir_s1d3);
+ dir_s1d3, false);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
@@ -1472,7 +1476,7 @@ TEST_F_FORK(layout1, inherit_superset)
add_path_beneath(_metadata, ruleset_fd,
LANDLOCK_ACCESS_FS_READ_FILE |
LANDLOCK_ACCESS_FS_READ_DIR,
- dir_s1d2);
+ dir_s1d2, false);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
@@ -4211,7 +4215,7 @@ static int ioctl_error(struct __test_metadata *const _metadata, int fd,
unsigned int cmd)
{
char buf[128]; /* sufficiently large */
- int res, stdinbak_fd;
+ int res, stdinbak_fd, err;
/*
* Depending on the IOCTL command, parts of the zeroed-out buffer might
@@ -4226,13 +4230,14 @@ static int ioctl_error(struct __test_metadata *const _metadata, int fd,
/* Invokes the IOCTL with a zeroed-out buffer. */
bzero(&buf, sizeof(buf));
res = ioctl(fd, cmd, &buf);
+ err = errno;
/* Restores the old FD 0 and closes the backup FD. */
ASSERT_EQ(0, dup2(stdinbak_fd, 0));
ASSERT_EQ(0, close(stdinbak_fd));
if (res < 0)
- return errno;
+ return err;
return 0;
}
@@ -4579,6 +4584,7 @@ FIXTURE(layout1_bind) {};
static const char bind_dir_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3";
static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1";
+static const char bind_file2_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f2";
/* Move targets for disconnected path tests. */
static const char dir_s4d1[] = TMP_DIR "/s4d1";
@@ -6927,8 +6933,8 @@ static int matches_log_fs_extra(struct __test_metadata *const _metadata,
return -E2BIG;
/*
- * It is assume that absolute_path does not contain control characters nor
- * spaces, see audit_string_contains_control().
+ * It is assumed that absolute_path does not contain control
+ * characters nor spaces, see audit_string_contains_control().
*/
absolute_path = realpath(path, NULL);
if (!absolute_path)
@@ -7500,4 +7506,2212 @@ TEST_F(audit_layout1, mount)
EXPECT_EQ(1, records.domain);
}
+static bool debug_quiet_tests;
+
+FIXTURE(audit_quiet_layout1)
+{
+ struct audit_filter audit_filter;
+ int audit_fd;
+};
+
+FIXTURE_SETUP(audit_quiet_layout1)
+{
+ prepare_layout(_metadata);
+ create_layout1(_metadata);
+
+ set_cap(_metadata, CAP_AUDIT_CONTROL);
+ self->audit_fd = audit_init_with_exe_filter(&self->audit_filter);
+ EXPECT_LE(0, self->audit_fd);
+ clear_cap(_metadata, CAP_AUDIT_CONTROL);
+
+ if (getenv("DEBUG_QUIET_TESTS"))
+ debug_quiet_tests = true;
+}
+
+FIXTURE_TEARDOWN_PARENT(audit_quiet_layout1)
+{
+ remove_layout1(_metadata);
+ cleanup_layout(_metadata);
+
+ set_cap(_metadata, CAP_AUDIT_CONTROL);
+ EXPECT_EQ(0, audit_cleanup(-1, NULL));
+ clear_cap(_metadata, CAP_AUDIT_CONTROL);
+}
+
+struct a_rule {
+ const char *path;
+ __u64 access;
+ bool quiet;
+};
+
+struct a_layer {
+ __u64 handled_access_fs;
+ __u64 quiet_access_fs;
+ struct a_rule rules[6];
+ __u64 restrict_flags;
+};
+
+struct a_target {
+ /* File/dir to try open. */
+ const char *target;
+ /* Open mode (one of O_RDONLY, O_WRONLY, or O_RDWR). */
+ int open_mode;
+ /* Should open succeed? */
+ bool expect_open_success;
+ /* If open fails, whether to expect an audit log for read. */
+ bool audit_read_blocked;
+ /* If open fails, whether to expect an audit log for write. */
+ bool audit_write_blocked;
+ /* If ftruncate() is expected to be allowed. */
+ bool expect_truncate_success;
+ /* If ftruncate fails, whether to expect an audit log. */
+ bool audit_truncate;
+ /*
+ * If ioctl() is expected to be allowed (ioctl not attempted if
+ * neither this nor expect_ioctl_denied is set).
+ */
+ bool expect_ioctl_allowed;
+ /* If ioctl() is expected to be denied. */
+ bool expect_ioctl_denied;
+ /* If ioctl fails, whether to expect an audit log. */
+ bool audit_ioctl;
+};
+
+#define AUDIT_QUIET_MAX_TARGETS 10
+
+FIXTURE_VARIANT(audit_quiet_layout1)
+{
+ struct a_layer layers[3];
+ struct a_target targets[AUDIT_QUIET_MAX_TARGETS];
+};
+
+#define FS_R LANDLOCK_ACCESS_FS_READ_FILE
+#define FS_W LANDLOCK_ACCESS_FS_WRITE_FILE
+#define FS_TRUNC LANDLOCK_ACCESS_FS_TRUNCATE
+#define FS_IOCTL LANDLOCK_ACCESS_FS_IOCTL_DEV
+
+static int sprint_access_bits(char *buf, size_t buflen, __u64 access)
+{
+ size_t offset = 0;
+
+ if (buflen < strlen("rwti make_reg remove_file refer") + 1)
+ abort();
+
+ buf[0] = '\0';
+ if (access & FS_R)
+ offset += snprintf(buf + offset, buflen - offset, "r");
+ if (access & FS_W)
+ offset += snprintf(buf + offset, buflen - offset, "w");
+ if (access & FS_TRUNC)
+ offset += snprintf(buf + offset, buflen - offset, "t");
+ if (access & FS_IOCTL)
+ offset += snprintf(buf + offset, buflen - offset, "i");
+ if (access & LANDLOCK_ACCESS_FS_MAKE_REG)
+ offset += snprintf(buf + offset, buflen - offset, ",make_reg");
+ if (access & LANDLOCK_ACCESS_FS_REMOVE_FILE)
+ offset +=
+ snprintf(buf + offset, buflen - offset, ",remove_file");
+ if (access & LANDLOCK_ACCESS_FS_REFER)
+ offset += snprintf(buf + offset, buflen - offset, ",refer");
+
+ if (buf[0] == ',') {
+ offset--;
+ memmove(buf, buf + 1, offset);
+ buf[offset] = '\0';
+ }
+
+ return offset;
+}
+
+static int apply_a_layer(struct __test_metadata *const _metadata,
+ const struct a_layer *l)
+{
+ struct landlock_ruleset_attr rs_attr = {
+ .handled_access_fs = l->handled_access_fs,
+ .quiet_access_fs = l->quiet_access_fs,
+ };
+ int rs_fd;
+ int i;
+ const struct a_rule *r;
+ char handled_access_s[33], quiet_access_s[33], rule_access_s[33];
+
+ if (!l->handled_access_fs)
+ return 0;
+
+ rs_fd = landlock_create_ruleset(&rs_attr, sizeof(rs_attr), 0);
+ ASSERT_LE(0, rs_fd);
+
+ for (i = 0; i < ARRAY_SIZE(l->rules); i++) {
+ r = &l->rules[i];
+ if (!r->path)
+ continue;
+
+ add_path_beneath(_metadata, rs_fd, r->access, r->path,
+ r->quiet);
+ }
+
+ ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
+ ASSERT_EQ(0, landlock_restrict_self(rs_fd, l->restrict_flags))
+ {
+ TH_LOG("Failed to enforce ruleset: %s", strerror(errno));
+ }
+ ASSERT_EQ(0, close(rs_fd));
+
+ if (debug_quiet_tests) {
+ sprint_access_bits(handled_access_s, sizeof(handled_access_s),
+ l->handled_access_fs);
+ sprint_access_bits(quiet_access_s, sizeof(quiet_access_s),
+ l->quiet_access_fs);
+ TH_LOG("applied layer: handled=%s quiet=%s restrict_flags=0x%llx",
+ handled_access_s, quiet_access_s,
+ (unsigned long long)l->restrict_flags);
+ for (i = 0; i < ARRAY_SIZE(l->rules); i++) {
+ r = &l->rules[i];
+ if (!r->path)
+ continue;
+
+ sprint_access_bits(rule_access_s, sizeof(rule_access_s),
+ r->access);
+ TH_LOG(" rule[%d]: path=%s access=%s quiet=%d", i,
+ r->path, rule_access_s, r->quiet);
+ }
+ }
+ return 0;
+}
+
+void audit_quiet_layout1_test_body(struct __test_metadata *const _metadata,
+ FIXTURE_DATA(audit_quiet_layout1) * self,
+ const struct a_target *targets)
+{
+ struct audit_records records = {};
+ int i;
+ const struct a_target *target;
+ int fd = -1;
+ int open_mode;
+ int ret;
+ bool expect_audit;
+ const char *blocker;
+
+ for (i = 0; i < AUDIT_QUIET_MAX_TARGETS; i++) {
+ target = &targets[i];
+ if (!target->target)
+ continue;
+
+ open_mode = target->open_mode & (O_RDONLY | O_WRONLY | O_RDWR);
+
+ EXPECT_TRUE(open_mode == O_RDONLY || open_mode == O_WRONLY ||
+ open_mode == O_RDWR);
+
+ if (target->expect_open_success) {
+ EXPECT_FALSE(target->audit_read_blocked);
+ EXPECT_FALSE(target->audit_write_blocked);
+ }
+ if (target->expect_truncate_success)
+ EXPECT_TRUE(target->expect_open_success &&
+ !target->audit_truncate);
+
+ if (debug_quiet_tests)
+ TH_LOG("Try open \"%s\" with %s%s", target->target,
+ open_mode != O_WRONLY ? "r" : "",
+ open_mode != O_RDONLY ? "w" : "");
+
+ fd = openat(AT_FDCWD, target->target, open_mode | O_CLOEXEC);
+ if (target->expect_open_success) {
+ ASSERT_LE(0, fd)
+ {
+ TH_LOG("Failed to open \"%s\": %s",
+ target->target, strerror(errno));
+ };
+ } else {
+ ASSERT_EQ(-1, fd);
+ ASSERT_EQ(EACCES, errno);
+ }
+
+ expect_audit = true;
+
+ if (target->audit_read_blocked && target->audit_write_blocked)
+ blocker = "fs\\.write_file,fs\\.read_file";
+ else if (target->audit_read_blocked)
+ blocker = "fs\\.read_file";
+ else if (target->audit_write_blocked)
+ blocker = "fs\\.write_file";
+ else
+ expect_audit = false;
+
+ if (expect_audit)
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ blocker, target->target));
+
+ /*
+ * Check that we see no (other) logs.
+ *
+ * We explicitly do not check records.domain here because sometimes, a
+ * domain deallocation log from a previous test (or even a previous
+ * run of the test binary when running in a loop) might run over and
+ * show up here. Since this is not a test about domain alloc/dealloc
+ * messages, we ignore them.
+ */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+
+ if (target->expect_open_success && fd >= 0) {
+ if (debug_quiet_tests)
+ TH_LOG("Try ftruncate \"%s\"", target->target);
+
+ ret = ftruncate(fd, 0);
+ if (target->expect_truncate_success) {
+ ASSERT_EQ(0, ret);
+ } else {
+ ASSERT_EQ(-1, ret);
+ if (open_mode != O_RDONLY)
+ ASSERT_EQ(EACCES, errno);
+ }
+
+ if (target->audit_truncate)
+ ASSERT_EQ(0, matches_log_fs(_metadata,
+ self->audit_fd,
+ "fs\\.truncate",
+ target->target));
+
+ if (target->expect_ioctl_allowed || target->expect_ioctl_denied) {
+ if (debug_quiet_tests)
+ TH_LOG("Try ioctl FIONREAD on \"%s\"",
+ target->target);
+
+ ret = ioctl_error(_metadata, fd, FIONREAD);
+ if (target->expect_ioctl_allowed) {
+ ASSERT_NE(EACCES, ret);
+ } else {
+ ASSERT_EQ(EACCES, ret);
+ }
+ }
+
+ if (target->audit_ioctl)
+ ASSERT_EQ(0,
+ matches_log_fs_extra(
+ _metadata, self->audit_fd,
+ "fs\\.ioctl_dev",
+ target->target,
+ " ioctlcmd=0x541b\\+"));
+
+ /* No other logs. records.domain not checked per reasoning above. */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+ ASSERT_EQ(0, close(fd));
+ }
+ }
+}
+
+TEST_F(audit_quiet_layout1, base)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(variant->layers); i++)
+ ASSERT_EQ(0, apply_a_layer(_metadata, &variant->layers[i]));
+
+ audit_quiet_layout1_test_body(_metadata, self, variant->targets);
+}
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_simple) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC,
+ .quiet_access_fs = FS_R,
+ .rules = {
+ { .path = dir_s1d1, .access = 0, .quiet = true },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ },
+ /* Not covered by quiet */
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ /* Access not quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .audit_write_blocked = true,
+ },
+ /*
+ * Quiet flag only takes effect if all blocked access bits are
+ * quieted, otherwise audit log emitted as normal (with all blockers)
+ */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ .audit_read_blocked = true,
+ .audit_write_blocked = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_allow_read) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC,
+ .quiet_access_fs = FS_W,
+ .rules = {
+ { .path = dir_s1d1, .access = FS_R, .quiet = true },
+ /* Quiet flags inherit down and is not overridden */
+ { .path = file1_s1d1, .access = FS_R, .quiet = false },
+ { .path = file1_s2d3, .access = 0, .quiet = true },
+ },
+ },
+ },
+ .targets = {
+ /* Read ok */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ },
+ /* Write quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ },
+ /* Read allowed, write quieted so no audit */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ },
+ /* Not covered by quiet */
+ {
+ .target = file1_s2d2,
+ .open_mode = O_WRONLY,
+ .audit_write_blocked = true,
+ },
+ {
+ .target = file1_s2d2,
+ .open_mode = O_RDWR,
+ .audit_read_blocked = true,
+ .audit_write_blocked = true,
+ },
+ /* Single file quiet */
+ {
+ .target = file1_s2d3,
+ .open_mode = O_WRONLY,
+ },
+ /* Wrong file */
+ {
+ .target = file2_s2d3,
+ .open_mode = O_WRONLY,
+ .audit_write_blocked = true,
+ },
+ /* Access not quieted */
+ {
+ .target = file1_s2d3,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ /* Some access not quieted */
+ {
+ .target = file1_s2d3,
+ .open_mode = O_RDWR,
+ .audit_read_blocked = true,
+ .audit_write_blocked = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_allow_write) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC,
+ .quiet_access_fs = FS_R,
+ .rules = {
+ { .path = dir_s1d1, .access = FS_W, .quiet = true },
+ },
+ },
+ },
+ .targets = {
+ /* Read quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ },
+ /* Truncate not quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ /* Not covered by quiet */
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ /* Write allowed, read quieted so no audit */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, allow_write_quiet_trunc) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC,
+ .quiet_access_fs = FS_TRUNC,
+ .rules = {
+ { .path = dir_s1d1, .access = FS_W, .quiet = true },
+ { .path = dir_s2d1, .access = FS_W, .quiet = false },
+ },
+ },
+ },
+ .targets = {
+ /* Read not allowed and not quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ /* Truncate quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ },
+ /* Not covered by quiet (truncate) */
+ {
+ .target = file1_s2d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ /* Not covered by quiet (read/write) */
+ {
+ .target = file1_s3d1,
+ .open_mode = O_RDWR,
+ .audit_read_blocked = true,
+ .audit_write_blocked = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, allow_rw_quiet_trunc) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC,
+ .quiet_access_fs = FS_TRUNC,
+ .rules = {
+ { .path = dir_s1d1, .access = FS_R | FS_W, .quiet = true },
+ { .path = dir_s2d1, .access = FS_R | FS_W, .quiet = false },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_all) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ { .path = dir_s1d1, .access = 0, .quiet = true },
+ { .path = file1_s2d1, .access = FS_R | FS_W, .quiet = true },
+ { .path = file1_s2d3, .access = 0, .quiet = true },
+ { .path = dir_s3d1, .access = FS_W, .quiet = false },
+ { .path = "/dev/zero", .access = FS_R, .quiet = false },
+ { .path = "/dev/null", .access = FS_R, .quiet = true },
+ },
+ },
+ },
+ .targets = {
+ /* No logs */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ },
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ },
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ },
+ /* Truncate quieted - no log */
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ },
+ /* Truncate not covered by quiet */
+ {
+ .target = file1_s3d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ /* Not covered by quiet */
+ {
+ .target = file1_s3d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ /* Single file quiet */
+ {
+ .target = file1_s2d3,
+ .open_mode = O_RDWR,
+ },
+ /* Wrong file */
+ {
+ .target = file2_s2d3,
+ .open_mode = O_RDWR,
+ .audit_read_blocked = true,
+ .audit_write_blocked = true,
+ },
+ /* Ioctl quieted */
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ },
+ /* Ioctl not quieted */
+ {
+ .target = "/dev/zero",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ .audit_ioctl = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_across_mountpoint) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC,
+ .quiet_access_fs = FS_R,
+ .rules = {
+ { .path = dir_s3d1, .access = 0, .quiet = true },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s3d3,
+ .open_mode = O_RDONLY,
+ },
+ /* Not covered by quiet */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ .audit_read_blocked = true,
+ .audit_write_blocked = true,
+ },
+ /* Access not quieted */
+ {
+ .target = file1_s3d3,
+ .open_mode = O_WRONLY,
+ .audit_write_blocked = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, allow_all_quiet) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = true
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = true
+ },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ .expect_truncate_success = true,
+ },
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_allowed = true,
+ },
+ },
+};
+
+/*
+ * With LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF, it doesn't matter what
+ * the quiet flags below the layer says
+ */
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, subdomains_off) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R,
+ .restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF,
+ .rules = {
+ { .path = "/", .access = FS_R, .quiet = false },
+ }
+ },
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R,
+ .rules = {
+ { .path = dir_s1d1, .access = 0, .quiet = true },
+ { .path = file1_s2d2, .access = FS_R | FS_W, .quiet = true },
+ { .path = file1_s2d3, .access = FS_R | FS_W, .quiet = false },
+ { .path = "/dev/null", .access = FS_R | FS_W, .quiet = true },
+ { .path = "/dev/zero", .access = FS_R | FS_W, .quiet = false },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDWR,
+ },
+ {
+ .target = file1_s2d2,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ /* No audit_truncate */
+ },
+ {
+ .target = file1_s2d3,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ /* No audit_truncate */
+ },
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ /* No audit_ioctl */
+ },
+ {
+ .target = "/dev/zero",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ /* No audit_ioctl */
+ },
+ },
+};
+
+/*
+ * With LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF, it doesn't matter what
+ * the quiet flags on the layer says
+ */
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, same_exec_off) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R,
+ .restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF,
+ .rules = {
+ { .path = dir_s1d1, .access = 0, .quiet = true },
+ { .path = file1_s2d2, .access = FS_R | FS_W, .quiet = true },
+ { .path = file1_s2d3, .access = FS_R | FS_W, .quiet = false },
+ { .path = "/dev/null", .access = FS_R | FS_W, .quiet = true },
+ { .path = "/dev/zero", .access = FS_R | FS_W, .quiet = false },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDWR,
+ },
+ {
+ .target = file1_s2d2,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ /* No audit_truncate */
+ },
+ {
+ .target = file1_s2d3,
+ .open_mode = O_RDWR,
+ .expect_open_success = true,
+ /* No audit_truncate */
+ },
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ /* No audit_ioctl */
+ },
+ {
+ .target = "/dev/zero",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ /* No audit_ioctl */
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_two_layers_1) {
+ /* Here, rules that deny access is always quiet. */
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_W,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = false,
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R,
+ .quiet = true,
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = false,
+ },
+ },
+ },
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_W,
+ .quiet = true,
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = false,
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R,
+ .quiet = true,
+ },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ },
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDONLY,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ },
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ },
+ {
+ .target = "/dev/zero",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_two_layers_2) {
+ /* Here, rules that deny access is never quiet. */
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_W,
+ .quiet = false
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = true
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R,
+ .quiet = false
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = true
+ },
+ },
+ },
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = true
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_W,
+ .quiet = false
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = true
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R,
+ .quiet = false
+ },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ .audit_ioctl = true,
+ },
+ {
+ .target = "/dev/zero",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ .audit_ioctl = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_two_layers_3) {
+ /* This time only the second layer quiets things. */
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_W,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = false,
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R,
+ .quiet = false,
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = false,
+ },
+ },
+ },
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_W,
+ .quiet = true,
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = false,
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R,
+ .quiet = true,
+ },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDONLY,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ },
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ .audit_ioctl = true,
+ },
+ {
+ .target = "/dev/zero",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, quiet_two_layers_different_quiet_access) {
+ /* Here, rules that deny access is always quiet. */
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_W,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = false,
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R,
+ .quiet = true,
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = false,
+ },
+ },
+ },
+ {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_IOCTL,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = FS_R | FS_W | FS_TRUNC,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = FS_W,
+ .quiet = true,
+ },
+ {
+ .path = "/dev/null",
+ .access = FS_R | FS_W | FS_IOCTL,
+ .quiet = false,
+ },
+ {
+ .path = "/dev/zero",
+ .access = FS_R,
+ .quiet = true,
+ },
+ },
+ },
+ },
+ .targets = {
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ },
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ {
+ .target = file1_s2d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .audit_truncate = true,
+ },
+ {
+ .target = "/dev/null",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ },
+ {
+ .target = "/dev/zero",
+ .open_mode = O_RDONLY,
+ .expect_open_success = true,
+ .expect_ioctl_denied = true,
+ },
+ },
+};
+
+FIXTURE_VARIANT_ADD(audit_quiet_layout1, without_quiet_then_with_quiet) {
+ .layers = {
+ {
+ .handled_access_fs = FS_R | FS_W,
+ .quiet_access_fs = FS_R,
+ .rules = {
+ { .path = dir_s1d1, .access = FS_W, .quiet = false },
+ { .path = dir_s1d1, .access = 0, .quiet = true },
+ },
+ },
+ },
+ .targets = {
+ /* Read denied and quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDONLY,
+ },
+ /* Write ok */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_WRONLY,
+ .expect_open_success = true,
+ .expect_truncate_success = true,
+ },
+ /* Write ok, read denied and quieted */
+ {
+ .target = file1_s1d1,
+ .open_mode = O_RDWR,
+ },
+ /* Not covered by quiet */
+ {
+ .target = file1_s2d1,
+ .open_mode = O_RDONLY,
+ .audit_read_blocked = true,
+ },
+ },
+};
+
+/*
+ * The following TEST_F extend the above test cases to test more layers,
+ * with the inserted layers having varying configurations.
+ */
+
+/* Extra allow all layers, quiet or not, does not change any behaviour. */
+TEST_F(audit_quiet_layout1, allow_all_layer)
+{
+ struct a_layer allow_all_layer = {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = 0,
+ .rules = {
+ {
+ .path = "/",
+ .access = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet = false,
+ },
+ },
+ };
+ int i;
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &allow_all_layer));
+ for (i = 0; i < ARRAY_SIZE(variant->layers); i++)
+ ASSERT_EQ(0, apply_a_layer(_metadata, &variant->layers[i]));
+
+ audit_quiet_layout1_test_body(_metadata, self, variant->targets);
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &allow_all_layer));
+
+ audit_quiet_layout1_test_body(_metadata, self, variant->targets);
+
+ /*
+ * SELF_LOG flags or quiet bits from inner allowing layers should not
+ * affect behaviour.
+ */
+ allow_all_layer.quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL;
+ allow_all_layer.rules[0].quiet = true;
+ /*
+ * Note: this only works because we're not checking counts of domain
+ * alloc/dealloc logs
+ */
+ allow_all_layer.restrict_flags =
+ LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
+ LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF;
+ ASSERT_EQ(0, apply_a_layer(_metadata, &allow_all_layer));
+
+ audit_quiet_layout1_test_body(_metadata, self, variant->targets);
+}
+
+/*
+ * Add useless outer layers until we reach the layer limit. Should not
+ * change anything.
+ */
+TEST_F(audit_quiet_layout1, many_outer_layers)
+{
+ struct a_layer useless_layer = {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC,
+ .rules = {
+ { .path = "/", .access = FS_R | FS_W | FS_TRUNC, .quiet = true },
+ },
+ };
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(variant->layers); i++) {
+ if (variant->layers[i].handled_access_fs == 0)
+ break;
+ }
+
+ for (; i < LANDLOCK_MAX_NUM_LAYERS; i++)
+ ASSERT_EQ(0, apply_a_layer(_metadata, &useless_layer));
+
+ for (i = 0; i < ARRAY_SIZE(variant->layers); i++)
+ ASSERT_EQ(0, apply_a_layer(_metadata, &variant->layers[i]));
+
+ audit_quiet_layout1_test_body(_metadata, self, variant->targets);
+}
+
+/*
+ * An inner layer that denies and quiets everything should result in no
+ * logs.
+ */
+TEST_F(audit_quiet_layout1, deny_all_quiet_layer)
+{
+ struct a_layer deny_all_layer = {
+ .handled_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .quiet_access_fs = FS_R | FS_W | FS_TRUNC | FS_IOCTL,
+ .rules = {
+ { .path = "/", .access = 0, .quiet = true },
+ },
+ };
+ int i;
+ FIXTURE_VARIANT(audit_quiet_layout1) variant_2 = {};
+
+ /* Any open should fail with no logs. */
+ for (i = 0; i < ARRAY_SIZE(variant->targets); i++) {
+ const struct a_target *target = &variant->targets[i];
+
+ variant_2.targets[i] = (struct a_target){
+ .target = target->target,
+ .open_mode = target->open_mode,
+ /* We denied everything, open should always fail. */
+ .expect_open_success = false,
+ };
+ }
+
+ for (i = 0; i < ARRAY_SIZE(variant->layers); i++)
+ ASSERT_EQ(0, apply_a_layer(_metadata, &variant->layers[i]));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &deny_all_layer));
+
+ audit_quiet_layout1_test_body(_metadata, self, variant_2.targets);
+}
+
+/*
+ * An inner layer that denies everything without quiet should produce logs
+ * for all access.
+ */
+TEST_F(audit_quiet_layout1, deny_all_layer)
+{
+ struct a_layer deny_all_layer = {
+ .handled_access_fs = FS_R | FS_W,
+ .quiet_access_fs = FS_R | FS_W,
+ };
+ int i;
+ FIXTURE_VARIANT(audit_quiet_layout1) variant_2 = {};
+ bool test_has_subdomains_off = false;
+
+ for (i = 0; i < ARRAY_SIZE(variant->layers); i++) {
+ if (variant->layers[i].restrict_flags &
+ LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF) {
+ test_has_subdomains_off = true;
+ break;
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(variant->targets); i++) {
+ const struct a_target *target = &variant->targets[i];
+
+ variant_2.targets[i] = (struct a_target){
+ .target = target->target,
+ .open_mode = target->open_mode,
+
+ /* We denied everything, open should always fail. */
+ .expect_open_success = false,
+ /* Audit should always happen as long as open request contains read. */
+ .audit_read_blocked = !test_has_subdomains_off &&
+ target->open_mode != O_WRONLY,
+ /* Audit should always happen as long as open request contains write. */
+ .audit_write_blocked = !test_has_subdomains_off &&
+ target->open_mode != O_RDONLY,
+ };
+ }
+
+ for (i = 0; i < ARRAY_SIZE(variant->layers); i++)
+ ASSERT_EQ(0, apply_a_layer(_metadata, &variant->layers[i]));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &deny_all_layer));
+
+ audit_quiet_layout1_test_body(_metadata, self, variant_2.targets);
+}
+
+/* Uses layout1_bind hierarchy */
+FIXTURE(audit_quiet_rename)
+{
+ struct audit_filter audit_filter;
+ int audit_fd;
+};
+
+FIXTURE_SETUP(audit_quiet_rename)
+{
+ prepare_layout(_metadata);
+ create_layout1(_metadata);
+
+ set_cap(_metadata, CAP_SYS_ADMIN);
+ ASSERT_EQ(0, mount(dir_s1d2, dir_s2d2, NULL, MS_BIND, NULL));
+ clear_cap(_metadata, CAP_SYS_ADMIN);
+
+ set_cap(_metadata, CAP_AUDIT_CONTROL);
+ self->audit_fd = audit_init_with_exe_filter(&self->audit_filter);
+ EXPECT_LE(0, self->audit_fd);
+ clear_cap(_metadata, CAP_AUDIT_CONTROL);
+
+ if (getenv("DEBUG_QUIET_TESTS"))
+ debug_quiet_tests = true;
+}
+
+FIXTURE_TEARDOWN_PARENT(audit_quiet_rename)
+{
+ remove_layout1(_metadata);
+ cleanup_layout(_metadata);
+
+ /* umount(dir_s2d2)) is handled by namespace lifetime. */
+
+ remove_path(file1_s4d1);
+ remove_path(file2_s4d1);
+
+ set_cap(_metadata, CAP_AUDIT_CONTROL);
+ EXPECT_EQ(0, audit_cleanup(-1, NULL));
+ clear_cap(_metadata, CAP_AUDIT_CONTROL);
+}
+
+static void simple_quiet_rename(struct __test_metadata *const _metadata,
+ FIXTURE_DATA(audit_quiet_rename) *const self,
+ __u64 handled_access, __u64 quiet_access,
+ bool source_allow, bool dest_allow,
+ bool source_quiet, bool dest_quiet,
+ const char *source_blockers,
+ const char *dest_blockers)
+{
+ /* We will move file1_s1d1 to file1_s2d1 */
+ struct a_layer layer = {
+ .handled_access_fs = handled_access,
+ .quiet_access_fs = quiet_access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = source_allow ? handled_access : 0,
+ .quiet = source_quiet,
+ },
+ {
+ .path = dir_s2d1,
+ .access = dest_allow ? handled_access : 0,
+ .quiet = dest_quiet,
+ },
+ },
+ };
+ struct audit_records records = {};
+ int ret, err;
+
+ /* Skip landlock_add_rule for useless rules. */
+ if (!source_allow && !source_quiet)
+ layer.rules[0].path = NULL;
+ if (!dest_allow && !dest_quiet)
+ layer.rules[1].path = NULL;
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+ EXPECT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ if (debug_quiet_tests)
+ TH_LOG("Try renameat \"%s\" to \"%s\"", file1_s1d1, file1_s2d1);
+ ret = renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1);
+ err = errno;
+ if (ret != 0 && debug_quiet_tests) {
+ TH_LOG("renameat error: %s", err == EXDEV ? "EXDEV" :
+ err == EACCES ? "EACCES" :
+ strerror(err));
+ }
+ if (source_allow && dest_allow) {
+ ASSERT_EQ(0, ret);
+ } else {
+ ASSERT_EQ(-1, ret);
+ if (handled_access & (LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE)) {
+ ASSERT_EQ(EACCES, err);
+ } else {
+ ASSERT_EQ(EXDEV, err);
+ }
+
+ if (source_blockers)
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ source_blockers, dir_s1d1));
+ if (dest_blockers)
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ dest_blockers, dir_s2d1));
+ }
+ /*
+ * No other logs. records.domain not checked per reasoning in
+ * audit_quiet_layout1_test_body.
+ */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, rename_ok)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, access, true, true, false,
+ false, NULL, NULL);
+}
+
+TEST_F(audit_quiet_rename, no_quiet)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, access, false, false,
+ false, false, "fs\\.remove_file,fs\\.refer",
+ "fs\\.make_reg,fs\\.refer");
+}
+
+TEST_F(audit_quiet_rename, quiet)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, access, false, false, true,
+ true, NULL, NULL);
+}
+
+TEST_F(audit_quiet_rename, source_no_quiet_dest_quiet)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, access, false, false,
+ false, true, "fs\\.remove_file,fs\\.refer", NULL);
+}
+
+TEST_F(audit_quiet_rename, source_quiet_dest_no_quiet)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, access, false, false, true,
+ false, NULL, "fs\\.make_reg,fs\\.refer");
+}
+
+TEST_F(audit_quiet_rename, only_quiet_refer)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, LANDLOCK_ACCESS_FS_REFER,
+ false, false, true, true,
+ "fs\\.remove_file,fs\\.refer",
+ "fs\\.make_reg,fs\\.refer");
+}
+
+TEST_F(audit_quiet_rename, source_allow_dest_quiet)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, access, true, false, false,
+ true, NULL, NULL);
+}
+
+TEST_F(audit_quiet_rename, source_quiet_dest_allow)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+
+ simple_quiet_rename(_metadata, self, access, access, false, true, true,
+ false, NULL, NULL);
+}
+
+TEST_F(audit_quiet_rename, handle_all_deny_quiet_refer)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = LANDLOCK_ACCESS_FS_REFER,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* No logs */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, handle_all_deny_not_quiet_refer)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = 0,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ .quiet = false,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EXDEV, errno);
+
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd, "fs\\.refer",
+ dir_s1d1));
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd, "fs\\.refer",
+ dir_s2d1));
+
+ /* No other logs */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, handle_all_deny_refer_quiet_source_not_quiet_dest)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = LANDLOCK_ACCESS_FS_REFER,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ .quiet = false,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EXDEV, errno);
+
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd, "fs\\.refer",
+ dir_s2d1));
+
+ /* No other logs */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, quiet_same_dir)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file2_s1d1));
+ ASSERT_EQ(EACCES, errno);
+
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, quiet_flag_on_file_ignored)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = file1_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ {
+ .path = file1_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.remove_file,fs\\.refer", dir_s1d1));
+ /* We didn't unlink destination file */
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.remove_file,fs\\.make_reg,fs\\.refer", dir_s2d1));
+
+ /* No other logs */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, quiet_flag_on_file_ignored_same_dir)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = file1_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ {
+ .path = file2_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file2_s1d1));
+ ASSERT_EQ(EACCES, errno);
+
+ ASSERT_EQ(0,
+ matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.remove_file,fs\\.make_reg", dir_s1d1));
+
+ /* No other logs */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, two_layers_different_quiet1)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer1 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = access,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct a_layer layer2 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = LANDLOCK_ACCESS_FS_REFER,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = access,
+ .quiet = false,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer2));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * The youngest denial will be layer 2. Refer is quieted but we are
+ * also missing remove_file on source.
+ */
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.remove_file,fs\\.refer", dir_s1d1));
+ /* No other logs */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, two_layers_different_quiet2)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer1 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = access,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct a_layer layer2 = {
+ .handled_access_fs = LANDLOCK_ACCESS_FS_REFER,
+ .quiet_access_fs = LANDLOCK_ACCESS_FS_REFER,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ .quiet = false,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer2));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * The youngest denial will be layer 2, but refer is quieted (and that
+ * layer does not handle any other accesses).
+ */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, two_layers_different_quiet3)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer1 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = access,
+ .quiet = false,
+ },
+ {
+ .path = dir_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct a_layer layer2 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = access,
+ .quiet = false,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer2));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * The youngest denial will be layer 2, in which everything is
+ * quieted.
+ */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, first_layer_quiet_deny_all_second_layer_not_quiet_deny_all)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer1 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct a_layer layer2 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {},
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer2));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.remove_file,fs\\.refer", dir_s1d1));
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.make_reg,fs\\.refer", dir_s2d1));
+ /* No other logs. */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, first_layer_quiet_deny_all_second_layer_dest_not_quiet)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer1 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct a_layer layer2 = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file1_s2d1));
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer1));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer2));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Source is quieted but destination is not.
+ */
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.make_reg,fs\\.refer", dir_s2d1));
+ /* No other logs. */
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, rename_xchg)
+{
+ struct a_layer layer = {
+ .handled_access_fs = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER,
+ .quiet_access_fs = LANDLOCK_ACCESS_FS_MAKE_REG,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER,
+ .quiet = true,
+ },
+ {
+ .path = dir_s2d1,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER,
+ .quiet = false,
+ }
+ },
+ };
+ struct audit_records records = {};
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s1d1, AT_FDCWD, file1_s2d1,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, quiet_on_parent_mount)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file2_s1d3));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, bind_file1_s1d3, AT_FDCWD, bind_file2_s1d3));
+ ASSERT_EQ(EACCES, errno);
+
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, quiet_behind_mountpoint_ignored)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s1d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+
+ EXPECT_EQ(0, unlink(file2_s1d3));
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(AT_FDCWD, bind_file1_s1d3, AT_FDCWD, bind_file2_s1d3));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(0,
+ matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.remove_file,fs\\.make_reg", bind_dir_s1d3));
+
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, quiet_on_parent_mount_disconnected)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s2d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+ int bind_s1d3_fd;
+
+ EXPECT_EQ(0, unlink(file2_s1d3));
+
+ bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_DIRECTORY);
+ ASSERT_GE(bind_s1d3_fd, 0);
+
+ /* Make s1d3 disconnected. */
+ create_directory(_metadata, dir_s4d1);
+ ASSERT_EQ(0, renameat(AT_FDCWD, dir_s1d3, AT_FDCWD, dir_s4d2));
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(bind_s1d3_fd, file1_name, bind_s1d3_fd, file2_name));
+ ASSERT_EQ(EACCES, errno);
+
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
+TEST_F(audit_quiet_rename, quiet_behind_mountpoint_ignored_disconnected)
+{
+ __u64 access = LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REFER;
+ struct a_layer layer = {
+ .handled_access_fs = access,
+ .quiet_access_fs = access,
+ .rules = {
+ {
+ .path = dir_s4d1,
+ .access = 0,
+ .quiet = true,
+ },
+ },
+ };
+ struct audit_records records = {};
+ int bind_s1d3_fd;
+
+ EXPECT_EQ(0, unlink(file2_s1d3));
+
+ bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_DIRECTORY);
+ ASSERT_GE(bind_s1d3_fd, 0);
+
+ /* Make s1d3 disconnected. */
+ create_directory(_metadata, dir_s4d1);
+ ASSERT_EQ(0, renameat(AT_FDCWD, dir_s1d3, AT_FDCWD, dir_s4d2));
+
+ ASSERT_EQ(0, apply_a_layer(_metadata, &layer));
+
+ ASSERT_EQ(-1, renameat(bind_s1d3_fd, file1_name, bind_s1d3_fd, file2_name));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Disconnected paths are logged as "/". */
+ ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.remove_file,fs\\.make_reg", "/"));
+
+ audit_count_records(self->audit_fd, &records);
+ ASSERT_EQ(0, records.access);
+}
+
TEST_HARNESS_MAIN
--
2.51.1
^ permalink raw reply related
* [PATCH v3 6/8] selftests/landlock: Replace hard-coded 16 with a constant
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
The next commit will reuse this number. Make it a shared constant to
future-proof changes.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- New patch
tools/testing/selftests/landlock/audit_test.c | 2 +-
tools/testing/selftests/landlock/common.h | 2 ++
tools/testing/selftests/landlock/fs_test.c | 2 +-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
index 46d02d49835a..4417cdedeadd 100644
--- a/tools/testing/selftests/landlock/audit_test.c
+++ b/tools/testing/selftests/landlock/audit_test.c
@@ -76,7 +76,7 @@ TEST_F(audit, layers)
.scoped = LANDLOCK_SCOPE_SIGNAL,
};
int status, ruleset_fd, i;
- __u64(*domain_stack)[16];
+ __u64(*domain_stack)[LANDLOCK_MAX_NUM_LAYERS];
__u64 prev_dom = 3;
pid_t child;
diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
index 230b75f6015b..719326f9e8f0 100644
--- a/tools/testing/selftests/landlock/common.h
+++ b/tools/testing/selftests/landlock/common.h
@@ -25,6 +25,8 @@
/* TEST_F_FORK() should not be used for new tests. */
#define TEST_F_FORK(fixture_name, test_name) TEST_F(fixture_name, test_name)
+#define LANDLOCK_MAX_NUM_LAYERS 16
+
static const char bin_sandbox_and_launch[] = "./sandbox-and-launch";
static const char bin_wait_pipe[] = "./wait-pipe";
static const char bin_wait_pipe_sandbox[] = "./wait-pipe-sandbox";
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 21dd95aaf5e4..943b6e2ac53d 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1497,7 +1497,7 @@ TEST_F_FORK(layout0, max_layers)
const int ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
ASSERT_LE(0, ruleset_fd);
- for (i = 0; i < 16; i++)
+ for (i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++)
enforce_ruleset(_metadata, ruleset_fd);
for (i = 0; i < 2; i++) {
--
2.51.1
^ permalink raw reply related
* [PATCH v3 5/8] samples/landlock: Add quiet flag support to sandboxer
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
Adds ability to set which access bits to quiet via LL_*_QUIET_ACCESS (FS,
NET or SCOPED), and attach quiet flags to individual objects via
LL_*_QUIET for FS and NET.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- Minor change to the above commit message.
Changes since v1:
- Added new environment variables to control which quiet access bits to
set on the rule, and populate quiet_access_* from it.
- Added support for quieting net rules and scoped access. Renamed patch
title.
- Increment ABI version
samples/landlock/sandboxer.c | 133 ++++++++++++++++++++++++++++++++---
1 file changed, 124 insertions(+), 9 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208..2d8e3e94b77b 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -58,9 +58,14 @@ static inline int landlock_restrict_self(const int ruleset_fd,
#define ENV_FS_RO_NAME "LL_FS_RO"
#define ENV_FS_RW_NAME "LL_FS_RW"
+#define ENV_FS_QUIET_NAME "LL_FS_QUIET"
+#define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
#define ENV_TCP_BIND_NAME "LL_TCP_BIND"
#define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
+#define ENV_NET_QUIET_NAME "LL_NET_QUIET"
+#define ENV_NET_QUIET_ACCESS_NAME "LL_NET_QUIET_ACCESS"
#define ENV_SCOPED_NAME "LL_SCOPED"
+#define ENV_SCOPED_QUIET_ACCESS_NAME "LL_SCOPED_QUIET_ACCESS"
#define ENV_FORCE_LOG_NAME "LL_FORCE_LOG"
#define ENV_DELIMITER ":"
@@ -116,7 +121,7 @@ static int parse_path(char *env_path, const char ***const path_list)
/* clang-format on */
static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
- const __u64 allowed_access)
+ const __u64 allowed_access, bool quiet)
{
int num_paths, i, ret = 1;
char *env_path_name;
@@ -166,7 +171,8 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
if (!S_ISDIR(statbuf.st_mode))
path_beneath.allowed_access &= ACCESS_FILE;
if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
- &path_beneath, 0)) {
+ &path_beneath,
+ quiet ? LANDLOCK_ADD_RULE_QUIET : 0)) {
fprintf(stderr,
"Failed to update the ruleset with \"%s\": %s\n",
path_list[i], strerror(errno));
@@ -184,7 +190,7 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
}
static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
- const __u64 allowed_access)
+ const __u64 allowed_access, bool quiet)
{
int ret = 1;
char *env_port_name, *env_port_name_next, *strport;
@@ -212,7 +218,8 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
}
net_port.port = port;
if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
- &net_port, 0)) {
+ &net_port,
+ quiet ? LANDLOCK_ADD_RULE_QUIET : 0)) {
fprintf(stderr,
"Failed to update the ruleset with port \"%llu\": %s\n",
net_port.port, strerror(errno));
@@ -299,7 +306,55 @@ static bool check_ruleset_scope(const char *const env_var,
/* clang-format on */
-#define LANDLOCK_ABI_LAST 7
+static int add_quiet_access(__u64 *const quiet_access,
+ const __u64 handled_access,
+ const char *const env_var, const bool default_all)
+{
+ char *env_quiet_access, *env_quiet_access_next, *str_access;
+
+ if (default_all)
+ *quiet_access = handled_access;
+ else
+ *quiet_access = 0;
+
+ env_quiet_access = getenv(env_var);
+ if (!env_quiet_access)
+ return 0;
+
+ env_quiet_access = strdup(env_quiet_access);
+ env_quiet_access_next = env_quiet_access;
+ unsetenv(env_var);
+ *quiet_access = 0;
+
+ while ((str_access = strsep(&env_quiet_access_next, ENV_DELIMITER))) {
+ if (strcmp(str_access, "") == 0)
+ continue;
+ else if (strcmp(str_access, "r") == 0)
+ *quiet_access |= ACCESS_FS_ROUGHLY_READ;
+ else if (strcmp(str_access, "w") == 0)
+ *quiet_access |= ACCESS_FS_ROUGHLY_WRITE;
+ else if (strcmp(str_access, "b") == 0)
+ *quiet_access |= LANDLOCK_ACCESS_NET_BIND_TCP;
+ else if (strcmp(str_access, "c") == 0)
+ *quiet_access |= LANDLOCK_ACCESS_NET_CONNECT_TCP;
+ else if (strcmp(str_access, "a") == 0)
+ *quiet_access |= LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
+ else if (strcmp(str_access, "s") == 0)
+ *quiet_access |= LANDLOCK_SCOPE_SIGNAL;
+ else {
+ fprintf(stderr, "Unknown quiet access \"%s\"\n",
+ str_access);
+ free(env_quiet_access);
+ return -1;
+ }
+ }
+
+ free(env_quiet_access);
+ *quiet_access &= handled_access;
+ return 0;
+}
+
+#define LANDLOCK_ABI_LAST 8
#define XSTR(s) #s
#define STR(s) XSTR(s)
@@ -328,6 +383,20 @@ static const char help[] =
"\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"
+ ENV_FS_QUIET_NAME " and " ENV_NET_QUIET_NAME ", both optional, can then be used "
+ "to make access to some denied paths or network ports not trigger audit logging.\n"
+ ENV_FS_QUIET_ACCESS_NAME " and " ENV_NET_QUIET_ACCESS_NAME " can be used to specify "
+ "which accesses should be quieted (defaults to all):\n"
+ "* " ENV_FS_QUIET_ACCESS_NAME ": file system accesses to quiet\n"
+ " - \"r\" to quiet all file/dir read accesses\n"
+ " - \"w\" to quiet all file/dir write accesses\n"
+ "* " ENV_NET_QUIET_ACCESS_NAME ": network accesses to quiet\n"
+ " - \"b\" to quiet bind denials\n"
+ " - \"c\" to quiet connect denials\n"
+ "In addition, " ENV_SCOPED_QUIET_ACCESS_NAME " can be set to quiet all denials for "
+ "scoped actions (defaults to none).\n"
+ " - \"a\" to quiet abstract unix socket denials\n"
+ " - \"s\" to quiet signal denials\n"
"\n"
"Example:\n"
ENV_FS_RO_NAME "=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
@@ -357,7 +426,12 @@ int main(const int argc, char *const argv[], char *const *const envp)
LANDLOCK_ACCESS_NET_CONNECT_TCP,
.scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
LANDLOCK_SCOPE_SIGNAL,
+ .quiet_access_fs = 0,
+ .quiet_access_net = 0,
+ .quiet_scoped = 0,
};
+
+ bool quiet_supported = true;
int supported_restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
int set_restrict_flags = 0;
@@ -444,6 +518,11 @@ 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:
+ /* Don't add quiet flags for ABI < 8 later on. */
+ quiet_supported = false;
+
+ __attribute__((fallthrough));
case LANDLOCK_ABI_LAST:
break;
default:
@@ -490,6 +569,25 @@ int main(const int argc, char *const argv[], char *const *const envp)
unsetenv(ENV_FORCE_LOG_NAME);
}
+ /*
+ * Add quiet for fs/net handled access bits. Doing this alone has no
+ * effect unless we later add quiet rules per FS_QUIET/NET_QUIET.
+ */
+ if (quiet_supported) {
+ if (add_quiet_access(&ruleset_attr.quiet_access_fs,
+ ruleset_attr.handled_access_fs,
+ ENV_FS_QUIET_ACCESS_NAME, true))
+ return 1;
+ if (add_quiet_access(&ruleset_attr.quiet_access_net,
+ ruleset_attr.handled_access_net,
+ ENV_NET_QUIET_ACCESS_NAME, true))
+ return 1;
+ if (add_quiet_access(&ruleset_attr.quiet_scoped,
+ ruleset_attr.scoped,
+ ENV_SCOPED_QUIET_ACCESS_NAME, false))
+ return 1;
+ }
+
ruleset_fd =
landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
if (ruleset_fd < 0) {
@@ -497,22 +595,39 @@ int main(const int argc, char *const argv[], char *const *const envp)
return 1;
}
- if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro)) {
+ if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro,
+ false)) {
goto err_close_ruleset;
}
- if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw)) {
+ if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw,
+ false)) {
goto err_close_ruleset;
}
+ /* Don't require this env to be present. */
+ if (quiet_supported && getenv(ENV_FS_QUIET_NAME)) {
+ if (populate_ruleset_fs(ENV_FS_QUIET_NAME, ruleset_fd, 0,
+ true)) {
+ goto err_close_ruleset;
+ }
+ }
if (populate_ruleset_net(ENV_TCP_BIND_NAME, ruleset_fd,
- LANDLOCK_ACCESS_NET_BIND_TCP)) {
+ LANDLOCK_ACCESS_NET_BIND_TCP, false)) {
goto err_close_ruleset;
}
if (populate_ruleset_net(ENV_TCP_CONNECT_NAME, ruleset_fd,
- LANDLOCK_ACCESS_NET_CONNECT_TCP)) {
+ LANDLOCK_ACCESS_NET_CONNECT_TCP, false)) {
goto err_close_ruleset;
}
+ /* Don't require this env to be present. */
+ if (quiet_supported && getenv(ENV_NET_QUIET_NAME)) {
+ if (populate_ruleset_net(ENV_NET_QUIET_NAME, ruleset_fd, 0,
+ true)) {
+ goto err_close_ruleset;
+ }
+ }
+
if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
perror("Failed to restrict privileges");
goto err_close_ruleset;
--
2.51.1
^ permalink raw reply related
* [PATCH v3 4/8] landlock: Fix wrong type usage
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
I think, based on my best understanding, that this type is likely a typo
(even though in the end both are u16)
Signed-off-by: Tingmao Wang <m@maowtm.org>
Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
---
Changes since v1:
- Added Fixes tag
security/landlock/audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 505b13c8f4c2..810b1665de0a 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -191,7 +191,7 @@ static size_t get_denied_layer(const struct landlock_ruleset *const domain,
long youngest_layer = -1;
for_each_set_bit(access_bit, &access_req, layer_masks_size) {
- const access_mask_t mask = (*layer_masks)[access_bit];
+ const layer_mask_t mask = (*layer_masks)[access_bit];
long layer;
if (!mask)
--
2.51.1
^ permalink raw reply related
* [PATCH v3 3/8] landlock: Suppress logging when quiet flag is present
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
The quietness behaviour is as documented in the previous patch.
For optional accesses, since the existing deny_masks can only store 2x4bit
of layer index, with no way to represent "no layer", we need to either
expand it or have another field to correctly handle quieting of those.
This commit uses the latter approach - we add another field to store which
optional access (of the 2) are covered by quiet rules in their respective
layers as stored in deny_masks.
We can avoid making struct landlock_file_security larger by converting the
existing fown_layer to a 4bit field. This commit does that, and adds test
to ensure that it is large enough for LANDLOCK_MAX_NUM_LAYERS-1.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- Renamed patch title from "Check for quiet flag in landlock_log_denial"
to this given the growth.
- Moved quiet bit check after domain_exec check
- Rename, style and comment fixes suggested by Mickaël.
- Squashed patch 6/6 from v2 "Implement quiet for optional accesses" into
this one. Changes to that below:
- Refactor the quiet flag setting in get_layer_from_deny_masks() to be
more clear.
- Add KUnit tests
- Fix comments, add WARN_ON_ONCE, use __const_hweight64() as suggested by
review
- Move build_check_file_security to fs.c
- Use a typedef for quiet_optional_accesses, add static_assert, and
improve docs on landlock_get_quiet_optional_accesses.
Changes since v1:
- Supports the new quiet access masks.
- Support quieting scope requests (but not ptrace and attempted mounting
for now)
security/landlock/access.h | 5 +
security/landlock/audit.c | 255 +++++++++++++++++++++++++++++++++---
security/landlock/audit.h | 4 +-
security/landlock/domain.c | 33 +++++
security/landlock/domain.h | 5 +
security/landlock/fs.c | 45 +++++--
security/landlock/fs.h | 17 ++-
security/landlock/net.c | 3 +-
security/landlock/ruleset.h | 5 +
security/landlock/task.c | 12 +-
10 files changed, 344 insertions(+), 40 deletions(-)
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 7961c6630a2d..db47edc88afa 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -97,4 +97,9 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
return access_masks;
}
+/* A bitmask that is large enough to hold set of optional accesses. */
+typedef u8 optional_access_t;
+static_assert(BITS_PER_TYPE(optional_access_t) >=
+ HWEIGHT(_LANDLOCK_ACCESS_FS_OPTIONAL));
+
#endif /* _SECURITY_LANDLOCK_ACCESS_H */
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index c52d079cdb77..505b13c8f4c2 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -273,7 +273,7 @@ static void test_get_denied_layer(struct kunit *const 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)
+ const deny_masks_t deny_masks, u8 quiet_optional_accesses, bool *quiet)
{
const unsigned long access_opt = all_existing_optional_access;
const unsigned long access_req = *access_request;
@@ -285,18 +285,29 @@ get_layer_from_deny_masks(access_mask_t *const access_request,
/* This will require change with new object types. */
WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
+ *quiet = false;
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);
+ const bool layer_has_quiet =
+ !!(quiet_optional_accesses & BIT(access_index));
if (layer > youngest_layer) {
youngest_layer = layer;
+ *quiet = layer_has_quiet;
missing = BIT(access_bit);
} else if (layer == youngest_layer) {
missing |= BIT(access_bit);
+ /*
+ * Whether the layer has rules with quiet flag covering
+ * the file accessed does not depend on the access, and so
+ * if this fails, quiet_optional_accesses is corrupted.
+ */
+ WARN_ON_ONCE(*quiet && !layer_has_quiet);
+ *quiet = layer_has_quiet;
}
}
access_index++;
@@ -312,42 +323,188 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
{
deny_masks_t deny_mask;
access_mask_t access;
+ u8 quiet_optional_accesses;
+ bool quiet;
/* truncate:0 ioctl_dev:2 */
deny_mask = 0x20;
+ quiet_optional_accesses = 0;
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_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ access = LANDLOCK_ACCESS_FS_IOCTL_DEV;
+ KUNIT_EXPECT_EQ(test, 2,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ 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, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ /* layer denying truncate: quiet, ioctl: not quiet */
+ quiet_optional_accesses = 0b01;
+
+ access = LANDLOCK_ACCESS_FS_TRUNCATE;
+ KUNIT_EXPECT_EQ(test, 0,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, true);
+
+ access = LANDLOCK_ACCESS_FS_IOCTL_DEV;
+ KUNIT_EXPECT_EQ(test, 2,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ 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, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ /* Reverse order - truncate:2 ioctl_dev:0 */
+ deny_mask = 0x02;
+ quiet_optional_accesses = 0;
+
+ access = LANDLOCK_ACCESS_FS_TRUNCATE;
+ KUNIT_EXPECT_EQ(test, 2,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ access = LANDLOCK_ACCESS_FS_IOCTL_DEV;
+ KUNIT_EXPECT_EQ(test, 0,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ 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, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ /* layer denying truncate: quiet, ioctl: not quiet */
+ quiet_optional_accesses = 0b01;
+
+ access = LANDLOCK_ACCESS_FS_TRUNCATE;
+ KUNIT_EXPECT_EQ(test, 2,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, true);
+
+ access = LANDLOCK_ACCESS_FS_IOCTL_DEV;
+ KUNIT_EXPECT_EQ(test, 0,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, false);
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_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, true);
+
+ /* layer denying truncate: not quiet, ioctl: quiet */
+ quiet_optional_accesses = 0b10;
+
+ access = LANDLOCK_ACCESS_FS_TRUNCATE;
+ KUNIT_EXPECT_EQ(test, 2,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ access = LANDLOCK_ACCESS_FS_IOCTL_DEV;
+ KUNIT_EXPECT_EQ(test, 0,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, true);
+
+ 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, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, false);
/* truncate:15 ioctl_dev:15 */
deny_mask = 0xff;
+ quiet_optional_accesses = 0;
+
+ access = LANDLOCK_ACCESS_FS_TRUNCATE;
+ KUNIT_EXPECT_EQ(test, 15,
+ get_layer_from_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ 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, quiet_optional_accesses, &quiet));
+ KUNIT_EXPECT_EQ(test, access,
+ LANDLOCK_ACCESS_FS_TRUNCATE |
+ LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, false);
+
+ /* Both quiet (same layer so quietness must be the same) */
+ quiet_optional_accesses = 0b11;
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_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+ KUNIT_EXPECT_EQ(test, quiet, true);
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_deny_masks(
+ &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
+ deny_mask, quiet_optional_accesses, &quiet));
KUNIT_EXPECT_EQ(test, access,
LANDLOCK_ACCESS_FS_TRUNCATE |
LANDLOCK_ACCESS_FS_IOCTL_DEV);
+ KUNIT_EXPECT_EQ(test, quiet, true);
}
#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
@@ -381,19 +538,39 @@ static bool is_valid_request(const struct landlock_request *const request)
return true;
}
+static access_mask_t
+pick_access_mask_for_request_type(const enum landlock_request_type type,
+ const struct access_masks access_masks)
+{
+ switch (type) {
+ case LANDLOCK_REQUEST_FS_ACCESS:
+ return access_masks.fs;
+ case LANDLOCK_REQUEST_NET_ACCESS:
+ return access_masks.net;
+ default:
+ WARN_ONCE(1, "Invalid request type %d passed to %s", type,
+ __func__);
+ return 0;
+ }
+}
+
/**
* landlock_log_denial - Create audit records related to a denial
*
* @subject: The Landlock subject's credential denying an action.
* @request: Detail of the user space request.
+ * @rule_flags: The flags for the matched rule, or no_rule_flags (zero) if
+ * this is a scope request (no particular object involved).
*/
void landlock_log_denial(const struct landlock_cred_security *const subject,
- const struct landlock_request *const request)
+ const struct landlock_request *const request,
+ const struct collected_rule_flags rule_flags)
{
struct audit_buffer *ab;
struct landlock_hierarchy *youngest_denied;
size_t youngest_layer;
- access_mask_t missing;
+ access_mask_t missing, quiet_mask;
+ bool object_quiet_flag = false, quiet_applicable_to_access = false;
if (WARN_ON_ONCE(!subject || !subject->domain ||
!subject->domain->hierarchy || !request))
@@ -409,10 +586,13 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
youngest_layer = get_denied_layer(
subject->domain, &missing, request->layer_masks,
request->layer_masks_size);
+ object_quiet_flag = !!(rule_flags.quiet_masks & BIT(youngest_layer));
} else {
youngest_layer = get_layer_from_deny_masks(
&missing, request->all_existing_optional_access,
- request->deny_masks);
+ request->deny_masks,
+ request->quiet_optional_accesses,
+ &object_quiet_flag);
}
youngest_denied =
get_hierarchy(subject->domain, youngest_layer);
@@ -447,6 +627,49 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
return;
}
+ /*
+ * Checks if the object is marked quiet by the layer that denied the
+ * request. If it's a different layer that marked it as quiet, but
+ * that layer is not the one that denied the request, we should still
+ * audit log the denial.
+ */
+ if (object_quiet_flag) {
+ /*
+ * We now check if the denied requests are all covered by the
+ * layer's quiet access bits.
+ */
+ quiet_mask = pick_access_mask_for_request_type(
+ request->type, youngest_denied->quiet_masks);
+ quiet_applicable_to_access = (quiet_mask & missing) == missing;
+ } else {
+ /*
+ * Either the object is not quiet, or this is a scope request. We
+ * check request->type to distinguish between the two cases.
+ */
+ quiet_mask = youngest_denied->quiet_masks.scope;
+ switch (request->type) {
+ case LANDLOCK_REQUEST_SCOPE_SIGNAL:
+ quiet_applicable_to_access =
+ !!(quiet_mask & LANDLOCK_SCOPE_SIGNAL);
+ break;
+ case LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET:
+ quiet_applicable_to_access =
+ !!(quiet_mask &
+ LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
+ break;
+ /*
+ * Leave LANDLOCK_REQUEST_PTRACE and
+ * LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY unhandled for now - they are
+ * never quiet.
+ */
+ default:
+ break;
+ }
+ }
+
+ if (quiet_applicable_to_access)
+ return;
+
/* Uses consistent allocation flags wrt common_lsm_audit(). */
ab = audit_log_start(audit_context(), GFP_ATOMIC | __GFP_NOWARN,
AUDIT_LANDLOCK_ACCESS);
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d8..950365cd223d 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -49,6 +49,7 @@ struct landlock_request {
/* Required fields for requests with deny masks. */
const access_mask_t all_existing_optional_access;
deny_masks_t deny_masks;
+ u8 quiet_optional_accesses;
};
#ifdef CONFIG_AUDIT
@@ -56,7 +57,8 @@ struct landlock_request {
void landlock_log_drop_domain(const struct landlock_hierarchy *const hierarchy);
void landlock_log_denial(const struct landlock_cred_security *const subject,
- const struct landlock_request *const request);
+ const struct landlock_request *const request,
+ const struct collected_rule_flags rule_flags);
#else /* CONFIG_AUDIT */
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index a647b68e8d06..8caf07250328 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -212,6 +212,39 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
return deny_masks;
}
+/**
+ * landlock_get_quiet_optional_accesses - Get optional accesses which are
+ * "covered" by quiet rule flags.
+ *
+ * Returns a bitmask of which optional access are denied by layers for
+ * which rule_flags.quiet_masks has the corresponding bit set.
+ */
+optional_access_t landlock_get_quiet_optional_accesses(
+ const access_mask_t all_existing_optional_access,
+ const deny_masks_t deny_masks,
+ const struct collected_rule_flags rule_flags)
+{
+ const unsigned long access_opt = all_existing_optional_access;
+ size_t access_index = 0;
+ unsigned long access_bit;
+ optional_access_t quiet_optional_accesses = 0;
+
+ /* This will require change with new object types. */
+ WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
+
+ for_each_set_bit(access_bit, &access_opt,
+ BITS_PER_TYPE(access_mask_t)) {
+ const u8 layer = (deny_masks >> (access_index * 4)) &
+ (LANDLOCK_MAX_NUM_LAYERS - 1);
+ const bool is_quiet = !!(rule_flags.quiet_masks & BIT(layer));
+
+ if (is_quiet)
+ quiet_optional_accesses |= BIT(access_index);
+ access_index++;
+ }
+ return quiet_optional_accesses;
+}
+
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
static void test_landlock_get_deny_masks(struct kunit *const test)
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index aadbf53505c0..ea487315580a 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -130,6 +130,11 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
const layer_mask_t (*const layer_masks)[],
size_t layer_masks_size);
+optional_access_t landlock_get_quiet_optional_accesses(
+ const access_mask_t all_existing_optional_access,
+ const deny_masks_t deny_masks,
+ const struct collected_rule_flags rule_flags);
+
int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
static inline void
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 13195b6ffed7..15b9da835eb5 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1105,7 +1105,7 @@ static int current_check_access_path(const struct path *const path,
NULL, 0, NULL, NULL, NULL, NULL))
return 0;
- landlock_log_denial(subject, &request);
+ landlock_log_denial(subject, &request, rule_flags);
return -EACCES;
}
@@ -1339,7 +1339,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
&request1, NULL, 0, NULL, NULL, NULL, NULL))
return 0;
- landlock_log_denial(subject, &request1);
+ landlock_log_denial(subject, &request1, rule_flags_parent1);
return -EACCES;
}
@@ -1387,11 +1387,11 @@ static int current_check_refer_path(struct dentry *const old_dentry,
if (request1.access) {
request1.audit.u.path.dentry = old_parent;
- landlock_log_denial(subject, &request1);
+ landlock_log_denial(subject, &request1, rule_flags_parent1);
}
if (request2.access) {
request2.audit.u.path.dentry = new_dir->dentry;
- landlock_log_denial(subject, &request2);
+ landlock_log_denial(subject, &request2, rule_flags_parent2);
}
/*
@@ -1547,7 +1547,7 @@ log_fs_change_topology_path(const struct landlock_cred_security *const subject,
.u.path = *path,
},
.layer_plus_one = handle_layer + 1,
- });
+ }, no_rule_flags);
}
static void log_fs_change_topology_dentry(
@@ -1561,7 +1561,7 @@ static void log_fs_change_topology_dentry(
.u.dentry = dentry,
},
.layer_plus_one = handle_layer + 1,
- });
+ }, no_rule_flags);
}
/*
@@ -1755,8 +1755,29 @@ get_required_file_open_access(const struct file *const file)
return access;
}
+static void build_check_file_security(void)
+{
+ const struct landlock_file_security file_sec = {
+ .quiet_optional_accesses = ~0,
+ .fown_layer = ~0,
+ };
+
+ /*
+ * Make sure quiet_optional_accesses has enough bits to cover all
+ * optional accesses. The use of __const_hweight64() rather than
+ * HWEIGHT() is due to GCC erroring about non-constants in
+ * BUILD_BUG_ON call when using the latter, and the use of the 64bit
+ * version is for future-proofing.
+ */
+ BUILD_BUG_ON(__const_hweight64((u64)file_sec.quiet_optional_accesses) <
+ __const_hweight64(_LANDLOCK_ACCESS_FS_OPTIONAL));
+ /* Makes sure all layers can be identified. */
+ BUILD_BUG_ON(file_sec.fown_layer < LANDLOCK_MAX_NUM_LAYERS - 1);
+}
+
static int hook_file_alloc_security(struct file *const file)
{
+ build_check_file_security();
/*
* Grants all access rights, even if most of them are not checked later
* on. It is more consistent.
@@ -1842,6 +1863,10 @@ static int hook_file_open(struct file *const file)
landlock_file(file)->deny_masks = landlock_get_deny_masks(
_LANDLOCK_ACCESS_FS_OPTIONAL, optional_access, &layer_masks,
ARRAY_SIZE(layer_masks));
+ landlock_file(file)->quiet_optional_accesses =
+ landlock_get_quiet_optional_accesses(
+ _LANDLOCK_ACCESS_FS_OPTIONAL,
+ landlock_file(file)->deny_masks, rule_flags);
#endif /* CONFIG_AUDIT */
if ((open_access_request & allowed_access) == open_access_request)
@@ -1849,7 +1874,7 @@ static int hook_file_open(struct file *const file)
/* Sets access to reflect the actual request. */
request.access = open_access_request;
- landlock_log_denial(subject, &request);
+ landlock_log_denial(subject, &request, rule_flags);
return -EACCES;
}
@@ -1878,8 +1903,9 @@ static int hook_file_truncate(struct file *const file)
.access = LANDLOCK_ACCESS_FS_TRUNCATE,
#ifdef CONFIG_AUDIT
.deny_masks = landlock_file(file)->deny_masks,
+ .quiet_optional_accesses = landlock_file(file)->quiet_optional_accesses,
#endif /* CONFIG_AUDIT */
- });
+ }, no_rule_flags);
return -EACCES;
}
@@ -1917,8 +1943,9 @@ static int hook_file_ioctl_common(const struct file *const file,
.access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
#ifdef CONFIG_AUDIT
.deny_masks = landlock_file(file)->deny_masks,
+ .quiet_optional_accesses = landlock_file(file)->quiet_optional_accesses,
#endif /* CONFIG_AUDIT */
- });
+ }, no_rule_flags);
return -EACCES;
}
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index cb7e654933ac..ac6e50216f87 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -63,11 +63,20 @@ struct landlock_file_security {
* _LANDLOCK_ACCESS_FS_OPTIONAL).
*/
deny_masks_t deny_masks;
+ /**
+ * @quiet_optional_accesses: Stores which optional accesses are
+ * covered by quiet rules within the layer referred to in deny_masks,
+ * one access per bit. Does not take into account whether the quiet
+ * access bits are actually set in the layer's corresponding
+ * landlock_hierarchy.
+ */
+ optional_access_t quiet_optional_accesses
+ : HWEIGHT(_LANDLOCK_ACCESS_FS_OPTIONAL);
/**
* @fown_layer: Layer level of @fown_subject->domain with
* LANDLOCK_SCOPE_SIGNAL.
*/
- u8 fown_layer;
+ u8 fown_layer:4;
#endif /* CONFIG_AUDIT */
/**
@@ -82,12 +91,6 @@ struct landlock_file_security {
#ifdef CONFIG_AUDIT
-/* Makes sure all layers can be identified. */
-/* clang-format off */
-static_assert((typeof_member(struct landlock_file_security, fown_layer))~0 >=
- LANDLOCK_MAX_NUM_LAYERS);
-/* clang-format off */
-
#endif /* CONFIG_AUDIT */
/**
diff --git a/security/landlock/net.c b/security/landlock/net.c
index bddbe93d69fd..0587aa3d6d0f 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -193,7 +193,8 @@ static int current_check_access_socket(struct socket *const sock,
.access = access_request,
.layer_masks = &layer_masks,
.layer_masks_size = ARRAY_SIZE(layer_masks),
- });
+ },
+ rule_flags);
return -EACCES;
}
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 57fa2e18a483..cd0434d8dc63 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -58,6 +58,11 @@ struct collected_rule_flags {
layer_mask_t quiet_masks;
};
+/**
+ * no_rule_flags - Convenience constant for an empty collected_rule_flags.
+ */
+static const struct collected_rule_flags no_rule_flags = {};
+
/**
* union landlock_key - Key of a ruleset's red-black tree
*/
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 2385017418ca..d5bd9a1b8467 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -115,7 +115,7 @@ static int hook_ptrace_access_check(struct task_struct *const child,
.u.tsk = child,
},
.layer_plus_one = parent_subject->domain->num_layers,
- });
+ }, no_rule_flags);
return err;
}
@@ -161,7 +161,7 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
.u.tsk = current,
},
.layer_plus_one = parent_subject->domain->num_layers,
- });
+ }, no_rule_flags);
return err;
}
@@ -290,7 +290,7 @@ static int hook_unix_stream_connect(struct sock *const sock,
},
},
.layer_plus_one = handle_layer + 1,
- });
+ }, no_rule_flags);
return -EPERM;
}
@@ -327,7 +327,7 @@ static int hook_unix_may_send(struct socket *const sock,
},
},
.layer_plus_one = handle_layer + 1,
- });
+ }, no_rule_flags);
return -EPERM;
}
@@ -383,7 +383,7 @@ static int hook_task_kill(struct task_struct *const p,
.u.tsk = p,
},
.layer_plus_one = handle_layer + 1,
- });
+ }, no_rule_flags);
return -EPERM;
}
@@ -426,7 +426,7 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
#ifdef CONFIG_AUDIT
.layer_plus_one = landlock_file(fown->file)->fown_layer + 1,
#endif /* CONFIG_AUDIT */
- });
+ }, no_rule_flags);
return -EPERM;
}
--
2.51.1
^ permalink raw reply related
* [PATCH v3 2/8] landlock: Add API support and docs for the quiet flags
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
As for kselftests, for now we just change add_rule_checks_ordering to use
a different invalid flag. I will add tests for the quiet flag in a later
version.
According to pahole, even after adding the struct access_masks quiet_masks
in struct landlock_hierarchy, the u32 log_* bitfield still only has a size
of 2 bytes, so there's minimal wasted space.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- Updated docs from Mickaël's suggestions.
Changes since v1:
- Per suggestion, added support for quieting only certain access bits,
controlled by extra quiet_access_* fields in the ruleset_attr.
- Added docs for the extra fields and made updates to doc changes in v1.
In particular, call out that the effect of LANDLOCK_ADD_RULE_QUIET is
independent from the access bits passed in rule_attr
- landlock_add_rule will return -EINVAL when LANDLOCK_ADD_RULE_QUIET is
used but the ruleset does not have any quiet access bits set for the
given rule type.
- ABI version bump to v8
- Syntactic and comment changes per suggestion.
include/uapi/linux/landlock.h | 64 +++++++++++++++++
security/landlock/domain.h | 5 ++
security/landlock/fs.c | 4 +-
security/landlock/fs.h | 2 +-
security/landlock/net.c | 5 +-
security/landlock/net.h | 3 +-
security/landlock/ruleset.c | 10 ++-
security/landlock/ruleset.h | 8 ++-
security/landlock/syscalls.c | 72 +++++++++++++++-----
tools/testing/selftests/landlock/base_test.c | 4 +-
10 files changed, 150 insertions(+), 27 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f030adc462ee..50f0806b7e33 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -32,6 +32,19 @@
* *handle* a wide range or all access rights that they know about at build time
* (and that they have tested with a kernel that supported them all).
*
+ * @quiet_access_fs and @quiet_access_net are bitmasks of actions for
+ * which a denial by this layer will not trigger an audit log if the
+ * corresponding object (or its children, for filesystem rules) is marked
+ * with the "quiet" bit via %LANDLOCK_ADD_RULE_QUIET, even if logging
+ * would normally take place per landlock_restrict_self() flags.
+ * quiet_scoped is similar, except that it does not require marking any
+ * objects as quiet - if the ruleset is created with any bits set in
+ * quiet_scoped, then denial of such scoped resources will not trigger any
+ * log. These 3 fields are available since Landlock ABI version 8.
+ *
+ * @quiet_access_fs, @quiet_access_net and @quiet_scoped must be a subset
+ * of @handled_access_fs, @handled_access_net and @scoped respectively.
+ *
* This structure can grow in future Landlock versions.
*/
struct landlock_ruleset_attr {
@@ -51,6 +64,24 @@ struct landlock_ruleset_attr {
* resources (e.g. IPCs).
*/
__u64 scoped;
+
+ /* Since ABI 8: */
+
+ /**
+ * @quiet_access_fs: Bitmask of filesystem actions which should not be
+ * audit logged if per-object quiet flag is set.
+ */
+ __u64 quiet_access_fs;
+ /**
+ * @quiet_access_net: Bitmask of network actions which should not be
+ * audit logged if per-object quiet flag is set.
+ */
+ __u64 quiet_access_net;
+ /**
+ * @quiet_scoped: Bitmask of scoped actions which should not be audit
+ * logged.
+ */
+ __u64 quiet_scoped;
};
/**
@@ -69,6 +100,39 @@ struct landlock_ruleset_attr {
#define LANDLOCK_CREATE_RULESET_ERRATA (1U << 1)
/* clang-format on */
+/**
+ * DOC: landlock_add_rule_flags
+ *
+ * **Flags**
+ *
+ * %LANDLOCK_ADD_RULE_QUIET
+ * Together with the quiet_* fields in struct landlock_ruleset_attr,
+ * this flag controls whether Landlock will log audit messages when
+ * access to the objects covered by this rule is denied by this layer.
+ *
+ * If audit logging is enabled, when Landlock denies an access, it will
+ * suppress the audit log if all of the following are true:
+ *
+ * - This layer is the innermost layer that denied the access;
+ * - All requested accesses are part of the quiet_* fields in the
+ * related struct landlock_ruleset_attr;
+ * - The object (or one of its parents, for filesystem rules) is
+ * marked as "quiet" via %LANDLOCK_ADD_RULE_QUIET.
+ *
+ * Because logging is only suppressed by a layer if the layer denies
+ * access, a sandboxed program cannot use this flag to "hide" access
+ * denials, without denying itself the access in the first place.
+ *
+ * The effect of this flag does not depend on the value of
+ * allowed_access in the passed in rule_attr. When this flag is
+ * present, the caller is also allowed to pass in an empty
+ * allowed_access.
+ */
+
+/* clang-format off */
+#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
+/* clang-format on */
+
/**
* DOC: landlock_restrict_self_flags
*
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 7fb70b25f85a..aadbf53505c0 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -114,6 +114,11 @@ struct landlock_hierarchy {
* %LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON. Set to false by default.
*/
log_new_exec : 1;
+ /**
+ * @quiet_masks: Bitmasks of access that should be quieted (i.e. not
+ * logged) if the related object is marked as quiet.
+ */
+ struct access_masks quiet_masks;
#endif /* CONFIG_AUDIT */
};
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 721cd63a4de8..13195b6ffed7 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -322,7 +322,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
*/
int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
const struct path *const path,
- access_mask_t access_rights)
+ access_mask_t access_rights, const int flags)
{
int err;
struct landlock_id id = {
@@ -343,7 +343,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
if (IS_ERR(id.key.object))
return PTR_ERR(id.key.object);
mutex_lock(&ruleset->lock);
- err = landlock_insert_rule(ruleset, id, access_rights);
+ err = landlock_insert_rule(ruleset, id, access_rights, flags);
mutex_unlock(&ruleset->lock);
/*
* No need to check for an error because landlock_insert_rule()
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index bf9948941f2f..cb7e654933ac 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -126,6 +126,6 @@ __init void landlock_add_fs_hooks(void);
int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
const struct path *const path,
- access_mask_t access_hierarchy);
+ access_mask_t access_hierarchy, const int flags);
#endif /* _SECURITY_LANDLOCK_FS_H */
diff --git a/security/landlock/net.c b/security/landlock/net.c
index fc6369dffa51..bddbe93d69fd 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -20,7 +20,8 @@
#include "ruleset.h"
int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
- const u16 port, access_mask_t access_rights)
+ const u16 port, access_mask_t access_rights,
+ const int flags)
{
int err;
const struct landlock_id id = {
@@ -35,7 +36,7 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
~landlock_get_net_access_mask(ruleset, 0);
mutex_lock(&ruleset->lock);
- err = landlock_insert_rule(ruleset, id, access_rights);
+ err = landlock_insert_rule(ruleset, id, access_rights, flags);
mutex_unlock(&ruleset->lock);
return err;
diff --git a/security/landlock/net.h b/security/landlock/net.h
index 09960c237a13..799cedd5d0b7 100644
--- a/security/landlock/net.h
+++ b/security/landlock/net.h
@@ -16,7 +16,8 @@
__init void landlock_add_net_hooks(void);
int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
- const u16 port, access_mask_t access_rights);
+ const u16 port, access_mask_t access_rights,
+ const int flags);
#else /* IS_ENABLED(CONFIG_INET) */
static inline void landlock_add_net_hooks(void)
{
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 81cdf87d1c79..e0bb7e795574 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -21,6 +21,7 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
+#include <uapi/linux/landlock.h>
#include "access.h"
#include "audit.h"
@@ -255,6 +256,7 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
if (WARN_ON_ONCE(this->layers[0].level != 0))
return -EINVAL;
this->layers[0].access |= (*layers)[0].access;
+ this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
return 0;
}
@@ -305,12 +307,15 @@ static void build_check_layer(void)
/* @ruleset must be locked by the caller. */
int landlock_insert_rule(struct landlock_ruleset *const ruleset,
const struct landlock_id id,
- const access_mask_t access)
+ const access_mask_t access, const int flags)
{
struct landlock_layer layers[] = { {
.access = access,
/* When @level is zero, insert_rule() extends @ruleset. */
.level = 0,
+ .flags = {
+ .quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
+ }
} };
build_check_layer();
@@ -351,6 +356,7 @@ static int merge_tree(struct landlock_ruleset *const dst,
return -EINVAL;
layers[0].access = walker_rule->layers[0].access;
+ layers[0].flags = walker_rule->layers[0].flags;
err = insert_rule(dst, id, &layers, ARRAY_SIZE(layers));
if (err)
@@ -581,6 +587,8 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
if (err)
return ERR_PTR(err);
+ new_dom->hierarchy->quiet_masks = ruleset->quiet_masks;
+
return no_free_ptr(new_dom);
}
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 9790c60c0c00..57fa2e18a483 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -193,6 +193,12 @@ struct landlock_ruleset {
* non-merged ruleset (i.e. not a domain).
*/
u32 num_layers;
+ /**
+ * @quiet_masks: Stores the quiet flags for an unmerged
+ * ruleset. For a merged domain, this is stored in each
+ * layer's struct landlock_hierarchy instead.
+ */
+ struct access_masks quiet_masks;
/**
* @access_masks: Contains the subset of filesystem and
* network actions that are restricted by a ruleset.
@@ -223,7 +229,7 @@ DEFINE_FREE(landlock_put_ruleset, struct landlock_ruleset *,
int landlock_insert_rule(struct landlock_ruleset *const ruleset,
const struct landlock_id id,
- const access_mask_t access);
+ const access_mask_t access, const int flags);
struct landlock_ruleset *
landlock_merge_ruleset(struct landlock_ruleset *const parent,
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 0116e9f93ffe..93396bfc1500 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -102,8 +102,11 @@ static void build_check_abi(void)
ruleset_size = sizeof(ruleset_attr.handled_access_fs);
ruleset_size += sizeof(ruleset_attr.handled_access_net);
ruleset_size += sizeof(ruleset_attr.scoped);
+ ruleset_size += sizeof(ruleset_attr.quiet_access_fs);
+ ruleset_size += sizeof(ruleset_attr.quiet_access_net);
+ ruleset_size += sizeof(ruleset_attr.quiet_scoped);
BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
- BUILD_BUG_ON(sizeof(ruleset_attr) != 24);
+ BUILD_BUG_ON(sizeof(ruleset_attr) != 48);
path_beneath_size = sizeof(path_beneath_attr.allowed_access);
path_beneath_size += sizeof(path_beneath_attr.parent_fd);
@@ -161,7 +164,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
@@ -185,6 +188,8 @@ const int landlock_abi_version = 7;
*
* - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
* - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small @size;
+ * - %EINVAL: quiet_access_fs or quiet_fs_net is not a subset of the
+ * corresponding handled_access_fs or handled_access_net;
* - %E2BIG: @attr or @size inconsistencies;
* - %EFAULT: @attr or @size inconsistencies;
* - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
@@ -241,6 +246,21 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
if ((ruleset_attr.scoped | LANDLOCK_MASK_SCOPE) != LANDLOCK_MASK_SCOPE)
return -EINVAL;
+ /*
+ * Check that quiet masks are subsets of the respective handled masks.
+ * Because of the checks above this is sufficient to also ensure that
+ * the quiet masks are valid access masks.
+ */
+ if ((ruleset_attr.quiet_access_fs | ruleset_attr.handled_access_fs) !=
+ ruleset_attr.handled_access_fs)
+ return -EINVAL;
+ if ((ruleset_attr.quiet_access_net | ruleset_attr.handled_access_net) !=
+ ruleset_attr.handled_access_net)
+ return -EINVAL;
+ if ((ruleset_attr.quiet_scoped | ruleset_attr.scoped) !=
+ ruleset_attr.scoped)
+ return -EINVAL;
+
/* Checks arguments and transforms to kernel struct. */
ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs,
ruleset_attr.handled_access_net,
@@ -248,6 +268,10 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
if (IS_ERR(ruleset))
return PTR_ERR(ruleset);
+ ruleset->quiet_masks.fs = ruleset_attr.quiet_access_fs;
+ ruleset->quiet_masks.net = ruleset_attr.quiet_access_net;
+ ruleset->quiet_masks.scope = ruleset_attr.quiet_scoped;
+
/* Creates anonymous FD referring to the ruleset. */
ruleset_fd = anon_inode_getfd("[landlock-ruleset]", &ruleset_fops,
ruleset, O_RDWR | O_CLOEXEC);
@@ -312,7 +336,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
}
static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
- const void __user *const rule_attr)
+ const void __user *const rule_attr, int flags)
{
struct landlock_path_beneath_attr path_beneath_attr;
struct path path;
@@ -327,9 +351,10 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
/*
* Informs about useless rule: empty allowed_access (i.e. deny rules)
- * are ignored in path walks.
+ * are ignored in path walks. However, the rule is not useless if it
+ * is there to hold a quiet flag
*/
- if (!path_beneath_attr.allowed_access)
+ if (!flags && !path_beneath_attr.allowed_access)
return -ENOMSG;
/* Checks that allowed_access matches the @ruleset constraints. */
@@ -337,6 +362,10 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
if ((path_beneath_attr.allowed_access | mask) != mask)
return -EINVAL;
+ /* Check for useless quiet flag. */
+ if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.fs)
+ return -EINVAL;
+
/* Gets and checks the new rule. */
err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
if (err)
@@ -344,13 +373,13 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
/* Imports the new rule. */
err = landlock_append_fs_rule(ruleset, &path,
- path_beneath_attr.allowed_access);
+ path_beneath_attr.allowed_access, flags);
path_put(&path);
return err;
}
static int add_rule_net_port(struct landlock_ruleset *ruleset,
- const void __user *const rule_attr)
+ const void __user *const rule_attr, int flags)
{
struct landlock_net_port_attr net_port_attr;
int res;
@@ -363,9 +392,10 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
/*
* Informs about useless rule: empty allowed_access (i.e. deny rules)
- * are ignored by network actions.
+ * are ignored by network actions. However, the rule is not useless
+ * if it is there to hold a quiet flag
*/
- if (!net_port_attr.allowed_access)
+ if (!flags && !net_port_attr.allowed_access)
return -ENOMSG;
/* Checks that allowed_access matches the @ruleset constraints. */
@@ -373,13 +403,17 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
if ((net_port_attr.allowed_access | mask) != mask)
return -EINVAL;
+ /* Check for useless quiet flag. */
+ if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.net)
+ return -EINVAL;
+
/* Denies inserting a rule with port greater than 65535. */
if (net_port_attr.port > U16_MAX)
return -EINVAL;
/* Imports the new rule. */
return landlock_append_net_rule(ruleset, net_port_attr.port,
- net_port_attr.allowed_access);
+ net_port_attr.allowed_access, flags);
}
/**
@@ -390,7 +424,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
* @rule_type: Identify the structure type pointed to by @rule_attr:
* %LANDLOCK_RULE_PATH_BENEATH or %LANDLOCK_RULE_NET_PORT.
* @rule_attr: Pointer to a rule (matching the @rule_type).
- * @flags: Must be 0.
+ * @flags: Must be 0 or %LANDLOCK_ADD_RULE_QUIET.
*
* This system call enables to define a new rule and add it to an existing
* ruleset.
@@ -400,20 +434,25 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
* - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
* - %EAFNOSUPPORT: @rule_type is %LANDLOCK_RULE_NET_PORT but TCP/IP is not
* supported by the running kernel;
- * - %EINVAL: @flags is not 0;
+ * - %EINVAL: @flags is not valid;
* - %EINVAL: The rule accesses are inconsistent (i.e.
* &landlock_path_beneath_attr.allowed_access or
* &landlock_net_port_attr.allowed_access is not a subset of the ruleset
* handled accesses)
* - %EINVAL: &landlock_net_port_attr.port is greater than 65535;
+ * - %EINVAL: LANDLOCK_ADD_RULE_QUIET is passed but the ruleset has no
+ * quiet access bits set for the corresponding rule type.
* - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access is
- * 0);
+ * 0) and no flags;
* - %EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
* member of @rule_attr is not a file descriptor as expected;
* - %EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
* @rule_attr is not the expected file descriptor type;
* - %EPERM: @ruleset_fd has no write access to the underlying ruleset;
* - %EFAULT: @rule_attr was not a valid address.
+ *
+ * .. kernel-doc:: include/uapi/linux/landlock.h
+ * :identifiers: landlock_add_rule_flags
*/
SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
const enum landlock_rule_type, rule_type,
@@ -424,8 +463,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
if (!is_initialized())
return -EOPNOTSUPP;
- /* No flag for now. */
- if (flags)
+ if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
return -EINVAL;
/* Gets and checks the ruleset. */
@@ -435,9 +473,9 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
switch (rule_type) {
case LANDLOCK_RULE_PATH_BENEATH:
- return add_rule_path_beneath(ruleset, rule_attr);
+ return add_rule_path_beneath(ruleset, rule_attr, flags);
case LANDLOCK_RULE_NET_PORT:
- return add_rule_net_port(ruleset, rule_attr);
+ return add_rule_net_port(ruleset, rule_attr, flags);
default:
return -EINVAL;
}
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 7b69002239d7..b34b340c52a5 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,
@@ -201,7 +201,7 @@ TEST(add_rule_checks_ordering)
ASSERT_LE(0, ruleset_fd);
/* Checks invalid flags. */
- ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 1));
+ ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 100));
ASSERT_EQ(EINVAL, errno);
/* Checks invalid ruleset FD. */
--
2.51.1
^ permalink raw reply related
* [PATCH v3 1/8] landlock: Add a place for flags to layer rules
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
To avoid unnecessarily increasing the size of struct landlock_layer, we
make the layer level a u8 and use the space to store the flags struct.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- Comment changes, move local variables, simplify if branch
Changes since v1:
- Comment changes
- Rebased to include disconnected directory handling changes on mic/next
and add backing up of collected_rule_flags.
security/landlock/fs.c | 95 ++++++++++++++++++++++++++-----------
security/landlock/net.c | 3 +-
security/landlock/ruleset.c | 7 ++-
security/landlock/ruleset.h | 26 +++++++++-
4 files changed, 99 insertions(+), 32 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 4ed997f4a663..721cd63a4de8 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -757,10 +757,12 @@ static bool is_access_to_paths_allowed(
const struct path *const path,
const access_mask_t access_request_parent1,
layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
+ struct collected_rule_flags *const rule_flags_parent1,
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 collected_rule_flags *const rule_flags_parent2,
struct landlock_request *const log_request_parent2,
struct dentry *const dentry_child2)
{
@@ -775,6 +777,7 @@ static bool is_access_to_paths_allowed(
_layer_masks_parent2_bkp[LANDLOCK_NUM_ACCESS_FS];
layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
(*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
+ struct collected_rule_flags _rule_flag_parent1_bkp, _rule_flag_parent2_bkp;
if (!access_request_parent1 && !access_request_parent2)
return true;
@@ -800,6 +803,8 @@ static bool is_access_to_paths_allowed(
*/
memcpy(&_layer_masks_parent2_bkp, layer_masks_parent2,
sizeof(_layer_masks_parent2_bkp));
+ memcpy(&_rule_flag_parent2_bkp, rule_flags_parent2,
+ sizeof(_rule_flag_parent2_bkp));
allowed_parent2 = is_layer_masks_allowed(layer_masks_parent2);
/*
@@ -826,26 +831,38 @@ static bool is_access_to_paths_allowed(
*/
memcpy(&_layer_masks_parent1_bkp, layer_masks_parent1,
sizeof(_layer_masks_parent1_bkp));
+ memcpy(&_rule_flag_parent1_bkp, rule_flags_parent1,
+ sizeof(_rule_flag_parent1_bkp));
allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
is_dom_check_bkp = is_dom_check;
if (unlikely(dentry_child1)) {
+ /*
+ * Get the layer masks for the child dentries for use by domain
+ * check later. The rule_flags for child1 should have been
+ * included in rule_flags_parent1 already (cf.
+ * collect_domain_accesses), and is not relevant for domain check,
+ * so we don't have to pass it to landlock_unmask_layers.
+ */
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));
+ &_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1),
+ NULL);
layer_masks_child1 = &_layer_masks_child1;
child1_is_directory = d_is_dir(dentry_child1);
}
if (unlikely(dentry_child2)) {
+ /* See above comment for why NULL is passed as rule_flags_masks. */
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));
+ &_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2),
+ NULL);
layer_masks_child2 = &_layer_masks_child2;
child2_is_directory = d_is_dir(dentry_child2);
}
@@ -901,16 +918,18 @@ static bool is_access_to_paths_allowed(
NULL :
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, access_masked_parent1,
+ layer_masks_parent1,
+ ARRAY_SIZE(*layer_masks_parent1),
+ rule_flags_parent1);
+ allowed_parent2 =
+ allowed_parent2 ||
+ landlock_unmask_layers(rule, access_masked_parent2,
+ layer_masks_parent2,
+ ARRAY_SIZE(*layer_masks_parent2),
+ rule_flags_parent2);
/* Stops when a rule from each layer grants access. */
if (allowed_parent1 && allowed_parent2) {
@@ -947,10 +966,16 @@ static bool is_access_to_paths_allowed(
memcpy(&_layer_masks_parent1_bkp,
layer_masks_parent1,
sizeof(_layer_masks_parent1_bkp));
+ memcpy(&_rule_flag_parent1_bkp,
+ rule_flags_parent1,
+ sizeof(_rule_flag_parent1_bkp));
if (layer_masks_parent2) {
memcpy(&_layer_masks_parent2_bkp,
layer_masks_parent2,
sizeof(_layer_masks_parent2_bkp));
+ memcpy(&_rule_flag_parent2_bkp,
+ rule_flags_parent2,
+ sizeof(_rule_flag_parent2_bkp));
is_dom_check_bkp = is_dom_check;
}
@@ -999,11 +1024,15 @@ static bool is_access_to_paths_allowed(
*/
memcpy(layer_masks_parent1, &_layer_masks_parent1_bkp,
sizeof(_layer_masks_parent1_bkp));
+ memcpy(rule_flags_parent1, &_rule_flag_parent1_bkp,
+ sizeof(_rule_flag_parent1_bkp));
allowed_parent1 =
is_layer_masks_allowed(&_layer_masks_parent1_bkp);
if (layer_masks_parent2) {
memcpy(layer_masks_parent2, &_layer_masks_parent2_bkp,
sizeof(_layer_masks_parent2_bkp));
+ memcpy(rule_flags_parent2, &_rule_flag_parent2_bkp,
+ sizeof(_rule_flag_parent2_bkp));
allowed_parent2 = is_layer_masks_allowed(
&_layer_masks_parent2_bkp);
@@ -1063,6 +1092,7 @@ static int current_check_access_path(const struct path *const path,
landlock_get_applicable_subject(current_cred(), masks, NULL);
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
struct landlock_request request = {};
+ struct collected_rule_flags rule_flags = {};
if (!subject)
return 0;
@@ -1071,8 +1101,8 @@ static int current_check_access_path(const struct path *const path,
access_request, &layer_masks,
LANDLOCK_KEY_INODE);
if (is_access_to_paths_allowed(subject->domain, path, access_request,
- &layer_masks, &request, NULL, 0, NULL,
- NULL, NULL))
+ &layer_masks, &rule_flags, &request,
+ NULL, 0, NULL, NULL, NULL, NULL))
return 0;
landlock_log_denial(subject, &request);
@@ -1139,7 +1169,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
static bool collect_domain_accesses(
const struct landlock_ruleset *const domain,
const struct path *const mnt_dir, struct dentry *dir,
- layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS])
+ layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS],
+ struct collected_rule_flags *const rule_flags)
{
access_mask_t access_dom;
bool ret = false;
@@ -1158,9 +1189,9 @@ 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), access_dom, layer_masks_dom,
+ ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
/*
* Before allowing this side of the access request, checks that the
* walk was not in a disconnected directory.
@@ -1269,6 +1300,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
struct landlock_request request1 = {}, request2 = {};
+ struct collected_rule_flags rule_flags_parent1 = {},
+ rule_flags_parent2 = {};
if (!subject)
return 0;
@@ -1300,10 +1333,10 @@ static int current_check_refer_path(struct dentry *const old_dentry,
subject->domain,
access_request_parent1 | access_request_parent2,
&layer_masks_parent1, LANDLOCK_KEY_INODE);
- if (is_access_to_paths_allowed(subject->domain, new_dir,
- access_request_parent1,
- &layer_masks_parent1, &request1,
- NULL, 0, NULL, NULL, NULL))
+ if (is_access_to_paths_allowed(
+ subject->domain, new_dir, access_request_parent1,
+ &layer_masks_parent1, &rule_flags_parent1,
+ &request1, NULL, 0, NULL, NULL, NULL, NULL))
return 0;
landlock_log_denial(subject, &request1);
@@ -1327,11 +1360,14 @@ static int current_check_refer_path(struct dentry *const old_dentry,
old_dentry->d_parent;
/* new_dir->dentry is equal to new_dentry->d_parent */
- allow_parent1 = collect_domain_accesses(
- subject->domain, &mnt_dir, old_parent, &layer_masks_parent1);
+ allow_parent1 = collect_domain_accesses(subject->domain, &mnt_dir,
+ old_parent,
+ &layer_masks_parent1,
+ &rule_flags_parent1);
allow_parent2 = collect_domain_accesses(subject->domain, &mnt_dir,
new_dir->dentry,
- &layer_masks_parent2);
+ &layer_masks_parent2,
+ &rule_flags_parent2);
if (allow_parent1 && allow_parent2)
return 0;
@@ -1343,8 +1379,9 @@ static int current_check_refer_path(struct dentry *const old_dentry,
*/
if (is_access_to_paths_allowed(
subject->domain, &mnt_dir, access_request_parent1,
- &layer_masks_parent1, &request1, old_dentry,
- access_request_parent2, &layer_masks_parent2, &request2,
+ &layer_masks_parent1, &rule_flags_parent1, &request1,
+ old_dentry, access_request_parent2, &layer_masks_parent2,
+ &rule_flags_parent2, &request2,
exchange ? new_dentry : NULL))
return 0;
@@ -1747,6 +1784,7 @@ static int hook_file_open(struct file *const file)
const struct landlock_cred_security *const subject =
landlock_get_applicable_subject(file->f_cred, any_fs, NULL);
struct landlock_request request = {};
+ struct collected_rule_flags rule_flags = {};
if (!subject)
return 0;
@@ -1773,7 +1811,8 @@ static int hook_file_open(struct file *const file)
landlock_init_layer_masks(subject->domain,
full_access_request, &layer_masks,
LANDLOCK_KEY_INODE),
- &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
+ &layer_masks, &rule_flags, &request, NULL, 0, NULL, NULL,
+ NULL, NULL)) {
allowed_access = full_access_request;
} else {
unsigned long access_bit;
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 1f3915a90a80..fc6369dffa51 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -48,6 +48,7 @@ static int current_check_access_socket(struct socket *const sock,
{
__be16 port;
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+ struct collected_rule_flags rule_flags = {};
const struct landlock_rule *rule;
struct landlock_id id = {
.type = LANDLOCK_KEY_NET_PORT,
@@ -179,7 +180,7 @@ static int current_check_access_socket(struct socket *const sock,
access_request, &layer_masks,
LANDLOCK_KEY_NET_PORT);
if (landlock_unmask_layers(rule, access_request, &layer_masks,
- ARRAY_SIZE(layer_masks)))
+ ARRAY_SIZE(layer_masks), &rule_flags))
return 0;
audit_net.family = address->sa_family;
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index dfcdc19ea268..81cdf87d1c79 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -624,7 +624,8 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
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)
+ const size_t masks_array_size,
+ struct collected_rule_flags *const rule_flags)
{
size_t layer_level;
@@ -651,6 +652,10 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
unsigned long access_bit;
bool is_empty;
+ /* Collect rule flags for each layer. */
+ if (rule_flags && layer->flags.quiet)
+ rule_flags->quiet_masks |= layer_bit;
+
/*
* Records in @layer_masks which layer grants access to each requested
* access: bit cleared if the related layer grants access.
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 1a78cba662b2..9790c60c0c00 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -29,7 +29,18 @@ struct landlock_layer {
/**
* @level: Position of this layer in the layer stack. Starts from 1.
*/
- u16 level;
+ u8 level;
+ /**
+ * @flags: Bitfield for special flags attached to this rule.
+ */
+ struct {
+ /**
+ * @quiet: Suppresses denial audit logs for the object covered by
+ * this rule in this domain. For filesystem rules, this inherits
+ * down the file hierarchy.
+ */
+ bool quiet:1;
+ } flags;
/**
* @access: Bitfield of allowed actions on the kernel object. They are
* relative to the object type (e.g. %LANDLOCK_ACTION_FS_READ).
@@ -37,6 +48,16 @@ struct landlock_layer {
access_mask_t access;
};
+/**
+ * struct collected_rule_flags - Hold accumulated flags for each layer.
+ */
+struct collected_rule_flags {
+ /**
+ * @quiet_masks: Layers for which the quiet flag is effective.
+ */
+ layer_mask_t quiet_masks;
+};
+
/**
* union landlock_key - Key of a ruleset's red-black tree
*/
@@ -304,7 +325,8 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
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);
+ const size_t masks_array_size,
+ struct collected_rule_flags *const rule_flags);
access_mask_t
landlock_init_layer_masks(const struct landlock_ruleset *const domain,
--
2.51.1
^ permalink raw reply related
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