To be able to have useful traces, let's consolidate rule finding into
unmask checking. landlock_unmask_layers() now gets a landlock_rule_ref
instead of a rule pointer.
This enables us to not deal with Landlock rule pointers outside of
ruleset.c, to avoid two calls, and to get all required information
available to landlock_unmask_layers().
We could make struct landlock_rule private because it is now only used
in the ruleset.c file.
Cc: Günther Noack <gno...@google.com>
Signed-off-by: Mickaël Salaün <m...@digikod.net>
---
security/landlock/fs.c | 144 ++++++++++++++++++++++--------------
security/landlock/net.c | 6 +-
security/landlock/ruleset.c | 12 ++-
security/landlock/ruleset.h | 9 +--
4 files changed, 100 insertions(+), 71 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index f5087688190a..73a20a501c3c 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -356,30 +356,27 @@ int landlock_append_fs_rule(struct landlock_ruleset
*const ruleset,
/* 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.
+ * Returns true if an object is tied to @dentry, and updates @ref accordingly.
*/
-static const struct landlock_rule *
-find_rule(const struct landlock_ruleset *const domain,
- const struct dentry *const dentry)
+static bool find_rule_ref(const struct dentry *const dentry,
+ struct landlock_rule_ref *ref)
{
- const struct landlock_rule *rule;
const struct inode *inode;
- struct landlock_rule_ref ref = {
- .type = LANDLOCK_KEY_INODE,
- };
+
+ /*
+ * We do not strictly need an RCU read-side critical section if
+ * ref->key.object is not dereferenced or if a domain's rule own a
reference
+ * to it, but it is simpler and safer to always require one.
+ */
+ lockdep_assert_in_rcu_read_lock();
/* Ignores nonexistent leafs. */
- if (d_is_negative(dentry))
- return NULL;
+ if (!dentry || d_is_negative(dentry))
+ return false;
inode = d_backing_inode(dentry);
- rcu_read_lock();
- ref.key.object = rcu_dereference(landlock_inode(inode)->object);
- rule = landlock_find_rule(domain, ref);
- rcu_read_unlock();
- return rule;
+ ref->key.object = rcu_dereference(landlock_inode(inode)->object);
+ return true;
}
/*
@@ -809,25 +806,36 @@ static bool is_access_to_paths_allowed(
is_dom_check = false;
}
- if (unlikely(dentry_child1)) {
- landlock_unmask_layers(
- find_rule(domain, dentry_child1),
- landlock_init_layer_masks(
- domain, LANDLOCK_MASK_ACCESS_FS,
- &_layer_masks_child1, LANDLOCK_KEY_INODE),
- &_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1));
- layer_masks_child1 = &_layer_masks_child1;
- child1_is_directory = d_is_dir(dentry_child1);
- }
- if (unlikely(dentry_child2)) {
- landlock_unmask_layers(
- find_rule(domain, dentry_child2),
- landlock_init_layer_masks(
- domain, LANDLOCK_MASK_ACCESS_FS,
- &_layer_masks_child2, LANDLOCK_KEY_INODE),
- &_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2));
- layer_masks_child2 = &_layer_masks_child2;
- child2_is_directory = d_is_dir(dentry_child2);
+ scoped_guard(rcu)
+ {
+ struct landlock_rule_ref ref = {
+ .type = LANDLOCK_KEY_INODE,
+ };
+
+ if (unlikely(find_rule_ref(dentry_child1, &ref))) {
+ landlock_unmask_layers(domain, ref,
+ landlock_init_layer_masks(
+ domain,
+ LANDLOCK_MASK_ACCESS_FS,
+ &_layer_masks_child1,
+ LANDLOCK_KEY_INODE),
+ &_layer_masks_child1,
+ ARRAY_SIZE(_layer_masks_child1));
+ layer_masks_child1 = &_layer_masks_child1;
+ child1_is_directory = d_is_dir(dentry_child1);
+ }
+ if (unlikely(find_rule_ref(dentry_child2, &ref))) {
+ landlock_unmask_layers(domain, ref,
+ landlock_init_layer_masks(
+ domain,
+ LANDLOCK_MASK_ACCESS_FS,
+ &_layer_masks_child2,
+ LANDLOCK_KEY_INODE),
+ &_layer_masks_child2,
+ ARRAY_SIZE(_layer_masks_child2));
+ layer_masks_child2 = &_layer_masks_child2;
+ child2_is_directory = d_is_dir(dentry_child2);
+ }
}
walker_path = *path;
@@ -838,7 +846,6 @@ static bool is_access_to_paths_allowed(
*/
while (true) {
struct dentry *parent_dentry;
- const struct landlock_rule *rule;
/*
* If at least all accesses allowed on the destination are
@@ -880,17 +887,32 @@ static bool is_access_to_paths_allowed(
break;
}
- rule = find_rule(domain, walker_path.dentry);
- allowed_parent1 = allowed_parent1 ||
- landlock_unmask_layers(
- rule, access_masked_parent1,
- layer_masks_parent1,
- ARRAY_SIZE(*layer_masks_parent1));
- allowed_parent2 = allowed_parent2 ||
- landlock_unmask_layers(
- rule, access_masked_parent2,
- layer_masks_parent2,
- ARRAY_SIZE(*layer_masks_parent2));
+ scoped_guard(rcu)
+ {
+ struct landlock_rule_ref ref = {
+ .type = LANDLOCK_KEY_INODE,
+ };
+
+ if (find_rule_ref(walker_path.dentry, &ref)) {
+ allowed_parent1 =
+ allowed_parent1 ||
+ landlock_unmask_layers(
+ domain, ref,
+ access_masked_parent1,
+ layer_masks_parent1,
+ ARRAY_SIZE(
+ *layer_masks_parent1));
+
+ allowed_parent2 =
+ allowed_parent2 ||
+ landlock_unmask_layers(
+ domain, ref,
+ access_masked_parent2,
+ layer_masks_parent2,
+ ARRAY_SIZE(
+ *layer_masks_parent2));
+ }
+ }
/* Stops when a rule from each layer grants access. */
if (allowed_parent1 && allowed_parent2)
@@ -1050,15 +1072,23 @@ static bool collect_domain_accesses(
struct dentry *parent_dentry;
/* Gets all layers allowing all domain accesses. */
- if (landlock_unmask_layers(find_rule(domain, dir), access_dom,
- layer_masks_dom,
- ARRAY_SIZE(*layer_masks_dom))) {
- /*
- * Stops when all handled accesses are allowed by at
- * least one rule in each layer.
- */
- ret = true;
- break;
+ scoped_guard(rcu)
+ {
+ struct landlock_rule_ref ref = {
+ .type = LANDLOCK_KEY_INODE,
+ };
+
+ if (find_rule_ref(dir, &ref) &&
+ landlock_unmask_layers(
+ domain, ref, access_dom, layer_masks_dom,
+ ARRAY_SIZE(*layer_masks_dom))) {
+ /*
+ * Stops when all handled accesses are allowed
by at
+ * least one rule in each layer.
+ */
+ ret = true;
+ break;
+ }
}
/* We should not reach a root other than @mnt_root. */
diff --git a/security/landlock/net.c b/security/landlock/net.c
index cd7327b5f43e..44489760e8ec 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -48,7 +48,6 @@ static int current_check_access_socket(struct socket *const
sock,
{
__be16 port;
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
- const struct landlock_rule *rule;
struct landlock_rule_ref ref = {
.type = LANDLOCK_KEY_NET_PORT,
};
@@ -174,12 +173,11 @@ static int current_check_access_socket(struct socket
*const sock,
ref.key.data = (__force uintptr_t)port;
BUILD_BUG_ON(sizeof(port) > sizeof(ref.key.data));
- rule = landlock_find_rule(subject->domain, ref);
access_request = landlock_init_layer_masks(subject->domain,
access_request, &layer_masks,
LANDLOCK_KEY_NET_PORT);
- if (landlock_unmask_layers(rule, access_request, &layer_masks,
- ARRAY_SIZE(layer_masks)))
+ if (landlock_unmask_layers(subject->domain, ref, access_request,
+ &layer_masks, ARRAY_SIZE(layer_masks)))
return 0;
audit_net.family = address->sa_family;
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 647ee570475b..20a4bbb2526f 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -579,9 +579,9 @@ landlock_merge_ruleset(struct landlock_ruleset *const
parent,
/*
* The returned access has the same lifetime as @ruleset.
*/
-const struct landlock_rule *
-landlock_find_rule(const struct landlock_ruleset *const ruleset,
- const struct landlock_rule_ref ref)
+static const struct landlock_rule *
+find_rule(const struct landlock_ruleset *const ruleset,
+ const struct landlock_rule_ref ref)
{
const struct rb_root *root;
const struct rb_node *node;
@@ -613,15 +613,19 @@ landlock_find_rule(const struct landlock_ruleset *const
ruleset,
* Returns true if the request is allowed (i.e. relevant layer masks for the
* request are empty).
*/
-bool landlock_unmask_layers(const struct landlock_rule *const rule,
+bool landlock_unmask_layers(const struct landlock_ruleset *const domain,
+ const struct landlock_rule_ref ref,
const access_mask_t access_request,
layer_mask_t (*const layer_masks)[],
const size_t masks_array_size)
{
size_t layer_level;
+ const struct landlock_rule *rule;
if (!access_request || !layer_masks)
return true;
+
+ rule = find_rule(domain, ref);
if (!rule)
return false;
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 967d0123cb73..9f25dbd3022a 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -44,7 +44,7 @@ union landlock_key {
/**
* @object: Pointer to identify a kernel object (e.g. an inode).
*/
- struct landlock_object *object;
+ struct landlock_object __rcu *object;
/**
* @data: Raw data to identify an arbitrary 32-bit value
* (e.g. a TCP port).
@@ -208,10 +208,6 @@ struct landlock_ruleset *
landlock_merge_ruleset(struct landlock_ruleset *const parent,
struct landlock_ruleset *const ruleset);
-const struct landlock_rule *
-landlock_find_rule(const struct landlock_ruleset *const ruleset,
- const struct landlock_rule_ref ref);
-
static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
{
if (ruleset)
@@ -301,7 +297,8 @@ landlock_get_scope_mask(const struct landlock_ruleset
*const ruleset,
return ruleset->access_masks[layer_level].scope;
}
-bool landlock_unmask_layers(const struct landlock_rule *const rule,
+bool landlock_unmask_layers(const struct landlock_ruleset *const domain,
+ const struct landlock_rule_ref ref,
const access_mask_t access_request,
layer_mask_t (*const layer_masks)[],
const size_t masks_array_size);
--
2.49.0
