* Re: [RFC PATCH v4 06/19] landlock: Add hook on socket creation
From: Günther Noack @ 2025-11-22 11:41 UTC (permalink / raw)
To: Mikhail Ivanov
Cc: mic, gnoack, willemdebruijn.kernel, matthieu,
linux-security-module, netdev, netfilter-devel, yusongping,
artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-7-ivanov.mikhail1@huawei-partners.com>
On Tue, Nov 18, 2025 at 09:46:26PM +0800, Mikhail Ivanov wrote:
> Add hook on security_socket_create(), which checks whether the socket
> of requested protocol is allowed by domain.
>
> Due to support of masked protocols Landlock tries to find one of the
> 4 rules that can allow creation of requested protocol.
>
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> ---
> Changes since v3:
> * Changes LSM hook from socket_post_create to socket_create so
> creation would be blocked before socket allocation and initialization.
> * Uses credential instead of domain in hook_socket create.
> * Removes get_raw_handled_socket_accesses.
> * Adds checks for rules with wildcard type and protocol values.
> * Minor refactoring, fixes.
>
> Changes since v2:
> * Adds check in `hook_socket_create()` to not restrict kernel space
> sockets.
> * Inlines `current_check_access_socket()` in the `hook_socket_create()`.
> * Fixes commit message.
>
> Changes since v1:
> * Uses lsm hook arguments instead of struct socket fields as family-type
> values.
> * Packs socket family and type using helper.
> * Fixes commit message.
> * Formats with clang-format.
> ---
> security/landlock/setup.c | 2 +
> security/landlock/socket.c | 78 ++++++++++++++++++++++++++++++++++++++
> security/landlock/socket.h | 2 +
> 3 files changed, 82 insertions(+)
>
> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
> index bd53c7a56ab9..140a53b022f7 100644
> --- a/security/landlock/setup.c
> +++ b/security/landlock/setup.c
> @@ -17,6 +17,7 @@
> #include "fs.h"
> #include "id.h"
> #include "net.h"
> +#include "socket.h"
> #include "setup.h"
> #include "task.h"
>
> @@ -68,6 +69,7 @@ static int __init landlock_init(void)
> landlock_add_task_hooks();
> landlock_add_fs_hooks();
> landlock_add_net_hooks();
> + landlock_add_socket_hooks();
> landlock_init_id();
> landlock_initialized = true;
> pr_info("Up and running.\n");
> diff --git a/security/landlock/socket.c b/security/landlock/socket.c
> index 28a80dcad629..d7e6e7b92b7a 100644
> --- a/security/landlock/socket.c
> +++ b/security/landlock/socket.c
> @@ -103,3 +103,81 @@ int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
>
> return err;
> }
> +
> +static int check_socket_access(const struct landlock_ruleset *dom,
> + uintptr_t key,
> + layer_mask_t (*const layer_masks)[],
> + access_mask_t handled_access)
> +{
> + const struct landlock_rule *rule;
> + struct landlock_id id = {
> + .type = LANDLOCK_KEY_SOCKET,
> + };
> +
> + id.key.data = key;
This line can be made part of the designated initializer:
struct landlock_id id = {
.type = ...,
.key.data = ...,
};
> + rule = landlock_find_rule(dom, id);
> + if (landlock_unmask_layers(rule, handled_access, layer_masks,
> + LANDLOCK_NUM_ACCESS_SOCKET))
> + return 0;
> + return -EACCES;
> +}
> +
> +static int hook_socket_create(int family, int type, int protocol, int kern)
> +{
> + layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_SOCKET] = {};
> + access_mask_t handled_access;
> + const struct access_masks masks = {
> + .socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> + };
> + const struct landlock_cred_security *const subject =
> + landlock_get_applicable_subject(current_cred(), masks, NULL);
> + uintptr_t key;
> +
> + if (!subject)
> + return 0;
> + /* Checks only user space sockets. */
> + if (kern)
> + return 0;
> +
> + handled_access = landlock_init_layer_masks(
> + subject->domain, LANDLOCK_ACCESS_SOCKET_CREATE, &layer_masks,
> + LANDLOCK_KEY_SOCKET);
Nit: I had to double check to confirm that the same PF_INET/PF_PACKET
transformation (which net/socket.c refers to as the "uglymoron") has
already happened on the arguments before hook_socket_create() gets
called from there. Maybe it's worth a brief mention in a comment
here.
> + /*
> + * Error could happen due to parameters are outside of the allowed range,
Grammar nit: drop the "are"
Suggestion: "If this error happens, the parameters are outside of the
allowed range, so this combination can't have been added to the
ruleset previously."
> + * so this combination couldn't be added in ruleset previously.
> + * Therefore, it's not permitted.
> + */
> + if (pack_socket_key(family, type, protocol, &key) == -EACCES)
> + return -EACCES;
BUG: pack_socket_key() does never return -EACCES!
(Consider whether that function should really return an error? Maybe
a boolean would be better, if you anyway need a different error code
in both locations where it is called.)
Can this code path actually get hit, or do the entry points for
creating sockets refuse these wrong values at an earlier stage with
EINVAL already?
> + if (check_socket_access(subject->domain, key, &layer_masks,
> + handled_access) == 0)
> + return 0;
> +
> + /* Ranges were already checked. */
> + (void)pack_socket_key(family, TYPE_ALL, protocol, &key);
> + if (check_socket_access(subject->domain, key, &layer_masks,
> + handled_access) == 0)
> + return 0;
> +
> + (void)pack_socket_key(family, type, PROTOCOL_ALL, &key);
> + if (check_socket_access(subject->domain, key, &layer_masks,
> + handled_access) == 0)
> + return 0;
> +
> + (void)pack_socket_key(family, TYPE_ALL, PROTOCOL_ALL, &key);
> + if (check_socket_access(subject->domain, key, &layer_masks,
> + handled_access) == 0)
> + return 0;
> +
> + return -EACCES;
> +}
It initially doesn't look very nice to drop the error from
pack_socket_key() repeatedly. The call repeats the bounds checks and
requires more cross-function reasoning to understand.
Since 'key' is an uintptr_t anyway, and the wildcards are all ones,
maybe a simpler way is to define masks for the wildcards?
const uintptr_t any_type_mask = (union key){.data.type = UINT8_MAX}.packed;
const uintptr_t any_protocol_mask = (union key){.data.protocol = UINT16_MAX}.packed;
and then, after calling pack_socket_key() once with error check, use
the combinations
* key
* key | any_type
* key | any_protocol
* key | any_type | any_protocol
to construct the wildcard-enabled keys in the four calls to
check_socket_access()? You could have compile-time assertions or
tests to check that the masking does the same as packing it from
scratch when passing -1.
(That being said, I don't feel strongly about it.)
Remark on the side: I was briefly confused why we don't need to guard
on CONFIG_SECURITY_NETWORK, but this is already required by
CONFIG_LANDLOCK. So that looks good.
–Günther
^ permalink raw reply
* [PATCH RESEND] apparmor: replace sprintf with snprintf in aa_new_learning_profile
From: Thorsten Blum @ 2025-11-22 11:54 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Replace unbounded sprintf() calls with snprintf() to prevent potential
buffer overflows in aa_new_learning_profile(). While the current code
works correctly, snprintf() is safer and follows secure coding best
practices. No functional changes.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/policy.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 50d5345ff5cb..b09323867fea 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -697,24 +697,27 @@ struct aa_profile *aa_new_learning_profile(struct aa_profile *parent, bool hat,
struct aa_profile *p, *profile;
const char *bname;
char *name = NULL;
+ size_t name_sz;
AA_BUG(!parent);
if (base) {
- name = kmalloc(strlen(parent->base.hname) + 8 + strlen(base),
- gfp);
+ name_sz = strlen(parent->base.hname) + 8 + strlen(base);
+ name = kmalloc(name_sz, gfp);
if (name) {
- sprintf(name, "%s//null-%s", parent->base.hname, base);
+ snprintf(name, name_sz, "%s//null-%s",
+ parent->base.hname, base);
goto name;
}
/* fall through to try shorter uniq */
}
- name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, gfp);
+ name_sz = strlen(parent->base.hname) + 2 + 7 + 8;
+ name = kmalloc(name_sz, gfp);
if (!name)
return NULL;
- sprintf(name, "%s//null-%x", parent->base.hname,
- atomic_inc_return(&parent->ns->uniq_null));
+ snprintf(name, name_sz, "%s//null-%x", parent->base.hname,
+ atomic_inc_return(&parent->ns->uniq_null));
name:
/* lookup to see if this is a dup creation */
--
2.51.1
^ permalink raw reply related
* [PATCH RESEND] apparmor: Replace sprintf/strcpy with scnprintf/strscpy in aa_policy_init
From: Thorsten Blum @ 2025-11-22 11:55 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
strcpy() is deprecated and sprintf() does not perform bounds checking
either. Although an overflow is unlikely, it's better to proactively
avoid it by using the safer strscpy() and scnprintf(), respectively.
Additionally, unify memory allocation for 'hname' to simplify and
improve aa_policy_init().
Link: https://github.com/KSPP/linux/issues/88
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lib.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 82dbb97ad406..acf7f5189bec 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -478,19 +478,17 @@ bool aa_policy_init(struct aa_policy *policy, const char *prefix,
const char *name, gfp_t gfp)
{
char *hname;
+ size_t hname_sz;
+ hname_sz = (prefix ? strlen(prefix) + 2 : 0) + strlen(name) + 1;
/* freed by policy_free */
- if (prefix) {
- hname = aa_str_alloc(strlen(prefix) + strlen(name) + 3, gfp);
- if (hname)
- sprintf(hname, "%s//%s", prefix, name);
- } else {
- hname = aa_str_alloc(strlen(name) + 1, gfp);
- if (hname)
- strcpy(hname, name);
- }
+ hname = aa_str_alloc(hname_sz, gfp);
if (!hname)
return false;
+ if (prefix)
+ scnprintf(hname, hname_sz, "%s//%s", prefix, name);
+ else
+ strscpy(hname, name, hname_sz);
policy->hname = hname;
/* base.name is a substring of fqname */
policy->name = basename(policy->hname);
--
2.51.1
^ permalink raw reply related
* Re: [RFC PATCH v4 01/19] landlock: Support socket access-control
From: Günther Noack @ 2025-11-22 12:18 UTC (permalink / raw)
To: Mikhail Ivanov, mic
Cc: mic, gnoack, willemdebruijn.kernel, matthieu,
linux-security-module, netdev, netfilter-devel, yusongping,
artem.kuzin, konstantin.meskhidze
In-Reply-To: <af464773-b01b-f3a4-474d-0efb2cfae142@huawei-partners.com>
On Sat, Nov 22, 2025 at 02:13:08PM +0300, Mikhail Ivanov wrote:
> On 11/22/2025 1:49 PM, Günther Noack wrote:
> > (Remark, should those be exposed as constants?)
>
> I thought it could overcomplicate socket rules definition and Landlock
> API. Do you think introducing such constants will be better decision?
No, I am not convinced either. FWIW, there is a bit of prior art for
"wildcard-like" -1 constants (grepping include/uapi for 'define.*-1'),
but then again, the places where people did the opposite are hard to
grep for. I would also be OK if we documented "-1" in that place and
left out the constant.
Mickaël, maybe you have a preference for the API style here?
> > > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> > > index 33eafb71e4f3..e9f500f97c86 100644
> > > --- a/security/landlock/syscalls.c
> > > +++ b/security/landlock/syscalls.c
> > > @@ -407,6 +458,8 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> > > * &landlock_net_port_attr.allowed_access is not a subset of the ruleset
> > > * handled accesses)
> > > * - %EINVAL: &landlock_net_port_attr.port is greater than 65535;
> > > + * - %EINVAL: &landlock_socket_attr.{family, type} are greater than 254 or
> > > + * &landlock_socket_attr.protocol is greater than 65534;
> >
> > Hmm, this is a bit annoying that these values have such unusual
> > bounds, even though the input parameters are 32 bit. We are exposing
> > a little bit that we are internally storing this with only 8 and 16
> > bits... (I don't know a better solution immediately either, though. I
> > think we discussed this on a previous version of the patch set as well
> > and ended up with permitting larger values than the narrower SOCK_MAX
> > etc bounds.)
>
> I agree, one of the possible solutions may be to store larger values in
> socket keys (eg. s32), but this would require to make a separate
> interface for storing socket rules (in order to not change key size for
> other type of rules which is currently 32-64 bit depending on virtual
> address size).
Yes, I'd be OK with it.
Do I remember this correctly that we settled on enforcing the looser
UINT8_MAX and UINT16_MAX instead of SOCK_MAX, AF_MAX, which we used in
v3 and before? I tried to find the conversation but could not find it
any more. (Or did you have other reasons why you switched the
implementation to use these larger bounds?)
Thanks,
–Günther
^ permalink raw reply
* Re: [RFC PATCH v4 01/19] landlock: Support socket access-control
From: Mikhail Ivanov @ 2025-11-22 16:51 UTC (permalink / raw)
To: Günther Noack, mic
Cc: gnoack, willemdebruijn.kernel, matthieu, linux-security-module,
netdev, netfilter-devel, yusongping, artem.kuzin,
konstantin.meskhidze
In-Reply-To: <20251122.d391a246d7dd@gnoack.org>
On 11/22/2025 3:18 PM, Günther Noack wrote:
> On Sat, Nov 22, 2025 at 02:13:08PM +0300, Mikhail Ivanov wrote:
>> On 11/22/2025 1:49 PM, Günther Noack wrote:
>>> (Remark, should those be exposed as constants?)
>>
>> I thought it could overcomplicate socket rules definition and Landlock
>> API. Do you think introducing such constants will be better decision?
>
> No, I am not convinced either. FWIW, there is a bit of prior art for
> "wildcard-like" -1 constants (grepping include/uapi for 'define.*-1'),
> but then again, the places where people did the opposite are hard to
> grep for. I would also be OK if we documented "-1" in that place and
> left out the constant.
>
> Mickaël, maybe you have a preference for the API style here?
>
>
>>>> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
>>>> index 33eafb71e4f3..e9f500f97c86 100644
>>>> --- a/security/landlock/syscalls.c
>>>> +++ b/security/landlock/syscalls.c
>>>> @@ -407,6 +458,8 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
>>>> * &landlock_net_port_attr.allowed_access is not a subset of the ruleset
>>>> * handled accesses)
>>>> * - %EINVAL: &landlock_net_port_attr.port is greater than 65535;
>>>> + * - %EINVAL: &landlock_socket_attr.{family, type} are greater than 254 or
>>>> + * &landlock_socket_attr.protocol is greater than 65534;
>>>
>>> Hmm, this is a bit annoying that these values have such unusual
>>> bounds, even though the input parameters are 32 bit. We are exposing
>>> a little bit that we are internally storing this with only 8 and 16
>>> bits... (I don't know a better solution immediately either, though. I
>>> think we discussed this on a previous version of the patch set as well
>>> and ended up with permitting larger values than the narrower SOCK_MAX
>>> etc bounds.)
>>
>> I agree, one of the possible solutions may be to store larger values in
>> socket keys (eg. s32), but this would require to make a separate
>> interface for storing socket rules (in order to not change key size for
>> other type of rules which is currently 32-64 bit depending on virtual
>> address size).
>
> Yes, I'd be OK with it.
>
> Do I remember this correctly that we settled on enforcing the looser
> UINT8_MAX and UINT16_MAX instead of SOCK_MAX, AF_MAX, which we used in
> v3 and before? I tried to find the conversation but could not find it
> any more. (Or did you have other reasons why you switched the
> implementation to use these larger bounds?)
Mickaël mentioned that Landlock should accept rules defined even for
unsupported protocol families:
https://lore.kernel.org/all/20241128.um9voo5Woo3I@digikod.net/
>
> Thanks,
> –Günther
^ permalink raw reply
* Re: [PATCH v18] exec: Fix dead-lock in de_thread with ptrace_attach
From: Bernd Edlinger @ 2025-11-22 17:10 UTC (permalink / raw)
To: Eric W. Biederman
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, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet, Ryan Lee, apparmor, selinux
In-Reply-To: <87wm3ky5n9.fsf@email.froward.int.ebiederm.org>
On 11/20/25 18:29, Eric W. Biederman wrote:
> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>
>> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>>
>>> 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.
>>
>> A small quibble it isn't a dead lock. It isn't even really a live lock,
>> as it is possible to SIGKILL our way out.
>>
That is of course what I meant to say with that sentence. In my language an
application process can "dead-lock" by waiting on a mutex forever.
Indeed the original problem with the dead-lock (I think in mm_access) was
worse, as both involved processes were only killable by "kill -9", but with the
remaining problem in ptrace_attach, the debugger can be killed by a simple CTRL-C.
So if I understand you right, you want me use a different term here like
"the trace process may be blocked in trace_attach" or so?
Or could you please give me a concrete suggestion how to rephrase the patch
description.
BTW, unless there are objections I would also want to rephrase the description
of cred_guard_mutex to replace the term "Deprecated" with "Not recommended",
like this:
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -260,11 +260,11 @@ struct signal_struct {
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.
+ * Not recommended to use in new code.
* Use exec_update_lock instead.
*/
>> Thinking about this there is a really silly and simple way we can deal
>> with this situation for PTRACE_ATTACH. We can send SIGSTOP and wait for
>> the thread to stop before doing anything with cred_guard_mutex.
>>
>> PTRACE_ATTACH already implies sending SIGSTOP so as long as we have
>> enough permissions to send SIGSTOP I don't see that being a problem.
>>
>> The worst case I can see is that we get a case where we stop the
>> process, the permission check fails under cred_guard_mutex and
>> and ptrace attach has fails and has to send SIGCONT to undo it's
>> premature SIGSTOP. That might almost be visible, but it would still
>> be legitimate because we can still check that we have permission to
>> send SIGSTOP.
>
> Bah no I am full of it.
>
> The challenging behavior is in the semantics of the kernel operations.
> We need to describe it as such please.
>
> It is the same class of problem as a single threaded process calls exec
> with a pipe attached to both stdin and stdout of the new process.
>
> For the stdin and stdout we can say just use pull and nonblocking I/O.
>
> The problem is that both PTRACE_ATTACH and PTRACE_SEIZE block over
> the duration of exec, and if exec is waiting for a thread to exit,
> and that thread is blocked in PTRACE_EVENT_EXIT waiting for that very
> same tracer those processes will hang. Not deadlock.
>
>
> I haven't seen anyone clearly describe the problem lately so I am
> repeating it.
>
>
> Just looking at the code I don't think there is any fundamental reason
> to call commit_creds after de_thread. If we can change that we can sort
> this out without any change in userspace semantics.
>
> If we can't move commit_creds we have to either give
> PTRACE_ATTACH/PTRACE_SEIZE a non-block mode, or break out of
> PTRACE_EVENT_EXIT in de_thread.
>
> I will post a proof of concept of moving commit_creds in just a minute.
>
> Eric
Note: I forgot to add apparmor and selinux mailing list to this patch, previous
versions of this did try to avoid to touch the security engine code, and did
instead temporarily install the new credentials, mostiy for the benefit of the
security engines. But that is considered an unacceptable solution, therefore
I want to use instead a new option to ptrace_may_access.
All security engines have to handle this option, but the advantage is, that the
engines could detect and maybe also deny the unsafe execve.
This is an alternative to Eric's patch: "exec: Move cred computation under
exec_update_lock" that is supposed to solve the same problem, but tries instead
to avoid any user visible API change.
Thanks
Bernd.
^ permalink raw reply
* Re: [RFC PATCH v4 06/19] landlock: Add hook on socket creation
From: Mikhail Ivanov @ 2025-11-22 17:19 UTC (permalink / raw)
To: Günther Noack
Cc: mic, gnoack, willemdebruijn.kernel, matthieu,
linux-security-module, netdev, netfilter-devel, yusongping,
artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251122.78c6cd69a873@gnoack.org>
On 11/22/2025 2:41 PM, Günther Noack wrote:
> On Tue, Nov 18, 2025 at 09:46:26PM +0800, Mikhail Ivanov wrote:
>> Add hook on security_socket_create(), which checks whether the socket
>> of requested protocol is allowed by domain.
>>
>> Due to support of masked protocols Landlock tries to find one of the
>> 4 rules that can allow creation of requested protocol.
>>
>> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
>> ---
>> Changes since v3:
>> * Changes LSM hook from socket_post_create to socket_create so
>> creation would be blocked before socket allocation and initialization.
>> * Uses credential instead of domain in hook_socket create.
>> * Removes get_raw_handled_socket_accesses.
>> * Adds checks for rules with wildcard type and protocol values.
>> * Minor refactoring, fixes.
>>
>> Changes since v2:
>> * Adds check in `hook_socket_create()` to not restrict kernel space
>> sockets.
>> * Inlines `current_check_access_socket()` in the `hook_socket_create()`.
>> * Fixes commit message.
>>
>> Changes since v1:
>> * Uses lsm hook arguments instead of struct socket fields as family-type
>> values.
>> * Packs socket family and type using helper.
>> * Fixes commit message.
>> * Formats with clang-format.
>> ---
>> security/landlock/setup.c | 2 +
>> security/landlock/socket.c | 78 ++++++++++++++++++++++++++++++++++++++
>> security/landlock/socket.h | 2 +
>> 3 files changed, 82 insertions(+)
>>
>> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
>> index bd53c7a56ab9..140a53b022f7 100644
>> --- a/security/landlock/setup.c
>> +++ b/security/landlock/setup.c
>> @@ -17,6 +17,7 @@
>> #include "fs.h"
>> #include "id.h"
>> #include "net.h"
>> +#include "socket.h"
>> #include "setup.h"
>> #include "task.h"
>>
>> @@ -68,6 +69,7 @@ static int __init landlock_init(void)
>> landlock_add_task_hooks();
>> landlock_add_fs_hooks();
>> landlock_add_net_hooks();
>> + landlock_add_socket_hooks();
>> landlock_init_id();
>> landlock_initialized = true;
>> pr_info("Up and running.\n");
>> diff --git a/security/landlock/socket.c b/security/landlock/socket.c
>> index 28a80dcad629..d7e6e7b92b7a 100644
>> --- a/security/landlock/socket.c
>> +++ b/security/landlock/socket.c
>> @@ -103,3 +103,81 @@ int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
>>
>> return err;
>> }
>> +
>> +static int check_socket_access(const struct landlock_ruleset *dom,
>> + uintptr_t key,
>> + layer_mask_t (*const layer_masks)[],
>> + access_mask_t handled_access)
>> +{
>> + const struct landlock_rule *rule;
>> + struct landlock_id id = {
>> + .type = LANDLOCK_KEY_SOCKET,
>> + };
>> +
>> + id.key.data = key;
>
> This line can be made part of the designated initializer:
>
> struct landlock_id id = {
> .type = ...,
> .key.data = ...,
> };
>
Indeed, thats would be better.
>
>> + rule = landlock_find_rule(dom, id);
>> + if (landlock_unmask_layers(rule, handled_access, layer_masks,
>> + LANDLOCK_NUM_ACCESS_SOCKET))
>> + return 0;
>> + return -EACCES;
>> +}
>> +
>> +static int hook_socket_create(int family, int type, int protocol, int kern)
>> +{
>> + layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_SOCKET] = {};
>> + access_mask_t handled_access;
>> + const struct access_masks masks = {
>> + .socket = LANDLOCK_ACCESS_SOCKET_CREATE,
>> + };
>> + const struct landlock_cred_security *const subject =
>> + landlock_get_applicable_subject(current_cred(), masks, NULL);
>> + uintptr_t key;
>> +
>> + if (!subject)
>> + return 0;
>> + /* Checks only user space sockets. */
>> + if (kern)
>> + return 0;
>> +
>> + handled_access = landlock_init_layer_masks(
>> + subject->domain, LANDLOCK_ACCESS_SOCKET_CREATE, &layer_masks,
>> + LANDLOCK_KEY_SOCKET);
>
> Nit: I had to double check to confirm that the same PF_INET/PF_PACKET
> transformation (which net/socket.c refers to as the "uglymoron") has
> already happened on the arguments before hook_socket_create() gets
> called from there. Maybe it's worth a brief mention in a comment
> here.
Ok, thanks!
>
>> + /*
>> + * Error could happen due to parameters are outside of the allowed range,
>
> Grammar nit: drop the "are"
>
> Suggestion: "If this error happens, the parameters are outside of the
> allowed range, so this combination can't have been added to the
> ruleset previously."
Thanks, I'll use it.
>
>> + * so this combination couldn't be added in ruleset previously.
>> + * Therefore, it's not permitted.
>> + */
>> + if (pack_socket_key(family, type, protocol, &key) == -EACCES)
>> + return -EACCES;
>
> BUG: pack_socket_key() does never return -EACCES!
Thanks a lot, will be fixed!
>
> (Consider whether that function should really return an error? Maybe
> a boolean would be better, if you anyway need a different error code
> in both locations where it is called.)
Agreed
>
> Can this code path actually get hit, or do the entry points for
> creating sockets refuse these wrong values at an earlier stage with
> EINVAL already?
There are checks for family and type ranges in __sock_create. Protocol
ranges should be checked in methods specific to protocol family after
LSM hook is triggered. But it would be safer to keep this check in order
to be independent of the specific kernel version.
>
>> + if (check_socket_access(subject->domain, key, &layer_masks,
>> + handled_access) == 0)
>> + return 0;
>> +
>> + /* Ranges were already checked. */
>> + (void)pack_socket_key(family, TYPE_ALL, protocol, &key);
>> + if (check_socket_access(subject->domain, key, &layer_masks,
>> + handled_access) == 0)
>> + return 0;
>> +
>> + (void)pack_socket_key(family, type, PROTOCOL_ALL, &key);
>> + if (check_socket_access(subject->domain, key, &layer_masks,
>> + handled_access) == 0)
>> + return 0;
>> +
>> + (void)pack_socket_key(family, TYPE_ALL, PROTOCOL_ALL, &key);
>> + if (check_socket_access(subject->domain, key, &layer_masks,
>> + handled_access) == 0)
>> + return 0;
>> +
>> + return -EACCES;
>> +}
>
> It initially doesn't look very nice to drop the error from
> pack_socket_key() repeatedly. The call repeats the bounds checks and
> requires more cross-function reasoning to understand.
Agreed
>
> Since 'key' is an uintptr_t anyway, and the wildcards are all ones,
> maybe a simpler way is to define masks for the wildcards?
>
> const uintptr_t any_type_mask = (union key){.data.type = UINT8_MAX}.packed;
> const uintptr_t any_protocol_mask = (union key){.data.protocol = UINT16_MAX}.packed;
>
> and then, after calling pack_socket_key() once with error check, use
> the combinations
>
> * key
> * key | any_type
> * key | any_protocol
> * key | any_type | any_protocol
>
> to construct the wildcard-enabled keys in the four calls to
> check_socket_access()? You could have compile-time assertions or
> tests to check that the masking does the same as packing it from
> scratch when passing -1.
>
> (That being said, I don't feel strongly about it.)
It seems clearer and simpler to me, so I think we should use your
approach. Probably, pack_socket_key() should be changed to pack values
using bit operations instead of socket_key structure:
key = protocol << 16 | type << 8 | family;
>
> Remark on the side: I was briefly confused why we don't need to guard
> on CONFIG_SECURITY_NETWORK, but this is already required by
> CONFIG_LANDLOCK. So that looks good.
>
> –Günther
^ permalink raw reply
* Re: [GIT PULL] selinux/selinux-pr-20251121
From: pr-tracker-bot @ 2025-11-22 18:29 UTC (permalink / raw)
To: Paul Moore; +Cc: Linus Torvalds, selinux, linux-security-module, linux-kernel
In-Reply-To: <e6a2c26951147ce12c57ff1fd53651c0@paul-moore.com>
The pull request you sent on Fri, 21 Nov 2025 15:57:27 -0500:
> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git tags/selinux-pr-20251121
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/5703357ede59ce8b0af11e02c374a3db73c55ee8
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH 5/6] landlock: Fix compilation error for kunit tests when CONFIG_AUDIT is disabled.
From: Tingmao Wang @ 2025-11-22 23:35 UTC (permalink / raw)
To: Justin Suess
Cc: Günther Noack, Jan Kara, Abhinav Saxena,
Mickaël Salaün, linux-security-module
In-Reply-To: <20251120222346.1157004-6-utilityemal77@gmail.com>
On 11/20/25 22:23, Justin Suess wrote:
> This was necessary when fixing the no inherit patch in the implementation of kunit tests.
>
> When compiled without the audit flag enabled, kunit tests would fail to compile because of
> the missing quiet flag field. This fixes this issue.
>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
Thanks for spotting these issues! I will include your fix in the next
version of my patch (it's not merged yet, so mistakes can be fixed "at
source"). Feel free to reply to my patch directly if you spot more
problems :)
(For the landlock_log_denial one, we might not end up changing the
function's parameter after all - Mickaël has suggested the rule_flags be
embedded inside the landlock_request)
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Johansen @ 2025-11-23 2:08 UTC (permalink / raw)
To: Helge Deller, John Paul Adrian Glaubitz, Helge Deller
Cc: linux-kernel, apparmor, linux-security-module
In-Reply-To: <be9c143d-1d5e-4c5b-9078-4a7804489258@gmx.de>
On 11/18/25 04:49, Helge Deller wrote:
> Hi Adrian,
>
> On 11/18/25 12:43, John Paul Adrian Glaubitz wrote:
>> On Tue, 2025-11-18 at 12:09 +0100, Helge Deller wrote:
>>> My patch fixed two call sites, but I suspect you see another call site which
>>> hasn't been fixed yet.
>>>
>>> Can you try attached patch? It might indicate the caller of the function and
>>> maybe prints the struct name/address which isn't aligned.
>>>
>>> Helge
>>>
>>>
>>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>>> index c5a91600842a..b477430c07eb 100644
>>> --- a/security/apparmor/match.c
>>> +++ b/security/apparmor/match.c
>>> @@ -313,6 +313,9 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
>>> if (size < sizeof(struct table_set_header))
>>> goto fail;
>>> + if (WARN_ON(((unsigned long)data) & (BITS_PER_LONG/8 - 1)))
>>> + pr_warn("dfa blob stream %pS not aligned.\n", data);
>>> +
>>> if (ntohl(*(__be32 *) data) != YYTH_MAGIC)
>>> goto fail;
>>
>> Here is the relevant output with the patch applied:
>>
>> [ 73.840639] ------------[ cut here ]------------
>> [ 73.901376] WARNING: CPU: 0 PID: 341 at security/apparmor/match.c:316 aa_dfa_unpack+0x6cc/0x720
>> [ 74.015867] Modules linked in: binfmt_misc evdev flash sg drm drm_panel_orientation_quirks backlight i2c_core configfs nfnetlink autofs4 ext4 crc16 mbcache jbd2 hid_generic usbhid sr_mod hid cdrom
>> sd_mod ata_generic ohci_pci ehci_pci ehci_hcd ohci_hcd pata_ali libata sym53c8xx scsi_transport_spi tg3 scsi_mod usbcore libphy scsi_common mdio_bus usb_common
>> [ 74.428977] CPU: 0 UID: 0 PID: 341 Comm: apparmor_parser Not tainted 6.18.0-rc6+ #9 NONE
>> [ 74.536543] Call Trace:
>> [ 74.568561] [<0000000000434c24>] dump_stack+0x8/0x18
>> [ 74.633757] [<0000000000476438>] __warn+0xd8/0x100
>> [ 74.696664] [<00000000004296d4>] warn_slowpath_fmt+0x34/0x74
>> [ 74.771006] [<00000000008db28c>] aa_dfa_unpack+0x6cc/0x720
>> [ 74.843062] [<00000000008e643c>] unpack_pdb+0xbc/0x7e0
>> [ 74.910545] [<00000000008e7740>] unpack_profile+0xbe0/0x1300
>> [ 74.984888] [<00000000008e82e0>] aa_unpack+0xe0/0x6a0
>> [ 75.051226] [<00000000008e3ec4>] aa_replace_profiles+0x64/0x1160
>> [ 75.130144] [<00000000008d4d90>] policy_update+0xf0/0x280
>> [ 75.201057] [<00000000008d4fc8>] profile_replace+0xa8/0x100
>> [ 75.274258] [<0000000000766bd0>] vfs_write+0x90/0x420
>> [ 75.340594] [<00000000007670cc>] ksys_write+0x4c/0xe0
>> [ 75.406932] [<0000000000767174>] sys_write+0x14/0x40
>> [ 75.472126] [<0000000000406174>] linux_sparc_syscall+0x34/0x44
>> [ 75.548802] ---[ end trace 0000000000000000 ]---
>> [ 75.609503] dfa blob stream 0xfff0000008926b96 not aligned.
>> [ 75.682695] Kernel unaligned access at TPC[8db2a8] aa_dfa_unpack+0x6e8/0x720
>
> The non-8-byte-aligned address (0xfff0000008926b96) is coming from userspace
> (via the write syscall).
> Some apparmor userspace tool writes into the apparmor ".replace" virtual file with
> a source address which is not correctly aligned.
the userpace buffer passed to write(2) has to be aligned? Its certainly nice if it
is but the userspace tooling hasn't been treating it as aligned. With that said,
the dfa should be padded to be aligned. So this tripping in the dfa is a bug,
and there really should be some validation to catch it.
> You should be able to debug/find the problematic code with strace from userspace.
> Maybe someone with apparmor knowledge here on the list has an idea?
>
This is likely an unaligned 2nd profile, being split out and loaded separately
from the rest of the container. Basically the loader for some reason (there
are a few different possible reasons) is poking into the container format and
pulling out the profile at some offset, this gets loaded to the kernel but
it would seem that its causing an issue with the dfa alignment within the container,
which should be aligned to the original container.
Kernel side, we are going to need to add some extra verification checks, it should
be catching this, as unaligned as part of the unpack. Userspace side, we will have
to verify my guess and fix the loader.
^ permalink raw reply
* Re: [PATCH 5/6] landlock: Fix compilation error for kunit tests when CONFIG_AUDIT is disabled.
From: Justin Suess @ 2025-11-23 16:43 UTC (permalink / raw)
To: m; +Cc: gnoack, jack, linux-security-module, mic, utilityemal77, xandfury
In-Reply-To: <baa3449e-e238-4caa-a16e-c252016bf480@maowtm.org>
No problem it was an easy fix. I'll reply directly to your patch in the
future, I'm still figuring out mailing list development and kernel
development workflows.
I do realize now I have some behaviors in this series that impact the
quiet flag that I want your input on. I'll add those as replies to
your patch series as well.
I plan to keep rebasing this patch off the quiet flag series as you release
new versions. I also plan to introduce some selftests combining the two flags
and ensuring they interact with eachother as expected.
The next version of this patch is going to include some big refactorings
(most likely removing the xarrays) and fix some edge cases I discovered.
King Regards,
Justin Suess
^ permalink raw reply
* Re: [PATCH v4 00/10] Implement LANDLOCK_ADD_RULE_QUIET
From: Justin Suess @ 2025-11-23 17:01 UTC (permalink / raw)
To: m; +Cc: gnoack, jack, linux-security-module, mic, utilityemal77, xandfury
In-Reply-To: <5c0de8ee7e00aff1aceb3a80f5af162eeaaa06db.1763330228.git.m@maowtm.org>
I had a question in regards to the quiet flag in how it
should interact with my proposed flag LANDLOCK_ADD_RULE_NO_INHERIT.
Should this flag block inheritence of the LANDLOCK_ADD_RULE_QUIET flag?
It seems to me it should block inheritence of this flag, so you can
create more fine grained audit-suppression rules.
So for example you could quiet logs on /a/b with the exception of /a/b/c
by setting LANDLOCK_ADD_RULE_NO_INHERIT on /a/b/c.
If so, as we add more flags, should this be a general policy that
LANDLOCK_ADD_RULE_NO_INHERIT blocks access right inheritence AND flag
inheritence? With the obvious exception of LANDLOCK_ADD_RULE_NO_INHERIT
itself.
Alternatives could be a new flag to control whether NO_INHERIT also
suppresses flag inheritence.
Or simply having LANDLOCK_ADD_RULE_NO_INHERIT continue to only apply to
access masks.
The latest version of LANDLOCK_ADD_RULE_NO_INHERIT is below for
convienence.
v3:
https://lore.kernel.org/linux-security-module/20251120222346.1157004-1-utilityemal77@gmail.com/T/#t
Kind Regards,
Justin Suess
^ permalink raw reply
* Re: [PATCH v3 0/9] module: Introduce hash-based integrity checking
From: Sebastian Andrzej Siewior @ 2025-11-23 17:05 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: James Bottomley, Masahiro Yamada, Nathan Chancellor,
Arnd Bergmann, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Paul Moore, James Morris, Serge E. Hallyn,
Jonathan Corbet, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20251119154834.A-tQsLzh@linutronix.de>
On 2025-11-19 16:48:34 [+0100], Sebastian Andrzej Siewior wrote:
> I fully agree with this approach. I don't like the big hash array but I
> have an idea how to optimize that part. So I don't see a problem in the
> long term.
The following PoC creates a merkle tree from a set files ending with .ko
within the specified directory. It will write a .hash files containing
the required hash for each file for its validation. The root hash is
saved as "hash_root" and "hash_root.h" in the directory.
The Debian kernel shipps 4256 modules:
| $ time ./compute_hashes mods_deb
| Files 4256 levels: 13 root hash: 97f8f439d63938ed74f48ec46dbd75c2b5e5b49f012a414e89b6f0e0f06efe84
|
| real 0m0,732s
| user 0m0,304s
| sys 0m0,427s
This computes the hashes for all the modules it found in the mods_deb
folder.
The kernel needs the root hash (for sha256 32 bytes) and the depth of
the tree (4 bytes). That are 36 bytes regardless of the number of
modules that are built.
In this case, the attached hash for each module is 420 bytes. This is 4
bytes (position in the tree) + 13 (depth) * 32.
The verification process requires 13 hash operation to hash through the
tree and verify against the root hash.
For convience, the following PoC can also be found at
https://git.kernel.org/pub/scm/linux/kernel/git/bigeasy/mtree-hashed-mods.git/
which also includes a small testsuite.
diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000000000..e4a35c15f0a94
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,7 @@
+CC := gcc
+CFLAGS := -O2 -g -Wall
+LDLIBS := -lcrypto
+
+all: compute_hashes mk-files verify_hash
+test: compute_hashes mk-files verify_hash
+ ./verify_test.sh
diff --git a/compute_hashes.c b/compute_hashes.c
new file mode 100644
index 0000000000000..da61b214137b8
--- /dev/null
+++ b/compute_hashes.c
@@ -0,0 +1,407 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Compute hashes for individual files and build a merkle tree.
+ *
+ * Author: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
+ *
+ */
+#define _GNU_SOURCE 1
+#include <ftw.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include <sys/stat.h>
+#include <sys/mman.h>
+
+#include <openssl/evp.h>
+
+#include "helpers.h"
+
+struct file_entry {
+ char *name;
+ size_t fsize;
+ unsigned int pos;
+ unsigned char hash[EVP_MAX_MD_SIZE];
+};
+
+static struct file_entry *fh_list;
+static size_t num_files;
+
+struct leaf_hash {
+ unsigned char hash[EVP_MAX_MD_SIZE];
+};
+
+struct mtree {
+ struct leaf_hash **l;
+ unsigned int *entries;
+ unsigned int levels;
+};
+
+static unsigned int get_pow2(unsigned int val)
+{
+ return 31 - __builtin_clz(val);
+}
+
+static unsigned int roundup_pow2(unsigned int val)
+{
+ return 1 << (get_pow2(val - 1) + 1);
+}
+
+static unsigned int log2_roundup(unsigned int val)
+{
+ return get_pow2(roundup_pow2(val));
+}
+
+static int str_endswith(const char *s, const char *suffix)
+{
+ size_t ls, lf;
+
+ ls = strlen(s);
+ lf = strlen(suffix);
+
+ if (ls <= lf)
+ return -1;
+ return strcmp(s + ls - lf, suffix);
+}
+
+static void __print_hash(unsigned char *h)
+{
+ int i;
+
+ for (i = 0; i < hash_size; i++)
+ printf("%02x", h[i]);
+}
+
+static void print_hash(unsigned char *h)
+{
+ __print_hash(h);
+ printf("\n");
+}
+
+static int hash_file(struct file_entry *fe)
+{
+ void *mem;
+ int fd;
+
+ fd = open(fe->name, O_RDONLY);
+ if (fd < 0) {
+ printf("Failed to open %s: %m\n", fe->name);
+ exit(1);
+ }
+
+ mem = mmap(NULL, fe->fsize, PROT_READ, MAP_SHARED, fd, 0);
+ close(fd);
+
+ if (mem == MAP_FAILED) {
+ printf("Failed to mmap %s: %m\n", fe->name);
+ exit(1);
+ }
+
+ hash_data(mem, fe->pos, fe->fsize, fe->hash);
+
+ munmap(mem, fe->fsize);
+ return 0;
+}
+
+static int add_files_cb(const char *fpath, const struct stat *sb, int tflag,
+ struct FTW *ftwbuf)
+{
+ if (tflag != FTW_F)
+ return 0;
+
+ if (str_endswith(fpath, ".ko"))
+ return 0;
+
+ fh_list = xrealloc(fh_list, (num_files + 1) * sizeof (struct file_entry));
+
+ fh_list[num_files].name = strdup(fpath);
+ if (!fh_list[num_files].name) {
+ printf("Failed to allocate memory\n");
+ exit(1);
+ }
+
+ fh_list[num_files].fsize = sb->st_size;
+
+ num_files++;
+ return 0;
+}
+
+static int cmp_file_entry(const void *p1, const void *p2)
+{
+ const struct file_entry *f1, *f2;
+
+ f1 = p1;
+ f2 = p2;
+
+ return strcmp(f1->name, f2->name);
+}
+
+static struct mtree *build_merkle(struct file_entry *fh, size_t num)
+{
+ unsigned int i, le;
+ struct mtree *mt;
+
+ mt = xmalloc(sizeof(struct mtree));
+ mt->levels = log2_roundup(num);
+ mt->l = xcalloc(sizeof(struct leaf_hash *), mt->levels);
+
+ mt->entries = xcalloc(sizeof(unsigned int), mt->levels);
+ le = num / 2;
+ if (num & 1)
+ le++;
+ mt->entries[0] = le;
+ mt->l[0] = xcalloc(sizeof(struct leaf_hash), le);
+
+ /* First level of pairs */
+ for (i = 0; i < num; i+= 2) {
+ if (i == num - 1) {
+ /* Odd number of files, no pair. Hash with itself */
+ hash_entry(fh[i].hash, fh[i].hash, mt->l[0][i/2].hash);
+ } else {
+ hash_entry(fh[i].hash, fh[i + 1].hash, mt->l[0][i/2].hash);
+ }
+ }
+ for (i = 1; i < mt->levels; i++) {
+ int n;
+ int odd = 0;
+
+ if (le & 1) {
+ le++;
+ odd++;
+ }
+
+ mt->entries[i] = le / 2;
+ mt->l[i] = xcalloc(sizeof(struct leaf_hash), le);
+
+ for (n = 0; n < le; n += 2) {
+ if (n == le - 2 && odd) {
+ /* Odd number of pairs, no pair. Hash with itself */
+ hash_entry(mt->l[i - 1][n].hash, mt->l[i - 1][n].hash,
+ mt->l[i][n/2].hash);
+ } else {
+ hash_entry(mt->l[i - 1][n].hash, mt->l[i - 1][n +1].hash,
+ mt->l[i][n/2].hash);
+ }
+ }
+ le = mt->entries[i];
+ }
+ return mt;
+}
+
+static void free_mtree(struct mtree *mt)
+{
+ int i;
+
+ for (i = 0; i < mt->levels; i++)
+ free(mt->l[i]);
+
+ free(mt->l);
+ free(mt->entries);
+ free(mt);
+}
+
+static void write_be_int(int fd, unsigned int v)
+{
+ unsigned int be_val = host_to_be32(v);
+
+ if (write(fd, &be_val, sizeof(be_val)) != sizeof(be_val)) {
+ printf("Failed writting to file: %m\n");
+ exit(1);
+ }
+}
+
+static void write_hash(int fd, const void *h)
+{
+ ssize_t wr;
+
+ wr = write(fd, h, hash_size);
+ if (wr != hash_size) {
+ printf("Failed writting to file: %m\n");
+ exit(1);
+ }
+}
+
+static void build_proof(struct mtree *mt, unsigned int n, int fd)
+{
+ unsigned char cur[EVP_MAX_MD_SIZE];
+ unsigned char tmp[EVP_MAX_MD_SIZE];
+ struct file_entry *fe, *fe_sib;
+ unsigned int i;
+
+ fe = &fh_list[n];
+
+ if ((n & 1) == 0) {
+ /* No pair, hash with itself */
+ if (n + 1 == num_files)
+ fe_sib = fe;
+ else
+ fe_sib = &fh_list[n + 1];
+ } else {
+ fe_sib = &fh_list[n - 1];
+ }
+ /* First comes the node position into the file */
+ write_be_int(fd, n);
+
+ if ((n & 1) == 0)
+ hash_entry(fe->hash, fe_sib->hash, cur);
+ else
+ hash_entry(fe_sib->hash, fe->hash, cur);
+
+ /* Next is the sibling hash, followed by hashes in the tree */
+ write_hash(fd, fe_sib->hash);
+
+ for (i = 0; i < mt->levels - 1; i++) {
+ n >>= 1;
+ if ((n & 1) == 0) {
+ void *h;
+
+ /* No pair, hash with itself */
+ if (n + 1 == mt->entries[i])
+ h = cur;
+ else
+ h = mt->l[i][n + 1].hash;
+
+ hash_entry(cur, h, tmp);
+ write_hash(fd, h);
+ } else {
+ hash_entry(mt->l[i][n - 1].hash, cur, tmp);
+ write_hash(fd, mt->l[i][n - 1].hash);
+ }
+ memcpy(cur, tmp, hash_size);
+ }
+
+ /* After all that, the end hash should match the root hash */
+ if (memcmp(cur, mt->l[mt->levels - 1][0].hash, hash_size))
+ printf("MISS-MATCH\n");
+}
+
+static void write_merkle_root(struct mtree *mt, const char *fp)
+{
+ char buf[1024];
+ int fd;
+
+ if (snprintf(buf, sizeof(buf), "%s/hash_root", fp) >= sizeof(buf)) {
+ printf("Root dir too long\n");
+ exit(1);
+ }
+ fd = open(buf, O_WRONLY | O_CREAT | O_TRUNC, DEF_F_PERM);
+ if (fd < 0) {
+ printf("Failed to create %s: %m\n", buf);
+ exit(1);
+ }
+
+ write_be_int(fd, mt->levels);
+ write_hash(fd, mt->l[mt->levels - 1][0].hash);
+ close(fd);
+ printf("Files %ld levels: %d root hash: ", num_files, mt->levels);
+ print_hash(mt->l[mt->levels - 1][0].hash);
+}
+
+static void write_merkle_root_h(struct mtree *mt, const char *fp)
+{
+ char buf[1024];
+ unsigned int i;
+ unsigned char *h;
+ FILE *f;
+
+ if (snprintf(buf, sizeof(buf), "%s/hash_root.h", fp) >= sizeof(buf)) {
+ printf("Root dir too long\n");
+ exit(1);
+ }
+ f = fopen(buf, "w");
+ if (!f) {
+ printf("Failed to create %s: %m\n", buf);
+ exit(1);
+ }
+ h = mt->l[mt->levels - 1][0].hash;
+
+ fprintf(f, "#ifndef __HASH_ROOT_TREE_H__\n");
+ fprintf(f, "#define __HASH_ROOT_TREE_H__\n\n");
+ fprintf(f, "unsigned int hashed_mods_levels = %u;\n", mt->levels);
+ fprintf(f, "unsigned char hashed_mods_root[%d] = {", hash_size);
+ for (i = 0; i < hash_size; i++) {
+ char *space = "";
+
+ if (!(i % 8))
+ fprintf(f, "\n\t");
+
+ if ((i + 1) % 8)
+ space = " ";
+
+ fprintf(f, "0x%02x,%s", h[i], space);
+ }
+ fprintf(f, "\n};\n#endif\n");
+ fclose(f);
+}
+
+int main(int argc, char *argv[])
+{
+ const EVP_MD *hash_evp;
+ char *fp;
+ struct mtree *mt;
+ int i;
+
+ ctx = EVP_MD_CTX_new();
+ if (!ctx)
+ goto err_ossl;
+
+ if (argc != 2) {
+ printf("%s: folder\n", argv[0]);
+ return 1;
+ }
+ fp = argv[1];
+
+ hash_evp = EVP_sha256();
+ hash_size = EVP_MD_get_size(hash_evp);
+ if (hash_size <= 0)
+ goto err_ossl;
+
+ if (EVP_DigestInit_ex(ctx, hash_evp, NULL) != 1)
+ goto err_ossl;
+
+ nftw(fp, add_files_cb, 64, 0);
+
+ qsort(fh_list, num_files, sizeof(struct file_entry), cmp_file_entry);
+
+ for (i = 0; i < num_files; i++) {
+ fh_list[i].pos = i;
+ hash_file(&fh_list[i]);
+ }
+
+ mt = build_merkle(fh_list, num_files);
+ write_merkle_root(mt, fp);
+ write_merkle_root_h(mt, fp);
+ for (i = 0; i < num_files; i++) {
+ char signame[1024];
+ int fd;
+ int ret;
+
+ ret = snprintf(signame, sizeof(signame), "%s.hash", fh_list[i].name);
+ if (ret >= sizeof(signame)) {
+ printf("path + name too long\n");
+ return 1;
+ }
+ fd = open(signame, O_WRONLY | O_CREAT | O_TRUNC, DEF_F_PERM);
+ if (fd < 0) {
+ printf("Can't create %s: %m\n", signame);
+ return 1;
+ }
+ build_proof(mt, i, fd);
+ close(fd);
+ }
+
+ free_mtree(mt);
+ for (i = 0; i < num_files; i++)
+ free(fh_list[i].name);
+ free(fh_list);
+
+ EVP_MD_CTX_free(ctx);
+ return 0;
+
+err_ossl:
+ printf("libssl operation failed\n");
+ return 1;
+}
diff --git a/helpers.h b/helpers.h
new file mode 100644
index 0000000000000..f52ad3543f890
--- /dev/null
+++ b/helpers.h
@@ -0,0 +1,109 @@
+#ifndef __HELPERS_H__
+#define __HELPERS_H__
+
+static EVP_MD_CTX *ctx;
+static int hash_size;
+
+#define DEF_F_PERM (S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH) /* 0644*/
+#define DEF_D_PERM (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) /* 0755*/
+
+static unsigned int host_to_be32(unsigned int v)
+{
+#if __BYTE_ORDER__ == __LITTLE_ENDIAN
+ return __builtin_bswap32(v);
+#elif __BYTE_ORDER__ == __BIG_ENDIAN
+ return v;
+#else
+#error Missing endian define
+#endif
+}
+
+static inline void *xcalloc(size_t n, size_t size)
+{
+ void *p;
+
+ p = calloc(n, size);
+ if (p)
+ return p;
+ printf("Memory allocation failed\n");
+ exit(1);
+}
+
+static void *xmalloc(size_t size)
+{
+ void *p;
+
+ p = malloc(size);
+ if (p)
+ return p;
+ printf("Memory allocation failed\n");
+ exit(1);
+}
+
+static inline void *xrealloc(void *oldp, size_t size)
+{
+ void *p;
+
+ p = realloc(oldp, size);
+ if (p)
+ return p;
+ printf("Memory allocation failed\n");
+ exit(1);
+}
+
+static void hash_data(void *p, unsigned int pos, size_t size, void *ret_hash)
+{
+ unsigned char magic = 0x01;
+ unsigned int pos_be;
+
+ pos_be = host_to_be32(pos);
+ if (EVP_DigestInit_ex(ctx, NULL, NULL) != 1)
+ goto err;
+
+ if (EVP_DigestUpdate(ctx, &magic, sizeof(magic)) != 1)
+ goto err;
+
+ if (EVP_DigestUpdate(ctx, &pos_be, sizeof(pos_be)) != 1)
+ goto err;
+
+ if (EVP_DigestUpdate(ctx, p, size) != 1)
+ goto err;
+
+ if (EVP_DigestFinal_ex(ctx, ret_hash, NULL) != 1)
+ goto err;
+
+ return;
+
+err:
+ printf("libssl operation failed\n");
+ exit(1);
+}
+static void hash_entry(void *left, void *right, void *ret_hash)
+{
+ unsigned char magic = 0x02;
+
+ if (EVP_DigestInit_ex(ctx, NULL, NULL) != 1)
+ goto err;
+
+ if (EVP_DigestUpdate(ctx, &magic, sizeof(magic)) != 1)
+ goto err;
+
+ if (EVP_DigestUpdate(ctx, left, hash_size) != 1)
+ goto err;
+
+ if (EVP_DigestUpdate(ctx, right, hash_size) != 1)
+ goto err;
+
+ if (EVP_DigestFinal_ex(ctx, ret_hash, NULL) != 1)
+ goto err;
+
+ return;
+
+err:
+ printf("libssl operation failed\n");
+ exit(1);
+}
+
+
+
+#endif
diff --git a/verify_hash.c b/verify_hash.c
new file mode 100644
index 0000000000000..0a842f27f1ebc
--- /dev/null
+++ b/verify_hash.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Verify a file against and its hash against a merkle tree hash.
+ *
+ * Author: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
+ *
+ */
+#define _GNU_SOURCE 1
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include <sys/stat.h>
+#include <sys/mman.h>
+
+#include <openssl/evp.h>
+
+#include "helpers.h"
+
+struct hash_root {
+ unsigned int level;
+ unsigned char hash[EVP_MAX_MD_SIZE];
+};
+
+struct verify_sig {
+ unsigned int pos;
+ char hash_sigs[];
+};
+
+static int hash_file(const char *f, unsigned char *hash, unsigned int pos)
+{
+ struct stat sb;
+ int fd, ret;
+ void *mem;
+
+ fd = open(f, O_RDONLY);
+ if (fd < 0) {
+ printf("Failed to open %s: %m\n", f);
+ exit(1);
+ }
+
+ ret = fstat(fd, &sb);
+ if (ret) {
+ printf("stat failed %m\n");
+ exit(1);
+ }
+
+ mem = mmap(NULL, sb.st_size, PROT_READ, MAP_SHARED, fd, 0);
+ close(fd);
+
+ if (mem == MAP_FAILED) {
+ printf("Failed to mmap file: %m\n");
+ exit(1);
+ }
+
+ hash_data(mem, pos, sb.st_size, hash);
+
+ munmap(mem, sb.st_size);
+ return 0;
+}
+
+static void verify_hash(struct hash_root *hr, struct verify_sig *vs, unsigned char *cur,
+ const char *fn)
+{
+ unsigned char tmp[EVP_MAX_MD_SIZE];
+ unsigned sig_ofs = 0;
+ unsigned int i, n;
+
+ n = vs->pos;
+ if ((n & 1) == 0)
+ hash_entry(cur, &vs->hash_sigs[sig_ofs], tmp);
+ else
+ hash_entry(&vs->hash_sigs[sig_ofs], cur, tmp);
+
+ memcpy(cur, tmp, hash_size);
+ sig_ofs += hash_size;
+ for (i = 0; i < hr->level - 1; i++) {
+ n >>= 1;
+ if ((n & 1) == 0) {
+ hash_entry(cur, &vs->hash_sigs[sig_ofs], tmp);
+ } else {
+ hash_entry(&vs->hash_sigs[sig_ofs], cur, tmp);
+ }
+ memcpy(cur, tmp, hash_size);
+ sig_ofs += hash_size;
+ }
+
+ if (!memcmp(cur, hr->hash, hash_size)) {
+ exit(0);
+ } else {
+ printf("MISS-MATCH on %s\n", fn);
+ exit(1);
+ }
+}
+
+static void read_be_int(int fd, unsigned int *val)
+{
+ unsigned int val_be;
+
+ if (read(fd, &val_be, sizeof(val_be)) != sizeof(val_be)) {
+ printf("Can't read from file\n");
+ exit(1);
+ }
+ *val = host_to_be32(val_be);
+}
+
+struct hash_root *read_root_hash(const char *f)
+{
+ int fd;
+ struct hash_root *hr;
+
+ hr = xmalloc(sizeof(*hr));
+ fd = open(f, O_RDONLY);
+ if (fd < 0) {
+ printf("Can't open %s: %m\n", f);
+ exit(1);
+ }
+ read_be_int(fd, &hr->level);
+ if (read(fd, hr->hash, hash_size) != hash_size) {
+ printf("Can't read complete hash (%u): %m\n",
+ hash_size);
+ exit(1);
+ }
+ close(fd);
+ return hr;
+}
+
+static void load_hash_sig(const char *f, struct verify_sig *verify_sig,
+ unsigned int sig_num)
+{
+ ssize_t total_hash_size;
+ struct stat sb;
+ char buf[1024];
+ int fd;
+ int ret;
+
+ total_hash_size = sig_num * hash_size;
+
+ ret = snprintf(buf, sizeof(buf), "%s.hash", f);
+ if (ret >= sizeof(buf)) {
+ printf("Too long\n");
+ exit(1);
+ }
+ fd = open(buf, O_RDONLY);
+ if (fd < 0) {
+ printf("Failed to open %s\n", buf);
+ exit(1);
+ }
+ read_be_int(fd, &verify_sig->pos);
+
+ ret = fstat(fd, &sb);
+ if (ret < 0) {
+ printf("Failed to stat %s: %m\n", f);
+ exit(1);
+ }
+
+ if (sb.st_size != total_hash_size + 4) {
+ printf("Unexpected signature size: Expected %ld vs found %ld\n",
+ total_hash_size + 4, sb.st_size);
+ exit(1);
+ }
+ if (read(fd, verify_sig->hash_sigs, total_hash_size) != total_hash_size) {
+ printf("Failed to read the signature: %m\n");
+ exit(1);
+ }
+ close(fd);
+}
+
+int main(int argc, char *argv[])
+{
+ struct hash_root *hash_root;
+ struct verify_sig *vsig;
+ unsigned char fhash[EVP_MAX_MD_SIZE];
+ const EVP_MD *hash_evp;
+
+ ctx = EVP_MD_CTX_new();
+ if (!ctx)
+ goto err;
+
+ if (argc != 3) {
+ printf("%s: hash_root module\n", argv[0]);
+ return 1;
+ }
+
+ hash_evp = EVP_sha256();
+ hash_size = EVP_MD_get_size(hash_evp);
+ if (hash_size <= 0)
+ goto err;
+
+ if (EVP_DigestInit_ex(ctx, hash_evp, NULL) != 1)
+ goto err;
+
+ hash_root = read_root_hash(argv[1]);
+ vsig = xmalloc(sizeof(struct verify_sig) + hash_root->level * hash_size);
+
+ load_hash_sig(argv[2], vsig, hash_root->level);
+ hash_file(argv[2], fhash, vsig->pos);
+ verify_hash(hash_root, vsig, fhash, argv[2]);
+
+ EVP_MD_CTX_free(ctx);
+ return 0;
+err:
+ printf("libssl operation failed\n");
+ return 1;
+}
--
2.51.0
Sebastian
^ permalink raw reply related
* Re: [PATCH v3 7/9] module: Move lockdown check into generic module loader
From: Sebastian Andrzej Siewior @ 2025-11-23 17:10 UTC (permalink / raw)
To: Paul Moore
Cc: Thomas Weißschuh, Masahiro Yamada, Nathan Chancellor,
Arnd Bergmann, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, James Morris, Serge E. Hallyn, Jonathan Corbet,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <CAHC9VhTuf1u4B3uybZxdojcmz5sFG+_JHUCC=C0N=9gFDmurHg@mail.gmail.com>
On 2025-11-19 14:55:47 [-0500], Paul Moore wrote:
> On Wed, Nov 19, 2025 at 6:20 AM Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > On 2025-04-29 15:04:34 [+0200], Thomas Weißschuh wrote:
> > > The lockdown check buried in module_sig_check() will not compose well
> > > with the introduction of hash-based module validation.
> >
> > An explanation of why would be nice.
>
> /me shrugs
>
> I thought the explanation was sufficient.
Okay. So if it is just me and everyone is well aware then okay.
Sebastian
^ permalink raw reply
* Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Oleg Nesterov @ 2025-11-23 18:32 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Christian Brauner, Alexander Viro, Alexey Dobriyan, Kees Cook,
Andy Lutomirski, Will Drewry, Andrew Morton, Michal Hocko,
Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE6F59267B0E9F28F536D0E4C9A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
Hi Bernd,
sorry for delay, I am on PTO, didn't read emails this week...
On 11/17, Bernd Edlinger wrote:
>
> On 11/17/25 16:01, Oleg Nesterov wrote:
> > On 11/17, Bernd Edlinger wrote:
> >>
> >> On 11/11/25 10:21, Christian Brauner wrote:
> >>> On Wed, Nov 05, 2025 at 03:32:10PM +0100, Oleg Nesterov wrote:
> >>
> >>>> But this is minor. Why do we need "bool unsafe_execve_in_progress" ?
> >>>> If this patch is correct, de_thread() can drop/reacquire cred_guard_mutex
> >>>> unconditionally.
> >>>>
> >>
> >> I would not like to drop the mutex when no absolutely necessary for performance reasons.
> >
> > OK, I won't insist... But I don't really understand how this can help to
> > improve the performance. If nothing else, this adds another for_other_threads()
> > loop.
> >
>
> If no dead-lock is possible it is better to complete the de_thread without
> releasing the mutex. For the debugger it is also the better experience,
> no matter when the ptrace_attack happens it will succeed rather quickly either
> before the execve or after the execve.
I still disagree, I still don't understand the "performance reasons", but since I can't
convince you I won't really argue.
> >>>>> + if (unlikely(unsafe_execve_in_progress)) {
> >>>>> + spin_unlock_irq(lock);
> >>>>> + sig->exec_bprm = bprm;
> >>>>> + mutex_unlock(&sig->cred_guard_mutex);
> >>>>> + spin_lock_irq(lock);
> >>>>
> >>>> I don't think spin_unlock_irq() + spin_lock_irq() makes any sense...
> >>>>
> >>
> >> Since the spin lock was acquired while holding the mutex, both should be
> >> unlocked in reverse sequence and the spin lock re-acquired after releasing
> >> the mutex.
> >
> > Why?
> >
>
> It is generally more safe when each thread acquires its mutexes in order and
> releases them in reverse order.
> Consider this:
> Thread A:
> holds spin_lock_irq(siglock);
> does mutes_unlock(cred_guard_mutex); with irq disabled.
> task switch happens to Thread B which has irq enabled.
> and is waiting for cred_guard_mutex.
> Thrad B:
> does mutex_lock(cred_guard_mutex);
> but is interrupted this point and the interrupt handler I executes
> now iterrupt handler I wants to take siglock and is blocked,
> because the system one single CPU core.
I don't follow. Do you mean PREEMPT_RT ?
If yes. In this case spin_lock_irq() is rt_spin_lock() which doesn't disable irqs,
it does rt_lock_lock() (takes rt_mutex) + migrate_disable().
I do think that spin/mutex/whatever_unlock() is always safe. In any order, and
regardless of RT.
> > And just in case... Lets look at this code
> >
> > + rcu_assign_pointer(task->real_cred, bprm->cred);
> > + task->mm = bprm->mm;
> > + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> > + rcu_assign_pointer(task->real_cred, old_cred);
> > + task->mm = old_mm;
> >
> > again.
> >
> > This is mostly theoretical, but what if begin_new_exec() fails after de_thread()
> > and before exec_mmap() and/or commit_creds(bprm->cred) ? In this case the execing
> > thread will report SIGSEGV to debugger which can (say) read old_mm.
> >
> > No?
> >
>
> Yes, and that is the reason why the debugger has to prove the possession of access rights
> to the process before the execve which are necessary in case exeve fails, yes the debugger
> may inspect the result, and as well the debugger's access rights must be also sufficient
> to ptrace the process after execve succeeds, moreover the debugged process shall stop
> right at the first instruction where the new process starts.
Not sure I understand... OK, I see that you sent V18, and in this version ptrace_attach()
calls __ptrace_may_access() twice, so IIUC ptrace_attach() can only succeed if the debugger
has rights to trace the execing thread both before and after exec...
Oleg.
^ permalink raw reply
* Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock
From: Oleg Nesterov @ 2025-11-23 18:52 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Bernd Edlinger, Alexander Viro, Alexey Dobriyan, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <87h5uoxw06.fsf_-_@email.froward.int.ebiederm.org>
Eric,
sorry for delay, I am on PTO, didn't read emails this week...
On 11/20, Eric W. Biederman wrote:
>
> Instead of computing the new cred before we pass the point of no
> return compute the new cred just before we use it.
>
> This allows the removal of fs_struct->in_exec and cred_guard_mutex.
>
> I am not certain why we wanted to compute the cred for the new
> executable so early. Perhaps I missed something but I did not see any
> common errors being signaled. So I don't think we loose anything by
> computing the new cred later.
>
> We gain a lot.
Yes. I LIKE your approach after a quick glance. And I swear, I thought about
it too ;)
But is it correct? I don't know. I'll try to actually read your patch next
week (I am on PTO untill the end of November), but I am not sure I can
provide a valuable feedback.
One "obvious" problem is that, after this patch, the execing process can crash
in a case when currently exec() returns an error...
Oleg.
^ permalink raw reply
* Re: [PATCH v4 02/10] landlock: Add API support and docs for the quiet flags
From: Tingmao Wang @ 2025-11-23 21:00 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <20251120.Sae4geish0ei@digikod.net>
On 11/21/25 15:27, Mickaël Salaün wrote:
> On Sun, Nov 16, 2025 at 09:59:32PM +0000, Tingmao Wang wrote:
>> [...]
>> @@ -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;
>
> Because these items follows ":" they should not start with a capital
> letter (e.g. "- this layer ...").
>
>> + * - All requested accesses are part of the quiet_* fields in the
>> + * related struct landlock_ruleset_attr;
>
> This should be updated to reflect my comment in the next patch about
> landlock_log_denial().
Not sure I completely understand what needs to be changed - are you
suggesting clarifying that only the accesses denied by this layer (which
naturally has to be handled by this layer) needs to be in quiet_*? So
basically:
* - all accesses denied by this layer are part of the quiet_* fields
* in the related struct landlock_ruleset_attr;
or something else?
Note that quiet_* already has to be a subset of handled_access_*.
>
>> + * - 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;
>
> Please update the above @work_free doc.
I assume you meant the one in struct landlock_ruleset, not here. Will update.
>
>> #endif /* CONFIG_AUDIT */
>> };
>>
^ permalink raw reply
* Re: [PATCH v4 03/10] landlock: Suppress logging when quiet flag is present
From: Tingmao Wang @ 2025-11-23 21:01 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <20251120.eoghapeeGh7i@digikod.net>
On 11/21/25 15:27, Mickaël Salaün wrote:
> On Sun, Nov 16, 2025 at 09:59:33PM +0000, Tingmao Wang wrote:
>> [...]
>> 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;
>
> In this case, why update *quiet?
A legitimate case where we end up here is if layer = youngest_layer = 0,
and layer_has_quiet = true, in which case *quiet starts out as false and
we have to set it to true here.
The comment is saying the WARN_ON_ONCE should not fail because the quiet
flag does not depend on access (and hence we should not be trying to set
*quiet from true back to false if the youngest layer hasn't changed), but
*the line after that WARN is still necessary.
I've updated the comment to clarify.
>
>> }
>> }
>> 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)
>
> It would be simpler and limit code change to move rule_flags/quiet_flags
> into struct landlock_request, which means we can also remove
> no_rule_flags.
That's true, I can do that. In fact this way we also don't have to have 2
extra parameters for is_access_to_paths_allowed - it can just operate on
log_request_parent{1,2}->rule_flags. However I do see that
landlock_request is intended to only be used by audit/logging code (and
there is a comment in audit.h about not using it outside CONFIG_AUDIT to
enable it to be optimized away, although testing a fresh build on next it
doesn't look like it is taken out of vmlinux if compiled without
CONFIG_AUDIT). While this is fine for the purpose of this series as the
quiet flag only affects audit logging, I wonder if this might cause a
problem when we want to add more flags that might not be related to audit
(e.g. Justin's LANDLOCK_ADD_RULE_NO_INHERIT).
Alternatively maybe is_access_to_paths_allowed can still take extra
parameters for rule flags, and we can make it so that the new rule_flags
field in landlock_request is only assigned to right before
landlock_log_denial, not from is_access_to_paths_allowed? (I won't do
this for v5 which I will send in a minute)
>
>> {
>> 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(
>
> This quiet_mask is only used in this branch, so we can declare it here
> and make it const:
>
> const access_mask_t quiet_mask = pick_access_mask_for_request_type(
>
>
>> + request->type, youngest_denied->quiet_masks);
>> + quiet_applicable_to_access = (quiet_mask & missing) == missing;
>
> I think it should be:
>
> quiet_applicable_to_access = (quiet_mask & missing) == (handled_mask & missing);
There is no handled_mask in this context, so I assume you meant
handled_mask of the youngest_layer? But still - not sure I understand why -
missing contains requested access bits that are denied by the youngest
denying layer, and so missing would never be != youngest->handled_mask & missing,
right? Since a layer that doesn't handle an access can't deny it.
>
> We should have a test for this case: an access request (e.g. read-write)
> is denied, half by one layer (e.g. read) and half by another (e.g.
> write). Tests should cover this matrix.
Added as quiet_two_layers_different_handled_{1,2,3}
^ permalink raw reply
* [PATCH v5 00/10] Implement LANDLOCK_ADD_RULE_QUIET
From: Tingmao Wang @ 2025-11-23 20:57 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
Hi,
This is the v5 of the "quiet flag" series, implementing the feature as
proposed in [1].
v4: https://lore.kernel.org/all/cover.1763330228.git.m@maowtm.org/
v3: https://lore.kernel.org/all/cover.1761511023.git.m@maowtm.org/
v2: https://lore.kernel.org/all/cover.1759686613.git.m@maowtm.org/
v1: https://lore.kernel.org/all/cover.1757376311.git.m@maowtm.org/
v4..v5 addresses review feedbacks, most significantly:
- reduces code changes by pushing rule_flags into landlock_request.
- adding test cases for two layers handling different access bits.
v3..v4 is a one-character formatting change, plus more tests.
We now have 5 patches for the selftest - I'm happy to squash it into one
depending on preference (and happy for Mickaël to do the squash if no
other feedback):
- selftests/landlock: Replace hard-coded 16 with a constant
- selftests/landlock: add tests for quiet flag with fs rules
- selftests/landlock: add tests for quiet flag with net rules
- selftests/landlock: Add tests for quiet flag with scope
- selftests/landlock: Add tests for invalid use of quiet flag
v2..v3:
Not much has changed in the actual functionality except various comment,
typing, asserts and general style fixes based on feedback. The major new
thing here is tests (a bit of KUnit squashed into the optional access
commit, a lot of selftests especially in fs_tests.c).
The added fs_tests should exercise code path for optional and non-optional
access, renames, and mountpoint and disconnected directory handling. I
will add the above missing bits to v4.
Removed:
- "Implement quiet for optional accesses"
(squashed into "landlock: Suppress logging when quiet flag is present")
Old feature summary below:
The quiet flag allows a sandboxer to suppress audit logs for uninteresting
denials. The flag can be set on objects and inherits downward in the
filesystem hierarchy. On a denial, the youngest denying layer's quiet
flag setting decides whether to audit. The motivation for this feature is
to reduce audit noise, and also prepare for a future supervisor feature
which will use this bit to suppress supervisor notifications.
This patch introduces a new quiet access mask in the ruleset_attr, which
gets eventually stored in the hierarchy. This allows the user to specify
which access should be affected by quiet bits. One can then, for example,
make it such that read accesses to certain files are not audited (but
still denied), but all writes are still audited, regardless of location.
The sandboxer is extended to show example usage of this feature,
supporting quieting filesystem, network and scope accesses.
Demo:
/# LL_FS_RO=/usr LL_FS_RW= LL_FORCE_LOG=1 LL_FS_QUIET=/dev:/tmp:/etc LL_FS_QUIET_ACCESS=r ./sandboxer bash
...
audit: type=1423 audit(1759680175.562:195): domain=15bb25f6b blockers=fs.write_file,fs.read_file path="/dev/tty" dev="devtmpfs" ino=11
^^^^^^^^
# note: because write is not quieted, we see the above line. blockers
# contains read as well since that's the originally requested access.
audit: type=1424 audit(1759680175.562:195): domain=15bb25f6b status=allocated mode=enforcing pid=616 uid=0 exe="/sandboxer" comm="sandboxer"
audit: type=1300 audit(1759680175.562:195): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c86113d1 a2=802 a3=0 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680175.562:195): proctitle="bash"
bash: cannot set terminal process group (605): Inappropriate ioctl for device
bash: no job control in this shell
bash: /etc/bash.bashrc: Permission denied
audit: type=1423 audit(1759680175.570:196): domain=15bb25f6b blockers=fs.read_file path="/.bash_history" dev="virtiofs" ino=36963
^^^^^^^^
# read outside /dev:/tmp:/etc - not quieted
audit: type=1300 audit(1759680175.570:196): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c868e400 a2=0 a3=0 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680175.570:196): proctitle="bash"
audit: type=1423 audit(1759680175.570:197): domain=15bb25f6b blockers=fs.read_file path="/.bash_history" dev="virtiofs" ino=36963
audit: type=1300 audit(1759680175.570:197): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c868e400 a2=0 a3=0 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680175.570:197): proctitle="bash"
bash-5.2# head /etc/passwd
head: cannot open '/etc/passwd' for reading: Permission denied
^^^^^^^^
# reads to /etc are quieted
bash-5.2# echo evil >> /etc/passwd
bash: /etc/passwd: Permission denied
audit: type=1423 audit(1759680227.030:198): domain=15bb25f6b blockers=fs.write_file path="/etc/passwd" dev="virtiofs" ino=790
^^^^^^^^
# writes are not quieted
audit: type=1300 audit(1759680227.030:198): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c86ab030 a2=441 a3=1b6 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680227.030:198): proctitle="bash"
Design:
- The user can set the quiet flag for a layer on any part of the fs
hierarchy (whether it allows any access on it or not), and the flag
inherits down (no support for "cancelling" the inheritance of the flag
in specific subdirectories).
- The youngest layer that denies a request gets to decide whether the
denial is audited or not. This means that a compromised binary, for
example, cannot "turn off" Landlock auditing when it tries to access
files, unless it denies access to the files itself. There is some
debate to be had on whether, if a parent layer sets the quiet flag, but
the request is denied by a deeper layer, whether Landlock should still
audit anyway (since the rule author of the child layer likely did not
expect the denial, so it would be good diagnostic). The current
approach is to ignore the quiet on the parent layer and audit anyway.
[1]: https://github.com/landlock-lsm/linux/issues/44#issuecomment-2876500918
Kind regards,
Tingmao
Tingmao Wang (10):
landlock: Add a place for flags to layer rules
landlock: Add API support and docs for the quiet flags
landlock: Suppress logging when quiet flag is present
landlock: Fix wrong type usage
samples/landlock: Add quiet flag support to sandboxer
selftests/landlock: Replace hard-coded 16 with a constant
selftests/landlock: add tests for quiet flag with fs rules
selftests/landlock: add tests for quiet flag with net rules
selftests/landlock: Add tests for quiet flag with scope
selftests/landlock: Add tests for invalid use of quiet flag
include/uapi/linux/landlock.h | 64 +
samples/landlock/sandboxer.c | 133 +-
security/landlock/access.h | 5 +
security/landlock/audit.c | 259 +-
security/landlock/audit.h | 3 +
security/landlock/domain.c | 33 +
security/landlock/domain.h | 10 +
security/landlock/fs.c | 104 +-
security/landlock/fs.h | 19 +-
security/landlock/net.c | 10 +-
security/landlock/net.h | 5 +-
security/landlock/ruleset.c | 19 +-
security/landlock/ruleset.h | 38 +-
security/landlock/syscalls.c | 72 +-
tools/testing/selftests/landlock/audit_test.c | 27 +-
tools/testing/selftests/landlock/base_test.c | 61 +-
tools/testing/selftests/landlock/common.h | 2 +
tools/testing/selftests/landlock/fs_test.c | 2464 ++++++++++++++++-
tools/testing/selftests/landlock/net_test.c | 121 +-
.../landlock/scoped_abstract_unix_test.c | 77 +-
20 files changed, 3403 insertions(+), 123 deletions(-)
base-commit: d724c6f85e80a23ed46b7ebc6e38b527c09d64f5
--
2.52.0
^ permalink raw reply
* [PATCH v5 01/10] landlock: Add a place for flags to layer rules
From: Tingmao Wang @ 2025-11-23 20:57 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <cover.1763931318.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.
Cc: Justin Suess <utilityemal77@gmail.com>
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes in v5:
- Move rule_flags into landlock_request. This lets us get rid of the
extra parameters to is_access_to_paths_allowed (and later on,
landlock_log_denial), and thus less code changes.
Changes in v3:
- Comment changes, move local variables, simplify if branch
Changes in v2:
- Comment changes
- Rebased to include disconnected directory handling changes on mic/next
and add backing up of collected_rule_flags.
security/landlock/audit.h | 2 ++
security/landlock/fs.c | 71 +++++++++++++++++++++++++++----------
security/landlock/net.c | 3 +-
security/landlock/ruleset.c | 7 +++-
security/landlock/ruleset.h | 26 ++++++++++++--
5 files changed, 86 insertions(+), 23 deletions(-)
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d8..d66c6d936438 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -49,6 +49,8 @@ struct landlock_request {
/* Required fields for requests with deny masks. */
const access_mask_t all_existing_optional_access;
deny_masks_t deny_masks;
+
+ struct collected_rule_flags rule_flags;
};
#ifdef CONFIG_AUDIT
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index b33d8ac239c1..58dba2f7c6cd 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -775,6 +775,9 @@ 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_flags_parent1 = &log_request_parent1->rule_flags;
+ struct collected_rule_flags *rule_flags_parent2 = &log_request_parent2->rule_flags;
+ 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);
@@ -1139,7 +1168,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 +1188,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.
@@ -1327,11 +1357,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,
+ &request1.rule_flags);
allow_parent2 = collect_domain_accesses(subject->domain, &mnt_dir,
new_dir->dentry,
- &layer_masks_parent2);
+ &layer_masks_parent2,
+ &request2.rule_flags);
if (allow_parent1 && allow_parent2)
return 0;
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.52.0
^ permalink raw reply related
* [PATCH v5 02/10] landlock: Add API support and docs for the quiet flags
From: Tingmao Wang @ 2025-11-23 20:57 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <cover.1763931318.git.m@maowtm.org>
Adds the UAPI for the quiet flags feature (but not the implementation
yet).
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 in v5:
- Doc fixes.
- Fix build failure without CONFIG_AUDIT / CONFIG_INET (reported by Justin
Suess)
Changes in v4:
- Minor update to this commit message.
- Fix minor formatting
Changes in v3:
- Updated docs from Mickaël's suggestions.
Changes in v2:
- 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 | 5 +-
security/landlock/ruleset.c | 12 +++-
security/landlock/ruleset.h | 12 +++-
security/landlock/syscalls.c | 72 +++++++++++++++-----
tools/testing/selftests/landlock/base_test.c | 4 +-
10 files changed, 155 insertions(+), 30 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f030adc462ee..d4f47d20361a 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 accesses denied by this layer 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 58dba2f7c6cd..bf45a7f9e1d7 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..72c47f4d6803 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)
{
@@ -24,7 +25,7 @@ static inline void landlock_add_net_hooks(void)
static inline int
landlock_append_net_rule(struct landlock_ruleset *const ruleset, const u16 port,
- access_mask_t access_rights)
+ access_mask_t access_rights, const int flags)
{
return -EAFNOSUPPORT;
}
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 81cdf87d1c79..750a444e1983 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,10 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
if (err)
return ERR_PTR(err);
+#ifdef CONFIG_AUDIT
+ new_dom->hierarchy->quiet_masks = ruleset->quiet_masks;
+#endif /* CONFIG_AUDIT */
+
return no_free_ptr(new_dom);
}
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 9790c60c0c00..eb60db646422 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -166,8 +166,8 @@ struct landlock_ruleset {
* @work_free: Enables to free a ruleset within a lockless
* section. This is only used by
* landlock_put_ruleset_deferred() when @usage reaches zero.
- * The fields @lock, @usage, @num_rules, @num_layers and
- * @access_masks are then unused.
+ * The fields @lock, @usage, @num_rules, @num_layers, @quiet_masks
+ * and @access_masks are then unused.
*/
struct work_struct work_free;
struct {
@@ -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.52.0
^ permalink raw reply related
* [PATCH v5 03/10] landlock: Suppress logging when quiet flag is present
From: Tingmao Wang @ 2025-11-23 20:57 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <cover.1763931318.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 in v5:
- Update code style and comment in get_layer_from_deny_masks() and
landlock_log_denial()
- Now that rule_flags is moved into landlock_request, this version removes
the extra parameter for landlock_log_denial and gets rid of
no_rule_flags, simplifying some code.
- Fix build failure without CONFIG_AUDIT (reported by Justin Suess)
Changes in v3:
- 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 in v2:
- 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 | 257 +++++++++++++++++++++++++++++++++++--
security/landlock/audit.h | 1 +
security/landlock/domain.c | 33 +++++
security/landlock/domain.h | 5 +
security/landlock/fs.c | 29 +++++
security/landlock/fs.h | 17 ++-
security/landlock/net.c | 2 +-
8 files changed, 327 insertions(+), 22 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..1a9d3f4e3369 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -273,7 +273,8 @@ 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;
@@ -281,6 +282,7 @@ get_layer_from_deny_masks(access_mask_t *const access_request,
size_t youngest_layer = 0;
size_t access_index = 0;
unsigned long access_bit;
+ bool should_quiet = false;
/* This will require change with new object types. */
WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
@@ -291,18 +293,29 @@ get_layer_from_deny_masks(access_mask_t *const access_request,
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;
missing = BIT(access_bit);
+ should_quiet = layer_has_quiet;
} 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
+ * the following WARN_ON_ONCE() should not fail.
+ */
+ WARN_ON_ONCE(should_quiet && !layer_has_quiet);
+ should_quiet = layer_has_quiet;
}
}
access_index++;
}
*access_request = missing;
+ *quiet = should_quiet;
return youngest_layer;
}
@@ -312,42 +325,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,11 +540,29 @@ 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)
@@ -394,6 +571,7 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
struct landlock_hierarchy *youngest_denied;
size_t youngest_layer;
access_mask_t missing;
+ bool object_quiet_flag = false, quiet_applicable_to_access = false;
if (WARN_ON_ONCE(!subject || !subject->domain ||
!subject->domain->hierarchy || !request))
@@ -409,10 +587,14 @@ 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 = !!(request->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 +629,53 @@ 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.
+ */
+ const access_mask_t 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.
+ */
+ const access_mask_t 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 d66c6d936438..f0647ee6052c 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;
struct collected_rule_flags rule_flags;
};
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 bf45a7f9e1d7..29f10da32141 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1750,8 +1750,31 @@ get_required_file_open_access(const struct file *const file)
return access;
}
+static void build_check_file_security(void)
+{
+#ifdef CONFIG_AUDIT
+ 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);
+#endif /* CONFIG_AUDIT */
+}
+
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.
@@ -1835,6 +1858,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, request.rule_flags);
#endif /* CONFIG_AUDIT */
if ((open_access_request & allowed_access) == open_access_request)
@@ -1871,6 +1898,7 @@ 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 */
});
return -EACCES;
@@ -1910,6 +1938,7 @@ 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 */
});
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..a3a90a4de70a 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -193,7 +193,7 @@ 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 = rule_flags });
return -EACCES;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v5 04/10] landlock: Fix wrong type usage
From: Tingmao Wang @ 2025-11-23 20:57 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <cover.1763931318.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 in v2:
- Added Fixes tag
security/landlock/audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 1a9d3f4e3369..d51563712325 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -191,7 +191,7 @@ static size_t get_denied_layer(const struct landlock_ruleset *const domain,
long youngest_layer = -1;
for_each_set_bit(access_bit, &access_req, layer_masks_size) {
- const access_mask_t mask = (*layer_masks)[access_bit];
+ const layer_mask_t mask = (*layer_masks)[access_bit];
long layer;
if (!mask)
--
2.52.0
^ permalink raw reply related
* [PATCH v5 05/10] samples/landlock: Add quiet flag support to sandboxer
From: Tingmao Wang @ 2025-11-23 20:57 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <cover.1763931318.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 in v3:
- Minor change to the above commit message.
Changes in v2:
- 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.52.0
^ permalink raw reply related
* [PATCH v5 06/10] selftests/landlock: Replace hard-coded 16 with a constant
From: Tingmao Wang @ 2025-11-23 20:57 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <cover.1763931318.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 in v3:
- 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.52.0
^ 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