netdev - Re: [PATCH v11 03/12] landlock: Refactor landlock_find_rule/insert

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: Sat, 1 Jul 2023 17:37:08 +0300
From: "Konstantin Meskhidze (A)" <konstantin.meskhidze@...wei.com>
To: Mickaël Salaün <mic@...ikod.net>
CC: <willemdebruijn.kernel@...il.com>, <gnoack3000@...il.com>,
	<linux-security-module@...r.kernel.org>, <netdev@...r.kernel.org>,
	<netfilter-devel@...r.kernel.org>, <yusongping@...wei.com>,
	<artem.kuzin@...wei.com>
Subject: Re: [PATCH v11 03/12] landlock: Refactor
 landlock_find_rule/insert_rule



6/26/2023 9:40 PM, 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@...ikod.net>
>> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
>> ---
>> 
>> Changes since v10:
>> * None.
>> 
>> Changes since v9:
>> * Splits commit.
>> * Refactors commit message.
>> * Minor fixes.
>> 
>> Changes since v8:
>> * Refactors commit message.
>> * Removes inlining.
>> * Minor fixes.
>> 
>> Changes since v7:
>> * Completes all the new field descriptions landlock_key,
>>    landlock_key_type, landlock_id.
>> * Refactors commit message, adds a co-developer.
>> 
>> 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 | 134 ++++++++++++++++++++++++++----------
>>   security/landlock/ruleset.h |  65 ++++++++++++++---
>>   3 files changed, 166 insertions(+), 54 deletions(-)
> 
> [...]
> 
>> 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
> 
> [...]
> 
>> @@ -316,21 +368,29 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
>>   			   struct landlock_ruleset *const child)
>>   {
>>   	struct landlock_rule *walker_rule, *next_rule;
>> +	struct rb_root *parent_root;
>>   	int err = 0;
>> 
>>   	might_sleep();
>>   	if (!parent)
>>   		return 0;
>> 
>> +	parent_root = get_root(parent, LANDLOCK_KEY_INODE);
>> +	if (IS_ERR(parent_root))
>> +		return PTR_ERR(parent_root);
>> +
>>   	/* Locks @child first because we are its only owner. */
>>   	mutex_lock(&child->lock);
>>   	mutex_lock_nested(&parent->lock, SINGLE_DEPTH_NESTING);
>> 
>>   	/* Copies the @parent tree. */
>>   	rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
>> -					     &parent->root, node) {
>> -		err = insert_rule(child, walker_rule->object,
>> -				  &walker_rule->layers,
>> +					     parent_root, node) {
>> +		const struct landlock_id id = {
>> +			.key = walker_rule->key,
>> +			.type = LANDLOCK_KEY_INODE,
>> +		};
> 
> Please add a line break here instead of in a the following refactoring
> commit.

   Ok. Will be addded.
> 
> 
>> +		err = insert_rule(child, id, &walker_rule->layers,
>>   				  walker_rule->num_layers);
>>   		if (err)
>>   			goto out_unlock;
> .