Re: [PATCH v6 02/17] landlock: refactors landlock_find/insert_rule

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





7/8/2022 4:59 PM, Mickaël Salaün пишет:

On 08/07/2022 15:10, 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@xxxxxxxxxx>
---

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?

  Sorry. Do you mean logical error here? I got your point.
  You are right!

  I think it must be refactored like this:

     if (object_ptr && !object_data) {
         landlock_get_object(object_ptr);
         new_rule->object.ptr = object_ptr;
     } else if (object_ptr && object_data) {
         ...
     }

There is indeed a logical error but this doesn't fix everything. Please
include my previous suggestion instead.

   By the way, in the next commits I have fixed this logic error.
Anyway I will refactor this one also. Thanks.

Plus, I will add a test for this case.

That would be great but I don't think this code is reachable from user
space. I think that would require kunit but I may be missing something.
How would you test this?

You are correct. I checked it. It's impossible to reach this line from userpace (insert both object_ptr and object_data). But create_rule() must be used carefuly by other developers (if any in future). Do you think if its possible to have some internal kernel tests that could handle this issue?
.



[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux