Re: [PATCH v11 03/12] landlock: Refactor landlock_find_rule/insert_rule
From: Mickaël Salaün <mic@digikod.net>
Date: 2023-07-06 14:35:03
Also in:
netdev, netfilter-devel
On 15/05/2023 18:13, Konstantin Meskhidze wrote:
Add a new landlock_key union and landlock_id structure to support a socket port rule type. A struct landlock_id identifies a unique entry in a ruleset: either a kernel object (e.g inode) or typed data (e.g TCP port). There is one red-black tree per key type. This patch also adds is_object_pointer() and get_root() helpers. is_object_pointer() returns true if key type is LANDLOCK_KEY_INODE. get_root() helper returns a red_black tree root pointer according to a key type. Refactor landlock_insert_rule() and landlock_find_rule() to support coming network modifications. Adding or searching a rule in ruleset can now be done thanks to a Landlock ID argument passed to these helpers. Co-developed-by: Mickaël Salaün <mic@digikod.net> Signed-off-by: Mickaël Salaün <mic@digikod.net> Signed-off-by: Konstantin Meskhidze <redacted> ---
[...]
quoted hunk ↗ jump to hunk
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index 1f3188b4e313..deab37838f5b 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c@@ -35,7 +35,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) return ERR_PTR(-ENOMEM); refcount_set(&new_ruleset->usage, 1); mutex_init(&new_ruleset->lock); - new_ruleset->root = RB_ROOT; + new_ruleset->root_inode = RB_ROOT; new_ruleset->num_layers = num_layers; /* * hierarchy = NULL@@ -68,8 +68,18 @@ static void build_check_rule(void) BUILD_BUG_ON(rule.num_layers < LANDLOCK_MAX_NUM_LAYERS); } +static bool is_object_pointer(const enum landlock_key_type key_type) +{ + switch (key_type) { + case LANDLOCK_KEY_INODE: + return true;
+ }
Because of enum change [1], could you please put the following block inside this commit's switch with a new "default:" case, and add a line break after the previous return like this: \n default:
+ WARN_ON_ONCE(1); + return false;
break; }
quoted hunk ↗ jump to hunk
+} + static struct landlock_rule * -create_rule(struct landlock_object *const object, +create_rule(const struct landlock_id id, const struct landlock_layer (*const layers)[], const u32 num_layers, const struct landlock_layer *const new_layer) {@@ -90,8 +100,13 @@ create_rule(struct landlock_object *const object, if (!new_rule) return ERR_PTR(-ENOMEM); RB_CLEAR_NODE(&new_rule->node); - landlock_get_object(object); - new_rule->object = object; + if (is_object_pointer(id.type)) { + /* This should be catched by insert_rule(). */ + WARN_ON_ONCE(!id.key.object); + landlock_get_object(id.key.object); + } + + new_rule->key = id.key; new_rule->num_layers = new_num_layers; /* Copies the original layer stack. */ memcpy(new_rule->layers, layers,@@ -102,12 +117,29 @@ create_rule(struct landlock_object *const object, return new_rule; } -static void free_rule(struct landlock_rule *const rule) +static struct rb_root *get_root(struct landlock_ruleset *const ruleset, + const enum landlock_key_type key_type) +{
Same here, you can remove the "root" variable:
+ struct rb_root *root = NULL;
+
+ switch (key_type) {
+ case LANDLOCK_KEY_INODE:
+ root = &ruleset->root_inode;
+ break;return &ruleset->root_inode; \n default:
+ if (WARN_ON_ONCE(!root)) + return ERR_PTR(-EINVAL);
break; }
+}
Actually, I've pushed this change here: https://git.kernel.org/mic/c/8c96c7eee3ff (landlock-net-v11 branch)