* [PATCH v7 06/10] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2026-04-12 19:31 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: <20260412193214.87072-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.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Notes:
v6..v7 changes:
* Split landlock_walk_path_up, the is_access_to_paths_allowed conversion,
the collect_domain_accesses conversion, and the find_rule move into
separate preparatory patches.
* Fixed disconnected-directory handling in landlock_append_fs_rule() when
marking NO_INHERIT ancestors.
v5..v6 changes:
* Retain existing documentation for path traversal in
is_access_to_paths_allowed.
* Change conditional for path walk in is_access_to_paths_allowed
removing possibility of infinite loop and renamed constant.
* Remove (now) redundant mnt_root parameter from
collect_domain_accesses.
* Change path parameter to a dentry for
deny_no_inherit_topology_change because only the dentry was needed.
* Minor documentation fixes.
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.
* Removed redundant loop for single-layer rulesets.
* 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.
security/landlock/fs.c | 117 ++++++++++++++++++++++++++++++++++++
security/landlock/ruleset.c | 40 +++++++++---
security/landlock/ruleset.h | 26 ++++++++
3 files changed, 173 insertions(+), 10 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 86a3435ebbba..6af1043a941f 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -392,6 +392,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
struct landlock_id id = {
.type = LANDLOCK_KEY_INODE,
};
+ struct path walker = *path;
/* Files only get access rights that make sense. */
if (!d_is_dir(path->dentry) &&
@@ -406,8 +407,44 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
id.key.object = get_inode_object(d_backing_inode(path->dentry));
if (IS_ERR(id.key.object))
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;
+
+ path_get(&walker);
+ while (landlock_walk_path_up(&walker) != LANDLOCK_WALK_STOP_REAL_ROOT) {
+ struct landlock_rule *ancestor_rule;
+
+ 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;
+ }
+ /* Insert a "blank" rule for the ancestor. */
+ 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);
+ }
+ /* Marks the ancestor rule, whether we inserted it or found it. */
+ 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()
@@ -1108,6 +1145,57 @@ collect_domain_accesses(const struct landlock_ruleset *const domain,
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,
+ struct dentry *const dcache_entry)
+{
+ layer_mask_t sealed_layers = 0;
+ layer_mask_t override_layers = 0;
+ const struct landlock_rule *rule;
+ size_t layer_index;
+
+ if (WARN_ON_ONCE(!subject || !dcache_entry ||
+ d_is_negative(dcache_entry)))
+ return 0;
+
+ rule = find_rule(subject->domain, dcache_entry);
+ 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;
+
+ landlock_log_denial(subject, &(struct landlock_request) {
+ .type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
+ .audit = {
+ .type = LSM_AUDIT_DATA_DENTRY,
+ .u.dentry = dcache_entry,
+ },
+ .layer_plus_one = __ffs((unsigned long)sealed_layers) + 1,
+ });
+ return -EACCES;
+}
+
/**
* current_check_refer_path - Check if a rename or link action is allowed
*
@@ -1193,6 +1281,16 @@ 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, old_dentry);
+
+ if (err)
+ return err;
+ if (exchange) {
+ err = deny_no_inherit_topology_change(subject,
+ new_dentry);
+ if (err)
+ return err;
+ }
access_request_parent1 |= maybe_remove(old_dentry);
access_request_parent2 |= maybe_remove(new_dentry);
}
@@ -1589,12 +1687,31 @@ 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, 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, dentry);
+ if (err)
+ return err;
+ }
+
return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
}
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index d2d1e3fb6cf2..8fdba3a7f983 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -257,6 +257,10 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
return -EINVAL;
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;
}
@@ -309,14 +313,17 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
const struct landlock_id id,
const access_mask_t access, const int flags)
{
- struct landlock_layer layers[] = { {
- .access = access,
- /* When @level is zero, insert_rule() extends @ruleset. */
- .level = 0,
- .flags = {
- .quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
- },
- } };
+ struct landlock_layer layers
+ [] = { { .access = access,
+ /* When @level is zero, insert_rule() extends @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();
return insert_rule(ruleset, id, &layers, ARRAY_SIZE(layers));
@@ -660,12 +667,25 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
const struct landlock_layer *const layer = &rule->layers[i];
const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
+ /*
+ * Skip layers that already have no_inherit set - these layers
+ * should not inherit access rights from ancestor directories.
+ */
+ if (rule_flags && (rule_flags->no_inherit_masks & layer_bit))
+ continue;
+
/* Clear the bits where the layer in the rule grants access. */
masks->access[layer->level - 1] &= ~layer->access;
/* Collect rule flags for each layer. */
- if (rule_flags && layer->flags.quiet)
- rule_flags->quiet_masks |= layer_bit;
+ if (rule_flags) {
+ if (layer->flags.quiet)
+ rule_flags->quiet_masks |= layer_bit;
+ if (layer->flags.no_inherit)
+ rule_flags->no_inherit_masks |= layer_bit;
+ if (layer->flags.has_no_inherit_descendant)
+ rule_flags->no_inherit_desc_masks |= layer_bit;
+ }
}
for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index e369f15ae885..34b70da8bd50 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -40,6 +40,20 @@ struct landlock_layer {
* down the file hierarchy.
*/
bool quiet:1;
+ /**
+ * @no_inherit: Prevents this rule from inheriting access rights
+ * from ancestor inodes. 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 to deny topology changes.
+ */
+ bool has_no_inherit_descendant : 1;
} flags;
/**
* @access: Bitfield of allowed actions on the kernel object. They are
@@ -62,6 +76,18 @@ 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.53.0
^ permalink raw reply related
* [PATCH v7 05/10] landlock: Move find_rule definition above landlock_append_fs_rule
From: Justin Suess @ 2026-04-12 19:31 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: <20260412193214.87072-1-utilityemal77@gmail.com>
Move find_rule above landlock_append_fs_rule to allow usage of find_rule
from within without an additional prototype declaration
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Notes:
v6..v7 changes:
* New patch split out from the v6 core NO_INHERIT implementation.
security/landlock/fs.c | 58 +++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 7ec26c671f91..86a3435ebbba 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -352,6 +352,35 @@ static enum landlock_walk_result landlock_walk_path_up(struct path *const path)
return LANDLOCK_WALK_CONTINUE;
}
+/* Access-control management */
+
+/*
+ * The lifetime of the returned rule is tied to @domain.
+ *
+ * Returns NULL if no rule is found or if @dentry is negative.
+ */
+static const struct landlock_rule *
+find_rule(const struct landlock_ruleset *const domain,
+ const struct dentry *const dentry)
+{
+ const struct landlock_rule *rule;
+ const struct inode *inode;
+ struct landlock_id id = {
+ .type = LANDLOCK_KEY_INODE,
+ };
+
+ /* Ignores nonexistent leafs. */
+ if (d_is_negative(dentry))
+ return NULL;
+
+ inode = d_backing_inode(dentry);
+ rcu_read_lock();
+ id.key.object = rcu_dereference(landlock_inode(inode)->object);
+ rule = landlock_find_rule(domain, id);
+ rcu_read_unlock();
+ return rule;
+}
+
/*
* @path: Should have been checked by get_path_from_fd().
*/
@@ -388,35 +417,6 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
return err;
}
-/* Access-control management */
-
-/*
- * The lifetime of the returned rule is tied to @domain.
- *
- * Returns NULL if no rule is found or if @dentry is negative.
- */
-static const struct landlock_rule *
-find_rule(const struct landlock_ruleset *const domain,
- const struct dentry *const dentry)
-{
- const struct landlock_rule *rule;
- const struct inode *inode;
- struct landlock_id id = {
- .type = LANDLOCK_KEY_INODE,
- };
-
- /* Ignores nonexistent leafs. */
- if (d_is_negative(dentry))
- return NULL;
-
- inode = d_backing_inode(dentry);
- rcu_read_lock();
- id.key.object = rcu_dereference(landlock_inode(inode)->object);
- rule = landlock_find_rule(domain, id);
- rcu_read_unlock();
- return rule;
-}
-
/*
* Allows access to pseudo filesystems that will never be mountable (e.g.
* sockfs, pipefs), but can still be reachable through
--
2.53.0
^ permalink raw reply related
* [PATCH v7 04/10] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
From: Justin Suess @ 2026-04-12 19:31 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: <20260412193214.87072-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>
---
Notes:
v6..v7 changes:
* None
v5..v6 changes:
* None
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.
include/uapi/linux/landlock.h | 29 ++++++++++++++++++++++++++++-
security/landlock/syscalls.c | 15 +++++++++++----
2 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 9a41c65623a1..290f76774e3f 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -127,10 +127,37 @@ 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 flags 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/syscalls.c b/security/landlock/syscalls.c
index a71068c41f76..fbab6573df70 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -360,7 +360,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;
@@ -414,6 +414,9 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
/* Check for useless quiet flag. */
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)
@@ -432,7 +435,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 or %LANDLOCK_ADD_RULE_NO_INHERIT.
*
* This system call enables to define a new rule and add it to an existing
* ruleset.
@@ -470,8 +473,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_NO_INHERIT | LANDLOCK_ADD_RULE_QUIET))
+ 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.53.0
^ permalink raw reply related
* [PATCH v7 03/10] landlock: Use landlock_walk_path_up for collect_domain_accesses
From: Justin Suess @ 2026-04-12 19:31 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: <20260412193214.87072-1-utilityemal77@gmail.com>
Use common path walk helper for collect_domain_accesses. This
extends the new centralized traversal logic to the current_check_refer
path code flow, and maintains consistency with the is_access_to_paths
allowed traversal while allowing reuse of traversal semantics.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Notes:
v6..v7 changes:
* New patch split out from the v6 core NO_INHERIT implementation.
security/landlock/fs.c | 75 ++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 35 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index b31bd2980e5c..7ec26c671f91 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1036,49 +1036,52 @@ 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.
+ * @path: Path to start the walk from and whose mount root is the last
+ * directory to check.
* @layer_masks_dom: Where to store the collected accesses.
*
- * This helper is useful to begin a path walk from the @dir directory to a
- * @mnt_root directory used as a mount point. This mount point is the common
- * ancestor between the source and the destination of a renamed and linked
- * file. While walking from @dir to @mnt_root, we record all the domain's
- * allowed accesses in @layer_masks_dom.
+ * This helper is useful to begin a path walk from @path to the mount root
+ * directory used as a mount point. This mount point is the common ancestor
+ * between the source and the destination of a renamed and linked file. While
+ * walking from @path to that mount root, we record all the domain's allowed
+ * accesses in @layer_masks_dom.
*
- * Because of disconnected directories, this walk may not reach @mnt_dir. In
- * this case, the walk will continue to @mnt_dir after this call.
+ * Because of disconnected directories, this walk may not reach that mount
+ * root. In this case, the walk will continue to the mount root after this
+ * call.
*
* This is similar to is_access_to_paths_allowed() but much simpler because it
* only handles walking on the same mount point and only checks one set of
* accesses.
*
- * Return: True if all the domain access rights are allowed for @dir, false if
- * the walk reached @mnt_root.
+ * Return: True if all the domain access rights are allowed for @path, false if
+ * the walk reached the mount root.
*/
static bool
collect_domain_accesses(const struct landlock_ruleset *const domain,
- const struct dentry *const mnt_root, struct dentry *dir,
+ const struct path *const path,
struct layer_access_masks *layer_masks_dom,
struct collected_rule_flags *const rule_flags)
{
bool ret = false;
+ struct path walker_path;
- if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
+ if (WARN_ON_ONCE(!domain || !path || !path->dentry ||
+ !path->mnt || !layer_masks_dom))
return true;
- if (is_nouser_or_private(dir))
+ if (is_nouser_or_private(path->dentry))
return true;
if (!landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
layer_masks_dom, LANDLOCK_KEY_INODE))
return true;
- dget(dir);
+ walker_path = *path;
+ path_get(&walker_path);
while (true) {
- struct dentry *parent_dentry;
-
/* Gets all layers allowing all domain accesses. */
- if (landlock_unmask_layers(find_rule(domain, dir),
+ if (landlock_unmask_layers(find_rule(domain,
+ walker_path.dentry),
layer_masks_dom, rule_flags)) {
/*
* Stops when all handled accesses are allowed by at
@@ -1092,14 +1095,16 @@ collect_domain_accesses(const struct landlock_ruleset *const domain,
* Stops at the mount point or the filesystem root for a disconnected
* directory.
*/
- if (dir == mnt_root || unlikely(IS_ROOT(dir)))
+ if ((walker_path.dentry == path->mnt->mnt_root &&
+ walker_path.mnt == path->mnt) ||
+ unlikely(IS_ROOT(walker_path.dentry)))
break;
- parent_dentry = dget_parent(dir);
- dput(dir);
- dir = parent_dentry;
+ if (WARN_ON_ONCE(landlock_walk_path_up(&walker_path) !=
+ LANDLOCK_WALK_CONTINUE))
+ break;
}
- dput(dir);
+ path_put(&walker_path);
return ret;
}
@@ -1165,7 +1170,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
bool allow_parent1, allow_parent2;
access_mask_t access_request_parent1, access_request_parent2;
struct path mnt_dir;
- struct dentry *old_parent;
+ struct path old_parent_path;
struct layer_access_masks layer_masks_parent1 = {},
layer_masks_parent2 = {};
struct landlock_request request1 = {}, request2 = {};
@@ -1223,19 +1228,19 @@ static int current_check_refer_path(struct dentry *const old_dentry,
/*
* old_dentry may be the root of the common mount point and
* !IS_ROOT(old_dentry) at the same time (e.g. with open_tree() and
- * OPEN_TREE_CLONE). We do not need to call dget(old_parent) because
- * we keep a reference to old_dentry.
+ * OPEN_TREE_CLONE). We do not need to call path_get(&old_parent_path)
+ * because we keep a reference to old_dentry.
*/
- old_parent = (old_dentry == mnt_dir.dentry) ? old_dentry :
- old_dentry->d_parent;
+ old_parent_path.mnt = mnt_dir.mnt;
+ old_parent_path.dentry = (old_dentry == mnt_dir.dentry) ?
+ 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,
- &layer_masks_parent1,
- &rule_flags_parent1);
- allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
- new_dir->dentry,
+ allow_parent1 = collect_domain_accesses(
+ subject->domain, &old_parent_path,
+ &layer_masks_parent1, &rule_flags_parent1);
+ allow_parent2 = collect_domain_accesses(subject->domain, new_dir,
&layer_masks_parent2,
&rule_flags_parent2);
if (allow_parent1 && allow_parent2)
@@ -1256,7 +1261,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
return 0;
if (request1.access) {
- request1.audit.u.path.dentry = old_parent;
+ request1.audit.u.path.dentry = old_parent_path.dentry;
request1.rule_flags = rule_flags_parent1;
landlock_log_denial(subject, &request1);
}
--
2.53.0
^ permalink raw reply related
* [PATCH v7 02/10] landlock: Use landlock_walk_path_up for is_access_to_paths_allowed
From: Justin Suess @ 2026-04-12 19:31 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: <20260412193214.87072-1-utilityemal77@gmail.com>
Use common path walk helper for is_access_to_paths_allowed. This
maintains consistency with the existing implementation, and removes the
backward goto jump.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Notes:
v6..v7 changes:
* New patch split out from the v6 core NO_INHERIT implementation.
security/landlock/fs.c | 60 ++++++++++++++----------------------------
1 file changed, 20 insertions(+), 40 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 644637a8c0b5..b31bd2980e5c 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -923,47 +923,27 @@ static bool is_access_to_paths_allowed(
/* Stops when a rule from each layer grants access. */
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;
+ switch (landlock_walk_path_up(&walker_path)) {
+ /*
+ * 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>).
+ */
+ case LANDLOCK_WALK_INTERNAL:
+ allowed_parent1 = true;
+ allowed_parent2 = true;
+ break;
+ /*
+ * Stops at the real root. Denies access
+ * because not all layers have granted access
+ */
+ case LANDLOCK_WALK_STOP_REAL_ROOT:
+ break;
+ /* Otherwise, keep walking up to the root. */
+ case LANDLOCK_WALK_CONTINUE:
+ continue;
}
+ break;
}
path_put(&walker_path);
--
2.53.0
^ permalink raw reply related
* [PATCH v7 01/10] landlock: Add path walk helper
From: Justin Suess @ 2026-04-12 19:31 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: <20260412193214.87072-1-utilityemal77@gmail.com>
Add path walk helper landlock_walk_path_up. This helper takes a pointer
to a struct path and walks the path upward towards the VFS root, and
returns an enum corresponding whether the current position in the walk
is an internal mountpoint, the real root, or neither.
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Notes:
v6..v7 changes:
* New patch split out from the v6 core NO_INHERIT implementation.
* Added enum comments.
security/landlock/fs.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index bd7554d0b65a..644637a8c0b5 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -320,6 +320,38 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
/* clang-format on */
+/**
+ * enum landlock_walk_result - Result codes for landlock_walk_path_up()
+ * @LANDLOCK_WALK_CONTINUE: Path is now neither the real root nor an internal mount point.
+ * @LANDLOCK_WALK_STOP_REAL_ROOT: Path has reached the real VFS root.
+ * @LANDLOCK_WALK_INTERNAL: Path has reached an internal mount point.
+ */
+enum landlock_walk_result {
+ LANDLOCK_WALK_CONTINUE,
+ LANDLOCK_WALK_STOP_REAL_ROOT,
+ LANDLOCK_WALK_INTERNAL,
+};
+
+static enum landlock_walk_result landlock_walk_path_up(struct path *const path)
+{
+ struct dentry *old;
+
+ while (path->dentry == path->mnt->mnt_root) {
+ if (!follow_up(path))
+ return LANDLOCK_WALK_STOP_REAL_ROOT;
+ }
+ old = path->dentry;
+ if (unlikely(IS_ROOT(old))) {
+ if (likely(path->mnt->mnt_flags & MNT_INTERNAL))
+ return LANDLOCK_WALK_INTERNAL;
+ path->dentry = dget(path->mnt->mnt_root);
+ } else {
+ path->dentry = dget_parent(old);
+ }
+ dput(old);
+ return LANDLOCK_WALK_CONTINUE;
+}
+
/*
* @path: Should have been checked by get_path_from_fd().
*/
--
2.53.0
^ permalink raw reply related
* [PATCH v7 00/10] Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2026-04-12 19:31 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 7 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 again on cleanup, splitting out
some patches and fixing an edge case with disconnected directories.
Behavior of the flag is identical to the previous patch.
This series is rebased on v8 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 reparent, rename, or remove the tagged inode or
its ancestors (via rename, rmdir, link) are denied up to the VFS
root; and
* 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 v6:
1. The main implementation of NO_INHERIT was split into smaller more
reviewable patches, separating the landlock_walk_path_up
implementation, usages of landlock_walk_path_up, and the find_rule
move to separate patches
2. A small issue regarding disconnected directory handling, where rules
inserted with NO_INHERIT only had protection up to a disconnected
directory instead of the mountpoint was fixed. In practice, this
isn't a problem at the current time since landlock forbids the mount
syscall needed to move a mountpoint with MS_MOVE. However, for
future-proofing in the case landlock allows some mount operations,
restrictions on parent directories now apply to the real root.
Changes since v5:
1. Retain existing documentation for path traversal in
is_access_to_paths_allowed.
2. Change conditional for path walk in is_access_to_paths_allowed
removing possibility of infinite loop and renamed constant.
3. Remove (now) redundant mnt_root parameter from
collect_domain_accesses.
4. Change path parameter to a dentry for
deny_no_inherit_topology_change because only the dentry was needed.
5. Remove duplicated tree diagram comment from selftests.
6. Minor documentation fixes.
Credit to Tingmao Wang for pointing out 1, 2, 3, 4, and 6.
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.
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/
v5:
https://lore.kernel.org/linux-security-module/20251214170548.408142-1-utilityemal77@gmail.com/
quiet-flag v6:
https://lore.kernel.org/linux-security-module/cover.1765040503.git.m@maowtm.org/
quiet-flag v7:
https://lore.kernel.org/linux-security-module/cover.1766330134.git.m@maowtm.org/
quiet-flag v8:
https://lore.kernel.org/linux-security-module/cover.1775490344.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
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 (10):
landlock: Add path walk helper
landlock: Use landlock_walk_path_up for is_access_to_paths_allowed
landlock: Use landlock_walk_path_up for collect_domain_accesses
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
landlock: Move find_rule definition above landlock_append_fs_rule
landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
landlock: Add documentation for LANDLOCK_ADD_RULE_NO_INHERIT
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
Documentation/userspace-api/landlock.rst | 17 +
include/uapi/linux/landlock.h | 29 +-
samples/landlock/sandboxer.c | 11 +
security/landlock/fs.c | 342 +++++++---
security/landlock/ruleset.c | 125 +++-
security/landlock/ruleset.h | 26 +
security/landlock/syscalls.c | 15 +-
tools/testing/selftests/landlock/fs_test.c | 705 +++++++++++++++++++++
8 files changed, 1151 insertions(+), 119 deletions(-)
--
2.53.0
^ permalink raw reply
* Re: [PATCH v3] KEYS: trusted: Debugging as a feature
From: Nayna Jain @ 2026-04-12 18:47 UTC (permalink / raw)
To: Jarkko Sakinen, linux-integrity, keyrings
Cc: Srish Srinivasan, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ahmad Fatoum,
Pengutronix Kernel Team, linux-kernel, linux-security-module
In-Reply-To: <20260409160752.988713-1-jarkko@kernel.org>
On 4/9/26 12:07 PM, Jarkko Sakinen wrote:
> From: Jarkko Sakkinen <jarkko@kernel.org>
>
> TPM_DEBUG, and other similar flags, are a non-standard way to specify a
> feature in Linux kernel. Introduce CONFIG_TRUSTED_KEYS_DEBUG for trusted
> keys, and use it to replace these ad-hoc feature flags.
>
> Given that trusted keys debug dumps can contain sensitive data, harden the
> feature as follows:
>
> 1. In the Kconfig description postulate that pr_debug() statements must be
> used.
> 2. Use pr_debug() statements in TPM 1.x driver to print the protocol dump.
> 3. Require trusted.debug=1 on the kernel command line (default: 0) to
> activate dumps at runtime, even when CONFIG_TRUSTED_KEYS_DEBUG=y.
>
> Traces, when actually needed, can be easily enabled by providing
> trusted.dyndbg='+p' and trusted.debug=1 in the kernel command-line.
Thanks Jarkko. Additional changes looks good to me. I just realized that
the kernel command-line parameters document may need to be updated to
include these parameters.
Apart from that, feel free to add my
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [PATCH v2 01/17] landlock: Prepare ruleset and domain type split
From: Tingmao Wang @ 2026-04-12 16:29 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Christian Brauner, Günther Noack, Steven Rostedt, Jann Horn,
Jeff Xu, Justin Suess, Kees Cook, Masami Hiramatsu,
Mathieu Desnoyers, Matthieu Buffet, Mikhail Ivanov, kernel-team,
linux-fsdevel, linux-security-module, linux-trace-kernel
In-Reply-To: <20260406143717.1815792-2-mic@digikod.net>
On 4/6/26 15:36, Mickaël Salaün wrote:
> [...]
Hi Mickaël,
I like this approach, as I basically ended up doing similar refactoring
previously for the hashtable / array-based domain changes, and having this
done first should make it easier to adopt the domain data structure
changes in the future.
I assume it's fine for me to add:
Reviewed-by: Tingmao Wang <m@maowtm.org>
> @@ -175,19 +163,24 @@ static void free_rule(struct landlock_rule *const rule,
>
> static void build_check_ruleset(void)
> {
> - const struct landlock_ruleset ruleset = {
> + const struct landlock_rules rules = {
> .num_rules = ~0,
> + };
> + const struct landlock_ruleset ruleset = {
> .num_layers = ~0,
> };
>
> - BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES);
> + BUILD_BUG_ON(rules.num_rules < LANDLOCK_MAX_NUM_RULES);
> BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
> }
>
> /**
> - * insert_rule - Create and insert a rule in a ruleset
> + * insert_rule - Create and insert a rule in a rule set
^^^^^^^^
Should this be rule storage to be consistent with the next 2 lines?
Alternatively maybe we can just say "struct landlock_rules" to avoid
inventing new names?
> *
> - * @ruleset: The ruleset to be updated.
> + * @rules: The rule storage to be updated. The caller is responsible for
> + * any required locking. For rulesets, this means holding
> + * landlock_ruleset.lock. For domains under construction, no lock is
> + * needed because the domain is not yet visible to other tasks.
> * @id: The ID to build the new rule with. The underlying kernel object, if
> * any, must be held by the caller.
> * @layers: One or multiple layers to be copied into the new rule.
^ permalink raw reply
* Re: [PATCH v2 03/17] landlock: Split struct landlock_domain from struct landlock_ruleset
From: Tingmao Wang @ 2026-04-12 16:27 UTC (permalink / raw)
To: Mickaël Salaün, Günther Noack
Cc: Christian Brauner, Steven Rostedt, Jann Horn, Jeff Xu,
Justin Suess, Kees Cook, Masami Hiramatsu, Mathieu Desnoyers,
Matthieu Buffet, Mikhail Ivanov, kernel-team, linux-fsdevel,
linux-security-module, linux-trace-kernel
In-Reply-To: <20260406143717.1815792-4-mic@digikod.net>
On 4/6/26 15:37, Mickaël Salaün wrote:
> [...]
> @@ -197,10 +179,10 @@ static void build_check_ruleset(void)
> *
> * Return: 0 on success, -errno on failure.
> */
> -static int insert_rule(struct landlock_rules *const rules,
> - const struct landlock_id id,
> - const struct landlock_layer (*layers)[],
> - const size_t num_layers)
> +int landlock_rule_insert(struct landlock_rules *const rules,
> + const struct landlock_id id,
> + const struct landlock_layer (*layers)[],
> + const size_t num_layers)
Maybe this is slightly off topic, but previously I've found this function,
along with create_rule and merge_tree, to be quite confusing, because the
logic for three different use cases (creating a copy of an old domain,
merging a new layer into this domain, and inserting a new rule into an
unmerged ruleset) are mixed in the code, and personally now that I'm
reading it again I still find these functions to be hard to reason about.
Therefore, given that we're refactoring these areas, I think this might be
a good opportunity to rewrite them (while getting the necessary testing
for this rewrite "for free" as part of this whole domain refactor).
For example, for this snippet:
/* Only a single-level layer should match an existing rule. */
if (WARN_ON_ONCE(num_layers != 1))
return -EINVAL;
Someone unfamiliar with how this function is being used by its caller may
not realize that the reason the comment is true is because the only use
case where we have multiple layers in @layers being passed into this
function is when we're copying an existing domain into a new domain, and
so we should never match something that already exists.
Also, further along in this function, there is this snippet:
/*
* Intersects access rights when it is a merge between a
* ruleset and a domain.
*/
new_rule = create_rule(id, &this->layers, this->num_layers,
&(*layers)[0]);
I also found the comment to be confusing because it's not "intersect"ing
anything (it's adding a new layer to an existing rule in the domain when
the object pointed to by "id" exists already in the domain, and the
intersection is only a consequence of how Landlock works when there are
multiple layers). This realization is made harder by the fact that a few
lines above we just OR'd the access rights (for modification of an
unmerged ruleset), and so it makes it sound like it's doing a similar
thing except with AND instead of OR.
I think it might make sense to have separate functions, even if they result in some slight code duplication, for use by:
1. Copying a domain: inherit_ruleset() -> inherit_tree() -> _____()
RB tree search to find the insertion point + create_rule(). Maybe
this logic could just be in inherit_tree() without creating a separate
function?
2. Merging a rule into a domain: merge_ruleset() -> merge_tree() -> _____()
insert_or_append_rule()? RB tree search, call create_rule() to create
a new rule with the new layer added, then either
rb_link_node()+rb_insert_color() or rb_replace_node().
Neither functions above will contain any actual logical AND/OR.
3. Inserting a new rule into an unmerged ruleset: landlock_add_rule() -> ... -> _____()
insert_or_update_rule()? RB tree search, and either update the access
rights of an existing rule in-place (as we currently do), or create a
new rule if the search fails.
We could create a utility static function or macro for the shared RB tree
search code.
How does this sound?
^ permalink raw reply
* [PATCH 3/5] selftests/landlock: add tests for chmod and chown restrictions
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
This patch adds basic tests for the support of chmod and chown system
calls restriction in landlock.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
tools/testing/selftests/landlock/fs_test.c | 99 +++++++++++++++++++++-
1 file changed, 98 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index e5898dc7e53e..13d276558146 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -578,7 +578,9 @@ TEST_F_FORK(layout1, inval)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV
@@ -4111,6 +4113,101 @@ TEST_F_FORK(ftruncate, open_and_ftruncate_in_different_processes)
ASSERT_EQ(0, close(socket_fds[1]));
}
+static int test_chmod(const char *path, mode_t mode)
+{
+ if (chmod(path, mode) == -1)
+ return errno;
+ return 0;
+}
+
+TEST_F_FORK(layout1, chmod_file)
+{
+ const char *const file_rw_no_chmod = file1_s1d1;
+ const char *const file_chmod = file1_s1d2;
+
+ const struct rule rules[] = {
+ {
+ .path = file_rw_no_chmod,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {
+ .path = file_chmod,
+ .access = LANDLOCK_ACCESS_FS_CHMOD,
+ },
+ {},
+ };
+
+ const __u64 handled = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE |
+ LANDLOCK_ACCESS_FS_CHMOD;
+ int ruleset_fd;
+
+ /* Enables Landlock. */
+ ruleset_fd = create_ruleset(_metadata, handled, rules);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks chmod rights when it is not allowed, mode is arbitrary */
+ EXPECT_EQ(EACCES, test_chmod(file_rw_no_chmod, 777));
+
+ /* Checks chmod rights when it is allowed, mode is arbitrary */
+ EXPECT_EQ(0, test_chmod(file_chmod, 777));
+}
+
+static int test_chown(const char *path, uid_t owner, gid_t group)
+{
+ if (chown(path, owner, group) == -1)
+ return errno;
+ return 0;
+}
+
+TEST_F_FORK(layout1, chown_file)
+{
+ const char *const file_rw_no_chown = file1_s1d1;
+ const char *const file_chown = file1_s1d2;
+
+ const struct rule rules[] = {
+ {
+ .path = file_rw_no_chown,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {
+ .path = file_chown,
+ .access = LANDLOCK_ACCESS_FS_CHOWN,
+ },
+ {},
+ };
+
+ const __u64 handled = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE |
+ LANDLOCK_ACCESS_FS_CHOWN;
+ int ruleset_fd;
+
+ /* Enables Landlock. */
+ ruleset_fd = create_ruleset(_metadata, handled, rules);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Checks chown rights when it is not allowed, owner and group are
+ * arbitrary.
+ */
+ EXPECT_EQ(EACCES, test_chown(file_rw_no_chown, 0, 0));
+
+ /*
+ * Checks chown rights when it is allowed, owner and group are
+ * arbitrary.
+ */
+ EXPECT_EQ(0, test_chown(file_chown, 0, 0));
+}
+
+
/* Invokes the FS_IOC_GETFLAGS IOCTL and returns its errno or 0. */
static int test_fs_ioc_getflags_ioctl(int fd)
{
--
2.53.0
^ permalink raw reply related
* [PATCH 2/5] landlock: add support for chmod and chown
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
Modifying file permissions and owner are operation of interest when
exploiting applications. This patch adds support for both chmod and
chown system calls family in landlock, allowing one to restrict it for
a given userland application.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
include/uapi/linux/landlock.h | 13 ++++++++++---
security/landlock/access.h | 2 +-
security/landlock/audit.c | 2 ++
security/landlock/fs.c | 17 ++++++++++++++++-
security/landlock/limits.h | 2 +-
5 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f88fa1f68b77..815577bda274 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -248,6 +248,12 @@ struct landlock_net_port_attr {
*
* This access right is available since the fifth version of the Landlock
* ABI.
+ * - %LANDLOCK_ACCESS_FS_CHMOD: Modify permissions on a file with
+ * :manpage:`chmod(2)` family system calls (:manpage:`fchmod(2)`,
+ * :manpage:`fchmodat(2)`, :manpage:`fchmodat2(2)`).
+ * - %LANDLOCK_ACCESS_FS_CHOWN: Change owner of a file with
+ * :manpage:`chown(2)` family system calls (:manpage:`fchown(2)`,
+ * :manpage:`fchownat(2)`, :manpage:`lchown(2)`).
*
* Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
* with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
@@ -311,9 +317,8 @@ struct landlock_net_port_attr {
*
* It is currently not possible to restrict some file-related actions
* accessible through these syscall families: :manpage:`chdir(2)`,
- * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`,
- * :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`,
- * :manpage:`fcntl(2)`, :manpage:`access(2)`.
+ * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`setxattr(2)`,
+ * :manpage:`utime(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`.
* Future Landlock evolutions will enable to restrict them.
*/
/* clang-format off */
@@ -333,6 +338,8 @@ struct landlock_net_port_attr {
#define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
#define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15)
+#define LANDLOCK_ACCESS_FS_CHMOD (1ULL << 16)
+#define LANDLOCK_ACCESS_FS_CHOWN (1ULL << 17)
/* clang-format on */
/**
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 42c95747d7bd..89dc8e7b93da 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -34,7 +34,7 @@
LANDLOCK_ACCESS_FS_IOCTL_DEV)
/* clang-format on */
-typedef u16 access_mask_t;
+typedef u32 access_mask_t;
/* Makes sure all filesystem access rights can be stored. */
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 60ff217ab95b..a4dec40d5395 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -37,6 +37,8 @@ static const char *const fs_access_strings[] = {
[BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
+ [BIT_INDEX(LANDLOCK_ACCESS_FS_CHMOD)] = "fs.chmod",
+ [BIT_INDEX(LANDLOCK_ACCESS_FS_CHOWN)] = "fs.chown",
};
static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index e764470f588c..b32d91b733b9 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -314,7 +314,9 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
/* clang-format on */
/*
@@ -1561,6 +1563,17 @@ static int hook_path_truncate(const struct path *const path)
return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
}
+static int hook_path_chmod(const struct path *const path, umode_t mode)
+{
+ return current_check_access_path(path, LANDLOCK_ACCESS_FS_CHMOD);
+}
+
+static int hook_path_chown(const struct path *const path, kuid_t uid,
+ kgid_t gid)
+{
+ return current_check_access_path(path, LANDLOCK_ACCESS_FS_CHOWN);
+}
+
/* File hooks */
/**
@@ -1838,6 +1851,8 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
LSM_HOOK_INIT(path_unlink, hook_path_unlink),
LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
LSM_HOOK_INIT(path_truncate, hook_path_truncate),
+ LSM_HOOK_INIT(path_chmod, hook_path_chmod),
+ LSM_HOOK_INIT(path_chown, hook_path_chown),
LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
LSM_HOOK_INIT(file_open, hook_file_open),
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index eb584f47288d..231d60d5bf8b 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -19,7 +19,7 @@
#define LANDLOCK_MAX_NUM_LAYERS 16
#define LANDLOCK_MAX_NUM_RULES U32_MAX
-#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV
+#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_CHOWN
#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
--
2.53.0
^ permalink raw reply related
* [PATCH 1/5] selftests/landlock: fix return condition on create_directory
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
If path exists when calling create_directory() in tests, i-e. when
mkdir() return with EEXISTS, directory creation fails. This patch
fixes it by allowing create_directory to use eventual existing
directories.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
tools/testing/selftests/landlock/fs_test.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 968a91c927a4..e5898dc7e53e 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -218,8 +218,11 @@ static void mkdir_parents(struct __test_metadata *const _metadata,
static void create_directory(struct __test_metadata *const _metadata,
const char *const path)
{
+ int err;
+
mkdir_parents(_metadata, path);
- ASSERT_EQ(0, mkdir(path, 0700))
+ err = mkdir(path, 0700);
+ ASSERT_FALSE(err && errno != EEXIST)
{
TH_LOG("Failed to create directory \"%s\": %s", path,
strerror(errno));
base-commit: 82544d36b1729153c8aeb179e84750f0c085d3b1
--
2.53.0
^ permalink raw reply related
* [PATCH 4/5] samples/landlock: add support for chown and chmod
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
Update sandboxer.c sample code with restriction for chown and chmod
system calls families
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
samples/landlock/sandboxer.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208..551e9a33665a 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -111,7 +111,9 @@ static int parse_path(char *env_path, const char ***const path_list)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
/* clang-format on */
@@ -295,7 +297,9 @@ static bool check_ruleset_scope(const char *const env_var,
LANDLOCK_ACCESS_FS_MAKE_SYM | \
LANDLOCK_ACCESS_FS_REFER | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
/* clang-format on */
--
2.53.0
^ permalink raw reply related
* landlock: Add support for chmod and chown system calls families
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
Hi,
This patch serie add support for chmod and chown system calls families
in Landlock.
These system calls could be used when exploiting applications. Two new
flags are added for struct landlock_ruleset_attr:
* LANDLOCK_ACCESS_FS_CHMOD
* LANDLOCK_ACCESS_FS_CHOWN
Restriction is limited to files as the security.c hooks for both
system calls seem to only applies to files. More digging is needed
before being able to restrict calls to chmod and chown on directories.
It adds basic tests for both family operations, one for when it is
allowed, one for when it is not.
First patch also fixes a bug I encountered when writing the tests.
^ permalink raw reply
* [PATCH 5/5] landlock: Document chmod and chown support in example code
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
Add missing LANDLOCK_ACCESS_FS_* flags in Landlock documentation
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
Documentation/userspace-api/landlock.rst | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 13134bccdd39..4eb7ed3dcbfe 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -77,7 +77,9 @@ to be explicit about the denied-by-default access rights.
LANDLOCK_ACCESS_FS_MAKE_SYM |
LANDLOCK_ACCESS_FS_REFER |
LANDLOCK_ACCESS_FS_TRUNCATE |
- LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ LANDLOCK_ACCESS_FS_IOCTL_DEV |
+ LANDLOCK_ACCESS_FS_CHMOD |
+ LANDLOCK_ACCESS_FS_CHOWN,
.handled_access_net =
LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 0/4] Firmware LSM hook
From: Leon Romanovsky @ 2026-04-12 9:00 UTC (permalink / raw)
To: Paul Moore
Cc: Roberto Sassu, KP Singh, Matt Bobrowski, Alexei Starovoitov,
Daniel Borkmann, John Fastabend, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
Jason Gunthorpe, Saeed Mahameed, Itay Avraham, Dave Jiang,
Jonathan Cameron, bpf, linux-kernel, linux-kselftest, linux-rdma,
Chiara Meiohas, Maher Sanalla, linux-security-module
In-Reply-To: <CAHC9VhT1X4HX4bGrK=mEzu=g=mZ-Wg-LDXVgZVe-e6oM+W9aHg@mail.gmail.com>
On Thu, Apr 09, 2026 at 05:04:24PM -0400, Paul Moore wrote:
> On Thu, Apr 9, 2026 at 8:45 AM Leon Romanovsky <leon@kernel.org> wrote:
> > On Thu, Apr 09, 2026 at 02:27:43PM +0200, Roberto Sassu wrote:
> > > On Thu, 2026-04-09 at 15:12 +0300, Leon Romanovsky wrote:
> > > > On Tue, Mar 31, 2026 at 08:56:32AM +0300, Leon Romanovsky wrote:
> > > > > From Chiara:
> > > > >
> > > > > This patch set introduces a new BPF LSM hook to validate firmware commands
> > > > > triggered by userspace before they are submitted to the device. The hook
> > > > > runs after the command buffer is constructed, right before it is sent
> > > > > to firmware.
> > > >
> > > > <...>
> > > >
> > > > > ---
> > > > > Chiara Meiohas (4):
> > > > > bpf: add firmware command validation hook
> > > > > selftests/bpf: add test cases for fw_validate_cmd hook
> > > > > RDMA/mlx5: Externally validate FW commands supplied in DEVX interface
> > > > > fwctl/mlx5: Externally validate FW commands supplied in fwctl
> > > >
> > > > Hi,
> > > >
> > > > Can we get Ack from BPF/LSM side?
> > >
> > > + Paul, linux-security-module ML
> > >
> > > Hi
> > >
> > > probably you also want to get an Ack from the LSM maintainer (added in
> > > CC with the list). Most likely, he will also ask you to create the
> > > security_*() functions counterparts of the BPF hooks.
> >
> > We implemented this approach in v1:
> > https://patch.msgid.link/20260309-fw-lsm-hook-v1-0-4a6422e63725@nvidia.com
> > and were advised to pursue a different direction.
>
> I'm assuming you are referring to my comments? If so, that isn't exactly what I said,
> I mentioned at least one other option besides
> going directly to BPF. Ultimately, it is your choice to decide how
> you want to proceed, but to claim I advised you to avoid a LSM based
> solution isn't strictly correct.
Yes, this matches how we understood your comments:
https://lore.kernel.org/all/20260311081955.GS12611@unreal/
In the end, the goal is to build something practical and avoid adding
unnecessary complexity that brings no real benefit to users.
>
> Regardless, looking at your v2 patchset, it looks like you've taken an
> unusual approach of using some of the LSM mechanisms, e.g. LSM_HOOK(),
> but not actually exposing a LSM hook with proper callbacks.
> Unfortunately, that's not something we want to support. If you want
> to pursue an LSM based solution, complete with a security_XXX() hook,
> use of LSM_HOOK() macros, etc. then that's fine, I'm happy to work
> with you on that.
The issue is that the sentence below was the reason we did not merge v1 with v2:
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks
"pass through implementations, such as the BPF LSM, are not eligible for
LSM hook reference implementations."
> However, if you've decided that your preferred
> option is to create a BPF hook you should avoid using things like
> LSM_HOOK() and locating your hook/code in bpf_lsm.c.
We are not limited to LSM solution, the goal is to intercept commands
which are submitted to the FW and "security" bucket sounded right to us.
>
> The good news is that there are plenty of other examples of BPF
> plugable code that you could use as an example, one such thing is the
> update_socket_protocol() BPF hook that was originally proposed as a
> LSM hook, but moved to a dedicated BPF hook as we generally want to
> avoid changing non-LSM kernel objects within the scope of the LSMs.
> While your proposed case is slightly different, I think the basic idea
> and mechanism should still be useful.
>
> https://lore.kernel.org/all/cover.1692147782.git.geliang.tang@suse.com
Thanks
>
> --
> paul-moore.com
>
^ permalink raw reply
* [PATCH] trusted-keys: move pr_fmt out of trusted-type.h
From: Josh Snyder @ 2026-04-11 20:12 UTC (permalink / raw)
To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Ahmad Fatoum, Pengutronix Kernel Team, Paul Moore, James Morris,
Serge E. Hallyn, David Gstir, sigma star Kernel Team,
Srish Srinivasan, Nayna Jain, Sumit Garg
Cc: linux-integrity, keyrings, linux-kernel, linux-security-module,
Josh Snyder
Defining pr_fmt in a widely-included header leaks the "trusted_key: "
prefix into every translation unit that transitively includes
<keys/trusted-type.h>. dm-crypt, for example, ends up printing
trusted_key: device-mapper: crypt: dm-10: INTEGRITY AEAD ERROR ...
dm-crypt began including <keys/trusted-type.h> in commit 363880c4eb36
("dm crypt: support using trusted keys"), which predates the pr_fmt
addition, so the regression has been live from the moment the header
gained its own pr_fmt definition.
Move the pr_fmt definition into the trusted-keys source files that
actually want the prefix.
Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Josh Snyder <josh@code406.com>
---
| 6 ------
| 2 ++
| 2 ++
| 2 ++
| 2 ++
| 2 ++
| 2 ++
7 files changed, 12 insertions(+), 6 deletions(-)
--git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 03527162613f7..54da1f174aeab 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -11,12 +11,6 @@
#include <linux/rcupdate.h>
#include <linux/tpm.h>
-#ifdef pr_fmt
-#undef pr_fmt
-#endif
-
-#define pr_fmt(fmt) "trusted_key: " fmt
-
#define MIN_KEY_SIZE 32
#define MAX_KEY_SIZE 128
#if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
--git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
index 601943ce0d60f..a31fd89c0e5c5 100644
--- a/security/keys/trusted-keys/trusted_caam.c
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -4,6 +4,8 @@
* Copyright 2025 NXP
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/trusted_caam.h>
#include <keys/trusted-type.h>
#include <linux/build_bug.h>
--git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 0b142d941cd2e..159af9dcfc774 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -6,6 +6,8 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/user-type.h>
#include <keys/trusted-type.h>
#include <keys/trusted_tee.h>
--git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
index 7b6eb655df0cb..f15ec400848ce 100644
--- a/security/keys/trusted-keys/trusted_dcp.c
+++ b/security/keys/trusted-keys/trusted_dcp.c
@@ -3,6 +3,8 @@
* Copyright (C) 2021 sigma star gmbh
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <crypto/aead.h>
#include <crypto/aes.h>
#include <crypto/algapi.h>
--git a/security/keys/trusted-keys/trusted_pkwm.c b/security/keys/trusted-keys/trusted_pkwm.c
index bf42c6679245a..94c92b90d88da 100644
--- a/security/keys/trusted-keys/trusted_pkwm.c
+++ b/security/keys/trusted-keys/trusted_pkwm.c
@@ -3,6 +3,8 @@
* Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/trusted_pkwm.h>
#include <keys/trusted-type.h>
#include <linux/build_bug.h>
--git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 6ea728f1eae6f..69dac20e4bf23 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -6,6 +6,8 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <crypto/hash_info.h>
#include <crypto/sha1.h>
#include <crypto/utils.h>
--git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 6340823f8b53c..f47ae952a0e7c 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -4,6 +4,8 @@
* Copyright (C) 2014 Intel Corporation
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <linux/asn1_encoder.h>
#include <linux/oid_registry.h>
#include <linux/string.h>
---
base-commit: cc13002a9f984d37906e9476f3e532a8cdd126f5
change-id: 20260411-trusted-key-header-a544a4f149d2
Best regards,
--
Josh
^ permalink raw reply related
* [PATCH 3/3] selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_CHAR
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
In-Reply-To: <20260411090944.3131168-2-gnoack@google.com>
Even though OverlayFS uses vfs_rename() with RENAME_WHITEOUT under the
hood, and even though RENAME_WHITEOUT requires
LANDLOCK_ACCESS_FS_MAKE_CHAR, a process that renames files in an OverlayFS
can do so without having the LANDLOCK_ACCESS_FS_MAKE_CHAR right in that
location. This works because OverlayFS uses the credentials determined at
mount time for the internal vfs_rename() operation.
Signed-off-by: Günther Noack <gnoack@google.com>
---
security/landlock/fs.c | 11 +++++---
tools/testing/selftests/landlock/fs_test.c | 31 ++++++++++++++++++++++
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 2b84a229e4d8..9b49f6c3e5da 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1523,11 +1523,14 @@ static int hook_path_rename(const struct path *const old_dir,
int err;
/*
- * This check would better be done together with other path
- * walks which are already happening for the normal rename check
- * in current_check_refer_path().
+ * Rename with RENAME_WHITEOUT creates a whiteout object
+ * (character device file with major=minor=0) in the old
+ * location, so we check the access right for creating that.
+ *
+ * See Documentation/filesystems/overlayfs.rst and renameat2(2).
*/
- err = current_check_access_path(old_dir, LANDLOCK_ACCESS_FS_MAKE_CHAR);
+ err = current_check_access_path(old_dir,
+ LANDLOCK_ACCESS_FS_MAKE_CHAR);
if (err)
return err;
}
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index d867016e3fd3..4cf6fc0bcb71 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -6962,6 +6962,37 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
}
}
+TEST_F_FORK(layout2_overlay, rename_in_overlay_without_make_char)
+{
+ struct stat st;
+ const char *merge_fl1_renamed = MERGE_DATA "/fl1_renamed";
+
+ if (self->skip_test)
+ SKIP(return, "overlayfs is not supported (test)");
+
+ enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_CHAR, NULL);
+
+ /*
+ * Execute a regular file rename within OverlayFS.
+ * merge_fl1 originates from lower layer, so this triggers a copy-up
+ * and creation of a whiteout in the upper layer.
+ */
+ EXPECT_EQ(0, rename(merge_fl1, merge_fl1_renamed));
+
+ /* Check that the rename worked. */
+ EXPECT_EQ(0, stat(merge_fl1_renamed, &st));
+ EXPECT_EQ(-1, stat(merge_fl1, &st));
+ EXPECT_EQ(ENOENT, errno);
+
+ /*
+ * Check that the whiteout object on the underlying "upper" filesystem
+ * exists after the rename. This is OK because it was done with the
+ * credentials of the OverlayFS.
+ */
+ EXPECT_EQ(0, stat(UPPER_DATA "/fl1", &st));
+ EXPECT_TRUE(S_ISCHR(st.st_mode));
+ EXPECT_EQ(0, st.st_rdev);
+}
FIXTURE(layout3_fs)
{
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply related
* [PATCH 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
In-Reply-To: <20260411090944.3131168-2-gnoack@google.com>
Add a test to check that renames with RENAME_WHITEOUT are guarded by
LANDLOCK_ACCESS_FS_MAKE_CHAR.
Signed-off-by: Günther Noack <gnoack@google.com>
---
tools/testing/selftests/landlock/fs_test.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index cdb47fc1fc0a..d867016e3fd3 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2247,6 +2247,19 @@ TEST_F_FORK(layout1, rename_file)
RENAME_EXCHANGE));
}
+TEST_F_FORK(layout1, rename_whiteout_denied)
+{
+ enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_CHAR, NULL);
+
+ /*
+ * Try to rename a file with RENAME_WHITEOUT.
+ * file1_s3d3 is in dir_s3d2 (tmpfs), so it supports RENAME_WHITEOUT.
+ */
+ EXPECT_EQ(-1, renameat2(AT_FDCWD, file1_s3d3, AT_FDCWD,
+ TMP_DIR "/s3d1/s3d2/s3d3/f2", RENAME_WHITEOUT));
+ EXPECT_EQ(EACCES, errno);
+}
+
TEST_F_FORK(layout1, rename_dir)
{
const struct rule rules[] = {
@@ -6949,6 +6962,7 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
}
}
+
FIXTURE(layout3_fs)
{
bool has_created_dir;
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply related
* [PATCH 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_CHAR for RENAME_WHITEOUT
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
In-Reply-To: <20260411090944.3131168-2-gnoack@google.com>
renameat2(2) with the RENAME_WHITEOUT flag places a whiteout character
device file in the source file location in place of the moved file,
bypassing the LANDLOCK_ACCESS_FS_MAKE_CHAR right.
Fix this by checking for LANDLOCK_ACCESS_FS_MAKE_CHAR if RENAME_WHITEOUT is
passed.
This does not affect normal renames within layered OverlayFS mounts: When
OverlayFS invokes rename with RENAME_WHITEOUT as part of a "normal" rename
operation, it does so in ovl_rename() using the credentials that were set
at the time of mounting the OverlayFS.
Suggested-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Günther Noack <gnoack@google.com>
---
security/landlock/fs.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c1ecfe239032..2b84a229e4d8 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1519,6 +1519,19 @@ static int hook_path_rename(const struct path *const old_dir,
const unsigned int flags)
{
/* old_dir refers to old_dentry->d_parent and new_dir->mnt */
+ if (flags & RENAME_WHITEOUT) {
+ int err;
+
+ /*
+ * This check would better be done together with other path
+ * walks which are already happening for the normal rename check
+ * in current_check_refer_path().
+ */
+ err = current_check_access_path(old_dir, LANDLOCK_ACCESS_FS_MAKE_CHAR);
+ if (err)
+ return err;
+ }
+
return current_check_refer_path(old_dentry, new_dir, new_dentry, true,
!!(flags & RENAME_EXCHANGE));
}
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply related
* [PATCH 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
Hello!
As discussed in [1], the renameat2() syscall's RENAME_WHITEOUT flag allows
the creation of chardev directory entries with major=minor=0 as "whiteout
objects" in the location of the rename source file [2].
This functionality is available even without having any OverlayFS mounted
and can be invoked with the regular renameat2(2) syscall [3].
Motivation
==========
The RENAME_WHITEOUT flag side-steps Landlock's LANDLOCK_ACCESS_FS_MAKE_CHAR
right, which is designed to restrict the creation of chardev device files.
This patch set fixes that by adding a check in Landlock's path_rename hook.
Tradeoffs considered in the implementation
==========================================
Q: Should we guard it with a dedicated LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
right?
Pros:
* This would be the fully backwards compatible solution,
and Linux always strives for full backward compatibility.
Cons:
* Complicates the Landlock API surface for a very minor use case.
In Debian Code search, the only use of RENAME_WHITEOUT from userspace
seems to be for fuse-overlayfs. It is used there for the same purpose
as in the kernel OverlayFS and it likely does not run in a Landlock
domain.
The tradeoff does not seem worth it to me. The chances that we break
anyone with this seem very low, and I'm inclined to treat it as a bugfix
for the existing LANDLOCK_ACCESS_FS_MAKE_CHAR right.
Q: Should we add a Landlock erratum for this?
I punted on it for now, but we can do it if needed.
Q: Should the access right check be merged into the longer
current_check_refer_path() function?
I am leaning towards keeping it as a special case earlier. This means
that we traverse the source path twice, but as we have seen in Debian
Code Search, there are apparently no legitimate callers of renameat2()
with RENAME_WHITEOUT who are calling this from within a Landlock domain.
(fuse-overlayfs is legitimate, but is not landlocked)
It doesn't seem worth complicating our common rename code for a corner
case that doesn't happen in practice.
[1] https://lore.kernel.org/all/adUBCQXrt7kmgqJT@google.com/
[2] https://docs.kernel.org/filesystems/overlayfs.html#whiteouts-and-opaque-directories
[3] https://man7.org/linux/man-pages/man2/renameat2.2.html#DESCRIPTION
[4] https://codesearch.debian.net/search?q=rename.*RENAME_WHITEOUT&literal=0
Günther Noack (3):
landlock: Require LANDLOCK_ACCESS_FS_MAKE_CHAR for RENAME_WHITEOUT
selftests/landlock: Add test for RENAME_WHITEOUT denial
selftests/landlock: Test OverlayFS renames w/o
LANDLOCK_ACCESS_FS_MAKE_CHAR
security/landlock/fs.c | 16 ++++++++
tools/testing/selftests/landlock/fs_test.c | 45 ++++++++++++++++++++++
2 files changed, 61 insertions(+)
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply
* Re: LSM: Whiteout chardev creation sidesteps mknod hook
From: Günther Noack @ 2026-04-11 8:36 UTC (permalink / raw)
To: Christian Brauner
Cc: Serge Hallyn, Miklos Szeredi, Amir Goldstein,
Mickaël Salaün, Paul Moore, linux-security-module
In-Reply-To: <20260409-entbrennen-turnschuh-54af9b45610e@brauner>
On Thu, Apr 09, 2026 at 02:47:16PM +0200, Christian Brauner wrote:
> On Tue, Apr 07, 2026 at 12:15:00PM -0500, Serge Hallyn wrote:
> > Apr 7, 2026 08:05:43 Günther Noack <gnoack@google.com>:
> >
> > > Hello Christian, Paul, Mickaël and LSM maintainers!
> > >
> > > I discovered the following bug in Landlock, which potentially also
> > > affects other LSMs:
> > >
> > > With renameat2(2)'s RENAME_WHITEOUT flag, it is possible to create a
> > > "whiteout object" at the source of the rename. Whiteout objects are
> > > character devices with major/minor (0, 0) -- these devices are not
> > > bound to any driver, so they are harmless, but still, the creation of
> > > these files can sidestep the LANDLOCK_ACCESS_FS_MAKE_CHAR access right
> > > in Landlock.
>
> They aren't devices.
The LANDLOCK_ACCESS_FS_MAKE_CHAR access right is about the *creation of
character device directory entries*.
These files do not hook up to any of the kernel's device driver subsystems, but
they *are* directory entries of the chardev type, and the creation of these is
still sidestepping the LANDLOCK_ACCESS_FS_MAKE_CHAR right.
> > > I am unconvinced which is the right fix here -- do you have an opinion
> > > on this from the VFS/LSM side?
> > >
> > >
> > > Option 1: Make filesystems call security_path_mknod() during RENAME_WHITEOUT?
>
> No.
>
> > >
> > > Do it in the VFS rename hook.
> > >
> > > * Pro: Fixes it for all LSMs
> > > * Con: Call would have to be done in multiple filesystems
> > >
> > >
> > > Option 2: Handle it in security_{path,inode}_rename()
> > >
> > > Make Landlock handle it in security_inode_rename() by looking for the
> > > RENAME_WHITEOUT flag.
> > >
> > > * Con: Operation should only be denied if the file system even
> > > implements RENAME_WHITEOUT, and we would have to maintain a list of
>
> Why? Just deny RENAME_WHITEOUT. What does it matter if the filesystem
> implements it or not. Overlayfs would fall back to non-RENAME_WHITEOUT
> if not provided by the upper fs anway.
I'll send a patch with this approach for discussion.
It turns out it is less difficult than I feared:
* OverlayFS uses its own credentials object, and since that is not under a
Landlock policy, the OverlayFS-internal vfs_rename() invocations do not have
that problem. (Under a Landlock policy, mount(2) is not permitted, so the
OverlayFS-internal credentials are not Landlocked.)
* The remaining use case is only when a user calls renameat2(...,
RENAME_WHITEOUT) directly on a filesystem (which is not necessarily part of an
OverlayFS). That case can be restricted with Landlock.
We might have slight error code inconsistencies yes, but as Mickaël is saying on
the sibling mail thread, it would not be worth the tradeoff to maintain a list
of supported file systems just to get the error codes right.
> > > affected filesystems for that. (That feels like solving it at the
> > > wrong layer of abstraction.)
> > > * Con: Unclear whether other LSMs need a similar fix
> > >
> > >
> > > Option 3: Declare that this is working as intended?
> >
> > Option 3 has my vote.
>
> Seconded.
(See also discussion on sibling thread)
I also don't currently see how an attacker would abuse this, but I still see
this as a violation of Landlock's security model if we can create a policy that
denies the creation of character device directory entries, and then we still
have a way to make them appear there where we previously had a different file.
I'll send a tentative patch for option 2 for discussion. We can discuss more in
the context of that more concrete proposal, if needed.
—Günther
^ permalink raw reply
* Re: LSM: Whiteout chardev creation sidesteps mknod hook
From: Günther Noack @ 2026-04-11 8:26 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Christian Brauner, Paul Moore, linux-security-module,
John Johansen, Georgia Garcia, Kentaro Takeda, Tetsuo Handa
In-Reply-To: <20260408.beu1Eing5aFo@digikod.net>
On Wed, Apr 08, 2026 at 01:01:28PM +0200, Mickaël Salaün wrote:
> On Tue, Apr 07, 2026 at 03:05:13PM +0200, Günther Noack wrote:
> > Hello Christian, Paul, Mickaël and LSM maintainers!
> >
> > I discovered the following bug in Landlock, which potentially also
> > affects other LSMs:
> >
> > With renameat2(2)'s RENAME_WHITEOUT flag, it is possible to create a
> > "whiteout object" at the source of the rename. Whiteout objects are
> > character devices with major/minor (0, 0) -- these devices are not
> > bound to any driver, so they are harmless, but still, the creation of
> > these files can sidestep the LANDLOCK_ACCESS_FS_MAKE_CHAR access right
> > in Landlock.
>
> Any way to "write" on the filesystem should properly be controlled. The
> man page says that RENAME_WHITEOUT requires CAP_MKNOD, however, looking
> at vfs_mknod(), there is an explicit exception to not check CAP_MKNOD
> for whiteout devices. See commit a3c751a50fe6 ("vfs: allow unprivileged
> whiteout creation").
Agreed, it should be possible to restrict it.
> > Option 2: Handle it in security_{path,inode}_rename()
> >
> > Make Landlock handle it in security_inode_rename() by looking for the
> > RENAME_WHITEOUT flag.
> >
> > * Con: Operation should only be denied if the file system even
> > implements RENAME_WHITEOUT, and we would have to maintain a list of
> > affected filesystems for that. (That feels like solving it at the
> > wrong layer of abstraction.)
>
> Why would we need to maintain such list? If it's only about the errno,
> well, that would not be perfect be ok with a proper doc.
Yes, it would be only about the errno. At the time of writing the initial mail,
I was also worried that OverlayFS would get confused if its internal
vfs_rename() call would sometimes work and sometimes be denied, but as it turns
out, since OverlayFS uses its own credentials internally, this is a non-issue.
I'll send a tentative patch for option 2 for discussion.
> I'm mostly worried that there might be other (future) call paths to
> create whiteout devices.
>
> I think option 2 would be the most practical approach for Landlock, with
> a new LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right.
Given that this only affect immediate renameat2() calls, I would actually argue
that we can probably get away with guarding this with the existing
LANDLOCK_ACCESS_FS_MAKE_CHAR right?
I checked Debian code search for usages:
https://codesearch.debian.net/search?q=rename.*RENAME_WHITEOUT&literal=0
Apart from the usual proliferation of copied-around kernel headers and wrapper
pass-through wrapper libraries around renameat2(), the only actual use I found
for the immediate renameat2() syscall with RENAME_WHITEOUT was in fuse-overlayfs
(for the exact same reason). Fuse-overlayfs is likely not running under a
Landlock policy given that it doesn't have Landlock support itself and given
that it also has to do a mount(), which is forbidden under Landlock, so users
are also unlikely to wrap it in a Landlock domain.
> I'm also wondering how are the chances that other kind of special file
> type like a whiteout device could come up in the future. Any guess
> Christian?
>
> > * Con: Unclear whether other LSMs need a similar fix
>
> I guess at least AppArmor and Tomoyo would consider that an issue.
>
> >
> >
> > Option 3: Declare that this is working as intended?
>
> We need to be able to controle any file creation, which is not currently
> the case because of this whiteout exception.
Seconded. Landlock offers a long list of access rights to restrict the creation
of new directory entries, and this is a way to create a new directory entry
anyway. Even though it's not immediately clear to me how this can be abused for
an actual attack, it is a violation of the previously defined Landlock policy if
directory entries can be created this way.
—Günther
^ permalink raw reply
* Re: [PATCH] security: remove BUG_ON in security_skb_classify_flow
From: Serge E. Hallyn @ 2026-04-10 23:34 UTC (permalink / raw)
To: Jiayuan Chen
Cc: Serge E. Hallyn, Stephen Smalley, linux-security-module, paul,
jmorris, linux-kernel, Kaiyan Mei, Yinhao Hu, Dongliang Mu
In-Reply-To: <a17199c6-fb52-493b-b76a-505faf27cfa0@linux.dev>
On Fri, Apr 10, 2026 at 09:56:22AM +0800, Jiayuan Chen wrote:
>
> On 4/10/26 8:58 AM, Serge E. Hallyn wrote:
> > On Wed, Apr 08, 2026 at 07:42:57PM +0800, Jiayuan Chen wrote:
> > > A BPF program attached to the xfrm_decode_session hook can return a
> > > non-zero value, which causes BUG_ON(rc) in security_skb_classify_flow()
> > > to trigger a kernel panic.
> > It would seem worth it to have pointed at the previous discussion at
> >
> > https://lore.kernel.org/all/CAEjxPJ5aA01in+Z1yLF1cwe-3uqL_E8SKGK4J294D5eRG5__5Q@mail.gmail.com/
> >
> > Based on that, I guess this is probably ok, but still,
> >
> > > Remove the BUG_ON and change the return type from void to int, so that
> > > callers can optionally handle the error.
> > but you don't have the existing callers handling the error. It's
> > conceivable they won't care, but it's also possible that they were
> > counting on a BUG_ON in that case.
> >
> > What *should* callers (icmp_reply, etc) do if an error code is
> > returned? Should they ignore it? In that case, would it be
> > better to change security_skb_classify_flow() to return void?
> >
> Thanks for your pointer.
>
> So I think Feng's patch is sufficient and can by applied ?
Well, selinux_xfrm_decode_session() calls selinux_xfrm_skb_sid_ingress()
which *can* return -EINVAL.
So I'd like to know, what is supposed to happen in that case?
Stephen, do you know? Is it safe for callers to ignore this?
^ 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