netdev - Re: [PATCH v9 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]

Message-ID: <1d9afc6c-30fc-fc70-5ceb-85110cb57a5f@digikod.net>
Date:   Tue, 14 Feb 2023 13:09:02 +0100
From:   Mickaël Salaün <mic@...ikod.net>
To:     "Konstantin Meskhidze (A)" <konstantin.meskhidze@...wei.com>
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 v9 03/12] landlock: Refactor
 landlock_find_rule/insert_rule


On 14/02/2023 11:15, Konstantin Meskhidze (A) wrote:
> 
> 
> 2/10/2023 8:36 PM, Mickaël Salaün пишет:
>>
>> On 16/01/2023 09:58, 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.
>>>
>>> Remove unnecessary inlining.
>>>
>>
>> You need to keep the Co-developed-by before the Signed-off-by for my entry.
> 
>     Got it.
>>
>>> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
>>> ---
>>>
>>> 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      |  49 ++++++------
>>>    security/landlock/ruleset.c | 148 +++++++++++++++++++++++++-----------
>>>    security/landlock/ruleset.h |  65 +++++++++++++---
>>>    3 files changed, 185 insertions(+), 77 deletions(-)
>>>
>>> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
>>> index 0ae54a639e16..273ed8549da1 100644
>>> --- a/security/landlock/fs.c
>>> +++ b/security/landlock/fs.c
>>
>> [...]
>>
>>> @@ -191,12 +193,15 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>>>     *
>>>     * Returns NULL if no rule is found or if @dentry is negative.
>>>     */
>>> -static inline const struct landlock_rule *
>>> +static const struct landlock_rule *
>>
>> Can you please create a (previous) dedicated patch for all the inlining
>> changes?
>>   
>     a patch with just inlining changes?

Yes, a new patch with just the inlining changes extracted from this patch.