* [PATCH v5 0/6] Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-14 17:05 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
Hi,
This is version 5 of the LANDLOCK_ADD_RULE_NO_INHERIT series, which
implements a new flag to suppress inheritance of access rights and
flags from parent objects.
This version of the series focuses on simplification and cleanup.
Behavior of the flag is identical to the previous patch.
This version has 1136 insertions(+), 72 deletions(-), down from
1285 insertions(+), 9 deletions(-), while adding documentation
and retaining all existing tests.
This series is still based on v6 of Tingmao Wang's "quiet flag" series.
Previous patch summary:
The new flag enables policies where a parent directory needs broader
access than its children. For example, a sandbox may permit read-write
access to /home/user but still prohibit writes to ~/.bashrc or
~/.ssh, even though they are nested beneath the parent. Today this is
not possible because access rights always propagate from parent to
child inodes.
When a rule is added with LANDLOCK_ADD_RULE_NO_INHERIT:
* access rights on parent inodes are ignored for that inode and its
descendants; and
* operations that change the filesystem ancestors of such objects
(via rename, rmdir, link) are denied up to the VFS root.
* parent flags do not propagate below a NO_INHERIT rule.
These parent-directory restrictions help mitigate sandbox-restart
attacks: a sandboxed process could otherwise move a protected
directory before exit, causing the next sandbox instance to apply its
policy to the wrong path.
Changes since v4:
1. Trimmed ~120 lines from core implementation in fs.c.
2. Centralized path traversal logic with a helper function
landlock_walk_path_up.
3. Fixed bug in test on applying LANDLOCK_ADD_RULE_NO_INHERIT on
a file, giving it valid access rights.
4. Restructured commits to allow independent builds.
5. Adds userspace API documentation for the flag.
Changes since v3:
1. Trimmed core implementation in fs.c by removing redundant functions.
2. Fixed placement/inclusion of prototypes.
3. Added 4 new selftests for bind mount cases.
4. Protections now apply up to the VFS root instead of the mountpoint
root.
Changes since v2:
1. Add six new selftests for the new flag.
2. Add an optimization to stop permission harvesting when all
relevant layers are tagged with NO_INHERIT.
3. Suppress inheritance of parent flags.
4. Rebase onto v5 of the quiet-flag series.
5. Remove the xarray structure used for flag tracking in favor of
blank rule insertion, simplifying the implementation.
6. Fix edge cases involving flag inheritance across multiple
NO_INHERIT layers.
7. Add documenting comments to new functions.
Links:
v1:
https://lore.kernel.org/linux-security-module/20251105180019.1432367-1-utilityemal77@gmail.com/
v2:
https://lore.kernel.org/linux-security-module/20251120222346.1157004-1-utilityemal77@gmail.com/
v3:
https://lore.kernel.org/linux-security-module/20251126122039.3832162-1-utilityemal77@gmail.com/
v4:
https://lore.kernel.org/linux-security-module/20251207015132.800576-1-utilityemal77@gmail.com/
quiet-flag v6:
https://lore.kernel.org/linux-security-module/cover.1765040503.git.m@maowtm.org/
Example usage:
# LL_FS_RO="/a/b/c" LL_FS_RW="/" LL_FS_NO_INHERIT="/a/b/c"
landlock-sandboxer sh
# touch /a/b/c/fi # denied; / RW does not inherit
# rmdir /a/b/c # denied by ancestor protections
# mv /a /bad # denied
# mkdir /a/good; touch /a/good/fi # allowed; unrelated path
About 120 lines of code have been removed from the fs.c file, achieved by
removing/streamlining many of the previous functions, and adding shared
path traversal logic.
Simplifying the path handling has a nice side effect of making some hairy
functions (is_access_to_paths_allowed) more readable.
All tests added by this series, and all other existing landlock tests,
are passing. This patch was also validated through checkpatch.pl.
Special thanks to Tingmao Wang and Mickaël Salaün for your valuable
feedback.
Thank you for your time and review.
Regards,
Justin Suess
Justin Suess (6):
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to
landlock-sandboxer
selftests/landlock: Implement selftests for
LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Add documentation for LANDLOCK_ADD_RULE_NO_INHERIT
Documentation/userspace-api/landlock.rst | 17 +
include/uapi/linux/landlock.h | 29 +
samples/landlock/sandboxer.c | 13 +-
security/landlock/fs.c | 266 ++++++--
security/landlock/ruleset.c | 108 ++-
security/landlock/ruleset.h | 29 +-
security/landlock/syscalls.c | 16 +-
tools/testing/selftests/landlock/fs_test.c | 730 +++++++++++++++++++++
8 files changed, 1136 insertions(+), 72 deletions(-)
base-commit: 92f98eb2cc08c6e2d093d4682f1cd1204728e97e
--
2.51.0
^ permalink raw reply
* [PATCH v5 1/6] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-14 17:05 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251214170548.408142-1-utilityemal77@gmail.com>
Implements a flag to prevent access grant inheritance within the filesystem
hierarchy for landlock rules.
If a landlock rule on an inode has this flag, any access grants on parent
inodes will be ignored. Moreover, operations that involve altering the
ancestors of the subject with LANDLOCK_ADD_RULE_NO_INHERIT will be
denied up to the VFS root.
For example, if /a/b/c/ = read only + LANDLOCK_ADD_RULE_NO_INHERIT and
/ = read write, writes to files in /a/b/c will be denied. Moreover,
moving /a to /bad, removing /a/b/c, or creating links to /a will be
prohibited.
Parent flag inheritance is automatically suppressed by the permission
harvesting logic, which will finish processing early if all relevant
layers are tagged with NO_INHERIT.
The parent directory restrictions mitigate sandbox-restart attacks. For
example, if a sandboxed program is able to move a
LANDLOCK_ADD_RULE_NO_INHERIT restricted directory, upon sandbox restart,
the policy applied naively on the same filenames would be invalid.
Preventing these operations mitigates these attacks.
Because this requires path walking, to centralize the logic in a
single location, the path walking logic is moved into a helper function
landlock_walk_path_up, which takes a path as a parameter, and returns
an enum corresponding to whether the path is a mount root, real root,
or other.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
Cc: Tingmao Wang <m@maowtm.org>
Cc: Mickaël Salaün <mic@digikod.net>
---
v4..v5 changes:
* Centralized path walking logic with landlock_walk_path_up.
* Removed redundant functions in fs.c, and streamlined core
logic, removing ~120 lines of code.
* Removed mark_no_inherit_ancestors, replacing with direct flag
setting in append_fs_rule.
* Removed micro-optimization of skipping ancestor processing
when all layers have no_inherit, as it complicated the code
significantly for little gain.
v3..v4 changes:
* Rebased on v6 of Tingmao Wang's "quiet flag" series.
* Removed unnecessary mask_no_inherit_descendant_layers and related
code at Tingmao Wang's suggestion, simplifying patch.
* Updated to use new disconnected directory handling.
* Improved WARN_ON_ONCE usage. (Thanks Tingmao Wang!)
* Removed redundant loop for single-layer rulesets (again thanks Tingmao
Wang!)
* Protections now apply up to the VFS root, not just the mountpoint.
* Indentation fixes.
* Removed redundant flag marker blocked_flag_masks.
v2..v3 changes:
* Parent directory topology protections now work by lazily
inserting blank rules on parent inodes if they do not
exist. This replaces the previous xarray implementation
with simplified logic.
* Added an optimization to skip further processing if all layers
collected have no inherit.
* Added support to block flag inheritance.
include/uapi/linux/landlock.h | 29 ++++
security/landlock/fs.c | 266 +++++++++++++++++++++++++---------
security/landlock/ruleset.h | 29 +++-
3 files changed, 258 insertions(+), 66 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index d4f47d20361a..6ab3e7bd1c81 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -127,10 +127,39 @@ struct landlock_ruleset_attr {
* allowed_access in the passed in rule_attr. When this flag is
* present, the caller is also allowed to pass in an empty
* allowed_access.
+ * %LANDLOCK_ADD_RULE_NO_INHERIT
+ * When set on a rule being added to a ruleset, this flag disables the
+ * inheritance of access rights and flags from parent objects.
+ *
+ * This flag currently applies only to filesystem rules. Adding it to
+ * non-filesystem rules will return -EINVAL, unless future extensions
+ * of Landlock define other hierarchical object types.
+ *
+ * By default, Landlock filesystem rules inherit allowed accesses from
+ * ancestor directories: if a parent directory grants certain rights,
+ * those rights also apply to its children. A rule marked with
+ * LANDLOCK_ADD_RULE_NO_INHERIT stops this propagation at the directory
+ * covered by the rule. Descendants of that directory continue to inherit
+ * normally unless they also have rules using this flag.
+ *
+ * If a regular file is marked with this flag, it will not inherit any
+ * access rights from its parent directories; only the accesses explicitly
+ * allowed by the rule will apply to that file.
+ *
+ * This flag also enforces parent-directory restrictions: rename, rmdir,
+ * link, and other operations that would change the directory's immediate
+ * parent subtree are denied up to the VFS root. This prevents
+ * sandboxed processes from manipulating the filesystem hierarchy to evade
+ * restrictions (e.g., via sandbox-restart attacks).
+ *
+ * In addition, this flag blocks the inheritance of rule-layer flags
+ * (such as the quiet flag) from parent directories to the object covered
+ * by this rule.
*/
/* clang-format off */
#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
+#define LANDLOCK_ADD_RULE_NO_INHERIT (1U << 1)
/* clang-format on */
/**
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 0b589263ea42..8d8623ea857f 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -317,6 +317,37 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_IOCTL_DEV)
/* clang-format on */
+enum landlock_walk_result {
+ LANDLOCK_WALK_CONTINUE,
+ LANDLOCK_WALK_STOP_REAL_ROOT,
+ LANDLOCK_WALK_MOUNT_ROOT,
+};
+
+static enum landlock_walk_result landlock_walk_path_up(struct path *const path)
+{
+ while (path->dentry == path->mnt->mnt_root) {
+ if (!follow_up(path))
+ return LANDLOCK_WALK_STOP_REAL_ROOT;
+ }
+
+ if (unlikely(IS_ROOT(path->dentry))) {
+ if (likely(path->mnt->mnt_flags & MNT_INTERNAL))
+ return LANDLOCK_WALK_MOUNT_ROOT;
+ dput(path->dentry);
+ path->dentry = dget(path->mnt->mnt_root);
+ return LANDLOCK_WALK_CONTINUE;
+ }
+
+ struct dentry *const parent = dget_parent(path->dentry);
+
+ dput(path->dentry);
+ path->dentry = parent;
+ return LANDLOCK_WALK_CONTINUE;
+}
+
+static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
+ const struct dentry *const dentry);
+
/*
* @path: Should have been checked by get_path_from_fd().
*/
@@ -344,6 +375,48 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
return PTR_ERR(id.key.object);
mutex_lock(&ruleset->lock);
err = landlock_insert_rule(ruleset, id, access_rights, flags);
+ if (err || !(flags & LANDLOCK_ADD_RULE_NO_INHERIT))
+ goto out_unlock;
+
+ /* Create ancestor rules and set has_no_inherit_descendant flags */
+ struct path walker = *path;
+
+ path_get(&walker);
+ while (landlock_walk_path_up(&walker) != LANDLOCK_WALK_STOP_REAL_ROOT) {
+ struct landlock_rule *ancestor_rule;
+
+ if (WARN_ON_ONCE(!walker.dentry || d_is_negative(walker.dentry))) {
+ err = -EIO;
+ break;
+ }
+
+ ancestor_rule = (struct landlock_rule *)find_rule(ruleset, walker.dentry);
+ if (!ancestor_rule) {
+ struct landlock_id ancestor_id = {
+ .type = LANDLOCK_KEY_INODE,
+ .key.object = get_inode_object(d_backing_inode(walker.dentry)),
+ };
+
+ if (IS_ERR(ancestor_id.key.object)) {
+ err = PTR_ERR(ancestor_id.key.object);
+ break;
+ }
+ err = landlock_insert_rule(ruleset, ancestor_id, 0, 0);
+ landlock_put_object(ancestor_id.key.object);
+ if (err)
+ break;
+
+ ancestor_rule = (struct landlock_rule *)
+ find_rule(ruleset, walker.dentry);
+ }
+ if (WARN_ON_ONCE(!ancestor_rule || ancestor_rule->num_layers != 1)) {
+ err = -EIO;
+ break;
+ }
+ ancestor_rule->layers[0].flags.has_no_inherit_descendant = true;
+ }
+ path_put(&walker);
+out_unlock:
mutex_unlock(&ruleset->lock);
/*
* No need to check for an error because landlock_insert_rule()
@@ -772,8 +845,10 @@ static bool is_access_to_paths_allowed(
_layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
(*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
- struct collected_rule_flags *rule_flags_parent1 = &log_request_parent1->rule_flags;
- struct collected_rule_flags *rule_flags_parent2 = &log_request_parent2->rule_flags;
+ struct collected_rule_flags *rule_flags_parent1 =
+ &log_request_parent1->rule_flags;
+ struct collected_rule_flags *rule_flags_parent2 =
+ log_request_parent2 ? &log_request_parent2->rule_flags : NULL;
if (!access_request_parent1 && !access_request_parent2)
return true;
@@ -784,7 +859,7 @@ static bool is_access_to_paths_allowed(
if (is_nouser_or_private(path->dentry))
return true;
- if (WARN_ON_ONCE(!layer_masks_parent1))
+ if (WARN_ON_ONCE(!layer_masks_parent1 || !log_request_parent1))
return false;
allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
@@ -851,6 +926,7 @@ static bool is_access_to_paths_allowed(
*/
while (true) {
const struct landlock_rule *rule;
+ enum landlock_walk_result walk_res;
/*
* If at least all accesses allowed on the destination are
@@ -910,46 +986,14 @@ static bool is_access_to_paths_allowed(
if (allowed_parent1 && allowed_parent2)
break;
-jump_up:
- if (walker_path.dentry == walker_path.mnt->mnt_root) {
- if (follow_up(&walker_path)) {
- /* Ignores hidden mount points. */
- goto jump_up;
- } else {
- /*
- * Stops at the real root. Denies access
- * because not all layers have granted access.
- */
- break;
- }
- }
-
- if (unlikely(IS_ROOT(walker_path.dentry))) {
- if (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) {
- /*
- * Stops and allows access when reaching disconnected root
- * directories that are part of internal filesystems (e.g. nsfs,
- * which is reachable through /proc/<pid>/ns/<namespace>).
- */
- allowed_parent1 = true;
- allowed_parent2 = true;
- break;
- }
-
- /*
- * We reached a disconnected root directory from a bind mount.
- * Let's continue the walk with the mount point we missed.
- */
- dput(walker_path.dentry);
- walker_path.dentry = walker_path.mnt->mnt_root;
- dget(walker_path.dentry);
- } else {
- struct dentry *const parent_dentry =
- dget_parent(walker_path.dentry);
-
- dput(walker_path.dentry);
- walker_path.dentry = parent_dentry;
+ walk_res = landlock_walk_path_up(&walker_path);
+ if (walk_res == LANDLOCK_WALK_MOUNT_ROOT) {
+ allowed_parent1 = true;
+ allowed_parent2 = true;
+ break;
}
+ if (walk_res != LANDLOCK_WALK_CONTINUE)
+ break;
}
path_put(&walker_path);
@@ -963,7 +1007,7 @@ static bool is_access_to_paths_allowed(
ARRAY_SIZE(*layer_masks_parent1);
}
- if (!allowed_parent2) {
+ if (!allowed_parent2 && log_request_parent2) {
log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
log_request_parent2->audit.u.path = *path;
@@ -1037,8 +1081,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
* collect_domain_accesses - Walk through a file path and collect accesses
*
* @domain: Domain to check against.
- * @mnt_root: Last directory to check.
- * @dir: Directory to start the walk from.
+ * @mnt_root: Last path element to check.
+ * @dir: Directory path to start the walk from.
* @layer_masks_dom: Where to store the collected accesses.
*
* This helper is useful to begin a path walk from the @dir directory to a
@@ -1060,29 +1104,31 @@ 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,
+ const struct path *const mnt_root, const struct path *const dir,
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;
+ struct path walker;
if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
return true;
- if (is_nouser_or_private(dir))
+ if (is_nouser_or_private(dir->dentry))
return true;
access_dom = landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
layer_masks_dom,
LANDLOCK_KEY_INODE);
- dget(dir);
+ walker = *dir;
+ path_get(&walker);
while (true) {
- struct dentry *parent_dentry;
+ enum landlock_walk_result walk_res;
/* Gets all layers allowing all domain accesses. */
if (landlock_unmask_layers(
- find_rule(domain, dir), access_dom, layer_masks_dom,
+ find_rule(domain, walker.dentry), access_dom, layer_masks_dom,
ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
/*
* Stops when all handled accesses are allowed by at
@@ -1091,22 +1137,69 @@ static bool collect_domain_accesses(
ret = true;
break;
}
-
- /*
- * Stops at the mount point or the filesystem root for a disconnected
- * directory.
- */
- if (dir == mnt_root || unlikely(IS_ROOT(dir)))
+ if (walker.dentry == mnt_root->dentry && walker.mnt == mnt_root->mnt)
+ break;
+ walk_res = landlock_walk_path_up(&walker);
+ if (walk_res != LANDLOCK_WALK_CONTINUE)
break;
-
- parent_dentry = dget_parent(dir);
- dput(dir);
- dir = parent_dentry;
}
- dput(dir);
+ path_put(&walker);
return ret;
}
+/**
+ * deny_no_inherit_topology_change - deny topology changes on sealed paths
+ * @subject: Subject performing the operation (contains the domain).
+ * @path: Path whose dentry is the target of the topology modification.
+ *
+ * Checks whether any domain layers are sealed against topology changes at
+ * @path. If so, emit an audit record and return -EACCES. Otherwise return 0.
+ */
+static int deny_no_inherit_topology_change(const struct landlock_cred_security
+ *subject,
+ const struct path *const path)
+{
+ layer_mask_t sealed_layers = 0;
+ layer_mask_t override_layers = 0;
+ const struct landlock_rule *rule;
+ u32 layer_index;
+ unsigned long audit_layer_index;
+
+ if (WARN_ON_ONCE(!subject || !path || !path->dentry || !path->mnt ||
+ d_is_negative(path->dentry)))
+ return 0;
+
+ rule = find_rule(subject->domain, path->dentry);
+ if (!rule)
+ return 0;
+
+ for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+ const struct landlock_layer *layer = &rule->layers[layer_index];
+ layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
+
+ if (layer->flags.no_inherit ||
+ layer->flags.has_no_inherit_descendant)
+ sealed_layers |= layer_bit;
+ else
+ override_layers |= layer_bit;
+ }
+
+ sealed_layers &= ~override_layers;
+ if (!sealed_layers)
+ return 0;
+
+ audit_layer_index = __ffs((unsigned long)sealed_layers);
+ landlock_log_denial(subject, &(struct landlock_request) {
+ .type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
+ .audit = {
+ .type = LSM_AUDIT_DATA_DENTRY,
+ .u.dentry = path->dentry,
+ },
+ .layer_plus_one = audit_layer_index + 1,
+ });
+ return -EACCES;
+}
+
/**
* current_check_refer_path - Check if a rename or link action is allowed
*
@@ -1191,6 +1284,21 @@ static int current_check_refer_path(struct dentry *const old_dentry,
access_request_parent2 =
get_mode_access(d_backing_inode(old_dentry)->i_mode);
if (removable) {
+ int err = deny_no_inherit_topology_change(subject,
+ &(struct path)
+ { .mnt = new_dir->mnt,
+ .dentry = old_dentry });
+
+ if (err)
+ return err;
+ if (exchange) {
+ err = deny_no_inherit_topology_change(subject,
+ &(struct path)
+ { .mnt = new_dir->mnt,
+ .dentry = new_dentry });
+ if (err)
+ return err;
+ }
access_request_parent1 |= maybe_remove(old_dentry);
access_request_parent2 |= maybe_remove(new_dentry);
}
@@ -1232,12 +1340,15 @@ static int current_check_refer_path(struct dentry *const old_dentry,
old_dentry->d_parent;
/* new_dir->dentry is equal to new_dentry->d_parent */
- allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
- old_parent,
+ allow_parent1 = collect_domain_accesses(subject->domain,
+ &mnt_dir,
+ &(struct path){ .mnt = new_dir->mnt,
+ .dentry = old_parent },
&layer_masks_parent1,
&request1.rule_flags);
- allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
- new_dir->dentry,
+ allow_parent2 = collect_domain_accesses(subject->domain, &mnt_dir,
+ &(struct path){ .mnt = new_dir->mnt,
+ .dentry = new_dir->dentry },
&layer_masks_parent2,
&request2.rule_flags);
@@ -1583,12 +1694,37 @@ static int hook_path_symlink(const struct path *const dir,
static int hook_path_unlink(const struct path *const dir,
struct dentry *const dentry)
{
+ const struct landlock_cred_security *const subject =
+ landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+ int err;
+
+ if (subject) {
+ err = deny_no_inherit_topology_change(subject,
+ &(struct path)
+ { .mnt = dir->mnt,
+ .dentry = dentry });
+ if (err)
+ return err;
+ }
return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
}
static int hook_path_rmdir(const struct path *const dir,
struct dentry *const dentry)
{
+ const struct landlock_cred_security *const subject =
+ landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+ int err;
+
+ if (subject) {
+ err = deny_no_inherit_topology_change(subject,
+ &(struct path)
+ { .mnt = dir->mnt,
+ .dentry = dentry });
+ if (err)
+ return err;
+ }
+
return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
}
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index eb60db646422..81df6c56a152 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -40,6 +40,21 @@ struct landlock_layer {
* down the file hierarchy.
*/
bool quiet:1;
+ /**
+ * @no_inherit: Prevents this rule from being inherited by
+ * descendant directories in the filesystem layer. Only used
+ * for filesystem rules.
+ */
+ bool no_inherit:1;
+ /**
+ * @has_no_inherit_descendant: Marker to indicate that this layer
+ * has at least one descendant directory with a rule having the
+ * no_inherit flag. Only used for filesystem rules.
+ * This "flag" is not set by the user, but by Landlock on
+ * parent directories of rules when the child rule has
+ * a rule with the no_inherit flag.
+ */
+ bool has_no_inherit_descendant:1;
} flags;
/**
* @access: Bitfield of allowed actions on the kernel object. They are
@@ -49,13 +64,25 @@ struct landlock_layer {
};
/**
- * struct collected_rule_flags - Hold accumulated flags for each layer.
+ * struct collected_rule_flags - Hold accumulated flags and their markers for each layer.
*/
struct collected_rule_flags {
/**
* @quiet_masks: Layers for which the quiet flag is effective.
*/
layer_mask_t quiet_masks;
+ /**
+ * @no_inherit_masks: Layers for which the no_inherit flag is effective.
+ */
+ layer_mask_t no_inherit_masks;
+ /**
+ * @no_inherit_desc_masks: Layers for which the
+ * has_no_inherit_descendant tag is effective.
+ * This is not a flag itself, but a marker set on ancestors
+ * of rules with the no_inherit flag to deny topology changes
+ * in the direct parent path.
+ */
+ layer_mask_t no_inherit_desc_masks;
};
/**
--
2.51.0
^ permalink raw reply related
* [PATCH v5 2/6] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
From: Justin Suess @ 2025-12-14 17:05 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251214170548.408142-1-utilityemal77@gmail.com>
Implements the syscall side flag handling and kernel api headers for the
LANDLOCK_ADD_RULE_NO_INHERIT flag.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
v4..v5 changes:
* Moved syscall handling to this patch and moved out flag definition
to allow independent build.
v3..v4 changes:
* Changed documentation to reflect protections now apply to VFS root
instead of the mountpoint.
v2..v3 changes:
* Extended documentation for flag inheritance suppression on
LANDLOCK_ADD_RULE_NO_INHERIT.
* Extended the flag validation rules in the syscall.
* Added mention of no inherit in empty rules in add_rule_path_beneath
as per Tingmao Wang's suggestion.
* Added check for useless no-inherit flag in networking rules.
security/landlock/ruleset.c | 19 ++++++++++++++++++-
security/landlock/syscalls.c | 16 ++++++++++++----
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 750a444e1983..9152a939d79a 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -255,8 +255,13 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
return -EINVAL;
if (WARN_ON_ONCE(this->layers[0].level != 0))
return -EINVAL;
+ /* Merge the flags into the rules */
this->layers[0].access |= (*layers)[0].access;
this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
+ this->layers[0].flags.no_inherit |=
+ (*layers)[0].flags.no_inherit;
+ this->layers[0].flags.has_no_inherit_descendant |=
+ (*layers)[0].flags.has_no_inherit_descendant;
return 0;
}
@@ -315,7 +320,10 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
.level = 0,
.flags = {
.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
- },
+ .no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+ .has_no_inherit_descendant =
+ !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+ }
} };
build_check_layer();
@@ -662,9 +670,18 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
unsigned long access_bit;
bool is_empty;
+ /* Skip layers that already have no inherit flags. */
+ if (rule_flags &&
+ (rule_flags->no_inherit_masks & layer_bit))
+ continue;
+
/* Collect rule flags for each layer. */
if (rule_flags && layer->flags.quiet)
rule_flags->quiet_masks |= layer_bit;
+ if (rule_flags && layer->flags.no_inherit)
+ rule_flags->no_inherit_masks |= layer_bit;
+ if (rule_flags && layer->flags.has_no_inherit_descendant)
+ rule_flags->no_inherit_desc_masks |= layer_bit;
/*
* Records in @layer_masks which layer grants access to each requested
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 5cf1183bb596..cffe7d944ae5 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -352,7 +352,7 @@ 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
+ * is there to hold a quiet or no inherit flag.
*/
if (!flags && !path_beneath_attr.allowed_access)
return -ENOMSG;
@@ -407,6 +407,10 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.net)
return -EINVAL;
+ /* No inherit is always useless for this scope */
+ if (flags & LANDLOCK_ADD_RULE_NO_INHERIT)
+ return -EINVAL;
+
/* Denies inserting a rule with port greater than 65535. */
if (net_port_attr.port > U16_MAX)
return -EINVAL;
@@ -424,7 +428,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 or %LANDLOCK_ADD_RULE_QUIET.
+ * @flags: Must be 0 or %LANDLOCK_ADD_RULE_QUIET and/or %LANDLOCK_ADD_RULE_NO_INHERIT.
*
* This system call enables to define a new rule and add it to an existing
* ruleset.
@@ -462,8 +466,12 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
if (!is_initialized())
return -EOPNOTSUPP;
-
- if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
+ /* Checks flag existence */
+ if (flags & ~(LANDLOCK_ADD_RULE_QUIET | LANDLOCK_ADD_RULE_NO_INHERIT))
+ return -EINVAL;
+ /* No inherit may only apply on path_beneath rules. */
+ if ((flags & LANDLOCK_ADD_RULE_NO_INHERIT) &&
+ rule_type != LANDLOCK_RULE_PATH_BENEATH)
return -EINVAL;
/* Gets and checks the ruleset. */
--
2.51.0
^ permalink raw reply related
* [PATCH v5 3/6] samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to landlock-sandboxer
From: Justin Suess @ 2025-12-14 17:05 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251214170548.408142-1-utilityemal77@gmail.com>
Adds support to landlock-sandboxer with environment variable
LL_FS_NO_INHERIT, which can be tagged on any filesystem object to
suppress access right inheritance.
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
v4..v5 changes:
* None
v3..v4 changes:
* Modified LL_FS_R(O/W)_NO_INHERIT variables to a single variable
to allow access rule combination. (credit to Tingmao Wang)
v2..v3 changes:
* Minor formatting fixes
samples/landlock/sandboxer.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 07dc0013ff19..852ffa413c75 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -60,6 +60,7 @@ static inline int landlock_restrict_self(const int ruleset_fd,
#define ENV_FS_RW_NAME "LL_FS_RW"
#define ENV_FS_QUIET_NAME "LL_FS_QUIET"
#define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
+#define ENV_FS_NO_INHERIT_NAME "LL_FS_NO_INHERIT"
#define ENV_TCP_BIND_NAME "LL_TCP_BIND"
#define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
#define ENV_NET_QUIET_NAME "LL_NET_QUIET"
@@ -383,6 +384,7 @@ static const char help[] =
"but to test audit we can set " ENV_FORCE_LOG_NAME "=1\n"
ENV_FS_QUIET_NAME " and " ENV_NET_QUIET_NAME ", both optional, can then be used "
"to make access to some denied paths or network ports not trigger audit logging.\n"
+ ENV_FS_NO_INHERIT_NAME " can be used to suppress access right propagation (ABI >= 8).\n"
ENV_FS_QUIET_ACCESS_NAME " and " ENV_NET_QUIET_ACCESS_NAME " can be used to specify "
"which accesses should be quieted (defaults to all):\n"
"* " ENV_FS_QUIET_ACCESS_NAME ": file system accesses to quiet\n"
@@ -430,6 +432,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
};
bool quiet_supported = true;
+ bool no_inherit_supported = true;
int supported_restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
int set_restrict_flags = 0;
@@ -517,8 +520,9 @@ int main(const int argc, char *const argv[], char *const *const envp)
LANDLOCK_ABI_LAST, abi);
__attribute__((fallthrough));
case 7:
- /* Don't add quiet flags for ABI < 8 later on. */
+ /* Don't add quiet/no_inherit flags for ABI < 8 later on. */
quiet_supported = false;
+ no_inherit_supported = false;
__attribute__((fallthrough));
case LANDLOCK_ABI_LAST:
@@ -605,6 +609,13 @@ int main(const int argc, char *const argv[], char *const *const envp)
goto err_close_ruleset;
}
+ /* Don't require this env to be present. */
+ if (no_inherit_supported && getenv(ENV_FS_NO_INHERIT_NAME)) {
+ if (populate_ruleset_fs(ENV_FS_NO_INHERIT_NAME, ruleset_fd, 0,
+ LANDLOCK_ADD_RULE_NO_INHERIT))
+ goto err_close_ruleset;
+ }
+
if (populate_ruleset_net(ENV_TCP_BIND_NAME, ruleset_fd,
LANDLOCK_ACCESS_NET_BIND_TCP, 0)) {
goto err_close_ruleset;
--
2.51.0
^ permalink raw reply related
* [PATCH v5 4/6] selftests/landlock: Implement selftests for LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-14 17:05 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251214170548.408142-1-utilityemal77@gmail.com>
Implements 15 selftests for the flag, covering allowed and disallowed
operations on parent and child directories when this flag is set, as
well as multi-layer configurations and flag inheritance / audit
logging. Also tests a bind mount configuration.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
v4..v5 changes:
* Fixed a bug in a test applying invalid access rights
to a file.
v3..v4 changes:
* Added 4 new tests for bind mount handling, increasing selftests
from 11 -> 15.
v2..v3 changes:
* Also covers flag inheritance, audit logging and
LANDLOCK_ADD_RULE_QUIET suppression.
* Increases number of selftests from 5 -> 11.
tools/testing/selftests/landlock/fs_test.c | 730 +++++++++++++++++++++
1 file changed, 730 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 44e131957fba..211c3b206710 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1484,6 +1484,111 @@ TEST_F_FORK(layout1, inherit_superset)
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
}
+TEST_F_FORK(layout1, inherit_no_inherit_flag)
+{
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Parent directory still grants write access to its direct children. */
+ EXPECT_EQ(0, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d1, O_WRONLY));
+
+ /* dir_s1d2 gets only its explicit read-only access rights. */
+ EXPECT_EQ(0, test_open(dir_s1d2, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+
+ /* Descendants of dir_s1d2 inherit the reduced access mask. */
+ EXPECT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_nested_levels)
+{
+ int ruleset_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Level 1: s1d1 (RW + REFER + REMOVE + NO_INHERIT) */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ dir_s1d1, LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ /* Level 2: s1d2 (RO + NO_INHERIT) */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ /* Level 3: s1d3 (RW + REFER + REMOVE + NO_INHERIT) */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ dir_s1d3, LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Level 3: s1d3
+ * - RW allowed (unlink file)
+ * - REFER allowed (rename file)
+ * - REMOVE_DIR denied (parent s1d2 is part of direct parent tree)
+ */
+ ASSERT_EQ(0, unlink(file1_s1d3));
+ ASSERT_EQ(0, rename(file2_s1d3, file1_s1d3));
+ ASSERT_EQ(0, rename(file1_s1d3, file2_s1d3));
+ ASSERT_EQ(-1, rmdir(dir_s1d3));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Level 2: s1d2
+ * - RW denied (unlink file), layer is RO
+ * - REFER denied (rename file)
+ * - REMOVE_DIR of s1d2 not allowed (parent s1d1 is part of direct parent tree)
+ */
+ ASSERT_EQ(-1, unlink(file1_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(file2_s1d2, file1_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rmdir(dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Level 1: s1d1
+ * - RW allowed
+ * - Rename allowed (except for direct parent tree s1d2)
+ * - REMOVE_DIR denied (parent tmp is denied)
+ */
+ ASSERT_EQ(0, unlink(file1_s1d1));
+ ASSERT_EQ(0, rename(file2_s1d1, file1_s1d1));
+ ASSERT_EQ(0, rename(file1_s1d1, file2_s1d1));
+ ASSERT_EQ(-1, rmdir(dir_s1d1));
+ ASSERT_EQ(EACCES, errno);
+}
+
TEST_F_FORK(layout0, max_layers)
{
int i, err;
@@ -4408,6 +4513,266 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
ASSERT_EQ(0, close(cli_fd));
}
+TEST_F_FORK(layout1, inherit_no_inherit_topology_dir)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ },
+ {},
+ };
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds a no-inherit rule on a leaf directory. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Topology modifications of the rule path and its parents are denied.
+ */
+
+ /* Target directory s1d3 */
+ ASSERT_EQ(-1, rmdir(dir_s1d3));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d3, dir_s2d3));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Parent directory s1d2 */
+ ASSERT_EQ(-1, rmdir(dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Grandparent directory s1d1 */
+ ASSERT_EQ(-1, rmdir(dir_s1d1));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d1, dir_s2d1));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Sibling operations are allowed.
+ */
+ /* Sibling of s1d3 */
+ ASSERT_EQ(0, unlink(file1_s1d2));
+ /* Sibling of s1d2 */
+ ASSERT_EQ(0, unlink(file1_s1d1));
+
+ /*
+ * Content of the no-inherit directory is restricted by the rule (RO).
+ */
+ ASSERT_EQ(-1, unlink(file1_s1d3));
+ ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F_FORK(layout1, no_inherit_allow_inner_removal)
+{
+ int ruleset_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Content of the no-inherit directory is mutable (RW).
+ * This checks that the no-inherit flag does not seal the content.
+ */
+ ASSERT_EQ(0, unlink(file1_s1d2));
+
+ /*
+ * Topology modifications of the rule path are denied.
+ */
+ ASSERT_EQ(-1, rmdir(dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+ ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_unrelated)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+ static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds a no-inherit rule on a leaf directory unrelated to s2. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Ensure we can still create and delete files outside the sealed branch. */
+ ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+ ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(unrelated_file));
+ ASSERT_EQ(0, rmdir(unrelated_dir));
+
+ /* Existing siblings in s2 remain modifiable. */
+ ASSERT_EQ(0, unlink(file1_s2d1));
+ ASSERT_EQ(0, mknod(file1_s2d1, S_IFREG | 0700, 0));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_descendant_rw)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RO,
+ },
+ {},
+ };
+ const __u64 handled_access = ACCESS_RW | LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE;
+ static const char child_file[] =
+ TMP_DIR "/s1d1/s1d2/s1d3/rw_descendant";
+ int ruleset_fd;
+
+ ruleset_fd = create_ruleset(_metadata, handled_access, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ dir_s1d3, 0);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ ASSERT_EQ(0, mknod(child_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(child_file));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_file)
+{
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW,
+ },
+ {},
+ };
+ int ruleset_fd;
+
+ /*
+ * Both file1_s1d2 and file2_s1d2 already exist from the fixture.
+ * file2_s1d2 is in the same directory as file1_s1d2 and will be
+ * used to test inheritance vs. NO_INHERIT behavior.
+ */
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /*
+ * Add a NO_INHERIT rule on file1_s1d2 with READ_FILE access.
+ * This should succeed (files can have NO_INHERIT).
+ * Use READ_FILE (not ACCESS_RO which includes READ_DIR) since
+ * directory access rights don't make sense for files.
+ */
+ add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_READ_FILE,
+ file1_s1d2, LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * file1_s1d2 has NO_INHERIT with READ_FILE access only,
+ * so it should only be readable (not inheriting RW from parent TMP_DIR).
+ */
+ ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+
+ /*
+ * file2_s1d2 does not have NO_INHERIT, so it should inherit
+ * RW access from parent TMP_DIR rule.
+ */
+ ASSERT_EQ(0, test_open(file2_s1d2, O_RDONLY));
+ ASSERT_EQ(0, test_open(file2_s1d2, O_WRONLY));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_layered)
+{
+ const struct rule layer1_and_2[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ },
+ {},
+ };
+ int ruleset_fd;
+ static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+ static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+
+ /* Layer 1: RW on TMP_DIR */
+ ruleset_fd = create_ruleset(_metadata,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ layer1_and_2);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Layer 2: Add no-inherit RO rule on s1d2 */
+ ruleset_fd = create_ruleset(_metadata,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ layer1_and_2);
+ ASSERT_LE(0, ruleset_fd);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Operations in unrelated areas should still work */
+ ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+ ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(unrelated_file));
+ ASSERT_EQ(0, rmdir(unrelated_dir));
+
+ /* Creating in s1d1 should be allowed (parent still has RW) */
+ ASSERT_EQ(0, mknod(TMP_DIR "/s1d1/newfile", S_IFREG | 0600, 0));
+ ASSERT_EQ(0, unlink(TMP_DIR "/s1d1/newfile"));
+
+ /* Content of s1d2 should be read-only */
+ ASSERT_EQ(-1, unlink(file1_s1d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Topology changes to s1d2 should be denied */
+ ASSERT_EQ(-1, rename(dir_s1d2, TMP_DIR "/s2d1/renamed"));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Renaming s1d1 should also be denied (it's an ancestor) */
+ ASSERT_EQ(-1, rename(dir_s1d1, TMP_DIR "/s2d1/renamed"));
+ ASSERT_EQ(EACCES, errno);
+}
+
/* clang-format off */
FIXTURE(ioctl) {};
@@ -5747,6 +6112,277 @@ TEST_F_FORK(layout4_disconnected_leafs, read_rename_exchange)
test_renameat(s1d42_bind_fd, "f4", s1d42_bind_fd, "f5"));
}
+/*
+ * Test that LANDLOCK_ADD_RULE_NO_INHERIT on a directory accessed via a mount
+ * point protects the parent hierarchy within the mount from topology changes.
+ *
+ * Layout (after bind mount s1d2 -> s2d2):
+ * tmp
+ * ├── s1d1
+ * │ └── s1d2 [source of bind mount]
+ * │ ├── s1d31
+ * │ │ └── s1d41
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d32
+ * │ └── s1d42
+ * │ ├── f3
+ * │ └── f4
+ * └── s2d1
+ * └── s2d2 [bind mount destination from s1d2]
+ * ├── s1d31 <- parent of protected dir, should be immovable
+ * │ └── s1d41 <- protected with NO_INHERIT
+ * │ ├── f1
+ * │ └── f2
+ * └── s1d32
+ * └── s1d42
+ * ├── f3
+ * └── f4
+ *
+ * When s1d41 (accessed via the mount at s2d2) is protected with NO_INHERIT,
+ * its parent directories within the mount (s1d31) should be immovable.
+ */
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_mount_parent_rename)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. This should seal the parent hierarchy through the mount.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * s1d31 is the parent of s1d41 within the mount. Renaming it should
+ * be denied because it is part of the protected parent hierarchy.
+ * Test via the mount path.
+ */
+ ASSERT_EQ(-1, rename(TMP_DIR "/s2d1/s2d2/s1d31",
+ TMP_DIR "/s2d1/s2d2/s1d31_renamed"));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * s1d32 is a sibling directory (not in the protected parent chain),
+ * so renaming it should be allowed.
+ */
+ ASSERT_EQ(0, rename(TMP_DIR "/s2d1/s2d2/s1d32",
+ TMP_DIR "/s2d1/s2d2/s1d32_renamed"));
+ ASSERT_EQ(0, rename(TMP_DIR "/s2d1/s2d2/s1d32_renamed",
+ TMP_DIR "/s2d1/s2d2/s1d32"));
+
+ /*
+ * Renaming directories not in the protected parent hierarchy should
+ * still be allowed.
+ */
+ ASSERT_EQ(0, rename(TMP_DIR "/s3d1", TMP_DIR "/s3d1_renamed"));
+ ASSERT_EQ(0, rename(TMP_DIR "/s3d1_renamed", TMP_DIR "/s3d1"));
+}
+
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_mount_parent_rmdir)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. This should seal the parent hierarchy through the mount.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * s1d31 is the parent of s1d41 within the mount. Removing it should
+ * be denied because it is part of the protected parent hierarchy.
+ */
+ ASSERT_EQ(-1, rmdir(TMP_DIR "/s2d1/s2d2/s1d31"));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Removing an unrelated directory should still be allowed (if empty).
+ */
+ ASSERT_EQ(0, rmdir(TMP_DIR "/s3d1"));
+ ASSERT_EQ(0, mkdir(TMP_DIR "/s3d1", 0755));
+}
+
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_mount_parent_link)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. This should seal the parent hierarchy through the mount.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Creating a hard link within the protected NO_INHERIT directory should
+ * be denied because NO_INHERIT grants only ACCESS_RO (no MAKE_REG).
+ */
+ ASSERT_EQ(-1, linkat(AT_FDCWD, TMP_DIR "/s2d1/s2d2/s1d31/s1d41/f1",
+ AT_FDCWD, TMP_DIR "/s2d1/s2d2/s1d31/s1d41/f1_link",
+ 0));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Creating links within directories outside the protected chain
+ * (using the mount source path to avoid EXDEV) should still be allowed.
+ */
+ ASSERT_EQ(0, linkat(AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3",
+ AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3_link",
+ 0));
+ ASSERT_EQ(0, unlink(TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3_link"));
+}
+
+/*
+ * Test that NO_INHERIT protection extends to the mount source hierarchy.
+ * If a directory is protected via a mount path, its parents within the
+ * mount source should also be protected from topology changes.
+ */
+TEST_F_FORK(layout4_disconnected_leafs, no_inherit_source_parent_rename)
+{
+ int ruleset_fd, s1d41_bind_fd;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ };
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Allow full access to TMP_DIR. */
+ add_path_beneath(_metadata, ruleset_fd,
+ ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR,
+ TMP_DIR, 0);
+
+ /*
+ * Access s1d41 through the bind mount at s2d2 and protect it with
+ * NO_INHERIT. The source mount path parents should also be protected.
+ */
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = s1d41_bind_fd,
+ .allowed_access = ACCESS_RO,
+ },
+ LANDLOCK_ADD_RULE_NO_INHERIT));
+ EXPECT_EQ(0, close(s1d41_bind_fd));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * The mount source is s1d1/s1d2. The protected directory s1d41 is at
+ * s1d1/s1d2/s1d31/s1d41. The parent s1d31 within the mount source
+ * should be protected from topology changes.
+ */
+ ASSERT_EQ(-1, rename(TMP_DIR "/s1d1/s1d2/s1d31",
+ TMP_DIR "/s1d1/s1d2/s1d31_renamed"));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * s1d32 is a sibling, not in the protected parent chain. It should
+ * be renamable.
+ */
+ ASSERT_EQ(0, rename(TMP_DIR "/s1d1/s1d2/s1d32",
+ TMP_DIR "/s1d1/s1d2/s1d32_renamed"));
+ ASSERT_EQ(0, rename(TMP_DIR "/s1d1/s1d2/s1d32_renamed",
+ TMP_DIR "/s1d1/s1d2/s1d32"));
+}
+
/*
* layout5_disconnected_branch before rename:
*
@@ -7231,6 +7867,100 @@ TEST_F(audit_layout1, write_file)
EXPECT_EQ(1, records.domain);
}
+TEST_F(audit_layout1, no_inherit_parent_is_logged)
+{
+ struct audit_records records;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Base read-only rule at s1d1. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1, 0);
+ /* Descendant s1d1/s1d2/s1d3 forbids inheritance but should still log. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+ EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.write_file", file1_s1d2));
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+ EXPECT_EQ(1, records.domain);
+
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(audit_layout1, no_inherit_blocks_quiet_flag_inheritance)
+{
+ struct audit_records records;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ .quiet_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Base read-only rule at tmp/s1d1 with quiet flag. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1,
+ LANDLOCK_ADD_RULE_QUIET);
+ /* Descendant tmp/s1d1/s1d2/s1d3 forbids inheritance of quiet flag and should still log. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+ EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.write_file", file1_s1d3));
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+ EXPECT_EQ(1, records.domain);
+
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(audit_layout1, no_inherit_quiet_parent)
+{
+ struct audit_records records;
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ .quiet_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Base read-only rule at tmp/s1d1 with quiet flag. */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1,
+ LANDLOCK_ADD_RULE_QUIET);
+ /* Access to dir_s1d1 shouldn't log */
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
+ EXPECT_NE(0, matches_log_fs(_metadata, self->audit_fd,
+ "fs\\.write_file", file1_s1d1));
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+ EXPECT_EQ(0, records.domain);
+
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
TEST_F(audit_layout1, read_file)
{
struct audit_records records;
--
2.51.0
^ permalink raw reply related
* [PATCH v5 5/6] landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-14 17:05 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251214170548.408142-1-utilityemal77@gmail.com>
Add a unit test for rule_flag collection, ensuring that access masks
are properly propagated with the flags.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
v4..v5 changes:
* None
v2..v3 changes:
* Removing erroneously misplaced code and placed in the proper
patch.
security/landlock/ruleset.c | 89 +++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 9152a939d79a..8064139fde8f 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -22,6 +22,7 @@
#include <linux/spinlock.h>
#include <linux/workqueue.h>
#include <uapi/linux/landlock.h>
+#include <kunit/test.h>
#include "access.h"
#include "audit.h"
@@ -770,3 +771,91 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
}
return handled_accesses;
}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+/**
+ * test_unmask_layers_no_inherit - Test landlock_unmask_layers() with no_inherit
+ * @test: The KUnit test context.
+ */
+static void test_unmask_layers_no_inherit(struct kunit *const test)
+{
+ struct landlock_rule *rule;
+ layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS];
+ struct collected_rule_flags rule_flags;
+ const access_mask_t access_request = BIT_ULL(0) | BIT_ULL(1);
+ const layer_mask_t layers_initialized = BIT_ULL(0) | BIT_ULL(1);
+ size_t i;
+
+ rule = kzalloc(struct_size(rule, layers, 2), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, rule);
+
+ rule->num_layers = 2;
+
+ /* Layer 1: allows access 0, no_inherit */
+ rule->layers[0].level = 1;
+ rule->layers[0].access = BIT_ULL(0);
+ rule->layers[0].flags.no_inherit = 1;
+
+ /* Layer 2: allows access 1 */
+ rule->layers[1].level = 2;
+ rule->layers[1].access = BIT_ULL(1);
+
+ /* Case 1: No rule_flags provided (should behave normally) */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), NULL);
+
+ /* Access 0 should be unmasked by layer 1 */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ /* Case 2: rule_flags provided, no existing no_inherit_masks */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+ memset(&rule_flags, 0, sizeof(rule_flags));
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), &rule_flags);
+
+ /* Access 0 should be unmasked by layer 1 */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ /* rule_flags should collect no_inherit from layer 1 */
+ KUNIT_EXPECT_EQ(test, rule_flags.no_inherit_masks, (layer_mask_t)BIT_ULL(0));
+
+ /* Case 3: rule_flags provided, layer 1 is masked by no_inherit_masks */
+ for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+ layer_masks[i] = layers_initialized;
+ memset(&rule_flags, 0, sizeof(rule_flags));
+ rule_flags.no_inherit_masks = BIT_ULL(0); /* Mask layer 1 */
+
+ landlock_unmask_layers(rule, access_request, &layer_masks,
+ ARRAY_SIZE(layer_masks), &rule_flags);
+
+ /* Access 0 should NOT be unmasked by layer 1 because it is skipped */
+ KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized);
+ /* Access 1 should be unmasked by layer 2 */
+ KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+ kfree(rule);
+}
+
+static struct kunit_case ruleset_test_cases[] = {
+ KUNIT_CASE(test_unmask_layers_no_inherit),
+ {}
+};
+
+static struct kunit_suite ruleset_test_suite = {
+ .name = "landlock_ruleset",
+ .test_cases = ruleset_test_cases,
+};
+
+kunit_test_suite(ruleset_test_suite);
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
--
2.51.0
^ permalink raw reply related
* [PATCH v5 6/6] landlock: Add documentation for LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-14 17:05 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Justin Suess, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251214170548.408142-1-utilityemal77@gmail.com>
Adds documentation of the flag to the userspace api, describing
the functionality of the flag and parent directory protections.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
v1..v5 changes:
* Initial addition
Documentation/userspace-api/landlock.rst | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 1d0c2c15c22e..3671cd90fbe2 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -604,6 +604,23 @@ Landlock audit events with the ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``,
sys_landlock_restrict_self(). See Documentation/admin-guide/LSM/landlock.rst
for more details on audit.
+Filesystem inheritance suppression (ABI < 8)
+-----------------
+
+Starting with the Landlock ABI version 8, it is possible to prevent a directory
+or file from inheriting it's parent's access grants by using the
+``LANDLOCK_ADD_RULE_NO_INHERIT`` flag passed to sys_landlock_add_rule(). This
+can be useful for policies where a parent directory needs broader access than its
+children.
+
+To mitigate sandbox-restart attacks, the inode itself, and ancestors of inodes
+tagged with ``LANDLOCK_ADD_RULE_NO_INHERIT`` cannot be removed, renamed,
+reparented, or linked into/from other directories.
+
+These parent directory protections propagate up to the root. Further inheritance
+for grants originating beneath a ``LANDLOCK_ADD_RULE_NO_INHERIT`` tagged inode
+are not affected unless also tagged with this flag.
+
.. _kernel_support:
Kernel support
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] host/roots: Sandbox xdg-desktop-portal-spectrum-host
From: Mickaël Salaün @ 2025-12-14 19:50 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Alyssa Ross, Spectrum OS Development, linux-security-module,
landlock, Ryan B. Sullivan, Günther Noack
In-Reply-To: <515ff0f4-2ab3-46de-8d1e-5c66a93c6ede@gmail.com>
On Sat, Dec 13, 2025 at 11:49:11PM -0500, Demi Marie Obenour wrote:
> On 12/13/25 20:39, Alyssa Ross wrote:
> > Demi Marie Obenour <demiobenour@gmail.com> writes:
> >
> >> On 12/13/25 16:42, Alyssa Ross wrote:
> >>> Demi Marie Obenour <demiobenour@gmail.com> writes:
> >>>
> >>>> On 12/13/25 14:12, Alyssa Ross wrote:
> >>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
> >>>>>
> >>>>>> It is quite possible that these Landlock rules are unnecessarily
> >>>>>> permissive, but all of the paths to which read and execute access is
> >>>>>> granted are part of the root filesystem and therefore assumed to be
> >>>>>> public knowledge. Removing access from any of them would only increase
> >>>>>> the risk of accidental breakage in the future, and would not provide any
> >>>>>> security improvements. seccomp *could* provide some improvements, but
> >>>>>> the effort needed is too high for now.
> >>>>>>
> >>>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> >>>>>> ---
> >>>>>> .../template/data/service/xdg-desktop-portal-spectrum-host/run | 8 ++++++++
> >>>>>> 1 file changed, 8 insertions(+)
> >>>>>
> >>>>> Are you sure this is working as intended? There's no rule allowing
> >>>>> access to Cloud Hypervisor's VSOCK socket, and yet it still seems to be
> >>>>> able to access that. Don't you need to set a rule that *restricts*
> >>>>> filesystem access and then add holes? Did you ever see this deny
> >>>>> anything?
> >>>>
> >>>> 'man 1 setpriv' states that '--landlock-access fs' blocks all
> >>>> filesystem access unless a subsequent --landlock-rule permits it.
> >>>> I tried running with no --landlock-rule flags and the execve of
> >>>> xdg-desktop-portal-spectrum-host failed as expected.
> >>>>
> >>>> The socket is passed over stdin, and I'm pretty sure Landlock
> >>>> doesn't restrict using an already-open file descriptor.
> >>>> xdg-desktop-portal-spectrum-host does need to find the path to the
> >>>> socket, but I don't think it ever accesses that path.
> >>>
> >>> I've been looking into this a bit myself, and from what I can tell
> >>> Landlock just doesn't restrict connecting to sockets at all, even if
> >>> they're inside directories that would otherwise be inaccessible. It's
> >>> able to connect to both Cloud Hypervisor's VSOCK socket and the D-Bus
> >>> socket even with a maximally restrictive landlock rule. So you were
> >>> right after all, sorry!
> >>
> >> That's not good at all! It's a trivial sandbox escape in so many cases.
> >> For instance, with access to D-Bus I can just call `systemd-run`.
> >>
> >> I'm CCing the Landlock and LSM mailing lists because if you are
> >> correct, then this is a bad security hole.
> >
> > I don't find it that surprising given the way landlock works. "connect"
> > (to a non-abstract AF_UNIX socket) is not an operation there's a
> > landlock action for, and it's not like the other actions care about
> > access to parent directories and the like — I was able to execute a
> > program via a symlink after only giving access to the symlink's target,
> > without any access to the directory containing the symlink or the
> > symlink itself, for example. Landlock, as I understand it, is intended
> > to block a specified set of operations (on particular file hierarchies),
> > rather than to completely prevent access to those hierarchies like
> > permissions or mount namespaces could, so the lack of a way to block
> > connecting to a socket is more of a missing feature than a security
> > hole.
>
> 'man 7 unix' states:
>
> On Linux, connecting to a stream socket object requires write
> permission on that socket; sending a datagram to a datagram socket
> likewise requires write permission on that socket.
>
> Landlock is definitely being inconsistent with DAC here. Also, this
> allows real-world sandbox breakouts. On systemd systems, the simplest
> way to escape is to use systemd-run to execute arbitrary commands.
The Linux kernel is complex and the link between the VFS and named UNIX
sockets is "special" (see the linked GitHub issue). Landlock doesn't
handle named UNIX sockets yet for the same reason it doesn't handle some
other kind of kernel resources or access rights: someone needs to
implement it (including tests, doc...). There is definitely interest to
add this feature, it has been discussed for some time, but it's not
trivial. It is a work in progress though:
https://github.com/landlock-lsm/linux/issues/36
Contributions are welcome!
Regards,
Mickaël
^ permalink raw reply
* [PATCH] lsm: fix kernel-doc struct member names
From: Randy Dunlap @ 2025-12-14 20:15 UTC (permalink / raw)
To: linux-kernel
Cc: Randy Dunlap, Paul Moore, James Morris, Serge E. Hallyn,
linux-security-module
Use the correct struct member names to avoid kernel-doc warnings:
Warning: include/linux/lsm_hooks.h:83 struct member 'name' not described
in 'lsm_id'
Warning: include/linux/lsm_hooks.h:183 struct member 'initcall_device' not
described in 'lsm_info'
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
Cc: Paul Moore <paul@paul-moore.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: linux-security-module@vger.kernel.org
---
include/linux/lsm_hooks.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-next-20251201.orig/include/linux/lsm_hooks.h
+++ linux-next-20251201/include/linux/lsm_hooks.h
@@ -73,7 +73,7 @@ struct lsm_static_calls_table {
/**
* struct lsm_id - Identify a Linux Security Module.
- * @lsm: name of the LSM, must be approved by the LSM maintainers
+ * @name: name of the LSM, must be approved by the LSM maintainers
* @id: LSM ID number from uapi/linux/lsm.h
*
* Contains the information that identifies the LSM.
@@ -164,7 +164,7 @@ enum lsm_order {
* @initcall_core: LSM callback for core_initcall() setup, optional
* @initcall_subsys: LSM callback for subsys_initcall() setup, optional
* @initcall_fs: LSM callback for fs_initcall setup, optional
- * @nitcall_device: LSM callback for device_initcall() setup, optional
+ * @initcall_device: LSM callback for device_initcall() setup, optional
* @initcall_late: LSM callback for late_initcall() setup, optional
*/
struct lsm_info {
^ permalink raw reply
* [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-14 21:32 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
use should be pooled rather than directly used. This both reduces
latency and improves its predictability.
2. Linux is better off overall if every subsystem uses the same source for
the random bistream as the de-facto choice, unless *force majeure*
reasons point to some other direction.
In the case, of TPM there is no reason for trusted keys to invoke TPM
directly.
Thus, unset '.get_random', which causes fallback to kernel_get_random().
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
security/keys/trusted-keys/trusted_tpm1.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..33b7739741c3 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
return ret;
}
-static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
-{
- return tpm_get_random(chip, key, key_len);
-}
-
static int __init init_digests(void)
{
int i;
@@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
.init = trusted_tpm_init,
.seal = trusted_tpm_seal,
.unseal = trusted_tpm_unseal,
- .get_random = trusted_tpm_get_random,
.exit = trusted_tpm_exit,
};
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-14 21:35 UTC (permalink / raw)
To: linux-integrity
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
James Bottomley, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list, David S. Miller,
Herbert Xu
In-Reply-To: <20251214213236.339586-1-jarkko@kernel.org>
On Sun, Dec 14, 2025 at 11:32:36PM +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> the random bistream as the de-facto choice, unless *force majeure*
> reasons point to some other direction.
>
> In the case, of TPM there is no reason for trusted keys to invoke TPM
> directly.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 636acb66a4f6..33b7739741c3 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> return ret;
> }
>
> -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> -{
> - return tpm_get_random(chip, key, key_len);
> -}
> -
> static int __init init_digests(void)
> {
> int i;
> @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> .init = trusted_tpm_init,
> .seal = trusted_tpm_seal,
> .unseal = trusted_tpm_unseal,
> - .get_random = trusted_tpm_get_random,
> .exit = trusted_tpm_exit,
> };
> --
> 2.39.5
>
Additional cc's as this indirectly relates to hwrng.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: James Bottomley @ 2025-12-14 22:18 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM,
open list
In-Reply-To: <20251214213236.339586-1-jarkko@kernel.org>
On Sun, 2025-12-14 at 23:32 +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus
> its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same
> source for
> the random bistream as the de-facto choice, unless *force majeure*
> reasons point to some other direction.
>
> In the case, of TPM there is no reason for trusted keys to invoke TPM
> directly.
That assertion isn't correct: you seem to have forgotten we had this
argument six or seven years ago, but even that was a reprise of an even
earlier one. Lore doesn't go back far enough for the intermediate one
on the tpm list, but the original was cc'd to lkml:
https://lore.kernel.org/all/1378920168.26698.64.camel@localhost/
The decision then was to use the same random source as the key
protection. Unfortunately most of the active participants have moved
on from IBM and I don't have their current email addresses, but the
bottom line is there were good reasons to do trusted keys this way that
your assertions above don't overcome. I'm not saying we shouldn't
reconsider the situation, but we need a reasoned debate rather than
simply doing it by fiat.
Regards,
James
^ permalink raw reply
* Re: [PATCH v5 1/6] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Tingmao Wang @ 2025-12-14 22:53 UTC (permalink / raw)
To: Justin Suess
Cc: Mickaël Salaün, Günther Noack, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <20251214170548.408142-2-utilityemal77@gmail.com>
On 12/14/25 17:05, Justin Suess wrote:
> [...]
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index d4f47d20361a..6ab3e7bd1c81 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -127,10 +127,39 @@ struct landlock_ruleset_attr {
> * allowed_access in the passed in rule_attr. When this flag is
> * present, the caller is also allowed to pass in an empty
> * allowed_access.
> + * %LANDLOCK_ADD_RULE_NO_INHERIT
> + * When set on a rule being added to a ruleset, this flag disables the
> + * inheritance of access rights and flags from parent objects.
> + *
> + * This flag currently applies only to filesystem rules. Adding it to
> + * non-filesystem rules will return -EINVAL, unless future extensions
> + * of Landlock define other hierarchical object types.
> + *
> + * By default, Landlock filesystem rules inherit allowed accesses from
> + * ancestor directories: if a parent directory grants certain rights,
> + * those rights also apply to its children. A rule marked with
> + * LANDLOCK_ADD_RULE_NO_INHERIT stops this propagation at the directory
> + * covered by the rule. Descendants of that directory continue to inherit
> + * normally unless they also have rules using this flag.
> + *
> + * If a regular file is marked with this flag, it will not inherit any
> + * access rights from its parent directories; only the accesses explicitly
> + * allowed by the rule will apply to that file.
> + *
> + * This flag also enforces parent-directory restrictions: rename, rmdir,
> + * link, and other operations that would change the directory's immediate
> + * parent subtree are denied up to the VFS root. This prevents
> + * sandboxed processes from manipulating the filesystem hierarchy to evade
> + * restrictions (e.g., via sandbox-restart attacks).
> + *
> + * In addition, this flag blocks the inheritance of rule-layer flags
tbh I feel that it's less confusing to just say "rule flags" (instead of
"rule-layer flags").
> + * (such as the quiet flag) from parent directories to the object covered
> + * by this rule.
> */
>
> /* clang-format off */
> #define LANDLOCK_ADD_RULE_QUIET (1U << 0)
> +#define LANDLOCK_ADD_RULE_NO_INHERIT (1U << 1)
> /* clang-format on */
>
> /**
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 0b589263ea42..8d8623ea857f 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -317,6 +317,37 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> LANDLOCK_ACCESS_FS_IOCTL_DEV)
> /* clang-format on */
>
> +enum landlock_walk_result {
> + LANDLOCK_WALK_CONTINUE,
> + LANDLOCK_WALK_STOP_REAL_ROOT,
> + LANDLOCK_WALK_MOUNT_ROOT,
> +};
> +
> +static enum landlock_walk_result landlock_walk_path_up(struct path *const path)
> +{
> + while (path->dentry == path->mnt->mnt_root) {
> + if (!follow_up(path))
> + return LANDLOCK_WALK_STOP_REAL_ROOT;
> + }
> +
> + if (unlikely(IS_ROOT(path->dentry))) {
> + if (likely(path->mnt->mnt_flags & MNT_INTERNAL))
> + return LANDLOCK_WALK_MOUNT_ROOT;
imo, LANDLOCK_WALK_MOUNT_ROOT is a somewhat confusing name for this,
especially in the context that if we see this in
is_access_to_paths_allowed() we allow access unconditionally.
Would LANDLOCK_WALK_INTERNAL be a better name here?
> + dput(path->dentry);
> + path->dentry = dget(path->mnt->mnt_root);
> + return LANDLOCK_WALK_CONTINUE;
> + }
> +
> + struct dentry *const parent = dget_parent(path->dentry);
> +
> + dput(path->dentry);
> + path->dentry = parent;
> + return LANDLOCK_WALK_CONTINUE;
> +}
> +
> +static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
> + const struct dentry *const dentry);
> +
> /*
> * @path: Should have been checked by get_path_from_fd().
> */
> @@ -344,6 +375,48 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> return PTR_ERR(id.key.object);
> mutex_lock(&ruleset->lock);
> err = landlock_insert_rule(ruleset, id, access_rights, flags);
> + if (err || !(flags & LANDLOCK_ADD_RULE_NO_INHERIT))
> + goto out_unlock;
> +
> + /* Create ancestor rules and set has_no_inherit_descendant flags */
> + struct path walker = *path;
> +
> + path_get(&walker);
> + while (landlock_walk_path_up(&walker) != LANDLOCK_WALK_STOP_REAL_ROOT) {
Why not landlock_walk_path_up(&walker) == LANDLOCK_WALK_CONTINUE here?
I'm not sure if it's actually possible to end up with an infinite loop by
ignoring LANDLOCK_WALK_MOUNT_ROOT (i.e. not sure if "internal" mounts can
have disconnected dentries), but it seems safer to write to loop in a way
such that if that happens, we exit.
> + struct landlock_rule *ancestor_rule;
> +
> + if (WARN_ON_ONCE(!walker.dentry || d_is_negative(walker.dentry))) {
> + err = -EIO;
> + break;
> + }
> +
> + ancestor_rule = (struct landlock_rule *)find_rule(ruleset, walker.dentry);
> + if (!ancestor_rule) {
> + struct landlock_id ancestor_id = {
> + .type = LANDLOCK_KEY_INODE,
> + .key.object = get_inode_object(d_backing_inode(walker.dentry)),
> + };
> +
> + if (IS_ERR(ancestor_id.key.object)) {
> + err = PTR_ERR(ancestor_id.key.object);
> + break;
> + }
> + err = landlock_insert_rule(ruleset, ancestor_id, 0, 0);
> + landlock_put_object(ancestor_id.key.object);
> + if (err)
> + break;
> +
> + ancestor_rule = (struct landlock_rule *)
> + find_rule(ruleset, walker.dentry);
> + }
> + if (WARN_ON_ONCE(!ancestor_rule || ancestor_rule->num_layers != 1)) {
> + err = -EIO;
> + break;
> + }
> + ancestor_rule->layers[0].flags.has_no_inherit_descendant = true;
> + }
> + path_put(&walker);
> +out_unlock:
> mutex_unlock(&ruleset->lock);
> /*
> * No need to check for an error because landlock_insert_rule()
> @@ -772,8 +845,10 @@ static bool is_access_to_paths_allowed(
> _layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
> layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
> (*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
> - struct collected_rule_flags *rule_flags_parent1 = &log_request_parent1->rule_flags;
> - struct collected_rule_flags *rule_flags_parent2 = &log_request_parent2->rule_flags;
> + struct collected_rule_flags *rule_flags_parent1 =
> + &log_request_parent1->rule_flags;
> + struct collected_rule_flags *rule_flags_parent2 =
> + log_request_parent2 ? &log_request_parent2->rule_flags : NULL;
Good point, I think the original was still safe because it would not be
used by landlock_unmask_layers anyway, but this is better. I will take
this in the next version, thanks!
>
> if (!access_request_parent1 && !access_request_parent2)
> return true;
> @@ -784,7 +859,7 @@ static bool is_access_to_paths_allowed(
> if (is_nouser_or_private(path->dentry))
> return true;
>
> - if (WARN_ON_ONCE(!layer_masks_parent1))
> + if (WARN_ON_ONCE(!layer_masks_parent1 || !log_request_parent1))
> return false;
>
> allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
> @@ -851,6 +926,7 @@ static bool is_access_to_paths_allowed(
> */
> while (true) {
> const struct landlock_rule *rule;
> + enum landlock_walk_result walk_res;
>
> /*
> * If at least all accesses allowed on the destination are
> @@ -910,46 +986,14 @@ static bool is_access_to_paths_allowed(
> if (allowed_parent1 && allowed_parent2)
> break;
>
> -jump_up:
> - if (walker_path.dentry == walker_path.mnt->mnt_root) {
> - if (follow_up(&walker_path)) {
> - /* Ignores hidden mount points. */
> - goto jump_up;
> - } else {
> - /*
> - * Stops at the real root. Denies access
> - * because not all layers have granted access.
> - */
> - break;
> - }
> - }
> -
> - if (unlikely(IS_ROOT(walker_path.dentry))) {
> - if (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) {
> - /*
> - * Stops and allows access when reaching disconnected root
> - * directories that are part of internal filesystems (e.g. nsfs,
> - * which is reachable through /proc/<pid>/ns/<namespace>).
> - */
> - allowed_parent1 = true;
> - allowed_parent2 = true;
> - break;
> - }
> -
> - /*
> - * We reached a disconnected root directory from a bind mount.
> - * Let's continue the walk with the mount point we missed.
> - */
I think we might want to preserve these comments.
> - dput(walker_path.dentry);
> - walker_path.dentry = walker_path.mnt->mnt_root;
> - dget(walker_path.dentry);
> - } else {
> - struct dentry *const parent_dentry =
> - dget_parent(walker_path.dentry);
> -
> - dput(walker_path.dentry);
> - walker_path.dentry = parent_dentry;
> + walk_res = landlock_walk_path_up(&walker_path);
> + if (walk_res == LANDLOCK_WALK_MOUNT_ROOT) {
> + allowed_parent1 = true;
> + allowed_parent2 = true;
> + break;
> }
> + if (walk_res != LANDLOCK_WALK_CONTINUE)
> + break;
> }
> path_put(&walker_path);
>
> @@ -963,7 +1007,7 @@ static bool is_access_to_paths_allowed(
> ARRAY_SIZE(*layer_masks_parent1);
> }
>
> - if (!allowed_parent2) {
> + if (!allowed_parent2 && log_request_parent2) {
> log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
> log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent2->audit.u.path = *path;
> @@ -1037,8 +1081,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
> * collect_domain_accesses - Walk through a file path and collect accesses
> *
> * @domain: Domain to check against.
> - * @mnt_root: Last directory to check.
> - * @dir: Directory to start the walk from.
> + * @mnt_root: Last path element to check.
> + * @dir: Directory path to start the walk from.
> * @layer_masks_dom: Where to store the collected accesses.
> *
> * This helper is useful to begin a path walk from the @dir directory to a
> @@ -1060,29 +1104,31 @@ 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,
> + const struct path *const mnt_root, const struct path *const dir,
> layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS],
> struct collected_rule_flags *const rule_flags)
> {
This function only walks up to the mountpoint of dir. If dir is changed
from a *dentry to a *path, wouldn't mnt_root be redundant? Since
mnt_root->dentry is always going to be dir->mnt->mnt_root. This also
means that they can't accidentally not be the same.
> unsigned long access_dom;
> bool ret = false;
> + struct path walker;
>
> if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
> return true;
> - if (is_nouser_or_private(dir))
> + if (is_nouser_or_private(dir->dentry))
> return true;
>
> access_dom = landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
> layer_masks_dom,
> LANDLOCK_KEY_INODE);
>
> - dget(dir);
> + walker = *dir;
> + path_get(&walker);
> while (true) {
> - struct dentry *parent_dentry;
> + enum landlock_walk_result walk_res;
>
> /* Gets all layers allowing all domain accesses. */
> if (landlock_unmask_layers(
> - find_rule(domain, dir), access_dom, layer_masks_dom,
> + find_rule(domain, walker.dentry), access_dom, layer_masks_dom,
> ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
> /*
> * Stops when all handled accesses are allowed by at
> @@ -1091,22 +1137,69 @@ static bool collect_domain_accesses(
> ret = true;
> break;
> }
> -
> - /*
> - * Stops at the mount point or the filesystem root for a disconnected
> - * directory.
> - */
> - if (dir == mnt_root || unlikely(IS_ROOT(dir)))
> + if (walker.dentry == mnt_root->dentry && walker.mnt == mnt_root->mnt)
> + break;
> + walk_res = landlock_walk_path_up(&walker);
> + if (walk_res != LANDLOCK_WALK_CONTINUE)
> break;
> -
> - parent_dentry = dget_parent(dir);
> - dput(dir);
> - dir = parent_dentry;
> }
> - dput(dir);
> + path_put(&walker);
> return ret;
> }
>
> +/**
> + * deny_no_inherit_topology_change - deny topology changes on sealed paths
> + * @subject: Subject performing the operation (contains the domain).
> + * @path: Path whose dentry is the target of the topology modification.
> + *
> + * Checks whether any domain layers are sealed against topology changes at
> + * @path. If so, emit an audit record and return -EACCES. Otherwise return 0.
> + */
> +static int deny_no_inherit_topology_change(const struct landlock_cred_security
> + *subject,
> + const struct path *const path)
Since you're not using path->mnt here (except for a NULL check), would it
be easier to just pass the dentry instead? In that case you wouldn't have
to do an inline initializer in current_check_refer_path / hook_path_*
below as well.
> +{
> + layer_mask_t sealed_layers = 0;
> + layer_mask_t override_layers = 0;
> + const struct landlock_rule *rule;
> + u32 layer_index;
> + unsigned long audit_layer_index;
> +
> + if (WARN_ON_ONCE(!subject || !path || !path->dentry || !path->mnt ||
> + d_is_negative(path->dentry)))
> + return 0;
> +
> + rule = find_rule(subject->domain, path->dentry);
> + if (!rule)
> + return 0;
> +
> + for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
> + const struct landlock_layer *layer = &rule->layers[layer_index];
> + layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
> +
> + if (layer->flags.no_inherit ||
> + layer->flags.has_no_inherit_descendant)
> + sealed_layers |= layer_bit;
> + else
> + override_layers |= layer_bit;
> + }
> +
> + sealed_layers &= ~override_layers;
> + if (!sealed_layers)
> + return 0;
> +
> + audit_layer_index = __ffs((unsigned long)sealed_layers);
> + landlock_log_denial(subject, &(struct landlock_request) {
> + .type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
> + .audit = {
> + .type = LSM_AUDIT_DATA_DENTRY,
> + .u.dentry = path->dentry,
> + },
> + .layer_plus_one = audit_layer_index + 1,
> + });
> + return -EACCES;
> +}
> +
> /**
> * current_check_refer_path - Check if a rename or link action is allowed
> *
> @@ -1191,6 +1284,21 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> access_request_parent2 =
> get_mode_access(d_backing_inode(old_dentry)->i_mode);
> if (removable) {
> + int err = deny_no_inherit_topology_change(subject,
> + &(struct path)
> + { .mnt = new_dir->mnt,
> + .dentry = old_dentry });
> +
> + if (err)
> + return err;
> + if (exchange) {
> + err = deny_no_inherit_topology_change(subject,
> + &(struct path)
> + { .mnt = new_dir->mnt,
> + .dentry = new_dentry });
> + if (err)
> + return err;
> + }
> access_request_parent1 |= maybe_remove(old_dentry);
> access_request_parent2 |= maybe_remove(new_dentry);
> }
> @@ -1232,12 +1340,15 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> old_dentry->d_parent;
>
> /* new_dir->dentry is equal to new_dentry->d_parent */
> - allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
> - old_parent,
> + allow_parent1 = collect_domain_accesses(subject->domain,
> + &mnt_dir,
> + &(struct path){ .mnt = new_dir->mnt,
> + .dentry = old_parent },
> &layer_masks_parent1,
> &request1.rule_flags);
> - allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
> - new_dir->dentry,
> + allow_parent2 = collect_domain_accesses(subject->domain, &mnt_dir,
> + &(struct path){ .mnt = new_dir->mnt,
> + .dentry = new_dir->dentry },
> &layer_masks_parent2,
> &request2.rule_flags);
>
> @@ -1583,12 +1694,37 @@ static int hook_path_symlink(const struct path *const dir,
> static int hook_path_unlink(const struct path *const dir,
> struct dentry *const dentry)
> {
> + const struct landlock_cred_security *const subject =
> + landlock_get_applicable_subject(current_cred(), any_fs, NULL);
> + int err;
> +
> + if (subject) {
> + err = deny_no_inherit_topology_change(subject,
> + &(struct path)
> + { .mnt = dir->mnt,
> + .dentry = dentry });
> + if (err)
> + return err;
> + }
> return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
> }
>
> static int hook_path_rmdir(const struct path *const dir,
> struct dentry *const dentry)
> {
> + const struct landlock_cred_security *const subject =
> + landlock_get_applicable_subject(current_cred(), any_fs, NULL);
> + int err;
> +
> + if (subject) {
> + err = deny_no_inherit_topology_change(subject,
> + &(struct path)
> + { .mnt = dir->mnt,
> + .dentry = dentry });
> + if (err)
> + return err;
> + }
> +
> return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
> }
>
> [...]
^ permalink raw reply
* Re: An opinion about Linux security
From: Dr. Greg @ 2025-12-15 4:55 UTC (permalink / raw)
To: Casey Schaufler; +Cc: Timur Chernykh, torvalds, linux-security-module
In-Reply-To: <950ac630-c75e-4d4c-ac70-5e4b0e397989@schaufler-ca.com>
On Fri, Dec 12, 2025 at 03:43:07PM -0800, Casey Schaufler wrote:
Good morning Casey, pleasant as always to hear from you.
> On 12/11/2025 9:45 PM, Dr. Greg wrote:
> > On Wed, Dec 10, 2025 at 03:15:39AM +0300, Timur Chernykh wrote:
> >
> > Good morning Timur, I hope this note finds your week having gone well.
> >
> >> Hello Linus,
> >>
> >> I'm writing to ask for your opinion. What do you think about Linux's
> >> current readiness for security-focused commercial products? I'm
> >> particularly interested in several areas.
> > I don't expect you will receive an answer.
> >
> > Based on his previous comments and long standing position on this
> > issue, I believe it can be fairly stated that he looks at the LSM as
> > an unnecessary evil.
> >
> > So in his absence, some 'in loco parentis' reflections on the issues
> > you raise.
> >
> > I've been advised, more than once, that in this day and age, no one is
> > interested in reading more than a two sentence paragraph, so a short
> > response to your issues here and a bit more detail for anyone who
> > wants to read more, at the end.
> >
> > There is active art available to address the shortcomings you outline
> > in your post below. Our TSEM LSM was designed to service the
> > realitities of the modern security environment and where it is going.
> > In a manner that doesn't provide any restrictions on how 'security'
> > can be implemented.
> >
> > We've done four releases over three years and we believe an unbiased
> > observer would conclude they have received no substantive technical
> > review that would support interest in upstream integration.
> Stop. Really, I mean it. I put significant effort into trying to teach
> you how to submit a patch set that could be reviewed. You ignored it.
> I can't speak to what an "unbiased observer" would conclude because
> your behavior has certainly left me with bias. Rather than writing
> full length novels about why you submitted patches the way you've
> done it you might consider heeding the advice. Grrr.
No, we are not going to stop, see immediately below.
> > The challenge is that the security gatekeepers view LSM submissions
> > through a lens of whether or not the LSM implements security
> > consistent with what they believe is security.
>
> While there is some truth to that, we're really quite flexible.
> What we need to see is that there is some sort of security model,
> and that the proposed code implements it. We also need to be able
> to examine the proposed code to see that it implements the model.
> You have rejected all suggestions about how to make your proposal
> reviewable.
No we didn't.
I've managed engineers before, I've never found a successful strategy
to be:
"Go off and do what I told you to do".
Without some kind of explanation and dialogue.
The Linux community has a process problem with the LSM and the
security sub-system that needs to be addressed. Personally, I now
have the cycles to address it.
We assume that if Timur elects to port the Apple security API (Timur's
ESF implementation) into the kernel, he will face the same issue that
concerns us.
When Paul Moore came on-board three years ago as the security
maintainer, he was immediately VERY prescriptive with respect to
establishing policies and guidelines that he wanted followed for LSM
submissions.
We see that he sent those guidelines to Timur in case he was
interested in submitting ESF as a new LSM. Here is the link for those
who haven't seen it.
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsms
What those guidelines are missing are prescriptive practices for
bringing a major body of virgin code into the security review process.
As we did with TSEM, and presumably Timur will have to do with ESF, if
he moves forward with that initiative.
For the benefit of those following this conversation and to close the
record in this thread from our perspective, as tedious as that might
be.
From an architectural perspective, TSEM consists of a series of
compilation units, each dedicated to a specific functional role needed
to support the LSM. We used a single include file that holds all of
the enumerations and structure definitions that are referenced by
every compilation unit.
We chose a submssion model of introducing this include file as a
single patch in the series, after a complete sub-system documentation
patch. Introduction of virgin sub-systems isn't really common but this
strategy was consistent with what we had previously observed.
In addition, we believe that Knuth was quoted at one time about an
understanding of a program flows from an understanding of its data
structures.
You reviewed about half of one compilation unit and said you didn't
have time for anymore. For the record, we made the changes that you
requested and responded to your other concerns for that unit.
Your primary issue, which you have repeated often, as you did here,
was with the fact that we had a single header file in the patch series
with all of the LSM global definitions. You indicated that the patch
series should be broken up in order to 'tell a story' by including the
structure and enumeration definitions with the code.
Fair enough, you are a valued voice in the Linux security community,
but your right pinky finger is not on the RETURN key that sends
security sub-system pull requests to Linus, Paul Moore's is.
If anyone chooses to review the new LSM guidelines, they will note
that it never makes mention of the fact that a patch series should
'tell a story' and/or how that should be done.
So, before we committed a ton of development time into what could very
well end up being a tag team match to try and please two maintainers,
we reached out to Paul for advice on how he would like to see a virgin
patch series structured.
His response was that TSEM had significant issues, the least of which
was the organization of the patch series. We repeatedly asked him to
detail what those problems were, as by his own admission he had only
read the documentation and not offered any review comments on the code
itself.
The only thing we received was that, in certain security behavior
modeling configurations enabled by TSEM, the security response would
be asynchronous from the event or events that triggered it.
We attempted to pursue further dialogue surrounding this. The last
response, at least a year ago now, was that he didn't think he needed
to respond to us because of our 'tone'. If anyone hasn't noticed, our
'tone' is driven by the fact that we tend to not be easily bul*ted.
So, in the interest of furthering Linux security development while at
the same time promoting homeostatic harmony in the community, we will
be sending out a request to the security list to formally address this
issue with new LSM's.
It obviously doesn't make sense to waste your or other people's time
addressing the other issues you have below, other than to agree that
we disagree on where security is headed and how Linux needs to
respond.
> > Those views are
> > inconsistent with the realities of the modern security market,
>
> Oh, stop it. Look at how many LSMs have been added over the past
> few years. Sandboxing and application controlled security are
> "modern" security concepts that were unheard of when the LSM was
> introduced. As I said before, security is a dynamic technology.
> If it were not, we'd have a Bell & LaPadula kernel config option
> instead of an LSM infrastructure.
>
> > a
> > market that that is now predicated on detection rather than
> > enforcement. A trend that will only accelerate with advancements in
> > machine learning and AI.
>
> Without the underlying access controls detection is rather pointless.
>
> > It is worth noting that the history of the technology industry is
> > littered with examples of technology incumbents usually missing
> > disruptive innovation.
>
> The technology industry is not unique on this. Acme Buggy Whips, inc.
>
> > This restriction on suitability is actually inconsistent with Linus'
> > stated position on how Linux sub-systems can be used, as expressed in
> > his comment in the following post.
> >
> > https://lore.kernel.org/lkml/CAHk-=wgLbz1Bm8QhmJ4dJGSmTuV5w_R0Gwvg5kHrYr4Ko9dUHQ@mail.gmail.com/
> >
> > So the problem is not technical, it is a political eco-system problem.
> >
> > So, big picture, that is the challenge facing resolution of your
> > concerns.
> >
> > Apologies to everyone about the paragraph/sentence overflow and any
> > afront to sensibilities.
> >
> > More detail below if anyone is interested.
> >
> >> First, in today's 3rd-party (out-of-tree) EDR development EDR
> >> being the most common commercial class of security products eBPF
> >> has effectively become the main option. Yet eBPF is extremely
> >> restrictive. It is not possible to write fully expressive real-time
> >> analysis code: the verifier is overly strict, non-deterministic
> >> loops are not allowed, and older kernels lack BTF support. These
> >> issues create real limitations.
> >>
> >> Second, the removal of the out-of-tree LSM API in the 4.x kernel
> >> series caused significant problems for many AV/EDR vendors. I was
> >> unable to find an explanation in the mailing lists that convincingly
> >> justified that decision.
> >>
> >> The next closest mechanism, fanotify, was a genuine improvement.
> >> However, it does not allow an AV/EDR vendor to protect the integrity
> >> of its own product. Is Linux truly expecting modern AV/EDR solutions
> >> to rely on fanotify alone?
> >>
> >> My main question is: what are the future plans? Linux provides very
> >> few APIs for security and dynamic analysis. eBPF is still immature,
> >> fanotify is insufficient, and driver workarounds that bypass kernel
> >> restrictions are risky they introduce both stability and security
> >> problems. At the same time, properly implemented in-tree LSMs are not
> >> inherently dangerous and remain the safer, supported path for
> >> extending security functionality. Without safe, supported interfaces,
> >> however, commercial products struggle to be competitive. At the
> >> moment, macOS with its Endpoint Security Framework is significantly
> >> ahead.
> >>
> >> Yes, the kernel includes multiple in-tree LSM modules, but in
> >> practice SELinux does not simplify operations it often complicates
> >> them, despite its long-standing presence. Many of the other LSMs are
> >> rarely used in production. As an EDR developer, I seldom encounter
> >> them, and when I do, they usually provide little practical
> >> value. Across numerous real-world server intrusions, none of these
> >> LSM modules have meaningfully prevented attacks, despite many years
> >> of kernel development.
> >>
> >> Perhaps it is time for Linux to focus on more than a theoretical model
> >> of security.
> > The heart of the political eco-system challenge is best expressed by a
> > quote from Kyle Moffett, in which he stated that security should only
> > be developed and implemented by experts. Unfortunately that view is
> > inconsistent with the current state of the technology industry.
>
> Glad to hear I'm not an expert! - Not!
>
> > Classical security practititioners will defend complex subject/object
> > architectures with: "Google uses SeLinux for Android security".
>
> Yea gads. Subject/Object is about as simple as it gets. Look at Smack.
>
> > Our response to that is that the world doesn't have a security problem
> > because Google lacks sufficient resources to implement anything it
> > desires to implement, regardless of the development and maintenance
> > input costs.
> >
> > Unfortunately, that luxury is inconsistent with the rest of the
> > software development world that doesn't enjoy a 3.8 trillion dollar
> > market capitalization.
> >
> > The world simply lacks enough experts to make the 'security only by
> > experts' model work.
> >
> > Today, the fastest way to a product is to grab Linux and a development
> > team and write software for hardware that is now completely
> > commoditized. Everyone knows that security is not one of the
> > fundamental project predicates in this model.
> >
> > Both NIST and DHS/CISA are officially on record as indicating that
> > security needs to start with and be baked in through the development
> > process. One of the objectives of TSEM was to provide a framework for
> > enabling this concept for the implementation of analysis and mandatory
> > behavior controls for software workloads.
> >
> > A second fundamental problem is that the world has moved, in large
> > part, to containerized execution workloads. The Linux LSM, in its
> > current form, doesn't effectively support the application of workload
> > specific security policies.
> >
> > Further complicating this issue is the fact that LSM 'stacking'
> > requires reasoning as to what a final security policy will be when
> > multiple different security architectures/policies get to decide on
> > the outcome of a security event/hook. The concept of least surprise
> > would suggest the need for stacking to have idempotency, in other
> > words, the order in which LSM event consumers are called shouldn't
> > influence the effective policy, but this is generally acknowledged as
> > not being the case with 'stacking'.
>
> Any other approach, and they have been considered, fails miserably
> and introduces a host of complications. Not to mention performance
> de-optimization.
>
> > So we designed TSEM to provide an alternative, not a replacement, but
> > an alternative to how developers and system administrators can develop
> > and apply security policy, including integrity controls.
> >
> > TSEM is an LSM that implements containerized security infrastructure
> > rather than security policy. It is designed around the concept of a
> > security orchestrator that can execute security isolated workloads and
> > receive the LSM events and their parameters from that workload and
> > process them in any manner it wishes.
>
> I shan't repeat the objections that have been raised, but I will
> point out that you have done nothing to address them.
>
> > For example: A Docker/Kubernetes container can be run and all of the
> > security events by that workload exported up into an OpenSearch or
> > ElasticSearch instance for anomaly detection and analysis.
> >
> > So an EDR implemented on top of this has visibility into all of the
> > security events and their characteristics that are deemed security
> > relevant by the kernel developers.
> >
> > One of the pushbacks is that this can lead to asynchronous security
> > decisions, but as you note, that is the model that the commercial
> > security industry and the consumers of its products has embraced,
> > particularly in light of the advancements coming out of the AI
> > industry, detection rather than enforcement.
> >
> > If synchronous enforcement is required TSEM provides that as well,
> > including the use of standard kernel modules to implement analysis and
> > response to the LSM hooks. Internally we have implemented other LSM's
> > such as Tomoyo and IMA as loadable modules that can support multiple
> > and independent workload policies.
> >
> > If you or other EDR vendors are interested, we would be more than
> > happy to engage in conversations as to how to improve the capabilities
> > of this type of architecture, as an alternative to what is currently
> > available in Linux, which as you note, has significant limitations.
> >
> >> Everything above reflects only my personal opinion. I would greatly
> >> appreciate your response and any criticism you may have.
> > As I mentioned at the outset, you are unlikely to hear anything.
> >
> > For the necessary Linux infrastructure improvements to emerge we
> > believe there is the need to develop and engage a community effort
> > that independently pursues the advancements that are necessary,
> > particularly those that enable Linux to implement first class AI based
> > security controls.
> >
> > We believe that only this will result in sufficient 'market pull' at the
> > distribution level to help shape upstreaming decisions.
> >
> > Absent that, it is likely that Linux will continue to implement what
> > has failed to work in the past in the hope that it will somehow work
> > in the future.
>
> Wow. calling Linux a failure is a bit of a stretch, don't you think?
>
> > Comments and criticism welcome, we have had plenty of experience with
> > the latter.... :-)
>
> We've been over these issues many times. Go back and make some changes to
> your approach.
>
> >> Best regards,
> >> Timur Chernykh
> > Best wishes for the success of your work and a pleasant holiday season.
> >
> > As always,
> > Dr. Greg
> >
> > The Quixote Project - Flailing at the Travails of Cybersecurity
> > https://github.com/Quixote-Project
> >
Best wishes for a productive week and a pleasant holiday season.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 6:43 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <64e3e4e0a92848fd3b02a213c754f096d2026463.camel@HansenPartnership.com>
On Mon, Dec 15, 2025 at 07:18:41AM +0900, James Bottomley wrote:
> On Sun, 2025-12-14 at 23:32 +0200, Jarkko Sakkinen wrote:
> > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus
> > its
> > use should be pooled rather than directly used. This both reduces
> > latency and improves its predictability.
> >
> > 2. Linux is better off overall if every subsystem uses the same
> > source for
> > the random bistream as the de-facto choice, unless *force majeure*
> > reasons point to some other direction.
> >
> > In the case, of TPM there is no reason for trusted keys to invoke TPM
> > directly.
>
> That assertion isn't correct: you seem to have forgotten we had this
> argument six or seven years ago, but even that was a reprise of an even
> earlier one. Lore doesn't go back far enough for the intermediate one
> on the tpm list, but the original was cc'd to lkml:
>
> https://lore.kernel.org/all/1378920168.26698.64.camel@localhost/
>
> The decision then was to use the same random source as the key
> protection. Unfortunately most of the active participants have moved
> on from IBM and I don't have their current email addresses, but the
> bottom line is there were good reasons to do trusted keys this way that
> your assertions above don't overcome. I'm not saying we shouldn't
> reconsider the situation, but we need a reasoned debate rather than
> simply doing it by fiat.
The way I see this is that given that kernel is not running inside TPM,
FIPS certification of the RNG does not have any measurable value.
Random data generation should happen as part of object creation process
i.e. should be fully self-contained process within the TPM in order for
FIPS to matter.
In the case of sealed data objects, this not the case.
>
> Regards,
>
> James
>
BR, Jarkko
^ permalink raw reply
* A formal request for process clarifications.
From: Dr. Greg @ 2025-12-15 7:08 UTC (permalink / raw)
To: linux-security-module; +Cc: torvalds, corbet
Good morning, I hope the week has started well for everyone.
When Paul Moore took over as the security/LSM sub-system maintainer,
three years ago, he indicated a desire to be very prescriptive with
respect to the practices that should be followed for the sub-system,
particularly for the introduction of new LSM's.
The following URL documents the requirements for the introduction of a
new LSM:
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsms
We believe that LWN covered the discussion around this document as well.
The following discussion on whether or not the Linux kernel provides
proper support for modern Event Detection and Response Systems (EDR or
EDRS) suggests the need to clarify these recommendations:
https://lore.kernel.org/linux-security-module/CABZOZnS4im-wNK4jtGKvp3YT9hPobA503rgiptutOF8rZEwt_w@mail.gmail.com
In that thread Timur Chernykh noted a possible desire to port the
Apple security API to the kernel, which would presumably be a large
body of virgin code.
Three years ago our team had submitted for review our TSEM LSM that
provides a framework for generic security modeling, particularly to
support machine learning and direct functional modeling of security
behavior.
We haven't been able to receive any substantive review of TSEM over
that period of time.
Casey Schaufler has been vocal in his criticism of how we introduced
what is a virgin LSM implementation. Most particularly with the fact
that we chose to include, as a single patch, a header file that
contains the structure and enumeration definitions that are referenced
by all of the compilation units that make up TSEM.
He indicates that an LSM submission, should 'tell a story' by bringing
in the structure definitions with the code that uses those structures.
We can offer multiple examples of the challenges we see with doing
this if that would be helpful but will not do so at this time.
We had requested guidance from Paul on how a new submission should be
properly structured, since he is the ultimate judge and jury on a
submission, but he declined to provide guidance.
Given the current security climate, particularly with what is widely
cited as the potential impact of machine learning and AI on security
architectures and practices, there will undoubtedly be new LSM's
coming forward. It would seem in the best interests of everyone
involved, reviewers and submitters, that specific guidance should be
codified in the 'new-lsms' document of how a virgin body of code
should be introduced.
Optimally this should include links to previous submissions that the
security maintainers believe codify the desired method of story
telling.
Given the importance of security in today's environment we are
prepared to pursue this through the TAB if necessary.
We will look forward to comments from the community on this issue.
Have a good week.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* Re: [PATCH v1 14/17] KEYS: trusted: Migrate to use tee specific driver registration function
From: Sumit Garg @ 2025-12-15 7:36 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn, op-tee,
linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <0b3ce259fa26e59ef24a91ca070e2b08feeede82.1765472125.git.u.kleine-koenig@baylibre.com>
On Thu, Dec 11, 2025 at 06:15:08PM +0100, Uwe Kleine-König wrote:
> The tee subsystem recently got a set of dedicated functions to register
> (and unregister) a tee driver. Make use of them. These care for setting the
> driver's bus (so the explicit assignment can be dropped) and the driver
> owner (which is an improvement this driver benefits from).
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> security/keys/trusted-keys/trusted_tee.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
-Sumit
>
> diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
> index aa3d477de6db..3cea9a377955 100644
> --- a/security/keys/trusted-keys/trusted_tee.c
> +++ b/security/keys/trusted-keys/trusted_tee.c
> @@ -264,7 +264,6 @@ static struct tee_client_driver trusted_key_driver = {
> .id_table = trusted_key_id_table,
> .driver = {
> .name = DRIVER_NAME,
> - .bus = &tee_bus_type,
> .probe = trusted_key_probe,
> .remove = trusted_key_remove,
> },
> @@ -272,12 +271,12 @@ static struct tee_client_driver trusted_key_driver = {
>
> static int trusted_tee_init(void)
> {
> - return driver_register(&trusted_key_driver.driver);
> + return tee_client_driver_register(&trusted_key_driver);
> }
>
> static void trusted_tee_exit(void)
> {
> - driver_unregister(&trusted_key_driver.driver);
> + tee_client_driver_unregister(&trusted_key_driver);
> }
>
> struct trusted_key_ops trusted_key_tee_ops = {
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH v1 15/17] KEYS: trusted: Make use of tee bus methods
From: Sumit Garg @ 2025-12-15 7:36 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn, op-tee,
linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <aab4c00b7e89abce7bcd8241c47f3398fb7227f8.1765472125.git.u.kleine-koenig@baylibre.com>
On Thu, Dec 11, 2025 at 06:15:09PM +0100, Uwe Kleine-König wrote:
> The tee bus got dedicated callbacks for probe and remove.
> Make use of these. This fixes a runtime warning about the driver needing
> to be converted to the bus methods.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> security/keys/trusted-keys/trusted_tee.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
-Sumit
>
> diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
> index 3cea9a377955..6e465c8bef5e 100644
> --- a/security/keys/trusted-keys/trusted_tee.c
> +++ b/security/keys/trusted-keys/trusted_tee.c
> @@ -202,9 +202,9 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> return 0;
> }
>
> -static int trusted_key_probe(struct device *dev)
> +static int trusted_key_probe(struct tee_client_device *rng_device)
> {
> - struct tee_client_device *rng_device = to_tee_client_device(dev);
> + struct device *dev = &rng_device->dev;
> int ret;
> struct tee_ioctl_open_session_arg sess_arg;
>
> @@ -244,13 +244,11 @@ static int trusted_key_probe(struct device *dev)
> return ret;
> }
>
> -static int trusted_key_remove(struct device *dev)
> +static void trusted_key_remove(struct tee_client_device *dev)
> {
> unregister_key_type(&key_type_trusted);
> tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
> tee_client_close_context(pvt_data.ctx);
> -
> - return 0;
> }
>
> static const struct tee_client_device_id trusted_key_id_table[] = {
> @@ -261,11 +259,11 @@ static const struct tee_client_device_id trusted_key_id_table[] = {
> MODULE_DEVICE_TABLE(tee, trusted_key_id_table);
>
> static struct tee_client_driver trusted_key_driver = {
> + .probe = trusted_key_probe,
> + .remove = trusted_key_remove,
> .id_table = trusted_key_id_table,
> .driver = {
> .name = DRIVER_NAME,
> - .probe = trusted_key_probe,
> - .remove = trusted_key_remove,
> },
> };
>
> --
> 2.47.3
>
^ permalink raw reply
* Re: A formal request for process clarifications.
From: Linus Torvalds @ 2025-12-15 7:38 UTC (permalink / raw)
To: Dr. Greg; +Cc: linux-security-module, corbet
In-Reply-To: <20251215070838.GA7209@wind.enjellic.com>
On Mon, 15 Dec 2025 at 19:13, Dr. Greg <greg@enjellic.com> wrote:
>
> Three years ago our team had submitted for review our TSEM LSM that
> provides a framework for generic security modeling,
If you can't convince the LSM people to take your code, you sure can't
convince me.
I already think we have too many of those pointless things. There's a
fine line between diversity and "too much confusion because everybody
thinks they know best". And the linux security modules passed that
line years ago.
So my suggestion is to standardize on normal existing security models
instead of thinking that you can do better by making yet another one.
Or at least work with the existing people instead of trying to bypass
them and ignoring what they tell you.
Yes, I know that security people always think they know best, and they
all disagree with each other, which is why we already have tons of
security modules. Ask ten people what model is the right one, and you
get fifteen different answers.
I'm not in the least interested in becoming some kind of arbiter or
voice of sanity in this.
Linus
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-15 7:54 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Sumit Garg, Ilias Apalodimas,
Jan Kiszka, Sudeep Holla, Christophe JAILLET, Michael Chan,
Pavan Chebbi, Rafał Miłecki, James Bottomley,
Jarkko Sakkinen, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel,
linux-crypto, linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <cover.1765472125.git.u.kleine-koenig@baylibre.com>
On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> the objective of this series is to make tee driver stop using callbacks
> in struct device_driver. These were superseded by bus methods in 2006
> (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> methods.")) but nobody cared to convert all subsystems accordingly.
>
> Here the tee drivers are converted. The first commit is somewhat
> unrelated, but simplifies the conversion (and the drivers). It
> introduces driver registration helpers that care about setting the bus
> and owner. (The latter is missing in all drivers, so by using these
> helpers the drivers become more correct.)
>
> The patches #4 - #17 depend on the first two, so if they should be
> applied to their respective subsystem trees these must contain the first
> two patches first.
Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
drivers.
>
> Note that after patch #2 is applied, unconverted drivers provoke a
> warning in driver_register(), so it would be good for the user
> experience if the whole series goes in during a single merge window.
+1
I suggest the whole series goes via the Jens tree since there shouldn't
be any chances for conflict here.
> So
> I guess an immutable branch containing the frist three patches that can
> be merged into the other subsystem trees would be sensible.
>
> After all patches are applied, tee_bus_type can be made private to
> drivers/tee as it's not used in other places any more.
>
Feel free to make the tee_bus_type private as the last patch in the series
such that any followup driver follows this clean approach.
-Sumit
> Best regards
> Uwe
>
> Uwe Kleine-König (17):
> tee: Add some helpers to reduce boilerplate for tee client drivers
> tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> tee: Adapt documentation to cover recent additions
> hwrng: optee - Make use of module_tee_client_driver()
> hwrng: optee - Make use of tee bus methods
> rtc: optee: Migrate to use tee specific driver registration function
> rtc: optee: Make use of tee bus methods
> efi: stmm: Make use of module_tee_client_driver()
> efi: stmm: Make use of tee bus methods
> firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> firmware: arm_scmi: Make use of tee bus methods
> firmware: tee_bnxt: Make use of module_tee_client_driver()
> firmware: tee_bnxt: Make use of tee bus methods
> KEYS: trusted: Migrate to use tee specific driver registration
> function
> KEYS: trusted: Make use of tee bus methods
> tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> tpm/tpm_ftpm_tee: Make use of tee bus methods
>
> Documentation/driver-api/tee.rst | 18 +----
> drivers/char/hw_random/optee-rng.c | 26 ++----
> drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> drivers/rtc/rtc-optee.c | 27 ++-----
> drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> include/linux/tee_drv.h | 12 +++
> security/keys/trusted-keys/trusted_tee.c | 17 ++--
> 10 files changed, 164 insertions(+), 138 deletions(-)
>
>
> base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: James Bottomley @ 2025-12-15 7:55 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <aT-uHgyYw3XhFasi@kernel.org>
On Mon, 2025-12-15 at 08:43 +0200, Jarkko Sakkinen wrote:
> On Mon, Dec 15, 2025 at 07:18:41AM +0900, James Bottomley wrote:
> > On Sun, 2025-12-14 at 23:32 +0200, Jarkko Sakkinen wrote:
> > > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and
> > > thus its use should be pooled rather than directly used. This
> > > both reduces latency and improves its predictability.
> > >
> > > 2. Linux is better off overall if every subsystem uses the same
> > > source for the random bistream as the de-facto choice, unless
> > > *force majeure* reasons point to some other direction.
> > >
> > > In the case, of TPM there is no reason for trusted keys to invoke
> > > TPM directly.
> >
> > That assertion isn't correct: you seem to have forgotten we had
> > this argument six or seven years ago, but even that was a reprise
> > of an even earlier one. Lore doesn't go back far enough for the
> > intermediate one on the tpm list, but the original was cc'd to
> > lkml:
> >
> > https://lore.kernel.org/all/1378920168.26698.64.camel@localhost/
> >
> > The decision then was to use the same random source as the key
> > protection. Unfortunately most of the active participants have
> > moved on from IBM and I don't have their current email addresses,
> > but the bottom line is there were good reasons to do trusted keys
> > this way that your assertions above don't overcome. I'm not saying
> > we shouldn't reconsider the situation, but we need a reasoned
> > debate rather than simply doing it by fiat.
>
> The way I see this is that given that kernel is not running inside
> TPM, FIPS certification of the RNG does not have any measurable
> value.
>
> Random data generation should happen as part of object creation
> process i.e. should be fully self-contained process within the TPM in
> order for FIPS to matter.
In FIPS terms, there's no distinction between keeping the whole
generation process internal to the TPM and using the FIPS certified rng
of the TPM to source the contents of a kernel protected key. Both
provide equally valid, and FIPS certified data.
> In the case of sealed data objects, this not the case.
FIPS is concerned with origins and provenance, so it most certainly is
the case even for trusted keys. However, if the Kernel RNG is fips
certified (as can happen with certain FIPS modules) it is the case that
either the Kernel or TPM RNG would satisfy the FIPS requirement. The
question for trusted key users is really do they always want the TPM
FIPS RNG or should we allow mixing with the kernel RNG even in the non-
FIPS case.
Perhaps, rather than getting hung up on FIPS sources and to facilitate
debating the bedrock requirements, we could turn this around and ask
what the use case you have for using the in-kernel RNG is?
Regards,
James
^ permalink raw reply
* Re: [PATCH] host/roots: Sandbox xdg-desktop-portal-spectrum-host
From: Günther Noack @ 2025-12-15 8:20 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Demi Marie Obenour, Alyssa Ross, Spectrum OS Development,
linux-security-module, landlock, Ryan B. Sullivan,
Günther Noack
In-Reply-To: <20251214.aiW5oc2izaxa@digikod.net>
On Sun, Dec 14, 2025 at 08:50:45PM +0100, Mickaël Salaün wrote:
> On Sat, Dec 13, 2025 at 11:49:11PM -0500, Demi Marie Obenour wrote:
> > On 12/13/25 20:39, Alyssa Ross wrote:
> > > Demi Marie Obenour <demiobenour@gmail.com> writes:
> > >
> > >> On 12/13/25 16:42, Alyssa Ross wrote:
> > >>> Demi Marie Obenour <demiobenour@gmail.com> writes:
> > >>>
> > >>>> On 12/13/25 14:12, Alyssa Ross wrote:
> > >>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
> > >>>>>
> > >>>>>> It is quite possible that these Landlock rules are unnecessarily
> > >>>>>> permissive, but all of the paths to which read and execute access is
> > >>>>>> granted are part of the root filesystem and therefore assumed to be
> > >>>>>> public knowledge. Removing access from any of them would only increase
> > >>>>>> the risk of accidental breakage in the future, and would not provide any
> > >>>>>> security improvements. seccomp *could* provide some improvements, but
> > >>>>>> the effort needed is too high for now.
> > >>>>>>
> > >>>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> > >>>>>> ---
> > >>>>>> .../template/data/service/xdg-desktop-portal-spectrum-host/run | 8 ++++++++
> > >>>>>> 1 file changed, 8 insertions(+)
> > >>>>>
> > >>>>> Are you sure this is working as intended? There's no rule allowing
> > >>>>> access to Cloud Hypervisor's VSOCK socket, and yet it still seems to be
> > >>>>> able to access that. Don't you need to set a rule that *restricts*
> > >>>>> filesystem access and then add holes? Did you ever see this deny
> > >>>>> anything?
> > >>>>
> > >>>> 'man 1 setpriv' states that '--landlock-access fs' blocks all
> > >>>> filesystem access unless a subsequent --landlock-rule permits it.
> > >>>> I tried running with no --landlock-rule flags and the execve of
> > >>>> xdg-desktop-portal-spectrum-host failed as expected.
> > >>>>
> > >>>> The socket is passed over stdin, and I'm pretty sure Landlock
> > >>>> doesn't restrict using an already-open file descriptor.
> > >>>> xdg-desktop-portal-spectrum-host does need to find the path to the
> > >>>> socket, but I don't think it ever accesses that path.
> > >>>
> > >>> I've been looking into this a bit myself, and from what I can tell
> > >>> Landlock just doesn't restrict connecting to sockets at all, even if
> > >>> they're inside directories that would otherwise be inaccessible. It's
> > >>> able to connect to both Cloud Hypervisor's VSOCK socket and the D-Bus
> > >>> socket even with a maximally restrictive landlock rule. So you were
> > >>> right after all, sorry!
> > >>
> > >> That's not good at all! It's a trivial sandbox escape in so many cases.
> > >> For instance, with access to D-Bus I can just call `systemd-run`.
> > >>
> > >> I'm CCing the Landlock and LSM mailing lists because if you are
> > >> correct, then this is a bad security hole.
> > >
> > > I don't find it that surprising given the way landlock works. "connect"
> > > (to a non-abstract AF_UNIX socket) is not an operation there's a
> > > landlock action for, and it's not like the other actions care about
> > > access to parent directories and the like — I was able to execute a
> > > program via a symlink after only giving access to the symlink's target,
> > > without any access to the directory containing the symlink or the
> > > symlink itself, for example. Landlock, as I understand it, is intended
> > > to block a specified set of operations (on particular file hierarchies),
> > > rather than to completely prevent access to those hierarchies like
> > > permissions or mount namespaces could, so the lack of a way to block
> > > connecting to a socket is more of a missing feature than a security
> > > hole.
> >
> > 'man 7 unix' states:
> >
> > On Linux, connecting to a stream socket object requires write
> > permission on that socket; sending a datagram to a datagram socket
> > likewise requires write permission on that socket.
> >
> > Landlock is definitely being inconsistent with DAC here. Also, this
> > allows real-world sandbox breakouts. On systemd systems, the simplest
> > way to escape is to use systemd-run to execute arbitrary commands.
>
> The Linux kernel is complex and the link between the VFS and named UNIX
> sockets is "special" (see the linked GitHub issue). Landlock doesn't
> handle named UNIX sockets yet for the same reason it doesn't handle some
> other kind of kernel resources or access rights: someone needs to
> implement it (including tests, doc...). There is definitely interest to
> add this feature, it has been discussed for some time, but it's not
> trivial. It is a work in progress though:
> https://github.com/landlock-lsm/linux/issues/36
Agreed, this would be the change that would let us restrict connect()
on AF_UNIX sockets.
Additionally, *in the case that you do not actually need* Unix
sockets, the other patch set that would be of interest is the one for
restricting the creation of new socket FDs:
https://github.com/landlock-lsm/linux/issues/6
In this talk in 2014, I explained my mental model around the
network-related restrictions:
https://youtu.be/K2onopkMhuM?si=LCObEX6HhGdzPnks&t=2030
The discussed socket control feature continues to be a central missing
piece, as the TCP port restrictions do not make much sense as long as
you can still create sockets for other protocol types.
Issue #6 that should fix this is still under active development -- an
updated version of the patch was posted just last month.
To bridge the gap, the same thing can also be emulated with seccomp,
but as you noted above as well in this thread, this is harder.
–Günther
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 8:49 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <60cf8bd2afbad5e930119d73ccf069e95ee4fd9d.camel@HansenPartnership.com>
On Mon, Dec 15, 2025 at 04:55:58PM +0900, James Bottomley wrote:
> On Mon, 2025-12-15 at 08:43 +0200, Jarkko Sakkinen wrote:
> > On Mon, Dec 15, 2025 at 07:18:41AM +0900, James Bottomley wrote:
> > > On Sun, 2025-12-14 at 23:32 +0200, Jarkko Sakkinen wrote:
> > > > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and
> > > > thus its use should be pooled rather than directly used. This
> > > > both reduces latency and improves its predictability.
> > > >
> > > > 2. Linux is better off overall if every subsystem uses the same
> > > > source for the random bistream as the de-facto choice, unless
> > > > *force majeure* reasons point to some other direction.
> > > >
> > > > In the case, of TPM there is no reason for trusted keys to invoke
> > > > TPM directly.
> > >
> > > That assertion isn't correct: you seem to have forgotten we had
> > > this argument six or seven years ago, but even that was a reprise
> > > of an even earlier one. Lore doesn't go back far enough for the
> > > intermediate one on the tpm list, but the original was cc'd to
> > > lkml:
> > >
> > > https://lore.kernel.org/all/1378920168.26698.64.camel@localhost/
> > >
> > > The decision then was to use the same random source as the key
> > > protection. Unfortunately most of the active participants have
> > > moved on from IBM and I don't have their current email addresses,
> > > but the bottom line is there were good reasons to do trusted keys
> > > this way that your assertions above don't overcome. I'm not saying
> > > we shouldn't reconsider the situation, but we need a reasoned
> > > debate rather than simply doing it by fiat.
> >
> > The way I see this is that given that kernel is not running inside
> > TPM, FIPS certification of the RNG does not have any measurable
> > value.
> >
> > Random data generation should happen as part of object creation
> > process i.e. should be fully self-contained process within the TPM in
> > order for FIPS to matter.
>
> In FIPS terms, there's no distinction between keeping the whole
> generation process internal to the TPM and using the FIPS certified rng
> of the TPM to source the contents of a kernel protected key. Both
> provide equally valid, and FIPS certified data.
I understand being "FIPS certified" embedding the premise that kernel
is also FIPS certified, which covers also crypto etc. This is the case
with enterprise kernels.
I have understanding FIPS certification dies at the point when random
data is acquired by a kernel, which is not FIPS certified. It's not
really a safe closure.
Using same code path for RNG universally should actually help with any
certification processes.
>
> > In the case of sealed data objects, this not the case.
>
> FIPS is concerned with origins and provenance, so it most certainly is
> the case even for trusted keys. However, if the Kernel RNG is fips
> certified (as can happen with certain FIPS modules) it is the case that
> either the Kernel or TPM RNG would satisfy the FIPS requirement. The
> question for trusted key users is really do they always want the TPM
> FIPS RNG or should we allow mixing with the kernel RNG even in the non-
> FIPS case.
I don't disagree on benefits of FIPS certification.
>
> Perhaps, rather than getting hung up on FIPS sources and to facilitate
> debating the bedrock requirements, we could turn this around and ask
> what the use case you have for using the in-kernel RNG is?
Generally removing any non-mandatory TPM traffic is a feasible idea.
This was just something low-hanging fruit that I spotted while working
on larger patch set.
BR, Jarkko
>
> Regards,
>
> James
>
>
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] host/roots: Sandbox xdg-desktop-portal-spectrum-host
From: Demi Marie Obenour @ 2025-12-15 8:54 UTC (permalink / raw)
To: Günther Noack, Mickaël Salaün
Cc: Alyssa Ross, Spectrum OS Development, linux-security-module,
landlock, Ryan B. Sullivan, Günther Noack
In-Reply-To: <20251215.5d7e473daa34@gnoack.org>
[-- Attachment #1.1.1: Type: text/plain, Size: 6574 bytes --]
On 12/15/25 03:20, Günther Noack wrote:
> On Sun, Dec 14, 2025 at 08:50:45PM +0100, Mickaël Salaün wrote:
>> On Sat, Dec 13, 2025 at 11:49:11PM -0500, Demi Marie Obenour wrote:
>>> On 12/13/25 20:39, Alyssa Ross wrote:
>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>>
>>>>> On 12/13/25 16:42, Alyssa Ross wrote:
>>>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>>>>
>>>>>>> On 12/13/25 14:12, Alyssa Ross wrote:
>>>>>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>>>>>>
>>>>>>>>> It is quite possible that these Landlock rules are unnecessarily
>>>>>>>>> permissive, but all of the paths to which read and execute access is
>>>>>>>>> granted are part of the root filesystem and therefore assumed to be
>>>>>>>>> public knowledge. Removing access from any of them would only increase
>>>>>>>>> the risk of accidental breakage in the future, and would not provide any
>>>>>>>>> security improvements. seccomp *could* provide some improvements, but
>>>>>>>>> the effort needed is too high for now.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>>>>>>>> ---
>>>>>>>>> .../template/data/service/xdg-desktop-portal-spectrum-host/run | 8 ++++++++
>>>>>>>>> 1 file changed, 8 insertions(+)
>>>>>>>>
>>>>>>>> Are you sure this is working as intended? There's no rule allowing
>>>>>>>> access to Cloud Hypervisor's VSOCK socket, and yet it still seems to be
>>>>>>>> able to access that. Don't you need to set a rule that *restricts*
>>>>>>>> filesystem access and then add holes? Did you ever see this deny
>>>>>>>> anything?
>>>>>>>
>>>>>>> 'man 1 setpriv' states that '--landlock-access fs' blocks all
>>>>>>> filesystem access unless a subsequent --landlock-rule permits it.
>>>>>>> I tried running with no --landlock-rule flags and the execve of
>>>>>>> xdg-desktop-portal-spectrum-host failed as expected.
>>>>>>>
>>>>>>> The socket is passed over stdin, and I'm pretty sure Landlock
>>>>>>> doesn't restrict using an already-open file descriptor.
>>>>>>> xdg-desktop-portal-spectrum-host does need to find the path to the
>>>>>>> socket, but I don't think it ever accesses that path.
>>>>>>
>>>>>> I've been looking into this a bit myself, and from what I can tell
>>>>>> Landlock just doesn't restrict connecting to sockets at all, even if
>>>>>> they're inside directories that would otherwise be inaccessible. It's
>>>>>> able to connect to both Cloud Hypervisor's VSOCK socket and the D-Bus
>>>>>> socket even with a maximally restrictive landlock rule. So you were
>>>>>> right after all, sorry!
>>>>>
>>>>> That's not good at all! It's a trivial sandbox escape in so many cases.
>>>>> For instance, with access to D-Bus I can just call `systemd-run`.
>>>>>
>>>>> I'm CCing the Landlock and LSM mailing lists because if you are
>>>>> correct, then this is a bad security hole.
>>>>
>>>> I don't find it that surprising given the way landlock works. "connect"
>>>> (to a non-abstract AF_UNIX socket) is not an operation there's a
>>>> landlock action for, and it's not like the other actions care about
>>>> access to parent directories and the like — I was able to execute a
>>>> program via a symlink after only giving access to the symlink's target,
>>>> without any access to the directory containing the symlink or the
>>>> symlink itself, for example. Landlock, as I understand it, is intended
>>>> to block a specified set of operations (on particular file hierarchies),
>>>> rather than to completely prevent access to those hierarchies like
>>>> permissions or mount namespaces could, so the lack of a way to block
>>>> connecting to a socket is more of a missing feature than a security
>>>> hole.
>>>
>>> 'man 7 unix' states:
>>>
>>> On Linux, connecting to a stream socket object requires write
>>> permission on that socket; sending a datagram to a datagram socket
>>> likewise requires write permission on that socket.
>>>
>>> Landlock is definitely being inconsistent with DAC here. Also, this
>>> allows real-world sandbox breakouts. On systemd systems, the simplest
>>> way to escape is to use systemd-run to execute arbitrary commands.
>>
>> The Linux kernel is complex and the link between the VFS and named UNIX
>> sockets is "special" (see the linked GitHub issue). Landlock doesn't
>> handle named UNIX sockets yet for the same reason it doesn't handle some
>> other kind of kernel resources or access rights: someone needs to
>> implement it (including tests, doc...). There is definitely interest to
>> add this feature, it has been discussed for some time, but it's not
>> trivial. It is a work in progress though:
>> https://github.com/landlock-lsm/linux/issues/36
>
> Agreed, this would be the change that would let us restrict connect()
> on AF_UNIX sockets.
>
> Additionally, *in the case that you do not actually need* Unix
> sockets, the other patch set that would be of interest is the one for
> restricting the creation of new socket FDs:
> https://github.com/landlock-lsm/linux/issues/6
>
> In this talk in 2014, I explained my mental model around the
> network-related restrictions:
> https://youtu.be/K2onopkMhuM?si=LCObEX6HhGdzPnks&t=2030
>
> The discussed socket control feature continues to be a central missing
> piece, as the TCP port restrictions do not make much sense as long as
> you can still create sockets for other protocol types.
>
> Issue #6 that should fix this is still under active development -- an
> updated version of the patch was posted just last month.
>
> To bridge the gap, the same thing can also be emulated with seccomp,
> but as you noted above as well in this thread, this is harder.
>
> –Günther
I'm a bit surprised that this needs to be separate from other access
controls. To me, it seems like a bit of a misdesign in the core kernel
(not Landlock).
I would go as far as to state that enabling other filesystem
restrictions should also restrict AF_UNIX filesystem sockets
automatically, as that is what users and administrators will expect.
What surprises me somewhat is that Linux does not have any sort of
unified hook for filesystem path walks. My mental model of Landlock
is that from a filesystem perspective, the result should be equivalent
to creating an empty mount namespace, putting a tmpfs on its root,
and bind-mounting the allowed paths. That this mental model does
not match reality was quite surprising.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-15 9:32 UTC (permalink / raw)
To: Sumit Garg
Cc: Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Sumit Garg, Ilias Apalodimas,
Jan Kiszka, Sudeep Holla, Christophe JAILLET, Michael Chan,
Pavan Chebbi, Rafał Miłecki, James Bottomley,
Jarkko Sakkinen, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel,
linux-crypto, linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <aT--ox375kg2Mzh-@sumit-X1>
[-- Attachment #1: Type: text/plain, Size: 3035 bytes --]
Hello Sumit,
On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> > Hello,
> >
> > the objective of this series is to make tee driver stop using callbacks
> > in struct device_driver. These were superseded by bus methods in 2006
> > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > methods.")) but nobody cared to convert all subsystems accordingly.
> >
> > Here the tee drivers are converted. The first commit is somewhat
> > unrelated, but simplifies the conversion (and the drivers). It
> > introduces driver registration helpers that care about setting the bus
> > and owner. (The latter is missing in all drivers, so by using these
> > helpers the drivers become more correct.)
> >
> > The patches #4 - #17 depend on the first two, so if they should be
> > applied to their respective subsystem trees these must contain the first
> > two patches first.
>
> Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
> drivers.
Thanks for your feedback. I will prepare a v2 and address your comments
(whitespace issues and wrong callback in the shutdown method).
> > Note that after patch #2 is applied, unconverted drivers provoke a
> > warning in driver_register(), so it would be good for the user
> > experience if the whole series goes in during a single merge window.
>
> +1
>
> I suggest the whole series goes via the Jens tree since there shouldn't
> be any chances for conflict here.
>
> > So
> > I guess an immutable branch containing the frist three patches that can
> > be merged into the other subsystem trees would be sensible.
> >
> > After all patches are applied, tee_bus_type can be made private to
> > drivers/tee as it's not used in other places any more.
> >
>
> Feel free to make the tee_bus_type private as the last patch in the series
> such that any followup driver follows this clean approach.
There is a bit more to do for that than I'm willing to invest. With my
patch series applied `tee_bus_type` is still used in
drivers/tee/optee/device.c and drivers/tee/tee_core.c. Maybe it's
sensible to merge these two files into a single one.
The things I wonder about additionally are:
- if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
drivers but not devices.
- optee_register_device() calls device_create_file() on
&optee_device->dev after device_register(&optee_device->dev).
(Attention half-knowledge!) I think device_create_file() should not
be called on an already registered device (or you have to send a
uevent afterwards). This should probably use type attribute groups.
(Or the need_supplicant attribute should be dropped as it isn't very
useful. This would maybe be considered an ABI change however.)
- Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
optee devices in its error path (optee_unregister_devices())?
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox