]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
landlock: Refactor landlock_find_rule/insert_rule helpers
authorKonstantin Meskhidze <konstantin.meskhidze@huawei.com>
Thu, 26 Oct 2023 01:47:42 +0000 (09:47 +0800)
committerMickaël Salaün <mic@digikod.net>
Thu, 26 Oct 2023 19:07:11 +0000 (21:07 +0200)
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.

Add 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.

Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
Link: https://lore.kernel.org/r/20231026014751.414649-4-konstantin.meskhidze@huawei.com
[mic: Fix commit message typo]
Co-developed-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
security/landlock/fs.c
security/landlock/ruleset.c
security/landlock/ruleset.h

index e8d680c2c80af74d51b7b4b3b0a20cceae345721..9edb64ac8251d585331ea1546b4c3a2a412856c4 100644 (file)
@@ -158,7 +158,9 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
                            access_mask_t access_rights)
 {
        int err;
-       struct landlock_object *object;
+       struct landlock_id id = {
+               .type = LANDLOCK_KEY_INODE,
+       };
 
        /* Files only get access rights that make sense. */
        if (!d_is_dir(path->dentry) &&
@@ -170,17 +172,17 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
        /* Transforms relative access rights to absolute ones. */
        access_rights |= LANDLOCK_MASK_ACCESS_FS &
                         ~landlock_get_fs_access_mask(ruleset, 0);
-       object = get_inode_object(d_backing_inode(path->dentry));
-       if (IS_ERR(object))
-               return PTR_ERR(object);
+       id.key.object = get_inode_object(d_backing_inode(path->dentry));
+       if (IS_ERR(id.key.object))
+               return PTR_ERR(id.key.object);
        mutex_lock(&ruleset->lock);
-       err = landlock_insert_rule(ruleset, object, access_rights);
+       err = landlock_insert_rule(ruleset, id, access_rights);
        mutex_unlock(&ruleset->lock);
        /*
         * No need to check for an error because landlock_insert_rule()
         * increments the refcount for the new object if needed.
         */
-       landlock_put_object(object);
+       landlock_put_object(id.key.object);
        return err;
 }
 
@@ -197,6 +199,9 @@ find_rule(const struct landlock_ruleset *const domain,
 {
        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))
@@ -204,8 +209,8 @@ 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));
+       id.key.object = rcu_dereference(landlock_inode(inode)->object);
+       rule = landlock_find_rule(domain, id);
        rcu_read_unlock();
        return rule;
 }
index 1f3188b4e313c5ee7d0f57ed47fab2f0798b40ec..3553632e76d9344bb24a05510132555cad7f3b52 100644 (file)
@@ -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
@@ -68,8 +68,20 @@ static void build_check_rule(void)
        BUILD_BUG_ON(rule.num_layers < LANDLOCK_MAX_NUM_LAYERS);
 }
 
+static bool is_object_pointer(const enum landlock_key_type key_type)
+{
+       switch (key_type) {
+       case LANDLOCK_KEY_INODE:
+               return true;
+
+       default:
+               WARN_ON_ONCE(1);
+               return false;
+       }
+}
+
 static struct landlock_rule *
-create_rule(struct landlock_object *const object,
+create_rule(const struct landlock_id id,
            const struct landlock_layer (*const layers)[], const u32 num_layers,
            const struct landlock_layer *const new_layer)
 {
@@ -90,8 +102,13 @@ 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 (is_object_pointer(id.type)) {
+               /* This should be catched by insert_rule(). */
+               WARN_ON_ONCE(!id.key.object);
+               landlock_get_object(id.key.object);
+       }
+
+       new_rule->key = id.key;
        new_rule->num_layers = new_num_layers;
        /* Copies the original layer stack. */
        memcpy(new_rule->layers, layers,
@@ -102,12 +119,27 @@ create_rule(struct landlock_object *const object,
        return new_rule;
 }
 
-static void free_rule(struct landlock_rule *const rule)
+static struct rb_root *get_root(struct landlock_ruleset *const ruleset,
+                               const enum landlock_key_type key_type)
+{
+       switch (key_type) {
+       case LANDLOCK_KEY_INODE:
+               return &ruleset->root_inode;
+
+       default:
+               WARN_ON_ONCE(1);
+               return ERR_PTR(-EINVAL);
+       }
+}
+
+static void free_rule(struct landlock_rule *const rule,
+                     const enum landlock_key_type key_type)
 {
        might_sleep();
        if (!rule)
                return;
-       landlock_put_object(rule->object);
+       if (is_object_pointer(key_type))
+               landlock_put_object(rule->key.object);
        kfree(rule);
 }
 
@@ -129,8 +161,8 @@ static void build_check_ruleset(void)
  * insert_rule - Create and insert a rule in a ruleset
  *
  * @ruleset: The ruleset to be updated.
- * @object: The object to build the new rule with.  The underlying kernel
- *          object must be held by the caller.
+ * @id: The ID to build the new rule with.  The underlying kernel object, if
+ *      any, must be held by the caller.
  * @layers: One or multiple layers to be copied into the new rule.
  * @num_layers: The number of @layers entries.
  *
@@ -144,26 +176,35 @@ static void build_check_ruleset(void)
  * access rights.
  */
 static int insert_rule(struct landlock_ruleset *const ruleset,
-                      struct landlock_object *const object,
+                      const struct landlock_id id,
                       const struct landlock_layer (*const layers)[],
-                      size_t num_layers)
+                      const size_t num_layers)
 {
        struct rb_node **walker_node;
        struct rb_node *parent_node = NULL;
        struct landlock_rule *new_rule;
+       struct rb_root *root;
 
        might_sleep();
        lockdep_assert_held(&ruleset->lock);
-       if (WARN_ON_ONCE(!object || !layers))
+       if (WARN_ON_ONCE(!layers))
+               return -ENOENT;
+
+       if (is_object_pointer(id.type) && WARN_ON_ONCE(!id.key.object))
                return -ENOENT;
-       walker_node = &(ruleset->root.rb_node);
+
+       root = get_root(ruleset, id.type);
+       if (IS_ERR(root))
+               return PTR_ERR(root);
+
+       walker_node = &root->rb_node;
        while (*walker_node) {
                struct landlock_rule *const this =
                        rb_entry(*walker_node, struct landlock_rule, node);
 
-               if (this->object != object) {
+               if (this->key.data != id.key.data) {
                        parent_node = *walker_node;
-                       if (this->object < object)
+                       if (this->key.data < id.key.data)
                                walker_node = &((*walker_node)->rb_right);
                        else
                                walker_node = &((*walker_node)->rb_left);
@@ -195,24 +236,24 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
                 * Intersects access rights when it is a merge between a
                 * ruleset and a domain.
                 */
-               new_rule = create_rule(object, &this->layers, this->num_layers,
+               new_rule = create_rule(id, &this->layers, this->num_layers,
                                       &(*layers)[0]);
                if (IS_ERR(new_rule))
                        return PTR_ERR(new_rule);
-               rb_replace_node(&this->node, &new_rule->node, &ruleset->root);
-               free_rule(this);
+               rb_replace_node(&this->node, &new_rule->node, root);
+               free_rule(this, id.type);
                return 0;
        }
 
-       /* There is no match for @object. */
+       /* There is no match for @id. */
        build_check_ruleset();
        if (ruleset->num_rules >= LANDLOCK_MAX_NUM_RULES)
                return -E2BIG;
-       new_rule = create_rule(object, layers, num_layers, NULL);
+       new_rule = create_rule(id, layers, num_layers, NULL);
        if (IS_ERR(new_rule))
                return PTR_ERR(new_rule);
        rb_link_node(&new_rule->node, parent_node, walker_node);
-       rb_insert_color(&new_rule->node, &ruleset->root);
+       rb_insert_color(&new_rule->node, root);
        ruleset->num_rules++;
        return 0;
 }
@@ -230,7 +271,7 @@ static void build_check_layer(void)
 
 /* @ruleset must be locked by the caller. */
 int landlock_insert_rule(struct landlock_ruleset *const ruleset,
-                        struct landlock_object *const object,
+                        const struct landlock_id id,
                         const access_mask_t access)
 {
        struct landlock_layer layers[] = { {
@@ -240,7 +281,7 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
        } };
 
        build_check_layer();
-       return insert_rule(ruleset, object, &layers, ARRAY_SIZE(layers));
+       return insert_rule(ruleset, id, &layers, ARRAY_SIZE(layers));
 }
 
 static inline void get_hierarchy(struct landlock_hierarchy *const hierarchy)
@@ -263,6 +304,7 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
                         struct landlock_ruleset *const src)
 {
        struct landlock_rule *walker_rule, *next_rule;
+       struct rb_root *src_root;
        int err = 0;
 
        might_sleep();
@@ -273,6 +315,10 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
        if (WARN_ON_ONCE(!dst || !dst->hierarchy))
                return -EINVAL;
 
+       src_root = get_root(src, LANDLOCK_KEY_INODE);
+       if (IS_ERR(src_root))
+               return PTR_ERR(src_root);
+
        /* Locks @dst first because we are its only owner. */
        mutex_lock(&dst->lock);
        mutex_lock_nested(&src->lock, SINGLE_DEPTH_NESTING);
@@ -285,11 +331,15 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
        dst->access_masks[dst->num_layers - 1] = src->access_masks[0];
 
        /* Merges the @src tree. */
-       rbtree_postorder_for_each_entry_safe(walker_rule, next_rule, &src->root,
+       rbtree_postorder_for_each_entry_safe(walker_rule, next_rule, src_root,
                                             node) {
                struct landlock_layer layers[] = { {
                        .level = dst->num_layers,
                } };
+               const struct landlock_id id = {
+                       .key = walker_rule->key,
+                       .type = LANDLOCK_KEY_INODE,
+               };
 
                if (WARN_ON_ONCE(walker_rule->num_layers != 1)) {
                        err = -EINVAL;
@@ -300,8 +350,8 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
                        goto out_unlock;
                }
                layers[0].access = walker_rule->layers[0].access;
-               err = insert_rule(dst, walker_rule->object, &layers,
-                                 ARRAY_SIZE(layers));
+
+               err = insert_rule(dst, id, &layers, ARRAY_SIZE(layers));
                if (err)
                        goto out_unlock;
        }
@@ -316,21 +366,30 @@ 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,
+               };
+
+               err = insert_rule(child, id, &walker_rule->layers,
                                  walker_rule->num_layers);
                if (err)
                        goto out_unlock;
@@ -362,8 +421,9 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
        struct landlock_rule *freeme, *next;
 
        might_sleep();
-       rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root, node)
-               free_rule(freeme);
+       rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode,
+                                            node)
+               free_rule(freeme, LANDLOCK_KEY_INODE);
        put_hierarchy(ruleset->hierarchy);
        kfree(ruleset);
 }
@@ -454,20 +514,23 @@ out_put_dom:
  */
 const struct landlock_rule *
 landlock_find_rule(const struct landlock_ruleset *const ruleset,
-                  const struct landlock_object *const object)
+                  const struct landlock_id id)
 {
+       const struct rb_root *root;
        const struct rb_node *node;
 
-       if (!object)
+       root = get_root((struct landlock_ruleset *)ruleset, id.type);
+       if (IS_ERR(root))
                return NULL;
-       node = ruleset->root.rb_node;
+       node = root->rb_node;
+
        while (node) {
                struct landlock_rule *this =
                        rb_entry(node, struct landlock_rule, node);
 
-               if (this->object == object)
+               if (this->key.data == id.key.data)
                        return this;
-               if (this->object < object)
+               if (this->key.data < id.key.data)
                        node = node->rb_right;
                else
                        node = node->rb_left;
index 6e2ad1546ab30be77edc883dde1ce19da742ae76..9e04c666b23c596295373114d8f4d7762cc6ff42 100644 (file)
@@ -60,6 +60,47 @@ struct landlock_layer {
        access_mask_t access;
 };
 
+/**
+ * union landlock_key - Key of a ruleset's red-black tree
+ */
+union landlock_key {
+       /**
+        * @object: Pointer to identify a kernel object (e.g. an inode).
+        */
+       struct landlock_object *object;
+       /**
+        * @data: Raw data to identify an arbitrary 32-bit value
+        * (e.g. a TCP port).
+        */
+       uintptr_t data;
+};
+
+/**
+ * enum landlock_key_type - Type of &union landlock_key
+ */
+enum landlock_key_type {
+       /**
+        * @LANDLOCK_KEY_INODE: Type of &landlock_ruleset.root_inode's node
+        * keys.
+        */
+       LANDLOCK_KEY_INODE = 1,
+};
+
+/**
+ * struct landlock_id - Unique rule identifier for a ruleset
+ */
+struct landlock_id {
+       /**
+        * @key: Identifies either a kernel object (e.g. an inode) or
+        * a raw value (e.g. a TCP port).
+        */
+       union landlock_key key;
+       /**
+        * @type: Type of a landlock_ruleset's root tree.
+        */
+       const enum landlock_key_type type;
+};
+
 /**
  * struct landlock_rule - Access rights tied to an object
  */
@@ -69,12 +110,13 @@ struct landlock_rule {
         */
        struct rb_node node;
        /**
-        * @object: Pointer to identify a kernel object (e.g. an inode).  This
-        * is used as a key for this ruleset element.  This pointer is set once
-        * and never modified.  It always points to an allocated object because
-        * each rule increments the refcount of its object.
+        * @key: A union to identify either a kernel object (e.g. an inode) or
+        * a raw data value (e.g. a network socket port). This is used as a key
+        * for this ruleset element.  The pointer is set once and never
+        * modified.  It always points to an allocated object because each rule
+        * increments the refcount of its object.
         */
-       struct landlock_object *object;
+       union landlock_key key;
        /**
         * @num_layers: Number of entries in @layers.
         */
@@ -110,11 +152,12 @@ struct landlock_hierarchy {
  */
 struct landlock_ruleset {
        /**
-        * @root: Root of a red-black tree containing &struct landlock_rule
-        * nodes.  Once a ruleset is tied to a process (i.e. as a domain), this
-        * tree is immutable until @usage reaches zero.
+        * @root_inode: Root of a red-black tree containing &struct
+        * landlock_rule nodes with inode object.  Once a ruleset is tied to a
+        * process (i.e. as a domain), this tree is immutable until @usage
+        * reaches zero.
         */
-       struct rb_root root;
+       struct rb_root root_inode;
        /**
         * @hierarchy: Enables hierarchy identification even when a parent
         * domain vanishes.  This is needed for the ptrace protection.
@@ -176,7 +219,7 @@ void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
 void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
 
 int landlock_insert_rule(struct landlock_ruleset *const ruleset,
-                        struct landlock_object *const object,
+                        const struct landlock_id id,
                         const access_mask_t access);
 
 struct landlock_ruleset *
@@ -185,7 +228,7 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
 
 const struct landlock_rule *
 landlock_find_rule(const struct landlock_ruleset *const ruleset,
-                  const struct landlock_object *const object);
+                  const struct landlock_id id);
 
 static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
 {