From: "Konstantin Meskhidze (A)" <konstantin.meskhidze@huawei.com>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: <willemdebruijn.kernel@gmail.com>,
<linux-security-module@vger.kernel.org>, <netdev@vger.kernel.org>,
<netfilter-devel@vger.kernel.org>, <yusongping@huawei.com>,
<anton.sirazetdinov@huawei.com>
Subject: Re: [PATCH v6 02/17] landlock: refactors landlock_find/insert_rule
Date: Fri, 8 Jul 2022 17:20:42 +0300 [thread overview]
Message-ID: <76c7c92e-3377-0bb8-14d5-e5c286c67dc3@huawei.com> (raw)
In-Reply-To: <72375435-94d4-e3aa-c27b-b44382dde6ad@digikod.net>
7/8/2022 4:56 PM, Mickaël Salaün пишет:
>
> On 08/07/2022 14:53, Konstantin Meskhidze (A) wrote:
>>
>>
>> 7/7/2022 7:44 PM, Mickaël Salaün пишет:
>>>
>>> On 21/06/2022 10:22, Konstantin Meskhidze wrote:
>>>> Adds a new object union to support a socket port
>>>> rule type. Refactors landlock_insert_rule() and
>>>> landlock_find_rule() to support coming network
>>>> modifications. Now adding or searching a rule
>>>> in a ruleset depends on a rule_type argument
>>>> provided in refactored functions mentioned above.
>>>>
>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>>> ---
>>>>
>>>> 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 | 7 ++-
>>>> security/landlock/ruleset.c | 105 ++++++++++++++++++++++++++----------
>>>> security/landlock/ruleset.h | 27 +++++-----
>>>> 3 files changed, 96 insertions(+), 43 deletions(-)
>>>>
>>>> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
>>>> index e6da08ed99d1..46aedc2a05a8 100644
>>>> --- a/security/landlock/fs.c
>>>> +++ b/security/landlock/fs.c
>>>> @@ -173,7 +173,8 @@ int landlock_append_fs_rule(struct
>>>> landlock_ruleset *const ruleset,
>>>> if (IS_ERR(object))
>>>> return PTR_ERR(object);
>>>> mutex_lock(&ruleset->lock);
>>>> - err = landlock_insert_rule(ruleset, object, access_rights);
>>>> + err = landlock_insert_rule(ruleset, object, 0, access_rights,
>>>> + LANDLOCK_RULE_PATH_BENEATH);
>>>> mutex_unlock(&ruleset->lock);
>>>> /*
>>>> * No need to check for an error because landlock_insert_rule()
>>>> @@ -204,7 +205,9 @@ find_rule(const struct landlock_ruleset *const
>>>> domain,
>>>> inode = d_backing_inode(dentry);
>>>> rcu_read_lock();
>>>> rule = landlock_find_rule(
>>>> - domain, rcu_dereference(landlock_inode(inode)->object));
>>>> + domain,
>>>> + (uintptr_t)rcu_dereference(landlock_inode(inode)->object),
>>>> + LANDLOCK_RULE_PATH_BENEATH);
>>>> rcu_read_unlock();
>>>> return rule;
>>>> }
>>>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>>>> index a3fd58d01f09..5f13f8a12aee 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
>>>> @@ -69,7 +69,8 @@ static void build_check_rule(void)
>>>> }
>>>>
>>>> static struct landlock_rule *
>>>> -create_rule(struct landlock_object *const object,
>>>> +create_rule(struct landlock_object *const object_ptr,
>>>> + const uintptr_t object_data,
>>>> const struct landlock_layer (*const layers)[], const u32
>>>> num_layers,
>>>> const struct landlock_layer *const new_layer)
>>>> {
>>>> @@ -90,8 +91,15 @@ 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 (object_ptr) {
>>>> + landlock_get_object(object_ptr);
>>>> + new_rule->object.ptr = object_ptr;
>>>> + } else if (object_ptr && object_data) {
>>>
>>> Something is wrong with this second check: else + object_ptr?
>>
>> It was your suggestion to use it like this:
>> " ....You can also add a WARN_ON_ONCE(object_ptr && object_data)."
>>
>> Please check it here:
>> https://lore.kernel.org/linux-security-module/bc44f11f-0eaa-a5f6-c5dc-1d36570f1be1@digikod.net/
>
> Yes, but the error is in the "else", you should write:
> if (WARN_ON_ONCE(object_ptr && object_data))
> return ERR_PTR(-EINVAL);
>
> …and this should be before the `if (object_ptr) {` line (to avoid
> erronous landlock_get_object() call), just after the `if (!new_rule)` check.
Maybe we could delete this check here cause we have it in the upper
insert_rule() function??
...
if (WARN_ON_ONCE(!layers))
return -ENOENT;
------> if (WARN_ON_ONCE(object_ptr && object_data))
return -EINVAL;
/* Chooses rb_tree structure depending on a rule type. */
switch (rule_type) {
case LANDLOCK_RULE_PATH_BENEATH:
if (WARN_ON_ONCE(!object_ptr))
return -ENOENT;
object_data = (uintptr_t)object_ptr;
root = &ruleset->root_inode;
break;
default:
WARN_ON_ONCE(1);
return -EINVAL;
}
...
This is double check here. What do you think?
> .
next prev parent reply other threads:[~2022-07-08 14:20 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-21 8:22 [PATCH v6 00/17] Network support for Landlock Konstantin Meskhidze
2022-06-21 8:22 ` [PATCH v6 01/17] landlock: renames access mask Konstantin Meskhidze
2022-07-01 17:08 ` Mickaël Salaün
2022-07-04 9:23 ` Konstantin Meskhidze (A)
2022-07-05 11:29 ` Konstantin Meskhidze (A)
2022-07-05 13:26 ` Mickaël Salaün
2022-07-08 12:56 ` Konstantin Meskhidze (A)
2022-06-21 8:22 ` [PATCH v6 02/17] landlock: refactors landlock_find/insert_rule Konstantin Meskhidze
2022-07-07 16:44 ` Mickaël Salaün
2022-07-08 12:53 ` Konstantin Meskhidze (A)
2022-07-08 13:56 ` Mickaël Salaün
2022-07-08 14:14 ` Konstantin Meskhidze (A)
2022-07-08 14:20 ` Konstantin Meskhidze (A) [this message]
2022-07-08 16:57 ` Mickaël Salaün
2022-07-11 8:16 ` Konstantin Meskhidze (A)
2022-07-08 13:10 ` Konstantin Meskhidze (A)
2022-07-08 13:59 ` Mickaël Salaün
2022-07-08 14:14 ` Konstantin Meskhidze (A)
2022-07-08 14:35 ` Mickaël Salaün
2022-07-11 14:05 ` Konstantin Meskhidze (A)
2022-07-28 14:48 ` Mickaël Salaün
2022-07-07 16:46 ` Mickaël Salaün
2022-07-08 12:54 ` Konstantin Meskhidze (A)
2022-06-21 8:22 ` [PATCH v6 03/17] landlock: refactors merge and inherit functions Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 04/17] landlock: moves helper functions Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 05/17] landlock: refactors " Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 06/17] landlock: refactors landlock_add_rule syscall Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 07/17] landlock: user space API network support Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 08/17] landlock: adds support network rules Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 09/17] landlock: implements TCP network hooks Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 10/17] seltests/landlock: moves helper function Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 11/17] seltests/landlock: adds tests for bind() hooks Konstantin Meskhidze
2022-07-28 13:24 ` Mickaël Salaün
2022-06-21 8:23 ` [PATCH v6 12/17] seltests/landlock: adds tests for connect() hooks Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 13/17] seltests/landlock: adds AF_UNSPEC family test Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 14/17] seltests/landlock: adds rules overlapping test Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 15/17] seltests/landlock: adds ruleset expanding test Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 16/17] seltests/landlock: adds invalid input data test Konstantin Meskhidze
2022-06-21 8:23 ` [PATCH v6 17/17] samples/landlock: adds network demo Konstantin Meskhidze
2022-07-27 20:26 ` Mickaël Salaün
2022-07-28 9:21 ` Konstantin Meskhidze (A)
2022-07-26 17:43 ` [PATCH v6 00/17] Network support for Landlock Mickaël Salaün
2022-07-27 19:54 ` Mickaël Salaün
2022-07-28 9:19 ` Konstantin Meskhidze (A)
2022-07-28 9:25 ` Konstantin Meskhidze (A)
2022-07-28 10:12 ` Mickaël Salaün
2022-07-28 11:27 ` Konstantin Meskhidze (A)
2022-07-28 13:17 ` Mickaël Salaün
2022-08-23 9:10 ` Konstantin Meskhidze (A)
2022-08-27 13:30 ` Konstantin Meskhidze (A)
2022-08-29 13:10 ` Mickaël Salaün
2022-07-27 20:21 ` Mickaël Salaün
2022-07-28 9:20 ` Konstantin Meskhidze (A)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=76c7c92e-3377-0bb8-14d5-e5c286c67dc3@huawei.com \
--to=konstantin.meskhidze@huawei.com \
--cc=anton.sirazetdinov@huawei.com \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@digikod.net \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=willemdebruijn.kernel@gmail.com \
--cc=yusongping@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).