[PATCH v11 03/12] landlock: Refactor landlock_find_rule/insert_rule

[Mickaël Salaün]

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 at digikod.net>
> Signed-off-by: Mickaël Salaün <mic at digikod.net>
> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze at huawei.com>
> ---

[...]

> 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;
}

> +}
> +
>   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)