landlock: Allow FS topology changes for domains without such rule type

Allow mount point and root directory changes when there is no filesystem
rule tied to the current Landlock domain. This doesn't change anything
for now because a domain must have at least a (filesystem) rule, but
this will change when other rule types will come. For instance, a domain
only restricting the network should have no impact on filesystem
restrictions.

Add a new get_current_fs_domain() helper to quickly check filesystem
rule existence for all filesystem LSM hooks.

Remove unnecessary inlining.

Link: https://lore.kernel.org/r/20231026014751.414649-3-konstantin.meskhidze@huawei.com
Signed-off-by: Mickaël Salaün <mic@digikod.net>

diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index d8cd8cd9ce252de62faf1225ea3aa2d8b5276a35..f6a7da21708a15a47e9c00c695fbde1fd029de83 100644 (file)
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -387,9 +387,9 @@ Current limitations
 Filesystem topology modification
 --------------------------------
 
-As for file renaming and linking, a sandboxed thread cannot modify its
-filesystem topology, whether via :manpage:`mount(2)` or
-:manpage:`pivot_root(2)`.  However, :manpage:`chroot(2)` calls are not denied.
+Threads sandboxed with filesystem restrictions cannot modify filesystem
+topology, whether via :manpage:`mount(2)` or :manpage:`pivot_root(2)`.
+However, :manpage:`chroot(2)` calls are not denied.
 
 Special filesystems
 -------------------

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 6953801619ac785e0e7e2b6fe5ef8f1ebfaed37d..e8d680c2c80af74d51b7b4b3b0a20cceae345721 100644 (file)
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -150,16 +150,6 @@ retry:
        LANDLOCK_ACCESS_FS_TRUNCATE)
 /* clang-format on */
 
-/*
- * All access rights that are denied by default whether they are handled or not
- * by a ruleset/layer.  This must be ORed with all ruleset->fs_access_masks[]
- * entries when we need to get the absolute handled access masks.
- */
-/* clang-format off */
-#define ACCESS_INITIALLY_DENIED ( \
-       LANDLOCK_ACCESS_FS_REFER)
-/* clang-format on */
-
 /*
  * @path: Should have been checked by get_path_from_fd().
  */
@@ -179,8 +169,7 @@ 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) |
-                          ACCESS_INITIALLY_DENIED);
+                        ~landlock_get_fs_access_mask(ruleset, 0);
        object = get_inode_object(d_backing_inode(path->dentry));
        if (IS_ERR(object))
                return PTR_ERR(object);
@@ -287,15 +276,16 @@ static inline bool is_nouser_or_private(const struct dentry *dentry)
                unlikely(IS_PRIVATE(d_backing_inode(dentry))));
 }
 
-static inline access_mask_t
-get_handled_accesses(const struct landlock_ruleset *const domain)
+static access_mask_t
+get_raw_handled_fs_accesses(const struct landlock_ruleset *const domain)
 {
-       access_mask_t access_dom = ACCESS_INITIALLY_DENIED;
+       access_mask_t access_dom = 0;
        size_t layer_level;
 
        for (layer_level = 0; layer_level < domain->num_layers; layer_level++)
-               access_dom |= landlock_get_fs_access_mask(domain, layer_level);
-       return access_dom & LANDLOCK_MASK_ACCESS_FS;
+               access_dom |=
+                       landlock_get_raw_fs_access_mask(domain, layer_level);
+       return access_dom;
 }
 
 /**
@@ -331,13 +321,8 @@ init_layer_masks(const struct landlock_ruleset *const domain,
 
                for_each_set_bit(access_bit, &access_req,
                                 ARRAY_SIZE(*layer_masks)) {
-                       /*
-                        * Artificially handles all initially denied by default
-                        * access rights.
-                        */
                        if (BIT_ULL(access_bit) &
-                           (landlock_get_fs_access_mask(domain, layer_level) |
-                            ACCESS_INITIALLY_DENIED)) {
+                           landlock_get_fs_access_mask(domain, layer_level)) {
                                (*layer_masks)[access_bit] |=
                                        BIT_ULL(layer_level);
                                handled_accesses |= BIT_ULL(access_bit);
@@ -347,6 +332,25 @@ init_layer_masks(const struct landlock_ruleset *const domain,
        return handled_accesses;
 }
 
+static access_mask_t
+get_handled_fs_accesses(const struct landlock_ruleset *const domain)
+{
+       /* Handles all initially denied by default access rights. */
+       return get_raw_handled_fs_accesses(domain) |
+              LANDLOCK_ACCESS_FS_INITIALLY_DENIED;
+}
+
+static const struct landlock_ruleset *get_current_fs_domain(void)
+{
+       const struct landlock_ruleset *const dom =
+               landlock_get_current_domain();
+
+       if (!dom || !get_raw_handled_fs_accesses(dom))
+               return NULL;
+
+       return dom;
+}
+
 /*
  * Check that a destination file hierarchy has more restrictions than a source
  * file hierarchy.  This is only used for link and rename actions.
@@ -519,7 +523,7 @@ static bool is_access_to_paths_allowed(
                 * a superset of the meaningful requested accesses).
                 */
                access_masked_parent1 = access_masked_parent2 =
-                       get_handled_accesses(domain);
+                       get_handled_fs_accesses(domain);
                is_dom_check = true;
        } else {
                if (WARN_ON_ONCE(dentry_child1 || dentry_child2))
@@ -651,8 +655,7 @@ static inline int check_access_path(const struct landlock_ruleset *const domain,
 static inline int current_check_access_path(const struct path *const path,
                                            const access_mask_t access_request)
 {
-       const struct landlock_ruleset *const dom =
-               landlock_get_current_domain();
+       const struct landlock_ruleset *const dom = get_current_fs_domain();
 
        if (!dom)
                return 0;
@@ -815,8 +818,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
                                    struct dentry *const new_dentry,
                                    const bool removable, const bool exchange)
 {
-       const struct landlock_ruleset *const dom =
-               landlock_get_current_domain();
+       const struct landlock_ruleset *const dom = get_current_fs_domain();
        bool allow_parent1, allow_parent2;
        access_mask_t access_request_parent1, access_request_parent2;
        struct path mnt_dir;
@@ -1050,7 +1052,7 @@ static int hook_sb_mount(const char *const dev_name,
                         const struct path *const path, const char *const type,
                         const unsigned long flags, void *const data)
 {
-       if (!landlock_get_current_domain())
+       if (!get_current_fs_domain())
                return 0;
        return -EPERM;
 }
@@ -1058,7 +1060,7 @@ static int hook_sb_mount(const char *const dev_name,
 static int hook_move_mount(const struct path *const from_path,
                           const struct path *const to_path)
 {
-       if (!landlock_get_current_domain())
+       if (!get_current_fs_domain())
                return 0;
        return -EPERM;
 }
@@ -1069,14 +1071,14 @@ static int hook_move_mount(const struct path *const from_path,
  */
 static int hook_sb_umount(struct vfsmount *const mnt, const int flags)
 {
-       if (!landlock_get_current_domain())
+       if (!get_current_fs_domain())
                return 0;
        return -EPERM;
 }
 
 static int hook_sb_remount(struct super_block *const sb, void *const mnt_opts)
 {
-       if (!landlock_get_current_domain())
+       if (!get_current_fs_domain())
                return 0;
        return -EPERM;
 }
@@ -1092,7 +1094,7 @@ static int hook_sb_remount(struct super_block *const sb, void *const mnt_opts)
 static int hook_sb_pivotroot(const struct path *const old_path,
                             const struct path *const new_path)
 {
-       if (!landlock_get_current_domain())
+       if (!get_current_fs_domain())
                return 0;
        return -EPERM;
 }
@@ -1128,8 +1130,7 @@ static int hook_path_mknod(const struct path *const dir,
                           struct dentry *const dentry, const umode_t mode,
                           const unsigned int dev)
 {
-       const struct landlock_ruleset *const dom =
-               landlock_get_current_domain();
+       const struct landlock_ruleset *const dom = get_current_fs_domain();
 
        if (!dom)
                return 0;
@@ -1208,8 +1209,7 @@ static int hook_file_open(struct file *const file)
        layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
        access_mask_t open_access_request, full_access_request, allowed_access;
        const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE;
-       const struct landlock_ruleset *const dom =
-               landlock_get_current_domain();
+       const struct landlock_ruleset *const dom = get_current_fs_domain();
 
        if (!dom)
                return 0;

diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 8c99c323b06cc1f4a4008a61643ffec6e89661fe..6e2ad1546ab30be77edc883dde1ce19da742ae76 100644 (file)
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -15,10 +15,21 @@
 #include <linux/rbtree.h>
 #include <linux/refcount.h>
 #include <linux/workqueue.h>
+#include <uapi/linux/landlock.h>
 
 #include "limits.h"
 #include "object.h"
 
+/*
+ * All access rights that are denied by default whether they are handled or not
+ * by a ruleset/layer.  This must be ORed with all ruleset->access_masks[]
+ * entries when we need to get the absolute handled access masks.
+ */
+/* clang-format off */
+#define LANDLOCK_ACCESS_FS_INITIALLY_DENIED ( \
+       LANDLOCK_ACCESS_FS_REFER)
+/* clang-format on */
+
 typedef u16 access_mask_t;
 /* Makes sure all filesystem access rights can be stored. */
 static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
@@ -196,12 +207,21 @@ landlock_add_fs_access_mask(struct landlock_ruleset *const ruleset,
 }
 
 static inline access_mask_t
-landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset,
-                           const u16 layer_level)
+landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset,
+                               const u16 layer_level)
 {
        return (ruleset->access_masks[layer_level] >>
                LANDLOCK_SHIFT_ACCESS_FS) &
               LANDLOCK_MASK_ACCESS_FS;
 }
 
+static inline access_mask_t
+landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset,
+                           const u16 layer_level)
+{
+       /* Handles all initially denied by default access rights. */
+       return landlock_get_raw_fs_access_mask(ruleset, layer_level) |
+              LANDLOCK_ACCESS_FS_INITIALLY_DENIED;
+}
+
 #endif /* _SECURITY_LANDLOCK_RULESET_H */

diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 7ec6bbed711789fdd41e03b1f357f8a9dd6aee77..d35cd5d304db436415c72e8b4613071dc39c14e9 100644 (file)
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -349,7 +349,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
         * Checks that allowed_access matches the @ruleset constraints
         * (ruleset->access_masks[0] is automatically upgraded to 64-bits).
         */
-       mask = landlock_get_fs_access_mask(ruleset, 0);
+       mask = landlock_get_raw_fs_access_mask(ruleset, 0);
        if ((path_beneath_attr.allowed_access | mask) != mask) {
                err = -EINVAL;
                goto out_put_ruleset;