From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F05DC43217 for ; Fri, 18 Nov 2022 09:16:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241823AbiKRJQp (ORCPT ); Fri, 18 Nov 2022 04:16:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34596 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241853AbiKRJQ1 (ORCPT ); Fri, 18 Nov 2022 04:16:27 -0500 Received: from smtp-42a8.mail.infomaniak.ch (smtp-42a8.mail.infomaniak.ch [IPv6:2001:1600:4:17::42a8]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B69048B13B for ; Fri, 18 Nov 2022 01:16:24 -0800 (PST) Received: from smtp-2-0000.mail.infomaniak.ch (unknown [10.5.36.107]) by smtp-3-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 4NDB3q1R4yzMq9pl; Fri, 18 Nov 2022 10:16:23 +0100 (CET) Received: from ns3096276.ip-94-23-54.eu (unknown [23.97.221.149]) by smtp-2-0000.mail.infomaniak.ch (Postfix) with ESMTPA id 4NDB3p4Gr5zMppFG; Fri, 18 Nov 2022 10:16:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=digikod.net; s=20191114; t=1668762983; bh=WmJqtFGITHPNH6hvWB7131Vucaosx6JH+KXGbIiVYqA=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=gqdfNSoZDyPuRAE6h1t81Ua6sjsWVWGYoewzjtWyJbmH0k0mcJQGvGHw/Yb+cgaU3 OfkW9dOHZkSG5nztgac0CtGxj6/2lxYJUKBH2yA5qjAUK9hy05Hq7JwpgZXMoU97ci 8JTUocetwgGRQqT1psWiARphh47FyMTv/Zm08bsY= Message-ID: Date: Fri, 18 Nov 2022 10:16:21 +0100 MIME-Version: 1.0 User-Agent: Subject: Re: [PATCH] landlock: Allow filesystem layout changes for domains without such rule type Content-Language: en-US To: Konstantin Meskhidze Cc: artem.kuzin@huawei.com, gnoack3000@gmail.com, willemdebruijn.kernel@gmail.com, linux-security-module@vger.kernel.org, netdev@vger.kernel.org, netfilter-devel@vger.kernel.org References: <5c6c99f7-4218-1f79-477e-5d943c9809fd@digikod.net> <20221117185509.702361-1-mic@digikod.net> From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= In-Reply-To: <20221117185509.702361-1-mic@digikod.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: Konstantin, this patch should apply cleanly just after "01/12 landlock: Make ruleset's access masks more generic". You can easily get this patch with https://git.kernel.org/pub/scm/utils/b4/b4.git/ Some adjustments are needed for the following patches. Feel free to review this patch. On 17/11/2022 19:55, Mickaël Salaün wrote: > 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. > > Signed-off-by: Mickaël Salaün > --- > security/landlock/fs.c | 73 ++++++++++++++++++------------------ > security/landlock/ruleset.h | 25 +++++++++++- > security/landlock/syscalls.c | 6 +-- > 3 files changed, 62 insertions(+), 42 deletions(-) > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c > index 0d57c6479d29..0ae54a639e16 100644 > --- a/security/landlock/fs.c > +++ b/security/landlock/fs.c > @@ -150,16 +150,6 @@ static struct landlock_object *get_inode_object(struct inode *const inode) > 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,14 +276,15 @@ 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); > + access_dom |= > + landlock_get_raw_fs_access_mask(domain, layer_level); > return access_dom & LANDLOCK_MASK_ACCESS_FS; > } > > @@ -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,24 @@ 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) | 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 +522,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)) > @@ -648,11 +651,10 @@ static inline int check_access_path(const struct landlock_ruleset *const domain, > return -EACCES; > } > > -static inline int current_check_access_path(const struct path *const path, > +static 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 +817,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 +1051,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 +1059,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 +1070,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 +1093,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 +1129,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 +1208,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 f2ad932d396c..ca46393ef3bb 100644 > --- a/security/landlock/ruleset.h > +++ b/security/landlock/ruleset.h > @@ -15,10 +15,21 @@ > #include > #include > #include > +#include > > #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 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); > @@ -197,11 +208,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) | > + ACCESS_FS_INITIALLY_DENIED; > +} > + > #endif /* _SECURITY_LANDLOCK_RULESET_H */ > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c > index 71aca7f990bc..d35cd5d304db 100644 > --- a/security/landlock/syscalls.c > +++ b/security/landlock/syscalls.c > @@ -310,6 +310,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, > struct path path; > struct landlock_ruleset *ruleset; > int res, err; > + access_mask_t mask; > > if (!landlock_initialized) > return -EOPNOTSUPP; > @@ -348,9 +349,8 @@ 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). > */ > - if ((path_beneath_attr.allowed_access | > - landlock_get_fs_access_mask(ruleset, 0)) != > - 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; > }