diff options
Diffstat (limited to 'security')
| -rw-r--r-- | security/landlock/errata/abi-1.h | 16 | ||||
| -rw-r--r-- | security/landlock/fs.c | 46 | ||||
| -rw-r--r-- | security/landlock/ruleset.c | 12 | ||||
| -rw-r--r-- | security/landlock/ruleset.h | 2 |
4 files changed, 59 insertions, 17 deletions
diff --git a/security/landlock/errata/abi-1.h b/security/landlock/errata/abi-1.h new file mode 100644 index 000000000000..e8a2bff2e5b6 --- /dev/null +++ b/security/landlock/errata/abi-1.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/** + * DOC: erratum_3 + * + * Erratum 3: Disconnected directory handling + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * This fix addresses an issue with disconnected directories that occur when a + * directory is moved outside the scope of a bind mount. The change ensures + * that evaluated access rights include both those from the disconnected file + * hierarchy down to its filesystem root and those from the related mount point + * hierarchy. This prevents access right widening through rename or link + * actions. + */ +LANDLOCK_ERRATUM(3) diff --git a/security/landlock/fs.c b/security/landlock/fs.c index cee2b6f22c83..fe794875ad46 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -714,7 +714,8 @@ static void test_is_eacces_with_write(struct kunit *const test) * is_access_to_paths_allowed - Check accesses for requests with a common path * * @domain: Domain to check against. - * @path: File hierarchy to walk through. + * @path: File hierarchy to walk through. For refer checks, this would be + * the common mountpoint. * @access_request_parent1: Accesses to check, once @layer_masks_parent1 is * equal to @layer_masks_parent2 (if any). This is tied to the unique * requested path for most actions, or the source in case of a refer action @@ -837,7 +838,6 @@ static bool is_access_to_paths_allowed( * restriction. */ while (true) { - struct dentry *parent_dentry; const struct landlock_rule *rule; /* @@ -909,21 +909,33 @@ jump_up: break; } } + if (unlikely(IS_ROOT(walker_path.dentry))) { - /* - * Stops at disconnected root directories. Only allows - * access to internal filesystems (e.g. nsfs, which is - * reachable through /proc/<pid>/ns/<namespace>). - */ - if (walker_path.mnt->mnt_flags & MNT_INTERNAL) { + if (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) { + /* + * Stops and allows access when reaching disconnected root + * directories that are part of internal filesystems (e.g. nsfs, + * which is reachable through /proc/<pid>/ns/<namespace>). + */ allowed_parent1 = true; allowed_parent2 = true; + break; } - break; + + /* + * We reached a disconnected root directory from a bind mount. + * Let's continue the walk with the mount point we missed. + */ + dput(walker_path.dentry); + walker_path.dentry = walker_path.mnt->mnt_root; + dget(walker_path.dentry); + } else { + struct dentry *const parent_dentry = + dget_parent(walker_path.dentry); + + dput(walker_path.dentry); + walker_path.dentry = parent_dentry; } - parent_dentry = dget_parent(walker_path.dentry); - dput(walker_path.dentry); - walker_path.dentry = parent_dentry; } path_put(&walker_path); @@ -1021,6 +1033,9 @@ static access_mask_t maybe_remove(const struct dentry *const dentry) * file. While walking from @dir to @mnt_root, we record all the domain's * allowed accesses in @layer_masks_dom. * + * Because of disconnected directories, this walk may not reach @mnt_dir. In + * this case, the walk will continue to @mnt_dir after this call. + * * This is similar to is_access_to_paths_allowed() but much simpler because it * only handles walking on the same mount point and only checks one set of * accesses. @@ -1062,8 +1077,11 @@ static bool collect_domain_accesses( break; } - /* We should not reach a root other than @mnt_root. */ - if (dir == mnt_root || WARN_ON_ONCE(IS_ROOT(dir))) + /* + * Stops at the mount point or the filesystem root for a disconnected + * directory. + */ + if (dir == mnt_root || unlikely(IS_ROOT(dir))) break; parent_dentry = dget_parent(dir); diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index ce7940efea51..dfcdc19ea268 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -83,6 +83,10 @@ static void build_check_rule(void) .num_layers = ~0, }; + /* + * Checks that .num_layers is large enough for at least + * LANDLOCK_MAX_NUM_LAYERS layers. + */ BUILD_BUG_ON(rule.num_layers < LANDLOCK_MAX_NUM_LAYERS); } @@ -290,6 +294,10 @@ static void build_check_layer(void) .access = ~0, }; + /* + * Checks that .level and .access are large enough to contain their expected + * maximum values. + */ BUILD_BUG_ON(layer.level < LANDLOCK_MAX_NUM_LAYERS); BUILD_BUG_ON(layer.access < LANDLOCK_MASK_ACCESS_FS); } @@ -644,8 +652,8 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule, bool is_empty; /* - * Records in @layer_masks which layer grants access to each - * requested access. + * Records in @layer_masks which layer grants access to each requested + * access: bit cleared if the related layer grants access. */ is_empty = true; for_each_set_bit(access_bit, &access_req, masks_array_size) { diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index 5da9a64f5af7..1a78cba662b2 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -27,7 +27,7 @@ struct landlock_hierarchy; */ struct landlock_layer { /** - * @level: Position of this layer in the layer stack. + * @level: Position of this layer in the layer stack. Starts from 1. */ u16 level; /** |