netdev - Re: [PATCH v5 03/15] landlock: merge and inherit function refactoring

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 May 2022 10:14:59 +0200
From:   Mickaël Salaün <mic@...ikod.net>
To:     Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
Cc:     willemdebruijn.kernel@...il.com,
        linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
        netfilter-devel@...r.kernel.org, yusongping@...wei.com,
        anton.sirazetdinov@...wei.com
Subject: Re: [PATCH v5 03/15] landlock: merge and inherit function refactoring



On 16/05/2022 17:20, Konstantin Meskhidze wrote:
> Merge_ruleset() and inherit_ruleset() functions were
> refactored to support new rule types. This patch adds
> tree_merge() and tree_copy() helpers. Each has
> rule_type argument to choose a particular rb_tree
> structure in a ruleset.
> 
> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
> ---
> 
> Changes since v3:
> * Split commit.
> * Refactoring functions:
> 	-insert_rule.
> 	-merge_ruleset.
> 	-tree_merge.
> 	-inherit_ruleset.
> 	-tree_copy.
> 	-free_rule.
> 
> Changes since v4:
> * None
> 
> ---
>   security/landlock/ruleset.c | 144 ++++++++++++++++++++++++------------
>   1 file changed, 98 insertions(+), 46 deletions(-)
> 
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index f079a2a320f1..4b4c9953bb32 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -112,12 +112,16 @@ static struct landlock_rule *create_rule(
>   	return new_rule;
>   }
> 
> -static void free_rule(struct landlock_rule *const rule)
> +static void free_rule(struct landlock_rule *const rule, const u16 rule_type)
>   {
>   	might_sleep();
>   	if (!rule)
>   		return;
> -	landlock_put_object(rule->object.ptr);
> +	switch (rule_type) {
> +	case LANDLOCK_RULE_PATH_BENEATH:
> +		landlock_put_object(rule->object.ptr);
> +		break;
> +	}
>   	kfree(rule);
>   }
> 
> @@ -227,12 +231,12 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
>   			new_rule = create_rule(object_ptr, 0, &this->layers,
>   					       this->num_layers,
>   					       &(*layers)[0]);
> +			if (IS_ERR(new_rule))
> +				return PTR_ERR(new_rule);
> +			rb_replace_node(&this->node, &new_rule->node, &ruleset->root_inode);
> +			free_rule(this, rule_type);
>   			break;
>   		}
> -		if (IS_ERR(new_rule))
> -			return PTR_ERR(new_rule);
> -		rb_replace_node(&this->node, &new_rule->node, &ruleset->root_inode);
> -		free_rule(this);
>   		return 0;
>   	}
> 
> @@ -243,13 +247,12 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
>   	switch (rule_type) {
>   	case LANDLOCK_RULE_PATH_BENEATH:
>   		new_rule = create_rule(object_ptr, 0, layers, num_layers, NULL);
> +		if (IS_ERR(new_rule))
> +			return PTR_ERR(new_rule);
> +		rb_link_node(&new_rule->node, parent_node, walker_node);
> +		rb_insert_color(&new_rule->node, &ruleset->root_inode);
>   		break;
>   	}
> -	if (IS_ERR(new_rule))
> -		return PTR_ERR(new_rule);
> -	rb_link_node(&new_rule->node, parent_node, walker_node);
> -	rb_insert_color(&new_rule->node, &ruleset->root_inode);
> -	ruleset->num_rules++;

Why removing this last line?