Re: [PATCH v9 03/12] landlock: Refactor landlock_find_rule/insert_rule
From: Konstantin Meskhidze (A) <hidden>
Date: 2023-02-14 10:15:16
Also in:
netdev, netfilter-devel
2/10/2023 8:36 PM, Mickaël Salaün пишет:
On 16/01/2023 09:58, Konstantin Meskhidze wrote:quoted
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.
quoted
Signed-off-by: Mickaël Salaün <mic@digikod.net> Signed-off-by: Konstantin Meskhidze <redacted> --- 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[...]quoted
@@ -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?
quoted
find_rule(const struct landlock_ruleset *const domain, const struct dentry *const dentry) { const struct landlock_rule *rule; const struct inode *inode; + struct landlock_id id = { + .type = LANDLOCK_KEY_INODE, + }; /* Ignores nonexistent leafs. */ if (d_is_negative(dentry))[...]quoted
@@ -652,7 +657,7 @@ static inline int check_access_path(const struct landlock_ruleset *const domain, } static int current_check_access_path(const struct path *const path, - const access_mask_t access_request) + const access_mask_t access_request)This syntax fix should be moved to patch 2/12.
Ok. Got it.
[...]quoted
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index 1f3188b4e313..c5c88a100f74 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c[...]quoted
@@ -285,23 +333,23 @@ static int merge_ruleset(struct landlock_ruleset *const dst,quoted
- if (WARN_ON_ONCE(walker_rule->num_layers != 1)) { - err = -EINVAL; - goto out_unlock; - } - if (WARN_ON_ONCE(walker_rule->layers[0].level != 0)) { - err = -EINVAL; - goto out_unlock; - } + if (WARN_ON_ONCE(walker_rule->num_layers != 1)) + return -EINVAL; + if (WARN_ON_ONCE(walker_rule->layers[0].level != 0)) + return -EINVAL;This introduces two potential bugs. Why change this code?
My bad. These changes will appear in 4/12. Will be fixed. Thanks.
.