Re: [PATCH v7 02/18] landlock: refactor landlock_find_rule/insert_rule

["Mickaël Salaün" <mic@digikod.net>]

Good to see such clean commit!

On 29/08/2022 19:03, Konstantin Meskhidze wrote:
> Adds a new landlock_key union and landlock_id structure to support
> a socket port rule type. Refactors landlock_insert_rule() and
> landlock_find_rule() to support coming network modifications.

> This patch also adds is_object_pointer() and get_root() helpers.

Please explain a bit what these helpers do.


> Now adding or searching a rule in a ruleset depends on a landlock id
> argument provided in refactored functions mentioned above.

More explanation:
A struct landlock_id identifies a unique entry in a ruleset: either a 
kernel object (e.g inode) or a typed data (e.g. TCP port). There is one 
red-black tree per key type.

> 
> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>

Because most changes come from 
https://git.kernel.org/mic/c/8f4104b3dc59e7f110c9b83cdf034d010a2d006f 
and 
https://git.kernel.org/mic/c/7d6cf40a6f81adf607ad3cc17aaa11e256beeea4 
you can append
Co-developed-by: Mickaël Salaün <mic@digikod.net>

> ---
> 
> Changes since v6:
> * Adds union landlock_key, enum landlock_key_type, and struct
>    landlock_id.
> * Refactors ruleset functions and improves switch/cases: create_rule(),
>    insert_rule(), get_root(), is_object_pointer(), free_rule(),
>    landlock_find_rule().
> * Refactors landlock_append_fs_rule() functions to support new
>    landlock_id type.
> 
> Changes since v5:
> * Formats code with clang-format-14.
> 
> Changes since v4:
> * Refactors insert_rule() and create_rule() functions by deleting
> rule_type from their arguments list, it helps to reduce useless code.
> 
> Changes since v3:
> * Splits commit.
> * Refactors landlock_insert_rule and landlock_find_rule functions.
> * Rename new_ruleset->root_inode.
> 
> ---
>   security/landlock/fs.c      |  21 ++++--
>   security/landlock/ruleset.c | 146 +++++++++++++++++++++++++-----------
>   security/landlock/ruleset.h |  51 ++++++++++---
>   3 files changed, 156 insertions(+), 62 deletions(-)

[...]

> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 647d44284080..bb1408cc8dd2 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -49,6 +49,33 @@ struct landlock_layer {
>   	access_mask_t access;
>   };
> 
> +/**
> + * union landlock_key - Key of a ruleset's red-black tree
> + */
> +union landlock_key {
> +	struct landlock_object *object;
> +	uintptr_t data;
> +};
> +
> +/**
> + * enum landlock_key_type - Type of &union landlock_key
> + */
> +enum landlock_key_type {
> +	/**
> +	 * @LANDLOCK_KEY_INODE: Type of &landlock_ruleset.root_inode's node
> +	 * keys.
> +	 */
> +	LANDLOCK_KEY_INODE = 1,
> +};
> +
> +/**
> + * struct landlock_id - Unique rule identifier for a ruleset
> + */
> +struct landlock_id {
> +	union landlock_key key;
> +	const enum landlock_key_type type;
> +};

You can add these new types to Documentation/security/landlock.rst (with 
this commit). You need to complete all the new field descriptions though 
(otherwise you'll get Sphinx warnings): object, data, key, type.