* [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET
@ 2025-09-09 0:06 Tingmao Wang
2025-09-09 0:06 ` [RFC PATCH 1/6] landlock: Add a place for flags to layer rules Tingmao Wang
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Tingmao Wang @ 2025-09-09 0:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, linux-security-module
Hi Mickaël,
This RFC patch series implements a first pass patch of the "quiet flag"
feature as proposed in [1]. I've evolved the design beyond the original
discussion to come up with what I believe would be most useful. For this
implementation:
- The user can set the quiet flag for a layer on any part of the fs
hierarchy, 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)
This series does not add any tests yet (and also no support for
suppressing optional access denial audit yet due to complexity). If
you're happy with this design I can write some tests (and add the missing
support). Here is a sandboxer demo:
# LL_FS_RO=/ LL_FS_RW= LL_FORCE_LOG=1 LL_FS_QUIET=/tmp linux/samples/landlock/sandboxer /bin/bash
Executing the sandboxed command...
[ 135.126499][ T60] audit: type=1423 audit(1757374868.281:942): domain=1a435130e blockers=fs.write_file path="/dev/tty" dev="devtmpfs" ino=11
[ 135.133298][ T60] audit: type=1424 audit(1757374868.281:942): domain=1a435130e status=allocated mode=enforcing pid=959 uid=0 exe="/linux/samples/landlock/sandboxer" comm="sandboxer"
[ 135.141869][ T60] audit: type=1300 audit(1757374868.281:942): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=557a9cda83d1 a2=802 a3=0 items=0 ppid=958 pid=959 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)
[ 135.156620][ T60] audit: type=1327 audit(1757374868.281:942): proctitle="/bin/bash"
bash: cannot set terminal process group (958): Inappropriate ioctl for device
bash: no job control in this shell
# echo quiet > /tmp/aa
bash: /tmp/aa: Permission denied
# echo not quiet > /usr/aa
[ 165.358804][ T60] audit: type=1423 audit(1757374898.513:943): domain=1a435130e blockers=fs.make_reg path="/usr" dev="virtiofs" ino=840
[ 165.363746][ T60] audit: type=1300 audit(1757374898.513:943): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=557a9ce447c0 a2=241 a3=1b6 items=0 ppid=958 pid=959 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)
[ 165.375594][ T60] audit: type=1327 audit(1757374898.513:943): proctitle="/bin/bash"
bash: /usr/aa: Permission denied
## (still in sandboxer)
# LL_FS_RO= LL_FS_RW=/ LL_FS_QUIET=/ linux/samples/landlock/sandboxer /bin/bash
Executing the sandboxed command...
[ 203.490417][ T60] audit: type=1423 audit(1757374936.645:944): domain=1a435130e blockers=fs.write_file path="/dev/tty" dev="devtmpfs" ino=11
...
# echo "child can't suppress audit logs" > /usr/a
[ 219.948543][ T60] audit: type=1423 audit(1757374953.101:945): domain=1a435130e blockers=fs.make_reg path="/usr" dev="virtiofs" ino=840
[ 219.953918][ T60] audit: type=1300 audit(1757374953.101:945): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5651ea7875c0 a2=241 a3=1b6 items=0 ppid=959 pid=960 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)
[ 219.969167][ T60] audit: type=1327 audit(1757374953.101:945): proctitle="/bin/bash"
bash: /usr/a: Permission denied
# echo "/tmp is still quiet" > /tmp/a
bash: /tmp/a: Permission denied
# exit
(still in first layer sandboxer)
# LL_FS_RO=/ LL_FS_RW= LL_FS_QUIET= LL_FORCE_LOG=1 linux/samples/landlock/sandboxer /bin/bash
Executing the sandboxed command...
...
root@fced6595bd01:/# echo "not quiet now" > /tmp/a
[ 492.130486][ T60] audit: type=1423 audit(1757375225.285:949): domain=1a435132a blockers=fs.make_reg path="/tmp" dev="tmpfs" ino=1
[ 492.136729][ T60] audit: type=1300 audit(1757375225.285:949): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=55fc4c168450 a2=241 a3=1b6 items=0 ppid=959 pid=964 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)
[ 492.151727][ T60] audit: type=1327 audit(1757375225.285:949): proctitle="/bin/bash"
bash: /tmp/a: Permission denied
All existing kselftests pass.
[1]: https://github.com/landlock-lsm/linux/issues/44#issuecomment-2876500918
Kind regards,
Tingmao
Tingmao Wang (6):
landlock: Add a place for flags to layer rules
landlock: Add API support for the quiet flag
landlock/audit: Check for quiet flag in landlock_log_denial
landlock/audit: Fix wrong type usage
landlock/access: Improve explanation on the deny_masks_t
samples/landlock: Add FS quiet flag support to sandboxer
include/uapi/linux/landlock.h | 25 +++++
samples/landlock/sandboxer.c | 20 +++-
security/landlock/access.h | 6 +-
security/landlock/audit.c | 18 +++-
security/landlock/audit.h | 3 +-
security/landlock/fs.c | 99 ++++++++++++--------
security/landlock/fs.h | 2 +-
security/landlock/net.c | 11 ++-
security/landlock/net.h | 3 +-
security/landlock/ruleset.c | 17 +++-
security/landlock/ruleset.h | 29 +++++-
security/landlock/syscalls.c | 28 +++---
security/landlock/task.c | 12 +--
tools/testing/selftests/landlock/base_test.c | 2 +-
14 files changed, 199 insertions(+), 76 deletions(-)
base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
--
2.51.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
@ 2025-09-09 0:06 ` Tingmao Wang
2025-09-19 16:02 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 2/6] landlock: Add API support for the quiet flag Tingmao Wang
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Tingmao Wang @ 2025-09-09 0:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, linux-security-module
To avoid unnecessarily increasing the size of struct landlock_layer, we
make the layer level a u8 and use the space to store the flags struct.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
security/landlock/fs.c | 75 ++++++++++++++++++++++++-------------
security/landlock/net.c | 3 +-
security/landlock/ruleset.c | 9 ++++-
security/landlock/ruleset.h | 27 ++++++++++++-
4 files changed, 83 insertions(+), 31 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c04f8879ad03..e7eaf55093e9 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -756,10 +756,12 @@ static bool is_access_to_paths_allowed(
const struct path *const path,
const access_mask_t access_request_parent1,
layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
+ struct collected_rule_flags *const rule_flags_parent1,
struct landlock_request *const log_request_parent1,
struct dentry *const dentry_child1,
const access_mask_t access_request_parent2,
layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
+ struct collected_rule_flags *const rule_flags_parent2,
struct landlock_request *const log_request_parent2,
struct dentry *const dentry_child2)
{
@@ -810,22 +812,30 @@ static bool is_access_to_paths_allowed(
}
if (unlikely(dentry_child1)) {
+ /*
+ * The rule_flags for child1 should have been included in
+ * rule_flags_masks_parent1 already. We do not bother about it
+ * for domain check.
+ */
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);
}
@@ -881,16 +891,18 @@ static bool is_access_to_paths_allowed(
}
rule = find_rule(domain, walker_path.dentry);
- allowed_parent1 = allowed_parent1 ||
- landlock_unmask_layers(
- rule, access_masked_parent1,
- layer_masks_parent1,
- ARRAY_SIZE(*layer_masks_parent1));
- allowed_parent2 = allowed_parent2 ||
- landlock_unmask_layers(
- rule, access_masked_parent2,
- layer_masks_parent2,
- ARRAY_SIZE(*layer_masks_parent2));
+ allowed_parent1 =
+ allowed_parent1 ||
+ landlock_unmask_layers(rule, 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)
@@ -958,6 +970,7 @@ static int current_check_access_path(const struct path *const path,
const struct landlock_cred_security *const subject =
landlock_get_applicable_subject(current_cred(), masks, NULL);
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+ struct collected_rule_flags rule_flags = {};
struct landlock_request request = {};
if (!subject)
@@ -967,8 +980,8 @@ static int current_check_access_path(const struct path *const path,
access_request, &layer_masks,
LANDLOCK_KEY_INODE);
if (is_access_to_paths_allowed(subject->domain, path, access_request,
- &layer_masks, &request, NULL, 0, NULL,
- NULL, NULL))
+ &layer_masks, &rule_flags, &request,
+ NULL, 0, NULL, NULL, NULL, NULL))
return 0;
landlock_log_denial(subject, &request);
@@ -1032,7 +1045,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 dentry *const mnt_root, 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)
{
unsigned long access_dom;
bool ret = false;
@@ -1051,9 +1065,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)) {
/*
* Stops when all handled accesses are allowed by at
* least one rule in each layer.
@@ -1140,6 +1154,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
struct dentry *old_parent;
layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
+ struct collected_rule_flags rule_flags_parent1 = {},
+ rule_flags_parent2 = {};
struct landlock_request request1 = {}, request2 = {};
if (!subject)
@@ -1172,10 +1188,10 @@ static int current_check_refer_path(struct dentry *const old_dentry,
subject->domain,
access_request_parent1 | access_request_parent2,
&layer_masks_parent1, LANDLOCK_KEY_INODE);
- if (is_access_to_paths_allowed(subject->domain, new_dir,
- access_request_parent1,
- &layer_masks_parent1, &request1,
- NULL, 0, NULL, NULL, NULL))
+ if (is_access_to_paths_allowed(
+ subject->domain, new_dir, access_request_parent1,
+ &layer_masks_parent1, &rule_flags_parent1,
+ &request1, NULL, 0, NULL, NULL, NULL, NULL))
return 0;
landlock_log_denial(subject, &request1);
@@ -1201,10 +1217,12 @@ static int current_check_refer_path(struct dentry *const old_dentry,
/* new_dir->dentry is equal to new_dentry->d_parent */
allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
old_parent,
- &layer_masks_parent1);
+ &layer_masks_parent1,
+ &rule_flags_parent1);
allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
new_dir->dentry,
- &layer_masks_parent2);
+ &layer_masks_parent2,
+ &rule_flags_parent2);
if (allow_parent1 && allow_parent2)
return 0;
@@ -1217,8 +1235,9 @@ static int current_check_refer_path(struct dentry *const old_dentry,
*/
if (is_access_to_paths_allowed(
subject->domain, &mnt_dir, access_request_parent1,
- &layer_masks_parent1, &request1, old_dentry,
- access_request_parent2, &layer_masks_parent2, &request2,
+ &layer_masks_parent1, &rule_flags_parent1, &request1,
+ old_dentry, access_request_parent2, &layer_masks_parent2,
+ &rule_flags_parent2, &request2,
exchange ? new_dentry : NULL))
return 0;
@@ -1616,6 +1635,7 @@ static bool is_device(const struct file *const file)
static int hook_file_open(struct file *const file)
{
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+ struct collected_rule_flags rule_flags = {};
access_mask_t open_access_request, full_access_request, allowed_access,
optional_access;
const struct landlock_cred_security *const subject =
@@ -1647,7 +1667,8 @@ static int hook_file_open(struct file *const file)
landlock_init_layer_masks(subject->domain,
full_access_request, &layer_masks,
LANDLOCK_KEY_INODE),
- &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
+ &layer_masks, &rule_flags, &request, NULL, 0, NULL, NULL,
+ NULL, NULL)) {
allowed_access = full_access_request;
} else {
unsigned long access_bit;
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 1f3915a90a80..fc6369dffa51 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -48,6 +48,7 @@ static int current_check_access_socket(struct socket *const sock,
{
__be16 port;
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+ struct collected_rule_flags rule_flags = {};
const struct landlock_rule *rule;
struct landlock_id id = {
.type = LANDLOCK_KEY_NET_PORT,
@@ -179,7 +180,7 @@ static int current_check_access_socket(struct socket *const sock,
access_request, &layer_masks,
LANDLOCK_KEY_NET_PORT);
if (landlock_unmask_layers(rule, access_request, &layer_masks,
- ARRAY_SIZE(layer_masks)))
+ ARRAY_SIZE(layer_masks), &rule_flags))
return 0;
audit_net.family = address->sa_family;
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index ce7940efea51..3aa4e33ac95b 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -616,7 +616,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;
@@ -643,6 +644,12 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
unsigned long access_bit;
bool is_empty;
+ if (rule_flags) {
+ /* Collect rule flags for each layer */
+ if (layer->flags.quiet)
+ rule_flags->quiet_masks |= layer_bit;
+ }
+
/*
* Records in @layer_masks which layer grants access to each
* requested access.
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 5da9a64f5af7..d4b70b6af137 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.
*/
- 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 fs 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,17 @@ 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 +326,8 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
bool landlock_unmask_layers(const struct landlock_rule *const rule,
const access_mask_t access_request,
layer_mask_t (*const layer_masks)[],
- const size_t masks_array_size);
+ const size_t masks_array_size,
+ struct collected_rule_flags *const rule_flags);
access_mask_t
landlock_init_layer_masks(const struct landlock_ruleset *const domain,
--
2.51.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 2/6] landlock: Add API support for the quiet flag
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
2025-09-09 0:06 ` [RFC PATCH 1/6] landlock: Add a place for flags to layer rules Tingmao Wang
@ 2025-09-09 0:06 ` Tingmao Wang
2025-09-19 16:02 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 3/6] landlock/audit: Check for quiet flag in landlock_log_denial Tingmao Wang
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Tingmao Wang @ 2025-09-09 0:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, linux-security-module
Also added documentation.
As for kselftests, for now we just change add_rule_checks_ordering to use
a different invalid flag. I will add tests for the quiet flag in a later
version.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
include/uapi/linux/landlock.h | 25 +++++++++++++++++
security/landlock/fs.c | 4 +--
security/landlock/fs.h | 2 +-
security/landlock/net.c | 5 ++--
security/landlock/net.h | 3 ++-
security/landlock/ruleset.c | 8 +++++-
security/landlock/ruleset.h | 2 +-
security/landlock/syscalls.c | 28 ++++++++++++--------
tools/testing/selftests/landlock/base_test.c | 2 +-
9 files changed, 59 insertions(+), 20 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f030adc462ee..3e5b2ce0b18b 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -69,6 +69,31 @@ struct landlock_ruleset_attr {
#define LANDLOCK_CREATE_RULESET_ERRATA (1U << 1)
/* clang-format on */
+/**
+ * DOC: landlock_add_rule_flags
+ *
+ * **Flags**
+ *
+ * %LANDLOCK_ADD_RULE_QUIET
+ * This flag controls whether Landlock will log audit messages when
+ * access to the objects covered by this rule is denied by this layer.
+ *
+ * When Landlock denies an access, if audit logging is enabled,
+ * Landlock will check if the youngest layer that denied the access
+ * has marked the object in question as "quiet". If so, no audit log
+ * will be generated. Note that logging is only suppressed if the
+ * layer that denied the access is this layer. This means that a
+ * sandboxed program cannot use this flag to "hide" access denials,
+ * unless it denies itself the access.
+ *
+ * When this flag is present, the caller is allowed to pass in a rule
+ * with 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/fs.c b/security/landlock/fs.c
index e7eaf55093e9..b566ae498df5 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -322,7 +322,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
*/
int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
const struct path *const path,
- access_mask_t access_rights)
+ access_mask_t access_rights, const int flags)
{
int err;
struct landlock_id id = {
@@ -343,7 +343,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
if (IS_ERR(id.key.object))
return PTR_ERR(id.key.object);
mutex_lock(&ruleset->lock);
- err = landlock_insert_rule(ruleset, id, access_rights);
+ err = landlock_insert_rule(ruleset, id, access_rights, flags);
mutex_unlock(&ruleset->lock);
/*
* No need to check for an error because landlock_insert_rule()
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index bf9948941f2f..cb7e654933ac 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -126,6 +126,6 @@ __init void landlock_add_fs_hooks(void);
int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
const struct path *const path,
- access_mask_t access_hierarchy);
+ access_mask_t access_hierarchy, const int flags);
#endif /* _SECURITY_LANDLOCK_FS_H */
diff --git a/security/landlock/net.c b/security/landlock/net.c
index fc6369dffa51..bddbe93d69fd 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -20,7 +20,8 @@
#include "ruleset.h"
int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
- const u16 port, access_mask_t access_rights)
+ const u16 port, access_mask_t access_rights,
+ const int flags)
{
int err;
const struct landlock_id id = {
@@ -35,7 +36,7 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
~landlock_get_net_access_mask(ruleset, 0);
mutex_lock(&ruleset->lock);
- err = landlock_insert_rule(ruleset, id, access_rights);
+ err = landlock_insert_rule(ruleset, id, access_rights, flags);
mutex_unlock(&ruleset->lock);
return err;
diff --git a/security/landlock/net.h b/security/landlock/net.h
index 09960c237a13..799cedd5d0b7 100644
--- a/security/landlock/net.h
+++ b/security/landlock/net.h
@@ -16,7 +16,8 @@
__init void landlock_add_net_hooks(void);
int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
- const u16 port, access_mask_t access_rights);
+ const u16 port, access_mask_t access_rights,
+ const int flags);
#else /* IS_ENABLED(CONFIG_INET) */
static inline void landlock_add_net_hooks(void)
{
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 3aa4e33ac95b..990aa1a2c120 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"
@@ -251,6 +252,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;
}
@@ -297,12 +299,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 ? 1 : 0,
+ }
} };
build_check_layer();
@@ -343,6 +348,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)
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index d4b70b6af137..4f184d2da382 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -224,7 +224,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..e46164246fdb 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -312,7 +312,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;
@@ -328,8 +328,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.
+ * (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. */
@@ -344,13 +346,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;
@@ -364,8 +366,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.
+ * (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. */
@@ -379,7 +383,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
/* 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 +394,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.
@@ -414,6 +418,9 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
* @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 +431,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 +441,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..d07a0bf6927c 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -201,7 +201,7 @@ TEST(add_rule_checks_ordering)
ASSERT_LE(0, ruleset_fd);
/* Checks invalid flags. */
- ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 1));
+ ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 100));
ASSERT_EQ(EINVAL, errno);
/* Checks invalid ruleset FD. */
--
2.51.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 3/6] landlock/audit: Check for quiet flag in landlock_log_denial
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
2025-09-09 0:06 ` [RFC PATCH 1/6] landlock: Add a place for flags to layer rules Tingmao Wang
2025-09-09 0:06 ` [RFC PATCH 2/6] landlock: Add API support for the quiet flag Tingmao Wang
@ 2025-09-09 0:06 ` Tingmao Wang
2025-09-19 16:02 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 4/6] landlock/audit: Fix wrong type usage Tingmao Wang
` (3 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Tingmao Wang @ 2025-09-09 0:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, linux-security-module
Suppresses logging if the flag is effective on the youngest layer.
This does not handle optional access logging yet - to do that correctly we will
need to expand deny_masks to support representing "don't log anything"
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
security/landlock/audit.c | 16 +++++++++++++++-
security/landlock/audit.h | 3 ++-
security/landlock/fs.c | 20 +++++++++++---------
security/landlock/net.c | 3 ++-
security/landlock/task.c | 12 ++++++------
5 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index c52d079cdb77..2b3edd1ab374 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -386,9 +386,12 @@ static bool is_valid_request(const struct landlock_request *const request)
*
* @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 NULL 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 *const rule_flags)
{
struct audit_buffer *ab;
struct landlock_hierarchy *youngest_denied;
@@ -436,6 +439,17 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
if (!audit_enabled)
return;
+ /*
+ * Check 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 (rule_flags &&
+ rule_flags->quiet_masks & BIT(youngest_layer)) {
+ return;
+ }
+
/* Checks if the current exec was restricting itself. */
if (subject->domain_exec & BIT(youngest_layer)) {
/* Ignores denials for the same execution. */
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d8..e6f76d417c2f 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -56,7 +56,8 @@ struct landlock_request {
void landlock_log_drop_domain(const struct landlock_hierarchy *const hierarchy);
void landlock_log_denial(const struct landlock_cred_security *const subject,
- const struct landlock_request *const request);
+ const struct landlock_request *const request,
+ const struct collected_rule_flags *const rule_flags);
#else /* CONFIG_AUDIT */
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index b566ae498df5..ba93b0de384c 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -984,7 +984,7 @@ static int current_check_access_path(const struct path *const path,
NULL, 0, NULL, NULL, NULL, NULL))
return 0;
- landlock_log_denial(subject, &request);
+ landlock_log_denial(subject, &request, &rule_flags);
return -EACCES;
}
@@ -1194,7 +1194,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
&request1, NULL, 0, NULL, NULL, NULL, NULL))
return 0;
- landlock_log_denial(subject, &request1);
+ landlock_log_denial(subject, &request1, &rule_flags_parent1);
return -EACCES;
}
@@ -1243,11 +1243,13 @@ static int current_check_refer_path(struct dentry *const old_dentry,
if (request1.access) {
request1.audit.u.path.dentry = old_parent;
- landlock_log_denial(subject, &request1);
+ landlock_log_denial(subject, &request1,
+ &rule_flags_parent1);
}
if (request2.access) {
request2.audit.u.path.dentry = new_dir->dentry;
- landlock_log_denial(subject, &request2);
+ landlock_log_denial(subject, &request2,
+ &rule_flags_parent2);
}
/*
@@ -1403,7 +1405,7 @@ log_fs_change_topology_path(const struct landlock_cred_security *const subject,
.u.path = *path,
},
.layer_plus_one = handle_layer + 1,
- });
+ }, NULL);
}
static void log_fs_change_topology_dentry(
@@ -1417,7 +1419,7 @@ static void log_fs_change_topology_dentry(
.u.dentry = dentry,
},
.layer_plus_one = handle_layer + 1,
- });
+ }, NULL);
}
/*
@@ -1705,7 +1707,7 @@ static int hook_file_open(struct file *const file)
/* Sets access to reflect the actual request. */
request.access = open_access_request;
- landlock_log_denial(subject, &request);
+ landlock_log_denial(subject, &request, &rule_flags);
return -EACCES;
}
@@ -1735,7 +1737,7 @@ static int hook_file_truncate(struct file *const file)
#ifdef CONFIG_AUDIT
.deny_masks = landlock_file(file)->deny_masks,
#endif /* CONFIG_AUDIT */
- });
+ }, NULL);
return -EACCES;
}
@@ -1774,7 +1776,7 @@ static int hook_file_ioctl_common(const struct file *const file,
#ifdef CONFIG_AUDIT
.deny_masks = landlock_file(file)->deny_masks,
#endif /* CONFIG_AUDIT */
- });
+ }, NULL);
return -EACCES;
}
diff --git a/security/landlock/net.c b/security/landlock/net.c
index bddbe93d69fd..d242bb9fa5b4 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -193,7 +193,8 @@ static int current_check_access_socket(struct socket *const sock,
.access = access_request,
.layer_masks = &layer_masks,
.layer_masks_size = ARRAY_SIZE(layer_masks),
- });
+ },
+ &rule_flags);
return -EACCES;
}
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 2385017418ca..dfea227ce1d7 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -115,7 +115,7 @@ static int hook_ptrace_access_check(struct task_struct *const child,
.u.tsk = child,
},
.layer_plus_one = parent_subject->domain->num_layers,
- });
+ }, NULL);
return err;
}
@@ -161,7 +161,7 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
.u.tsk = current,
},
.layer_plus_one = parent_subject->domain->num_layers,
- });
+ }, NULL);
return err;
}
@@ -290,7 +290,7 @@ static int hook_unix_stream_connect(struct sock *const sock,
},
},
.layer_plus_one = handle_layer + 1,
- });
+ }, NULL);
return -EPERM;
}
@@ -327,7 +327,7 @@ static int hook_unix_may_send(struct socket *const sock,
},
},
.layer_plus_one = handle_layer + 1,
- });
+ }, NULL);
return -EPERM;
}
@@ -383,7 +383,7 @@ static int hook_task_kill(struct task_struct *const p,
.u.tsk = p,
},
.layer_plus_one = handle_layer + 1,
- });
+ }, NULL);
return -EPERM;
}
@@ -426,7 +426,7 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
#ifdef CONFIG_AUDIT
.layer_plus_one = landlock_file(fown->file)->fown_layer + 1,
#endif /* CONFIG_AUDIT */
- });
+ }, NULL);
return -EPERM;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 4/6] landlock/audit: Fix wrong type usage
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
` (2 preceding siblings ...)
2025-09-09 0:06 ` [RFC PATCH 3/6] landlock/audit: Check for quiet flag in landlock_log_denial Tingmao Wang
@ 2025-09-09 0:06 ` Tingmao Wang
2025-09-19 16:03 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t Tingmao Wang
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Tingmao Wang @ 2025-09-09 0:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, linux-security-module
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>
---
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 2b3edd1ab374..a67155c7f0c3 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -191,7 +191,7 @@ static size_t get_denied_layer(const struct landlock_ruleset *const domain,
long youngest_layer = -1;
for_each_set_bit(access_bit, &access_req, layer_masks_size) {
- const access_mask_t mask = (*layer_masks)[access_bit];
+ const layer_mask_t mask = (*layer_masks)[access_bit];
long layer;
if (!mask)
--
2.51.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
` (3 preceding siblings ...)
2025-09-09 0:06 ` [RFC PATCH 4/6] landlock/audit: Fix wrong type usage Tingmao Wang
@ 2025-09-09 0:06 ` Tingmao Wang
2025-09-19 16:04 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 6/6] samples/landlock: Add FS quiet flag support to sandboxer Tingmao Wang
2025-09-19 16:01 ` [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Mickaël Salaün
6 siblings, 1 reply; 19+ messages in thread
From: Tingmao Wang @ 2025-09-09 0:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, linux-security-module
Not really related to this series, but just something which took me a
while to realize, and would probably be helpful as a comment.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
security/landlock/access.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 7961c6630a2d..5e2285575479 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -67,8 +67,10 @@ typedef u16 layer_mask_t;
static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
/*
- * Tracks domains responsible of a denied access. This is required to avoid
- * storing in each object the full layer_masks[] required by update_request().
+ * Tracks domains responsible of a denied access, stored in the form of
+ * two 4-bit layer numbers packed into a byte (one for each optional
+ * access). This is required to avoid storing in each object the full
+ * layer_masks[] required by update_request().
*/
typedef u8 deny_masks_t;
--
2.51.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 6/6] samples/landlock: Add FS quiet flag support to sandboxer
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
` (4 preceding siblings ...)
2025-09-09 0:06 ` [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t Tingmao Wang
@ 2025-09-09 0:06 ` Tingmao Wang
2025-09-19 16:01 ` [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Mickaël Salaün
6 siblings, 0 replies; 19+ messages in thread
From: Tingmao Wang @ 2025-09-09 0:06 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, linux-security-module
net rule support is TODO
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
samples/landlock/sandboxer.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208..77c99329b3ba 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -58,6 +58,7 @@ 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_TCP_BIND_NAME "LL_TCP_BIND"
#define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
#define ENV_SCOPED_NAME "LL_SCOPED"
@@ -116,7 +117,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 +167,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));
@@ -328,6 +330,7 @@ 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 " can then be used to make access to some denied paths not trigger audit logging.\n"
"\n"
"Example:\n"
ENV_FS_RO_NAME "=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
@@ -497,12 +500,21 @@ 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 (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)) {
--
2.51.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
` (5 preceding siblings ...)
2025-09-09 0:06 ` [RFC PATCH 6/6] samples/landlock: Add FS quiet flag support to sandboxer Tingmao Wang
@ 2025-09-19 16:01 ` Mickaël Salaün
6 siblings, 0 replies; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-19 16:01 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
Hi Tingmao,
I have a few comments but overall this series looks very good. Thanks!
On Tue, Sep 09, 2025 at 01:06:34AM +0100, Tingmao Wang wrote:
> Hi Mickaël,
>
> This RFC patch series implements a first pass patch of the "quiet flag"
> feature as proposed in [1]. I've evolved the design beyond the original
> discussion to come up with what I believe would be most useful. For this
> implementation:
>
> - The user can set the quiet flag for a layer on any part of the fs
> hierarchy, 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)
>
> This series does not add any tests yet (and also no support for
> suppressing optional access denial audit yet due to complexity). If
> you're happy with this design I can write some tests (and add the missing
> support).
Yes, please.
> Here is a sandboxer demo:
>
> # LL_FS_RO=/ LL_FS_RW= LL_FORCE_LOG=1 LL_FS_QUIET=/tmp linux/samples/landlock/sandboxer /bin/bash
> Executing the sandboxed command...
> [ 135.126499][ T60] audit: type=1423 audit(1757374868.281:942): domain=1a435130e blockers=fs.write_file path="/dev/tty" dev="devtmpfs" ino=11
> [ 135.133298][ T60] audit: type=1424 audit(1757374868.281:942): domain=1a435130e status=allocated mode=enforcing pid=959 uid=0 exe="/linux/samples/landlock/sandboxer" comm="sandboxer"
> [ 135.141869][ T60] audit: type=1300 audit(1757374868.281:942): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=557a9cda83d1 a2=802 a3=0 items=0 ppid=958 pid=959 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)
> [ 135.156620][ T60] audit: type=1327 audit(1757374868.281:942): proctitle="/bin/bash"
> bash: cannot set terminal process group (958): Inappropriate ioctl for device
> bash: no job control in this shell
>
> # echo quiet > /tmp/aa
> bash: /tmp/aa: Permission denied
>
> # echo not quiet > /usr/aa
> [ 165.358804][ T60] audit: type=1423 audit(1757374898.513:943): domain=1a435130e blockers=fs.make_reg path="/usr" dev="virtiofs" ino=840
> [ 165.363746][ T60] audit: type=1300 audit(1757374898.513:943): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=557a9ce447c0 a2=241 a3=1b6 items=0 ppid=958 pid=959 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)
> [ 165.375594][ T60] audit: type=1327 audit(1757374898.513:943): proctitle="/bin/bash"
> bash: /usr/aa: Permission denied
>
> ## (still in sandboxer)
> # LL_FS_RO= LL_FS_RW=/ LL_FS_QUIET=/ linux/samples/landlock/sandboxer /bin/bash
> Executing the sandboxed command...
> [ 203.490417][ T60] audit: type=1423 audit(1757374936.645:944): domain=1a435130e blockers=fs.write_file path="/dev/tty" dev="devtmpfs" ino=11
> ...
> # echo "child can't suppress audit logs" > /usr/a
> [ 219.948543][ T60] audit: type=1423 audit(1757374953.101:945): domain=1a435130e blockers=fs.make_reg path="/usr" dev="virtiofs" ino=840
> [ 219.953918][ T60] audit: type=1300 audit(1757374953.101:945): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5651ea7875c0 a2=241 a3=1b6 items=0 ppid=959 pid=960 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)
> [ 219.969167][ T60] audit: type=1327 audit(1757374953.101:945): proctitle="/bin/bash"
> bash: /usr/a: Permission denied
> # echo "/tmp is still quiet" > /tmp/a
> bash: /tmp/a: Permission denied
> # exit
>
> (still in first layer sandboxer)
> # LL_FS_RO=/ LL_FS_RW= LL_FS_QUIET= LL_FORCE_LOG=1 linux/samples/landlock/sandboxer /bin/bash
> Executing the sandboxed command...
> ...
> root@fced6595bd01:/# echo "not quiet now" > /tmp/a
> [ 492.130486][ T60] audit: type=1423 audit(1757375225.285:949): domain=1a435132a blockers=fs.make_reg path="/tmp" dev="tmpfs" ino=1
> [ 492.136729][ T60] audit: type=1300 audit(1757375225.285:949): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=55fc4c168450 a2=241 a3=1b6 items=0 ppid=959 pid=964 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)
> [ 492.151727][ T60] audit: type=1327 audit(1757375225.285:949): proctitle="/bin/bash"
> bash: /tmp/a: Permission denied
>
> All existing kselftests pass.
>
> [1]: https://github.com/landlock-lsm/linux/issues/44#issuecomment-2876500918
>
> Kind regards,
> Tingmao
>
> Tingmao Wang (6):
> landlock: Add a place for flags to layer rules
> landlock: Add API support for the quiet flag
> landlock/audit: Check for quiet flag in landlock_log_denial
> landlock/audit: Fix wrong type usage
> landlock/access: Improve explanation on the deny_masks_t
> samples/landlock: Add FS quiet flag support to sandboxer
>
> include/uapi/linux/landlock.h | 25 +++++
> samples/landlock/sandboxer.c | 20 +++-
> security/landlock/access.h | 6 +-
> security/landlock/audit.c | 18 +++-
> security/landlock/audit.h | 3 +-
> security/landlock/fs.c | 99 ++++++++++++--------
> security/landlock/fs.h | 2 +-
> security/landlock/net.c | 11 ++-
> security/landlock/net.h | 3 +-
> security/landlock/ruleset.c | 17 +++-
> security/landlock/ruleset.h | 29 +++++-
> security/landlock/syscalls.c | 28 +++---
> security/landlock/task.c | 12 +--
> tools/testing/selftests/landlock/base_test.c | 2 +-
> 14 files changed, 199 insertions(+), 76 deletions(-)
>
>
> base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
2025-09-09 0:06 ` [RFC PATCH 1/6] landlock: Add a place for flags to layer rules Tingmao Wang
@ 2025-09-19 16:02 ` Mickaël Salaün
2025-09-21 23:52 ` Tingmao Wang
0 siblings, 1 reply; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-19 16:02 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
On Tue, Sep 09, 2025 at 01:06:35AM +0100, Tingmao Wang wrote:
> To avoid unnecessarily increasing the size of struct landlock_layer, we
> make the layer level a u8 and use the space to store the flags struct.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> security/landlock/fs.c | 75 ++++++++++++++++++++++++-------------
> security/landlock/net.c | 3 +-
> security/landlock/ruleset.c | 9 ++++-
> security/landlock/ruleset.h | 27 ++++++++++++-
> 4 files changed, 83 insertions(+), 31 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index c04f8879ad03..e7eaf55093e9 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -756,10 +756,12 @@ static bool is_access_to_paths_allowed(
> const struct path *const path,
> const access_mask_t access_request_parent1,
> layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> + struct collected_rule_flags *const rule_flags_parent1,
> struct landlock_request *const log_request_parent1,
> struct dentry *const dentry_child1,
> const access_mask_t access_request_parent2,
> layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> + struct collected_rule_flags *const rule_flags_parent2,
> struct landlock_request *const log_request_parent2,
> struct dentry *const dentry_child2)
> {
> @@ -810,22 +812,30 @@ static bool is_access_to_paths_allowed(
> }
>
> if (unlikely(dentry_child1)) {
> + /*
> + * The rule_flags for child1 should have been included in
> + * rule_flags_masks_parent1 already. We do not bother about it
> + * for domain check.
> + */
> 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);
> }
> @@ -881,16 +891,18 @@ static bool is_access_to_paths_allowed(
> }
>
> rule = find_rule(domain, walker_path.dentry);
> - allowed_parent1 = allowed_parent1 ||
> - landlock_unmask_layers(
> - rule, access_masked_parent1,
> - layer_masks_parent1,
> - ARRAY_SIZE(*layer_masks_parent1));
> - allowed_parent2 = allowed_parent2 ||
> - landlock_unmask_layers(
> - rule, access_masked_parent2,
> - layer_masks_parent2,
> - ARRAY_SIZE(*layer_masks_parent2));
> + allowed_parent1 =
> + allowed_parent1 ||
> + landlock_unmask_layers(rule, 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)
> @@ -958,6 +970,7 @@ static int current_check_access_path(const struct path *const path,
> const struct landlock_cred_security *const subject =
> landlock_get_applicable_subject(current_cred(), masks, NULL);
> layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct collected_rule_flags rule_flags = {};
> struct landlock_request request = {};
>
> if (!subject)
> @@ -967,8 +980,8 @@ static int current_check_access_path(const struct path *const path,
> access_request, &layer_masks,
> LANDLOCK_KEY_INODE);
> if (is_access_to_paths_allowed(subject->domain, path, access_request,
> - &layer_masks, &request, NULL, 0, NULL,
> - NULL, NULL))
> + &layer_masks, &rule_flags, &request,
> + NULL, 0, NULL, NULL, NULL, NULL))
> return 0;
>
> landlock_log_denial(subject, &request);
> @@ -1032,7 +1045,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 dentry *const mnt_root, 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)
> {
> unsigned long access_dom;
> bool ret = false;
> @@ -1051,9 +1065,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)) {
> /*
> * Stops when all handled accesses are allowed by at
> * least one rule in each layer.
> @@ -1140,6 +1154,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> struct dentry *old_parent;
> layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
> layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct collected_rule_flags rule_flags_parent1 = {},
> + rule_flags_parent2 = {};
> struct landlock_request request1 = {}, request2 = {};
>
> if (!subject)
> @@ -1172,10 +1188,10 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> subject->domain,
> access_request_parent1 | access_request_parent2,
> &layer_masks_parent1, LANDLOCK_KEY_INODE);
> - if (is_access_to_paths_allowed(subject->domain, new_dir,
> - access_request_parent1,
> - &layer_masks_parent1, &request1,
> - NULL, 0, NULL, NULL, NULL))
> + if (is_access_to_paths_allowed(
> + subject->domain, new_dir, access_request_parent1,
> + &layer_masks_parent1, &rule_flags_parent1,
> + &request1, NULL, 0, NULL, NULL, NULL, NULL))
> return 0;
>
> landlock_log_denial(subject, &request1);
> @@ -1201,10 +1217,12 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> /* new_dir->dentry is equal to new_dentry->d_parent */
> allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
> old_parent,
> - &layer_masks_parent1);
> + &layer_masks_parent1,
> + &rule_flags_parent1);
> allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
> new_dir->dentry,
> - &layer_masks_parent2);
> + &layer_masks_parent2,
> + &rule_flags_parent2);
>
> if (allow_parent1 && allow_parent2)
> return 0;
> @@ -1217,8 +1235,9 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> */
> if (is_access_to_paths_allowed(
> subject->domain, &mnt_dir, access_request_parent1,
> - &layer_masks_parent1, &request1, old_dentry,
> - access_request_parent2, &layer_masks_parent2, &request2,
> + &layer_masks_parent1, &rule_flags_parent1, &request1,
> + old_dentry, access_request_parent2, &layer_masks_parent2,
> + &rule_flags_parent2, &request2,
> exchange ? new_dentry : NULL))
> return 0;
>
> @@ -1616,6 +1635,7 @@ static bool is_device(const struct file *const file)
> static int hook_file_open(struct file *const file)
> {
> layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct collected_rule_flags rule_flags = {};
> access_mask_t open_access_request, full_access_request, allowed_access,
> optional_access;
> const struct landlock_cred_security *const subject =
> @@ -1647,7 +1667,8 @@ static int hook_file_open(struct file *const file)
> landlock_init_layer_masks(subject->domain,
> full_access_request, &layer_masks,
> LANDLOCK_KEY_INODE),
> - &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
> + &layer_masks, &rule_flags, &request, NULL, 0, NULL, NULL,
> + NULL, NULL)) {
> allowed_access = full_access_request;
> } else {
> unsigned long access_bit;
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index 1f3915a90a80..fc6369dffa51 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -48,6 +48,7 @@ static int current_check_access_socket(struct socket *const sock,
> {
> __be16 port;
> layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
> + struct collected_rule_flags rule_flags = {};
> const struct landlock_rule *rule;
> struct landlock_id id = {
> .type = LANDLOCK_KEY_NET_PORT,
> @@ -179,7 +180,7 @@ static int current_check_access_socket(struct socket *const sock,
> access_request, &layer_masks,
> LANDLOCK_KEY_NET_PORT);
> if (landlock_unmask_layers(rule, access_request, &layer_masks,
> - ARRAY_SIZE(layer_masks)))
> + ARRAY_SIZE(layer_masks), &rule_flags))
> return 0;
>
> audit_net.family = address->sa_family;
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index ce7940efea51..3aa4e33ac95b 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -616,7 +616,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;
>
> @@ -643,6 +644,12 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
> unsigned long access_bit;
> bool is_empty;
>
> + if (rule_flags) {
> + /* Collect rule flags for each layer */
> + if (layer->flags.quiet)
> + rule_flags->quiet_masks |= layer_bit;
This patch makes quiet flags related to on object, but not tied to the
rule access rights (as explained in the next patch's doc). To tie it to
rule access rights would require to duplicate the access bits for each
rule (because multiple rules can be tied to the same object for the same
layer/ruleset).
So, the question is, what do we really need to mute?
I think the current approach is enough. We could still add a new flag in
the future, or maybe even a new field to each rule type. However, we
should rename the flag to make it clear that the it's related to the
rule's object which is muted instead of the whole rule. Maybe something
like LANDLOCK_ADD_RULE_QUIET_OBJECT?
If we want to have a more fine-grained control, a complementary patch
could add a bitfield for each access right type to quiet a denied access
right iff the object is also quiet (where rules are possible). That
could be a follow up to complete this quiet feature, but this patch
series is good on its own.
> + }
> +
> /*
> * Records in @layer_masks which layer grants access to each
> * requested access.
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 5da9a64f5af7..d4b70b6af137 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.
> */
> - 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 fs rules, this inherits down the
Please use filesystem instead of fs.
> + * 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,17 @@ struct landlock_layer {
> access_mask_t access;
> };
>
> +
Extra new line.
> +/**
> + * 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 +326,8 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
> bool landlock_unmask_layers(const struct landlock_rule *const rule,
> const access_mask_t access_request,
> layer_mask_t (*const layer_masks)[],
> - const size_t masks_array_size);
> + const size_t masks_array_size,
> + struct collected_rule_flags *const rule_flags);
>
> access_mask_t
> landlock_init_layer_masks(const struct landlock_ruleset *const domain,
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 2/6] landlock: Add API support for the quiet flag
2025-09-09 0:06 ` [RFC PATCH 2/6] landlock: Add API support for the quiet flag Tingmao Wang
@ 2025-09-19 16:02 ` Mickaël Salaün
0 siblings, 0 replies; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-19 16:02 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
On Tue, Sep 09, 2025 at 01:06:36AM +0100, Tingmao Wang wrote:
> Also added documentation.
>
> As for kselftests, for now we just change add_rule_checks_ordering to use
> a different invalid flag. I will add tests for the quiet flag in a later
> version.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> include/uapi/linux/landlock.h | 25 +++++++++++++++++
> security/landlock/fs.c | 4 +--
> security/landlock/fs.h | 2 +-
> security/landlock/net.c | 5 ++--
> security/landlock/net.h | 3 ++-
> security/landlock/ruleset.c | 8 +++++-
> security/landlock/ruleset.h | 2 +-
> security/landlock/syscalls.c | 28 ++++++++++++--------
> tools/testing/selftests/landlock/base_test.c | 2 +-
> 9 files changed, 59 insertions(+), 20 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f030adc462ee..3e5b2ce0b18b 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -69,6 +69,31 @@ struct landlock_ruleset_attr {
> #define LANDLOCK_CREATE_RULESET_ERRATA (1U << 1)
> /* clang-format on */
>
> +/**
> + * DOC: landlock_add_rule_flags
> + *
> + * **Flags**
> + *
> + * %LANDLOCK_ADD_RULE_QUIET
> + * This flag controls whether Landlock will log audit messages when
> + * access to the objects covered by this rule is denied by this layer.
> + *
> + * When Landlock denies an access, if audit logging is enabled,
> + * Landlock will check if the youngest layer that denied the access
> + * has marked the object in question as "quiet". If so, no audit log
> + * will be generated. Note that logging is only suppressed if the
> + * layer that denied the access is this layer. This means that a
> + * sandboxed program cannot use this flag to "hide" access denials,
> + * unless it denies itself the access.
> + *
> + * When this flag is present, the caller is allowed to pass in a rule
> + * with 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/fs.c b/security/landlock/fs.c
> index e7eaf55093e9..b566ae498df5 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -322,7 +322,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> */
> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> const struct path *const path,
> - access_mask_t access_rights)
> + access_mask_t access_rights, const int flags)
> {
> int err;
> struct landlock_id id = {
> @@ -343,7 +343,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> if (IS_ERR(id.key.object))
> return PTR_ERR(id.key.object);
> mutex_lock(&ruleset->lock);
> - err = landlock_insert_rule(ruleset, id, access_rights);
> + err = landlock_insert_rule(ruleset, id, access_rights, flags);
> mutex_unlock(&ruleset->lock);
> /*
> * No need to check for an error because landlock_insert_rule()
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index bf9948941f2f..cb7e654933ac 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -126,6 +126,6 @@ __init void landlock_add_fs_hooks(void);
>
> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> const struct path *const path,
> - access_mask_t access_hierarchy);
> + access_mask_t access_hierarchy, const int flags);
>
> #endif /* _SECURITY_LANDLOCK_FS_H */
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index fc6369dffa51..bddbe93d69fd 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -20,7 +20,8 @@
> #include "ruleset.h"
>
> int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> - const u16 port, access_mask_t access_rights)
> + const u16 port, access_mask_t access_rights,
> + const int flags)
> {
> int err;
> const struct landlock_id id = {
> @@ -35,7 +36,7 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> ~landlock_get_net_access_mask(ruleset, 0);
>
> mutex_lock(&ruleset->lock);
> - err = landlock_insert_rule(ruleset, id, access_rights);
> + err = landlock_insert_rule(ruleset, id, access_rights, flags);
> mutex_unlock(&ruleset->lock);
>
> return err;
> diff --git a/security/landlock/net.h b/security/landlock/net.h
> index 09960c237a13..799cedd5d0b7 100644
> --- a/security/landlock/net.h
> +++ b/security/landlock/net.h
> @@ -16,7 +16,8 @@
> __init void landlock_add_net_hooks(void);
>
> int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> - const u16 port, access_mask_t access_rights);
> + const u16 port, access_mask_t access_rights,
> + const int flags);
> #else /* IS_ENABLED(CONFIG_INET) */
> static inline void landlock_add_net_hooks(void)
> {
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 3aa4e33ac95b..990aa1a2c120 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"
> @@ -251,6 +252,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;
> }
>
> @@ -297,12 +299,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 ? 1 : 0,
This looks better to me (no hardcoded values):
.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET,
> + }
> } };
>
> build_check_layer();
> @@ -343,6 +348,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)
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index d4b70b6af137..4f184d2da382 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -224,7 +224,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..e46164246fdb 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -312,7 +312,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;
> @@ -328,8 +328,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.
> + * (However, the rule is not useless if it is there to hold a quiet
> + * flag)
There is no need for parenthesis, please just append to the previous
sentence.
> */
> - if (!path_beneath_attr.allowed_access)
> + if (!flags && !path_beneath_attr.allowed_access)
> return -ENOMSG;
>
> /* Checks that allowed_access matches the @ruleset constraints. */
> @@ -344,13 +346,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;
> @@ -364,8 +366,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.
> + * (However, the rule is not useless if it is there to hold a quiet
> + * flag)
ditto
> */
> - if (!net_port_attr.allowed_access)
> + if (!flags && !net_port_attr.allowed_access)
> return -ENOMSG;
>
> /* Checks that allowed_access matches the @ruleset constraints. */
> @@ -379,7 +383,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
>
> /* 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 +394,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.
> @@ -414,6 +418,9 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> * @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 +431,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 +441,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..d07a0bf6927c 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -201,7 +201,7 @@ TEST(add_rule_checks_ordering)
> ASSERT_LE(0, ruleset_fd);
>
> /* Checks invalid flags. */
> - ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 1));
> + ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 100));
> ASSERT_EQ(EINVAL, errno);
>
> /* Checks invalid ruleset FD. */
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 3/6] landlock/audit: Check for quiet flag in landlock_log_denial
2025-09-09 0:06 ` [RFC PATCH 3/6] landlock/audit: Check for quiet flag in landlock_log_denial Tingmao Wang
@ 2025-09-19 16:02 ` Mickaël Salaün
0 siblings, 0 replies; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-19 16:02 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
On Tue, Sep 09, 2025 at 01:06:37AM +0100, Tingmao Wang wrote:
> Suppresses logging if the flag is effective on the youngest layer.
>
> This does not handle optional access logging yet - to do that correctly we will
> need to expand deny_masks to support representing "don't log anything"
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> security/landlock/audit.c | 16 +++++++++++++++-
> security/landlock/audit.h | 3 ++-
> security/landlock/fs.c | 20 +++++++++++---------
> security/landlock/net.c | 3 ++-
> security/landlock/task.c | 12 ++++++------
> 5 files changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index c52d079cdb77..2b3edd1ab374 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -386,9 +386,12 @@ static bool is_valid_request(const struct landlock_request *const request)
> *
> * @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 NULL 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 *const rule_flags)
No need for a pointer for this small struct. For scope requests,
rule_flags can just be 0, which should also simplify the following
check.
I think that's the only place where we could replace a pointer with the
(small) raw struct, but if there are other in the code, this rule should
also be applied.
> {
> struct audit_buffer *ab;
> struct landlock_hierarchy *youngest_denied;
> @@ -436,6 +439,17 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> if (!audit_enabled)
> return;
>
> + /*
> + * Check if the object is marked quiet by the layer that denied the
For consistency: "Checks"
> + * 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)
No need for parenthesis.
> + */
> + if (rule_flags &&
> + rule_flags->quiet_masks & BIT(youngest_layer)) {
> + return;
> + }
> +
> /* Checks if the current exec was restricting itself. */
> if (subject->domain_exec & BIT(youngest_layer)) {
> /* Ignores denials for the same execution. */
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 92428b7fc4d8..e6f76d417c2f 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -56,7 +56,8 @@ struct landlock_request {
> void landlock_log_drop_domain(const struct landlock_hierarchy *const hierarchy);
>
> void landlock_log_denial(const struct landlock_cred_security *const subject,
> - const struct landlock_request *const request);
> + const struct landlock_request *const request,
> + const struct collected_rule_flags *const rule_flags);
>
> #else /* CONFIG_AUDIT */
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index b566ae498df5..ba93b0de384c 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -984,7 +984,7 @@ static int current_check_access_path(const struct path *const path,
> NULL, 0, NULL, NULL, NULL, NULL))
> return 0;
>
> - landlock_log_denial(subject, &request);
> + landlock_log_denial(subject, &request, &rule_flags);
> return -EACCES;
> }
>
> @@ -1194,7 +1194,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> &request1, NULL, 0, NULL, NULL, NULL, NULL))
> return 0;
>
> - landlock_log_denial(subject, &request1);
> + landlock_log_denial(subject, &request1, &rule_flags_parent1);
> return -EACCES;
> }
>
> @@ -1243,11 +1243,13 @@ static int current_check_refer_path(struct dentry *const old_dentry,
>
> if (request1.access) {
> request1.audit.u.path.dentry = old_parent;
> - landlock_log_denial(subject, &request1);
> + landlock_log_denial(subject, &request1,
> + &rule_flags_parent1);
> }
> if (request2.access) {
> request2.audit.u.path.dentry = new_dir->dentry;
> - landlock_log_denial(subject, &request2);
> + landlock_log_denial(subject, &request2,
> + &rule_flags_parent2);
> }
>
> /*
> @@ -1403,7 +1405,7 @@ log_fs_change_topology_path(const struct landlock_cred_security *const subject,
> .u.path = *path,
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, NULL);
> }
>
> static void log_fs_change_topology_dentry(
> @@ -1417,7 +1419,7 @@ static void log_fs_change_topology_dentry(
> .u.dentry = dentry,
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, NULL);
> }
>
> /*
> @@ -1705,7 +1707,7 @@ static int hook_file_open(struct file *const file)
>
> /* Sets access to reflect the actual request. */
> request.access = open_access_request;
> - landlock_log_denial(subject, &request);
> + landlock_log_denial(subject, &request, &rule_flags);
> return -EACCES;
> }
>
> @@ -1735,7 +1737,7 @@ static int hook_file_truncate(struct file *const file)
> #ifdef CONFIG_AUDIT
> .deny_masks = landlock_file(file)->deny_masks,
> #endif /* CONFIG_AUDIT */
> - });
> + }, NULL);
> return -EACCES;
> }
>
> @@ -1774,7 +1776,7 @@ static int hook_file_ioctl_common(const struct file *const file,
> #ifdef CONFIG_AUDIT
> .deny_masks = landlock_file(file)->deny_masks,
> #endif /* CONFIG_AUDIT */
> - });
> + }, NULL);
> return -EACCES;
> }
>
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index bddbe93d69fd..d242bb9fa5b4 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -193,7 +193,8 @@ static int current_check_access_socket(struct socket *const sock,
> .access = access_request,
> .layer_masks = &layer_masks,
> .layer_masks_size = ARRAY_SIZE(layer_masks),
> - });
> + },
> + &rule_flags);
> return -EACCES;
> }
>
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 2385017418ca..dfea227ce1d7 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -115,7 +115,7 @@ static int hook_ptrace_access_check(struct task_struct *const child,
> .u.tsk = child,
> },
> .layer_plus_one = parent_subject->domain->num_layers,
> - });
> + }, NULL);
>
> return err;
> }
> @@ -161,7 +161,7 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> .u.tsk = current,
> },
> .layer_plus_one = parent_subject->domain->num_layers,
> - });
> + }, NULL);
> return err;
> }
>
> @@ -290,7 +290,7 @@ static int hook_unix_stream_connect(struct sock *const sock,
> },
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, NULL);
> return -EPERM;
> }
>
> @@ -327,7 +327,7 @@ static int hook_unix_may_send(struct socket *const sock,
> },
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, NULL);
> return -EPERM;
> }
>
> @@ -383,7 +383,7 @@ static int hook_task_kill(struct task_struct *const p,
> .u.tsk = p,
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, NULL);
> return -EPERM;
> }
>
> @@ -426,7 +426,7 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
> #ifdef CONFIG_AUDIT
> .layer_plus_one = landlock_file(fown->file)->fown_layer + 1,
> #endif /* CONFIG_AUDIT */
> - });
> + }, NULL);
> return -EPERM;
> }
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 4/6] landlock/audit: Fix wrong type usage
2025-09-09 0:06 ` [RFC PATCH 4/6] landlock/audit: Fix wrong type usage Tingmao Wang
@ 2025-09-19 16:03 ` Mickaël Salaün
0 siblings, 0 replies; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-19 16:03 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
On Tue, Sep 09, 2025 at 01:06:38AM +0100, Tingmao Wang wrote:
> I think, based on my best understanding, that this type is likely a typo
> (even though in the end both are u16)
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
Indeed, good catch!
You can add:
Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
> ---
> 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 2b3edd1ab374..a67155c7f0c3 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -191,7 +191,7 @@ static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> long youngest_layer = -1;
>
> for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> - const access_mask_t mask = (*layer_masks)[access_bit];
> + const layer_mask_t mask = (*layer_masks)[access_bit];
> long layer;
>
> if (!mask)
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t
2025-09-09 0:06 ` [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t Tingmao Wang
@ 2025-09-19 16:04 ` Mickaël Salaün
2025-09-21 23:52 ` Tingmao Wang
0 siblings, 1 reply; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-19 16:04 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
Looks good, I'll take it.
On Tue, Sep 09, 2025 at 01:06:39AM +0100, Tingmao Wang wrote:
> Not really related to this series, but just something which took me a
> while to realize, and would probably be helpful as a comment.
Please just describe the change in the main commit message and move this
kind of explanation bellow a "---", just after your SoB. This is useful
for review and avoid unrelated information when picking a patch out of
this context.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> security/landlock/access.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d..5e2285575479 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -67,8 +67,10 @@ typedef u16 layer_mask_t;
> static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
>
> /*
> - * Tracks domains responsible of a denied access. This is required to avoid
> - * storing in each object the full layer_masks[] required by update_request().
> + * Tracks domains responsible of a denied access, stored in the form of
> + * two 4-bit layer numbers packed into a byte (one for each optional
> + * access). This is required to avoid storing in each object the full
> + * layer_masks[] required by update_request().
> */
> typedef u8 deny_masks_t;
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
2025-09-19 16:02 ` Mickaël Salaün
@ 2025-09-21 23:52 ` Tingmao Wang
2025-09-24 9:20 ` Mickaël Salaün
0 siblings, 1 reply; 19+ messages in thread
From: Tingmao Wang @ 2025-09-21 23:52 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
On 9/19/25 17:02, Mickaël Salaün wrote:
> On Tue, Sep 09, 2025 at 01:06:35AM +0100, Tingmao Wang wrote:
>> To avoid unnecessarily increasing the size of struct landlock_layer, we
>> make the layer level a u8 and use the space to store the flags struct.
>>
>> Signed-off-by: Tingmao Wang <m@maowtm.org>
>> ---
>> security/landlock/fs.c | 75 ++++++++++++++++++++++++-------------
>> security/landlock/net.c | 3 +-
>> security/landlock/ruleset.c | 9 ++++-
>> security/landlock/ruleset.h | 27 ++++++++++++-
>> 4 files changed, 83 insertions(+), 31 deletions(-)
>>
>> [...]
>> @@ -643,6 +644,12 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
>> unsigned long access_bit;
>> bool is_empty;
>>
>> + if (rule_flags) {
>> + /* Collect rule flags for each layer */
>> + if (layer->flags.quiet)
>> + rule_flags->quiet_masks |= layer_bit;
>
> This patch makes quiet flags related to on object, but not tied to the
> rule access rights (as explained in the next patch's doc). To tie it to
> rule access rights would require to duplicate the access bits for each
> rule (because multiple rules can be tied to the same object for the same
> layer/ruleset).
(imo the use of "rule" here as a terminology is a bit confusing, I would
have thought that a "rule" is a collection of access rights associated
with an object, and therefore you can of course only have one rule per
object in a ruleset. Otherwise landlock_add_rule should really have been
called landlock_add_rules...?)
>
> So, the question is, what do we really need to mute?
>
> I think the current approach is enough. We could still add a new flag in
> the future, or maybe even a new field to each rule type. However, we
> should rename the flag to make it clear that the it's related to the
> rule's object which is muted instead of the whole rule. Maybe something
> like LANDLOCK_ADD_RULE_QUIET_OBJECT?
I don't see what benefit a new field to each rule type would bring, since
different rule types targets different objects and live in different
rbtrees, and so they are already separable.
>
> If we want to have a more fine-grained control, a complementary patch
> could add a bitfield for each access right type to quiet a denied access
> right iff the object is also quiet (where rules are possible). That
> could be a follow up to complete this quiet feature, but this patch
> series is good on its own.
Worth noting that if one really wants to suppress logging for only some
access bits and we do not add support for it (due to the extra overhead
etc), that is still doable with just this patch by using two layers - the
outer one would contain the intended rules but not have any quiet flags,
whereas the inner one would contain the same set of rules but with quiet
flags set, except that for access bits which the sandbox does not want to
be quiet, it would also "allow" them in the second layer (access would
still be denied by the first layer, but would get audit logged due to
quiet flag not applying when the younger layer allows the access).
But this gets very tricky in the context of mutable domains, and does not
work at all for the purpose of controlling whether supervisor mode would
delegate to the supervisor or deny outright, since supervisors are
"accumulative". Therefore if this (different "quietness" for different
access bits) becomes a strong need, we should probably consider some way
of implementing it, rather than expecting a sandboxer to do this two-layer
hack. (But implementing this does have the potential to result in needing
to have a (number of access bits) x (number of layers) matrix...)
> [...]
Will add suggested changes in v2. Thanks for the review :)
Kind regards,
Tingmao
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t
2025-09-19 16:04 ` Mickaël Salaün
@ 2025-09-21 23:52 ` Tingmao Wang
0 siblings, 0 replies; 19+ messages in thread
From: Tingmao Wang @ 2025-09-21 23:52 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
On 9/19/25 17:04, Mickaël Salaün wrote:
> Looks good, I'll take it.
Thanks, will skip this in the next version.
>
> On Tue, Sep 09, 2025 at 01:06:39AM +0100, Tingmao Wang wrote:
>> Not really related to this series, but just something which took me a
>> while to realize, and would probably be helpful as a comment.
>
> Please just describe the change in the main commit message and move this
> kind of explanation bellow a "---", just after your SoB. This is useful
> for review and avoid unrelated information when picking a patch out of
> this context.
Got it :)
>
>>
>> Signed-off-by: Tingmao Wang <m@maowtm.org>
>> ---
>> security/landlock/access.h | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/landlock/access.h b/security/landlock/access.h
>> index 7961c6630a2d..5e2285575479 100644
>> --- a/security/landlock/access.h
>> +++ b/security/landlock/access.h
>> @@ -67,8 +67,10 @@ typedef u16 layer_mask_t;
>> static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
>>
>> /*
>> - * Tracks domains responsible of a denied access. This is required to avoid
>> - * storing in each object the full layer_masks[] required by update_request().
>> + * Tracks domains responsible of a denied access, stored in the form of
>> + * two 4-bit layer numbers packed into a byte (one for each optional
>> + * access). This is required to avoid storing in each object the full
>> + * layer_masks[] required by update_request().
>> */
>> typedef u8 deny_masks_t;
>>
>> --
>> 2.51.0
>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
2025-09-21 23:52 ` Tingmao Wang
@ 2025-09-24 9:20 ` Mickaël Salaün
2025-09-27 15:43 ` Tingmao Wang
0 siblings, 1 reply; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-24 9:20 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
On Mon, Sep 22, 2025 at 12:52:19AM +0100, Tingmao Wang wrote:
> On 9/19/25 17:02, Mickaël Salaün wrote:
> > On Tue, Sep 09, 2025 at 01:06:35AM +0100, Tingmao Wang wrote:
> >> To avoid unnecessarily increasing the size of struct landlock_layer, we
> >> make the layer level a u8 and use the space to store the flags struct.
> >>
> >> Signed-off-by: Tingmao Wang <m@maowtm.org>
> >> ---
> >> security/landlock/fs.c | 75 ++++++++++++++++++++++++-------------
> >> security/landlock/net.c | 3 +-
> >> security/landlock/ruleset.c | 9 ++++-
> >> security/landlock/ruleset.h | 27 ++++++++++++-
> >> 4 files changed, 83 insertions(+), 31 deletions(-)
> >>
> >> [...]
> >> @@ -643,6 +644,12 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
> >> unsigned long access_bit;
> >> bool is_empty;
> >>
> >> + if (rule_flags) {
> >> + /* Collect rule flags for each layer */
> >> + if (layer->flags.quiet)
> >> + rule_flags->quiet_masks |= layer_bit;
> >
> > This patch makes quiet flags related to on object, but not tied to the
> > rule access rights (as explained in the next patch's doc). To tie it to
> > rule access rights would require to duplicate the access bits for each
> > rule (because multiple rules can be tied to the same object for the same
> > layer/ruleset).
>
> (imo the use of "rule" here as a terminology is a bit confusing, I would
> have thought that a "rule" is a collection of access rights associated
> with an object, and therefore you can of course only have one rule per
> object in a ruleset. Otherwise landlock_add_rule should really have been
> called landlock_add_rules...?)
A rule is indeed a set of access rights tied to an object. However,
when the same object is used with two calls to landlock_add_rule(), the
ruleset's rule is the union of both access rights.
What I wanted to highlight is that the quiet flag is unrelated to the
access rights, which makes sense because the otherwise it would never be
denied and then never logged.
>
> >
> > So, the question is, what do we really need to mute?
> >
> > I think the current approach is enough. We could still add a new flag in
> > the future, or maybe even a new field to each rule type. However, we
> > should rename the flag to make it clear that the it's related to the
> > rule's object which is muted instead of the whole rule. Maybe something
> > like LANDLOCK_ADD_RULE_QUIET_OBJECT?
>
> I don't see what benefit a new field to each rule type would bring, since
> different rule types targets different objects and live in different
> rbtrees, and so they are already separable.
Adding a new field to rule types would allow to only mute denied
requests if they match an access right, instead of muting all
non-allowed access rights. For instance, if an app tries to do an ioctl
because of legacy/compatibility reasons, we may want to ignore such
request, but we may still want to log malicious attempts to modify
regular files (in the same file hierarchy).
>
> >
> > If we want to have a more fine-grained control, a complementary patch
> > could add a bitfield for each access right type to quiet a denied access
> > right iff the object is also quiet (where rules are possible). That
> > could be a follow up to complete this quiet feature, but this patch
> > series is good on its own.
>
> Worth noting that if one really wants to suppress logging for only some
> access bits and we do not add support for it (due to the extra overhead
> etc), that is still doable with just this patch by using two layers - the
This hack should definitely not be encouraged, otherwise the limit of 16
layers will shrink to 8 (or less) in practice.
> outer one would contain the intended rules but not have any quiet flags,
> whereas the inner one would contain the same set of rules but with quiet
> flags set, except that for access bits which the sandbox does not want to
> be quiet, it would also "allow" them in the second layer (access would
> still be denied by the first layer, but would get audit logged due to
> quiet flag not applying when the younger layer allows the access).
>
> But this gets very tricky in the context of mutable domains, and does not
> work at all for the purpose of controlling whether supervisor mode would
> delegate to the supervisor or deny outright, since supervisors are
> "accumulative". Therefore if this (different "quietness" for different
> access bits) becomes a strong need, we should probably consider some way
> of implementing it, rather than expecting a sandboxer to do this two-layer
> hack. (But implementing this does have the potential to result in needing
> to have a (number of access bits) x (number of layers) matrix...)
Yes, that will indeed increase the size of rules, which is why I'm not
sure if it worth it.
The alternative I was thinking about is to only increase the size of
struct landlock_ruleset (which would be negligible) to include sets of
quiet access rights. A request to such access right *on* a quiet object
would never be logged. I think this approach is flexible enough and
doesn't increase much the complexity. This would also be useful to not
log access rights that don't have associated rules (e.g. scopes), and
then no identified objects. To avoid the kind of hack you pointed out,
this feature should probably be part of this patch series though. What
do you think?
>
> > [...]
>
> Will add suggested changes in v2. Thanks for the review :)
>
> Kind regards,
> Tingmao
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
2025-09-24 9:20 ` Mickaël Salaün
@ 2025-09-27 15:43 ` Tingmao Wang
2025-09-27 19:00 ` Mickaël Salaün
0 siblings, 1 reply; 19+ messages in thread
From: Tingmao Wang @ 2025-09-27 15:43 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
On 9/24/25 10:20, Mickaël Salaün wrote:
> On Mon, Sep 22, 2025 at 12:52:19AM +0100, Tingmao Wang wrote:
>> [...]
>> "accumulative". Therefore if this (different "quietness" for different
>> access bits) becomes a strong need, we should probably consider some way
>> of implementing it, rather than expecting a sandboxer to do this two-layer
>> hack. (But implementing this does have the potential to result in needing
>> to have a (number of access bits) x (number of layers) matrix...)
>
> Yes, that will indeed increase the size of rules, which is why I'm not
> sure if it worth it.
In addition to the size of each rule, another concern is that we would
need another layer_mask_t[LANDLOCK_NUM_ACCESS_FS] matrix on the stack in
order to correctly accumulate quiet bits when multiple access bits are
requested, and the quiet bits are only set on some of them / spread out
between different objects. For example, for this 1 layer domain:
/ quiet: r
/etc quiet: w
request: /etc/file rw
# This should not audit log, but if we only keep one accumulated bit
# per layer, we would not be able to tell that all access bits are
# eventually "covered" by quiet flags.
The alternative approach you suggested below would get rid of this
situation as well.
>
> The alternative I was thinking about is to only increase the size of
> struct landlock_ruleset (which would be negligible) to include sets of
> quiet access rights. A request to such access right *on* a quiet object
> would never be logged. I think this approach is flexible enough and
> doesn't increase much the complexity. This would also be useful to not
> log access rights that don't have associated rules (e.g. scopes), and
> then no identified objects. To avoid the kind of hack you pointed out,
> this feature should probably be part of this patch series though. What
> do you think?
This seems reasonable to me, especially if we don't think that having
separate quiet access bit controls for each object would be a common need.
Although if for some reason such control is needed, one might still be
tempted to use the kind of two layer hack I mentioned. Maybe some program
would like to quiet reads globally but only quiet writes to /run...?
But maybe later on when we get to have supervised domains, the supervisor
can tell Landlock whether to audit log or not for individual denied
requests, or do such logging itself, therefore offering fine-grained
control without hacks, and could potentially be more flexible e.g. only
log once per request per process, etc.
I think the most obvious way to implement this is to add a field to struct
landlock_ruleset_attr, and landlock_create_ruleset would use the passed in
size to determine if the quiet access bits field should be read or not?
Also, do we want to consider calling this something else instead, like
"suppress_audit"?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
2025-09-27 15:43 ` Tingmao Wang
@ 2025-09-27 19:00 ` Mickaël Salaün
2025-09-27 23:12 ` Tingmao Wang
0 siblings, 1 reply; 19+ messages in thread
From: Mickaël Salaün @ 2025-09-27 19:00 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
On Sat, Sep 27, 2025 at 04:43:50PM +0100, Tingmao Wang wrote:
> On 9/24/25 10:20, Mickaël Salaün wrote:
> > On Mon, Sep 22, 2025 at 12:52:19AM +0100, Tingmao Wang wrote:
> >> [...]
> >> "accumulative". Therefore if this (different "quietness" for different
> >> access bits) becomes a strong need, we should probably consider some way
> >> of implementing it, rather than expecting a sandboxer to do this two-layer
> >> hack. (But implementing this does have the potential to result in needing
> >> to have a (number of access bits) x (number of layers) matrix...)
> >
> > Yes, that will indeed increase the size of rules, which is why I'm not
> > sure if it worth it.
>
> In addition to the size of each rule, another concern is that we would
> need another layer_mask_t[LANDLOCK_NUM_ACCESS_FS] matrix on the stack in
> order to correctly accumulate quiet bits when multiple access bits are
> requested, and the quiet bits are only set on some of them / spread out
> between different objects. For example, for this 1 layer domain:
>
> / quiet: r
> /etc quiet: w
>
> request: /etc/file rw
> # This should not audit log, but if we only keep one accumulated bit
> # per layer, we would not be able to tell that all access bits are
> # eventually "covered" by quiet flags.
>
> The alternative approach you suggested below would get rid of this
> situation as well.
>
> >
> > The alternative I was thinking about is to only increase the size of
> > struct landlock_ruleset (which would be negligible) to include sets of
> > quiet access rights. A request to such access right *on* a quiet object
> > would never be logged. I think this approach is flexible enough and
> > doesn't increase much the complexity. This would also be useful to not
> > log access rights that don't have associated rules (e.g. scopes), and
> > then no identified objects. To avoid the kind of hack you pointed out,
> > this feature should probably be part of this patch series though. What
> > do you think?
>
> This seems reasonable to me, especially if we don't think that having
> separate quiet access bit controls for each object would be a common need.
> Although if for some reason such control is needed, one might still be
> tempted to use the kind of two layer hack I mentioned. Maybe some program
> would like to quiet reads globally but only quiet writes to /run...?
>
> But maybe later on when we get to have supervised domains, the supervisor
> can tell Landlock whether to audit log or not for individual denied
> requests, or do such logging itself, therefore offering fine-grained
> control without hacks, and could potentially be more flexible e.g. only
> log once per request per process, etc.
>
> I think the most obvious way to implement this is to add a field to struct
> landlock_ruleset_attr, and landlock_create_ruleset would use the passed in
> size to determine if the quiet access bits field should be read or not?
Yes
>
> Also, do we want to consider calling this something else instead, like
> "suppress_audit"?
Quiet is simpler (similar to the LANDLOCK_RESTRICT_SELF_LOG_* flags) and
if we get other ways to log actions this will also be used. For the
supervisor case, that would be useful to not forward a request to the
supervisor. The *_LOG_* flags could be used the same way too (even if
"LOG" may be a subset of the supervisor capabilities). Do you think
that would be OK? Dedicated flags would be more flexible but also a bit
more complex. Is it worth it? In any case, the semantic and need
should be quite similar.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 1/6] landlock: Add a place for flags to layer rules
2025-09-27 19:00 ` Mickaël Salaün
@ 2025-09-27 23:12 ` Tingmao Wang
0 siblings, 0 replies; 19+ messages in thread
From: Tingmao Wang @ 2025-09-27 23:12 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
On 9/27/25 20:00, Mickaël Salaün wrote:
> On Sat, Sep 27, 2025 at 04:43:50PM +0100, Tingmao Wang wrote:
>> [..]
>>
>> Also, do we want to consider calling this something else instead, like
>> "suppress_audit"?
>
> Quiet is simpler (similar to the LANDLOCK_RESTRICT_SELF_LOG_* flags) and
> if we get other ways to log actions this will also be used. For the
> supervisor case, that would be useful to not forward a request to the
> supervisor. The *_LOG_* flags could be used the same way too (even if
> "LOG" may be a subset of the supervisor capabilities). Do you think
> that would be OK? Dedicated flags would be more flexible but also a bit
> more complex. Is it worth it? In any case, the semantic and need
> should be quite similar.
I don't think we need a dedicated flag, I was just wondering if "QUIET" is
the right name, but I guess I don't have a better suggestion either. On
second thought SUPPRESS_AUDIT would no longer be accurate if we later use
it to control supervisor forwarding (it would be doing more than just
suppressing audit logs).
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-09-27 23:12 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-09 0:06 [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Tingmao Wang
2025-09-09 0:06 ` [RFC PATCH 1/6] landlock: Add a place for flags to layer rules Tingmao Wang
2025-09-19 16:02 ` Mickaël Salaün
2025-09-21 23:52 ` Tingmao Wang
2025-09-24 9:20 ` Mickaël Salaün
2025-09-27 15:43 ` Tingmao Wang
2025-09-27 19:00 ` Mickaël Salaün
2025-09-27 23:12 ` Tingmao Wang
2025-09-09 0:06 ` [RFC PATCH 2/6] landlock: Add API support for the quiet flag Tingmao Wang
2025-09-19 16:02 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 3/6] landlock/audit: Check for quiet flag in landlock_log_denial Tingmao Wang
2025-09-19 16:02 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 4/6] landlock/audit: Fix wrong type usage Tingmao Wang
2025-09-19 16:03 ` Mickaël Salaün
2025-09-09 0:06 ` [RFC PATCH 5/6] landlock/access: Improve explanation on the deny_masks_t Tingmao Wang
2025-09-19 16:04 ` Mickaël Salaün
2025-09-21 23:52 ` Tingmao Wang
2025-09-09 0:06 ` [RFC PATCH 6/6] samples/landlock: Add FS quiet flag support to sandboxer Tingmao Wang
2025-09-19 16:01 ` [RFC PATCH 0/6] Implement LANDLOCK_ADD_RULE_QUIET Mickaël Salaün
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).