From 49440290a0935f428a1e43a5ac8dc275a647ff80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Fri, 10 Jan 2025 16:39:13 +0100 Subject: landlock: Handle weird files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A corrupted filesystem (e.g. bcachefs) might return weird files. Instead of throwing a warning and allowing access to such file, treat them as regular files. Cc: Dave Chinner Cc: Kent Overstreet Cc: Paul Moore Reported-by: syzbot+34b68f850391452207df@syzkaller.appspotmail.com Closes: https://lore.kernel.org/r/000000000000a65b35061cffca61@google.com Reported-by: syzbot+360866a59e3c80510a62@syzkaller.appspotmail.com Closes: https://lore.kernel.org/r/67379b3f.050a0220.85a0.0001.GAE@google.com Reported-by: Ubisectech Sirius Closes: https://lore.kernel.org/r/c426821d-8380-46c4-a494-7008bbd7dd13.bugreport@ubisectech.com Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control") Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20250110153918.241810-1-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/fs.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/landlock/fs.c b/security/landlock/fs.c index e31b97a9f175..7adb25150488 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -937,10 +937,6 @@ static access_mask_t get_mode_access(const umode_t mode) switch (mode & S_IFMT) { case S_IFLNK: return LANDLOCK_ACCESS_FS_MAKE_SYM; - case 0: - /* A zero mode translates to S_IFREG. */ - case S_IFREG: - return LANDLOCK_ACCESS_FS_MAKE_REG; case S_IFDIR: return LANDLOCK_ACCESS_FS_MAKE_DIR; case S_IFCHR: @@ -951,9 +947,12 @@ static access_mask_t get_mode_access(const umode_t mode) return LANDLOCK_ACCESS_FS_MAKE_FIFO; case S_IFSOCK: return LANDLOCK_ACCESS_FS_MAKE_SOCK; + case S_IFREG: + case 0: + /* A zero mode translates to S_IFREG. */ default: - WARN_ON_ONCE(1); - return 0; + /* Treats weird files as regular files. */ + return LANDLOCK_ACCESS_FS_MAKE_REG; } } -- cgit v1.2.3 From 25ccc75f5de6684fd6a497e44297497ccc7e0603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Fri, 10 Jan 2025 16:39:14 +0100 Subject: landlock: Constify get_mode_access() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use __attribute_const__ for get_mode_access(). Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20250110153918.241810-2-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 7adb25150488..f81d0335b825 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -932,7 +932,7 @@ static int current_check_access_path(const struct path *const path, return check_access_path(dom, path, access_request); } -static access_mask_t get_mode_access(const umode_t mode) +static __attribute_const__ access_mask_t get_mode_access(const umode_t mode) { switch (mode & S_IFMT) { case S_IFLNK: -- cgit v1.2.3 From d32f79a59ae1a90f27735c75f9920c585e6ceb8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 13 Jan 2025 17:11:09 +0100 Subject: landlock: Use scoped guards for ruleset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify error handling by replacing goto statements with automatic calls to landlock_put_ruleset() when going out of scope. This change will be easy to backport to v6.6 if needed, only the kernel.h include line conflicts. As for any other similar changes, we should be careful when backporting without goto statements. Add missing include file. Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20250113161112.452505-2-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/ruleset.c | 22 ++++++++++------------ security/landlock/ruleset.h | 5 +++++ security/landlock/syscalls.c | 25 ++++++++----------------- 3 files changed, 23 insertions(+), 29 deletions(-) (limited to 'security') diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index a93bdbf52fff..f27b7bdb19b9 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -8,11 +8,13 @@ #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -537,7 +539,7 @@ struct landlock_ruleset * landlock_merge_ruleset(struct landlock_ruleset *const parent, struct landlock_ruleset *const ruleset) { - struct landlock_ruleset *new_dom; + struct landlock_ruleset *new_dom __free(landlock_put_ruleset) = NULL; u32 num_layers; int err; @@ -557,29 +559,25 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent, new_dom = create_ruleset(num_layers); if (IS_ERR(new_dom)) return new_dom; + new_dom->hierarchy = kzalloc(sizeof(*new_dom->hierarchy), GFP_KERNEL_ACCOUNT); - if (!new_dom->hierarchy) { - err = -ENOMEM; - goto out_put_dom; - } + if (!new_dom->hierarchy) + return ERR_PTR(-ENOMEM); + refcount_set(&new_dom->hierarchy->usage, 1); /* ...as a child of @parent... */ err = inherit_ruleset(parent, new_dom); if (err) - goto out_put_dom; + return ERR_PTR(err); /* ...and including @ruleset. */ err = merge_ruleset(new_dom, ruleset); if (err) - goto out_put_dom; - - return new_dom; + return ERR_PTR(err); -out_put_dom: - landlock_put_ruleset(new_dom); - return ERR_PTR(err); + return no_free_ptr(new_dom); } /* diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index 631e24d4ffe9..70e5b53d1c71 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -11,6 +11,8 @@ #include #include +#include +#include #include #include #include @@ -252,6 +254,9 @@ landlock_create_ruleset(const access_mask_t access_mask_fs, void landlock_put_ruleset(struct landlock_ruleset *const ruleset); void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset); +DEFINE_FREE(landlock_put_ruleset, struct landlock_ruleset *, + if (!IS_ERR_OR_NULL(_T)) landlock_put_ruleset(_T)) + int landlock_insert_rule(struct landlock_ruleset *const ruleset, const struct landlock_id id, const access_mask_t access); diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index 4ed8e70c25ed..5a7f1f77292e 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -456,10 +457,10 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, flags) { - struct landlock_ruleset *new_dom, *ruleset; + struct landlock_ruleset *new_dom, + *ruleset __free(landlock_put_ruleset) = NULL; struct cred *new_cred; struct landlock_cred_security *new_llcred; - int err; if (!is_initialized()) return -EOPNOTSUPP; @@ -483,10 +484,9 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, /* Prepares new credentials. */ new_cred = prepare_creds(); - if (!new_cred) { - err = -ENOMEM; - goto out_put_ruleset; - } + if (!new_cred) + return -ENOMEM; + new_llcred = landlock_cred(new_cred); /* @@ -495,21 +495,12 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, */ new_dom = landlock_merge_ruleset(new_llcred->domain, ruleset); if (IS_ERR(new_dom)) { - err = PTR_ERR(new_dom); - goto out_put_creds; + abort_creds(new_cred); + return PTR_ERR(new_dom); } /* Replaces the old (prepared) domain. */ landlock_put_ruleset(new_llcred->domain); new_llcred->domain = new_dom; - - landlock_put_ruleset(ruleset); return commit_creds(new_cred); - -out_put_creds: - abort_creds(new_cred); - -out_put_ruleset: - landlock_put_ruleset(ruleset); - return err; } -- cgit v1.2.3 From 16a6f4d3b558bd55b52892f2becad8f33cb62ed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 13 Jan 2025 17:11:10 +0100 Subject: landlock: Use scoped guards for ruleset in landlock_add_rule() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify error handling by replacing goto statements with automatic calls to landlock_put_ruleset() when going out of scope. This change depends on the TCP support. Cc: Konstantin Meskhidze Cc: Mikhail Ivanov Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20250113161112.452505-3-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/syscalls.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) (limited to 'security') diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index 5a7f1f77292e..a9760d252fc2 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -399,8 +399,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, const enum landlock_rule_type, rule_type, const void __user *const, rule_attr, const __u32, flags) { - struct landlock_ruleset *ruleset; - int err; + struct landlock_ruleset *ruleset __free(landlock_put_ruleset) = NULL; if (!is_initialized()) return -EOPNOTSUPP; @@ -416,17 +415,12 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, switch (rule_type) { case LANDLOCK_RULE_PATH_BENEATH: - err = add_rule_path_beneath(ruleset, rule_attr); - break; + return add_rule_path_beneath(ruleset, rule_attr); case LANDLOCK_RULE_NET_PORT: - err = add_rule_net_port(ruleset, rule_attr); - break; + return add_rule_net_port(ruleset, rule_attr); default: - err = -EINVAL; - break; + return -EINVAL; } - landlock_put_ruleset(ruleset); - return err; } /* Enforcement */ -- cgit v1.2.3 From 924f4403d869ad24bd2c54ad97ad87d4b838d09c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Wed, 8 Jan 2025 16:43:11 +0100 Subject: landlock: Factor out check_access_path() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge check_access_path() into current_check_access_path() and make hook_path_mknod() use it. Cc: Günther Noack Link: https://lore.kernel.org/r/20250108154338.1129069-4-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/fs.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) (limited to 'security') diff --git a/security/landlock/fs.c b/security/landlock/fs.c index f81d0335b825..4023354dd8e3 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -908,28 +908,22 @@ jump_up: return allowed_parent1 && allowed_parent2; } -static int check_access_path(const struct landlock_ruleset *const domain, - const struct path *const path, - access_mask_t access_request) -{ - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {}; - - access_request = landlock_init_layer_masks( - domain, access_request, &layer_masks, LANDLOCK_KEY_INODE); - if (is_access_to_paths_allowed(domain, path, access_request, - &layer_masks, NULL, 0, NULL, NULL)) - return 0; - return -EACCES; -} - static int current_check_access_path(const struct path *const path, - const access_mask_t access_request) + access_mask_t access_request) { const struct landlock_ruleset *const dom = get_current_fs_domain(); + layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {}; if (!dom) return 0; - return check_access_path(dom, path, access_request); + + access_request = landlock_init_layer_masks( + dom, access_request, &layer_masks, LANDLOCK_KEY_INODE); + if (is_access_to_paths_allowed(dom, path, access_request, &layer_masks, + NULL, 0, NULL, NULL)) + return 0; + + return -EACCES; } static __attribute_const__ access_mask_t get_mode_access(const umode_t mode) @@ -1413,11 +1407,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 = get_current_fs_domain(); - - if (!dom) - return 0; - return check_access_path(dom, dir, get_mode_access(mode)); + return current_check_access_path(dir, get_mode_access(mode)); } static int hook_path_symlink(const struct path *const dir, -- cgit v1.2.3 From 622e2f5954763385c4fa1f9a11a11366952a9b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Wed, 8 Jan 2025 16:43:13 +0100 Subject: landlock: Move access types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move LANDLOCK_ACCESS_FS_INITIALLY_DENIED, access_mask_t, struct access_mask, and struct access_masks_all to a dedicated access.h file. Rename LANDLOCK_ACCESS_FS_INITIALLY_DENIED to _LANDLOCK_ACCESS_FS_INITIALLY_DENIED to make it clear that it's not part of UAPI. Add some newlines when appropriate. This file will be extended with following commits, and it will help to avoid dependency loops. Cc: Günther Noack Link: https://lore.kernel.org/r/20250108154338.1129069-6-mic@digikod.net [mic: Fix rebase conflict because of the new cleanup headers] Signed-off-by: Mickaël Salaün --- security/landlock/access.h | 62 +++++++++++++++++++++++++++++++++++++++++++++ security/landlock/fs.c | 3 ++- security/landlock/fs.h | 1 + security/landlock/ruleset.c | 1 + security/landlock/ruleset.h | 47 ++-------------------------------- 5 files changed, 68 insertions(+), 46 deletions(-) create mode 100644 security/landlock/access.h (limited to 'security') diff --git a/security/landlock/access.h b/security/landlock/access.h new file mode 100644 index 000000000000..9ee4b30a87e6 --- /dev/null +++ b/security/landlock/access.h @@ -0,0 +1,62 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Landlock LSM - Access types and helpers + * + * Copyright © 2016-2020 Mickaël Salaün + * Copyright © 2018-2020 ANSSI + * Copyright © 2024-2025 Microsoft Corporation + */ + +#ifndef _SECURITY_LANDLOCK_ACCESS_H +#define _SECURITY_LANDLOCK_ACCESS_H + +#include +#include +#include +#include + +#include "limits.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); +/* Makes sure all network access rights can be stored. */ +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET); +/* Makes sure all scoped rights can be stored. */ +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE); +/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */ +static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); + +/* Ruleset access masks. */ +struct access_masks { + access_mask_t fs : LANDLOCK_NUM_ACCESS_FS; + access_mask_t net : LANDLOCK_NUM_ACCESS_NET; + access_mask_t scope : LANDLOCK_NUM_SCOPE; +}; + +union access_masks_all { + struct access_masks masks; + u32 all; +}; + +/* Makes sure all fields are covered. */ +static_assert(sizeof(typeof_member(union access_masks_all, masks)) == + sizeof(typeof_member(union access_masks_all, all))); + +typedef u16 layer_mask_t; + +/* Makes sure all layers can be checked. */ +static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS); + +#endif /* _SECURITY_LANDLOCK_ACCESS_H */ diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 4023354dd8e3..e323f7fb5a98 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -36,6 +36,7 @@ #include #include +#include "access.h" #include "common.h" #include "cred.h" #include "fs.h" @@ -393,7 +394,7 @@ get_handled_fs_accesses(const struct landlock_ruleset *const domain) { /* Handles all initially denied by default access rights. */ return landlock_union_access_masks(domain).fs | - LANDLOCK_ACCESS_FS_INITIALLY_DENIED; + _LANDLOCK_ACCESS_FS_INITIALLY_DENIED; } static const struct access_masks any_fs = { diff --git a/security/landlock/fs.h b/security/landlock/fs.h index 1487e1f023a1..d445f411c26a 100644 --- a/security/landlock/fs.h +++ b/security/landlock/fs.h @@ -13,6 +13,7 @@ #include #include +#include "access.h" #include "ruleset.h" #include "setup.h" diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index f27b7bdb19b9..4aeab215d7c5 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -22,6 +22,7 @@ #include #include +#include "access.h" #include "limits.h" #include "object.h" #include "ruleset.h" diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index 70e5b53d1c71..52f4f0af6ab0 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -9,60 +9,17 @@ #ifndef _SECURITY_LANDLOCK_RULESET_H #define _SECURITY_LANDLOCK_RULESET_H -#include -#include #include #include -#include #include #include #include #include -#include +#include "access.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); -/* Makes sure all network access rights can be stored. */ -static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET); -/* Makes sure all scoped rights can be stored. */ -static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE); -/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */ -static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); - -/* Ruleset access masks. */ -struct access_masks { - access_mask_t fs : LANDLOCK_NUM_ACCESS_FS; - access_mask_t net : LANDLOCK_NUM_ACCESS_NET; - access_mask_t scope : LANDLOCK_NUM_SCOPE; -}; - -union access_masks_all { - struct access_masks masks; - u32 all; -}; - -/* Makes sure all fields are covered. */ -static_assert(sizeof(typeof_member(union access_masks_all, masks)) == - sizeof(typeof_member(union access_masks_all, all))); - -typedef u16 layer_mask_t; -/* Makes sure all layers can be checked. */ -static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS); - /** * struct landlock_layer - Access rights for a given layer */ @@ -371,7 +328,7 @@ landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset, { /* Handles all initially denied by default access rights. */ return ruleset->access_masks[layer_level].fs | - LANDLOCK_ACCESS_FS_INITIALLY_DENIED; + _LANDLOCK_ACCESS_FS_INITIALLY_DENIED; } static inline access_mask_t -- cgit v1.2.3 From d6c7cf84a24fff332ff65ffe236302216474b834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Wed, 8 Jan 2025 16:43:14 +0100 Subject: landlock: Simplify initially denied access rights MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upgrade domain's handled access masks when creating a domain from a ruleset, instead of converting them at runtime. This is more consistent and helps with audit support. Cc: Günther Noack Link: https://lore.kernel.org/r/20250108154338.1129069-7-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/access.h | 17 ++++++++++++++++- security/landlock/fs.c | 10 +--------- security/landlock/ruleset.c | 3 ++- 3 files changed, 19 insertions(+), 11 deletions(-) (limited to 'security') diff --git a/security/landlock/access.h b/security/landlock/access.h index 9ee4b30a87e6..74fd8f399fbd 100644 --- a/security/landlock/access.h +++ b/security/landlock/access.h @@ -20,7 +20,8 @@ /* * 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. + * entries when we need to get the absolute handled access masks, see + * landlock_upgrade_handled_access_masks(). */ /* clang-format off */ #define _LANDLOCK_ACCESS_FS_INITIALLY_DENIED ( \ @@ -59,4 +60,18 @@ typedef u16 layer_mask_t; /* Makes sure all layers can be checked. */ static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS); +/* Upgrades with all initially denied by default access rights. */ +static inline struct access_masks +landlock_upgrade_handled_access_masks(struct access_masks access_masks) +{ + /* + * All access rights that are denied by default whether they are + * explicitly handled or not. + */ + if (access_masks.fs) + access_masks.fs |= _LANDLOCK_ACCESS_FS_INITIALLY_DENIED; + + return access_masks; +} + #endif /* _SECURITY_LANDLOCK_ACCESS_H */ diff --git a/security/landlock/fs.c b/security/landlock/fs.c index e323f7fb5a98..4eb972f2292f 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -389,14 +389,6 @@ static bool is_nouser_or_private(const struct dentry *dentry) unlikely(IS_PRIVATE(d_backing_inode(dentry)))); } -static access_mask_t -get_handled_fs_accesses(const struct landlock_ruleset *const domain) -{ - /* Handles all initially denied by default access rights. */ - return landlock_union_access_masks(domain).fs | - _LANDLOCK_ACCESS_FS_INITIALLY_DENIED; -} - static const struct access_masks any_fs = { .fs = ~0, }; @@ -788,7 +780,7 @@ static bool is_access_to_paths_allowed( * a superset of the meaningful requested accesses). */ access_masked_parent1 = access_masked_parent2 = - get_handled_fs_accesses(domain); + landlock_union_access_masks(domain).fs; is_dom_check = true; } else { if (WARN_ON_ONCE(dentry_child1 || dentry_child2)) diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index 4aeab215d7c5..241ce44375b6 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -387,7 +387,8 @@ static int merge_ruleset(struct landlock_ruleset *const dst, err = -EINVAL; goto out_unlock; } - dst->access_masks[dst->num_layers - 1] = src->access_masks[0]; + dst->access_masks[dst->num_layers - 1] = + landlock_upgrade_handled_access_masks(src->access_masks[0]); /* Merges the @src inode tree. */ err = merge_tree(dst, src, LANDLOCK_KEY_INODE); -- cgit v1.2.3 From 058518c2092081f224edb37cbc236bed5c28852d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Wed, 8 Jan 2025 16:43:19 +0100 Subject: landlock: Align partial refer access checks with final ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a logical issue that could have been visible if the source or the destination of a rename/link action was allowed for either the source or the destination but not both. However, this logical bug is unreachable because either: - the rename/link action is allowed by the access rights tied to the same mount point (without relying on access rights in a parent mount point) and the access request is allowed (i.e. allow_parent1 and allow_parent2 are true in current_check_refer_path), - or a common rule in a parent mount point updates the access check for the source and the destination (cf. is_access_to_paths_allowed). See the following layout1.refer_part_mount_tree_is_allowed test that work with and without this fix. This fix does not impact current code but it is required for the audit support. Cc: Günther Noack Link: https://lore.kernel.org/r/20250108154338.1129069-12-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/fs.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 4eb972f2292f..110b8cfaab9c 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -565,6 +565,12 @@ static void test_no_more_access(struct kunit *const test) #undef NMA_TRUE #undef NMA_FALSE +static bool is_layer_masks_allowed( + layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]) +{ + return !memchr_inv(layer_masks, 0, sizeof(*layer_masks)); +} + /* * Removes @layer_masks accesses that are not requested. * @@ -582,7 +588,8 @@ scope_to_request(const access_mask_t access_request, for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks)) (*layer_masks)[access_bit] = 0; - return !memchr_inv(layer_masks, 0, sizeof(*layer_masks)); + + return is_layer_masks_allowed(layer_masks); } #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST @@ -771,9 +778,14 @@ static bool is_access_to_paths_allowed( if (WARN_ON_ONCE(domain->num_layers < 1 || !layer_masks_parent1)) return false; + allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1); + if (unlikely(layer_masks_parent2)) { if (WARN_ON_ONCE(!dentry_child1)) return false; + + allowed_parent2 = is_layer_masks_allowed(layer_masks_parent2); + /* * For a double request, first check for potential privilege * escalation by looking at domain handled accesses (which are -- cgit v1.2.3 From d617f0d72d8041c7099fd04a62db0f0fa5331c1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Wed, 8 Jan 2025 16:43:21 +0100 Subject: landlock: Optimize file path walks and prepare for audit support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always synchronize access_masked_parent* with access_request_parent* according to allowed_parent*. This is required for audit support to be able to get back to the reason of denial. In a rename/link action, instead of always checking a rule two times for the same parent directory of the source and the destination files, only check it when an action on a child was not already allowed. This also enables us to keep consistent allowed_parent* status, which is required to get back to the reason of denial. For internal mount points, only upgrade allowed_parent* to true but do not wrongfully set both of them to false otherwise. This is also required to get back to the reason of denial. This does not impact the current behavior but slightly optimize code and prepare for audit support that needs to know the exact reason why an access was denied. Cc: Günther Noack Link: https://lore.kernel.org/r/20250108154338.1129069-14-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/fs.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 110b8cfaab9c..71b9dc331aae 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -852,15 +852,6 @@ static bool is_access_to_paths_allowed( child1_is_directory, layer_masks_parent2, layer_masks_child2, child2_is_directory))) { - allowed_parent1 = scope_to_request( - access_request_parent1, layer_masks_parent1); - allowed_parent2 = scope_to_request( - access_request_parent2, layer_masks_parent2); - - /* Stops when all accesses are granted. */ - if (allowed_parent1 && allowed_parent2) - break; - /* * Now, downgrades the remaining checks from domain * handled accesses to requested accesses. @@ -868,15 +859,32 @@ static bool is_access_to_paths_allowed( is_dom_check = false; access_masked_parent1 = access_request_parent1; access_masked_parent2 = access_request_parent2; + + allowed_parent1 = + allowed_parent1 || + scope_to_request(access_masked_parent1, + layer_masks_parent1); + allowed_parent2 = + allowed_parent2 || + scope_to_request(access_masked_parent2, + layer_masks_parent2); + + /* Stops when all accesses are granted. */ + if (allowed_parent1 && allowed_parent2) + break; } rule = find_rule(domain, walker_path.dentry); - allowed_parent1 = landlock_unmask_layers( - rule, access_masked_parent1, layer_masks_parent1, - ARRAY_SIZE(*layer_masks_parent1)); - allowed_parent2 = landlock_unmask_layers( - rule, access_masked_parent2, layer_masks_parent2, - ARRAY_SIZE(*layer_masks_parent2)); + allowed_parent1 = allowed_parent1 || + landlock_unmask_layers( + rule, access_masked_parent1, + layer_masks_parent1, + ARRAY_SIZE(*layer_masks_parent1)); + allowed_parent2 = allowed_parent2 || + landlock_unmask_layers( + rule, access_masked_parent2, + layer_masks_parent2, + ARRAY_SIZE(*layer_masks_parent2)); /* Stops when a rule from each layer grants access. */ if (allowed_parent1 && allowed_parent2) @@ -900,8 +908,10 @@ jump_up: * access to internal filesystems (e.g. nsfs, which is * reachable through /proc//ns/). */ - allowed_parent1 = allowed_parent2 = - !!(walker_path.mnt->mnt_flags & MNT_INTERNAL); + if (walker_path.mnt->mnt_flags & MNT_INTERNAL) { + allowed_parent1 = true; + allowed_parent2 = true; + } break; } parent_dentry = dget_parent(walker_path.dentry); -- cgit v1.2.3