From: "Mickaël Salaün" <mic@digikod.net>
To: Paul Moore <paul@paul-moore.com>
Cc: "James Morris" <jmorris@namei.org>,
"Serge E . Hallyn" <serge@hallyn.com>,
"Al Viro" <viro@zeniv.linux.org.uk>,
"Jann Horn" <jannh@google.com>,
"Kees Cook" <keescook@chromium.org>,
"Konstantin Meskhidze" <konstantin.meskhidze@huawei.com>,
"Shuah Khan" <shuah@kernel.org>,
linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
"Mickaël Salaün" <mic@linux.microsoft.com>
Subject: Re: [PATCH v1 01/11] landlock: Define access_mask_t to enforce a consistent access mask size
Date: Thu, 17 Mar 2022 09:36:36 +0100 [thread overview]
Message-ID: <ed8467f2-dcd0-bc2f-8e98-1d9129fb2c30@digikod.net> (raw)
In-Reply-To: <CAHC9VhQEEKGgCn7fYgUt-_WhXc-vrKq9TVm=cfwJUyWaUgY2Vw@mail.gmail.com>
On 17/03/2022 02:26, Paul Moore wrote:
> On Mon, Feb 21, 2022 at 4:15 PM Mickaël Salaün <mic@digikod.net> wrote:
>>
>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>
>> Create and use the access_mask_t typedef to enforce a consistent access
>> mask size and uniformly use a 16-bits type. This will helps transition
>> to a 32-bits value one day.
>>
>> Add a build check to make sure all (filesystem) access rights fit in.
>> This will be extended with a following commit.
>>
>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
>> Link: https://lore.kernel.org/r/20220221212522.320243-2-mic@digikod.net
>> ---
>> security/landlock/fs.c | 19 ++++++++++---------
>> security/landlock/fs.h | 2 +-
>> security/landlock/limits.h | 2 ++
>> security/landlock/ruleset.c | 6 ++++--
>> security/landlock/ruleset.h | 17 +++++++++++++----
>> 5 files changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
>> index 97b8e421f617..9de2a460a762 100644
>> --- a/security/landlock/fs.c
>> +++ b/security/landlock/fs.c
>> @@ -150,7 +150,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
>> * @path: Should have been checked by get_path_from_fd().
>> */
>> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>> - const struct path *const path, u32 access_rights)
>> + const struct path *const path, access_mask_t access_rights)
>> {
>> int err;
>> struct landlock_object *object;
>> @@ -182,8 +182,8 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>>
>> static inline u64 unmask_layers(
>> const struct landlock_ruleset *const domain,
>> - const struct path *const path, const u32 access_request,
>> - u64 layer_mask)
>> + const struct path *const path,
>> + const access_mask_t access_request, u64 layer_mask)
>> {
>> const struct landlock_rule *rule;
>> const struct inode *inode;
>> @@ -223,7 +223,8 @@ static inline u64 unmask_layers(
>> }
>>
>> static int check_access_path(const struct landlock_ruleset *const domain,
>> - const struct path *const path, u32 access_request)
>> + const struct path *const path,
>> + const access_mask_t access_request)
>> {
>> bool allowed = false;
>> struct path walker_path;
>> @@ -308,7 +309,7 @@ static int check_access_path(const struct landlock_ruleset *const domain,
>> }
>>
>> static inline int current_check_access_path(const struct path *const path,
>> - const u32 access_request)
>> + const access_mask_t access_request)
>> {
>> const struct landlock_ruleset *const dom =
>> landlock_get_current_domain();
>> @@ -511,7 +512,7 @@ static int hook_sb_pivotroot(const struct path *const old_path,
>>
>> /* Path hooks */
>>
>> -static inline u32 get_mode_access(const umode_t mode)
>> +static inline access_mask_t get_mode_access(const umode_t mode)
>> {
>> switch (mode & S_IFMT) {
>> case S_IFLNK:
>> @@ -563,7 +564,7 @@ static int hook_path_link(struct dentry *const old_dentry,
>> get_mode_access(d_backing_inode(old_dentry)->i_mode));
>> }
>>
>> -static inline u32 maybe_remove(const struct dentry *const dentry)
>> +static inline access_mask_t maybe_remove(const struct dentry *const dentry)
>> {
>> if (d_is_negative(dentry))
>> return 0;
>> @@ -631,9 +632,9 @@ static int hook_path_rmdir(const struct path *const dir,
>>
>> /* File hooks */
>>
>> -static inline u32 get_file_access(const struct file *const file)
>> +static inline access_mask_t get_file_access(const struct file *const file)
>> {
>> - u32 access = 0;
>> + access_mask_t access = 0;
>>
>> if (file->f_mode & FMODE_READ) {
>> /* A directory can only be opened in read mode. */
>> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
>> index 187284b421c9..74be312aad96 100644
>> --- a/security/landlock/fs.h
>> +++ b/security/landlock/fs.h
>> @@ -65,6 +65,6 @@ static inline struct landlock_superblock_security *landlock_superblock(
>> __init void landlock_add_fs_hooks(void);
>>
>> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>> - const struct path *const path, u32 access_hierarchy);
>> + const struct path *const path, access_mask_t access_hierarchy);
>>
>> #endif /* _SECURITY_LANDLOCK_FS_H */
>> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
>> index 2a0a1095ee27..458d1de32ed5 100644
>> --- a/security/landlock/limits.h
>> +++ b/security/landlock/limits.h
>> @@ -9,6 +9,7 @@
>> #ifndef _SECURITY_LANDLOCK_LIMITS_H
>> #define _SECURITY_LANDLOCK_LIMITS_H
>>
>> +#include <linux/bitops.h>
>> #include <linux/limits.h>
>> #include <uapi/linux/landlock.h>
>>
>> @@ -17,5 +18,6 @@
>>
>> #define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_MAKE_SYM
>> #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
>> +#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>
> The line above, and the static_assert() in ruleset.h are clever. I'll
> admit I didn't even know the hweightX() macros existed until looking
> at this code :)
>
> However, the LANDLOCK_NUM_ACCESS_FS is never really going to be used
> outside the static_assert() in ruleset.h is it? I wonder if it would
> be better to skip the extra macro and rewrite the static_assert like
> this:
>
> static_assert(BITS_PER_TYPE(access_mask_t) >=
> __const_hweight64(LANDLOCK_MASK_ACCESS_FS));
>
> If not, I might suggest changing LANDLOCK_NUM_ACCESS_FS to
> LANDLOCK_BITS_ACCESS_FS or something similar.
I declared LANDLOCK_NUM_ACCESS_FS in this patch to be able to have the
static_assert() here and ease the review, but LANDLOCK_NUM_ACCESS_FS is
really used in patch 6/11 to define an array size:
get_handled_acceses(), init_layer_masks(), is_superset(),
check_access_path_dual()…
>
>
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index 2d3ed7ec5a0a..7e7cac68e443 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -9,13 +9,20 @@
>> #ifndef _SECURITY_LANDLOCK_RULESET_H
>> #define _SECURITY_LANDLOCK_RULESET_H
>>
>> +#include <linux/bitops.h>
>> +#include <linux/build_bug.h>
>> #include <linux/mutex.h>
>> #include <linux/rbtree.h>
>> #include <linux/refcount.h>
>> #include <linux/workqueue.h>
>>
>> +#include "limits.h"
>> #include "object.h"
>>
>> +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);
>
> --
> paul-moore.com
next prev parent reply other threads:[~2022-03-17 8:36 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-21 21:25 [PATCH v1 00/11] Landlock: file linking and renaming support Mickaël Salaün
2022-02-21 21:25 ` [PATCH v1 01/11] landlock: Define access_mask_t to enforce a consistent access mask size Mickaël Salaün
2022-03-17 1:26 ` Paul Moore
2022-03-17 8:36 ` Mickaël Salaün [this message]
2022-03-17 21:31 ` Paul Moore
2022-02-21 21:25 ` [PATCH v1 02/11] landlock: Reduce the maximum number of layers to 16 Mickaël Salaün
2022-03-17 1:26 ` Paul Moore
2022-02-21 21:25 ` [PATCH v1 03/11] landlock: Create find_rule() from unmask_layers() Mickaël Salaün
2022-03-17 1:26 ` Paul Moore
2022-02-21 21:25 ` [PATCH v1 04/11] landlock: Fix same-layer rule unions Mickaël Salaün
2022-03-17 1:26 ` Paul Moore
2022-03-17 10:41 ` Mickaël Salaün
2022-03-17 21:34 ` Paul Moore
2022-02-21 21:25 ` [PATCH v1 05/11] landlock: Move filesystem helpers and add a new one Mickaël Salaün
2022-03-17 1:26 ` Paul Moore
2022-03-17 10:42 ` Mickaël Salaün
2022-02-21 21:25 ` [PATCH v1 06/11] landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER Mickaël Salaün
2022-02-22 3:16 ` kernel test robot
2022-02-22 10:18 ` Mickaël Salaün
2022-03-17 1:26 ` Paul Moore
2022-03-17 12:04 ` Mickaël Salaün
2022-03-17 21:42 ` Paul Moore
2022-03-24 10:31 ` Mickaël Salaün
2022-02-21 21:25 ` [PATCH v1 07/11] selftest/landlock: Add 6 new test suites dedicated to file reparenting Mickaël Salaün
2022-02-21 21:25 ` [PATCH v1 08/11] samples/landlock: Add support for " Mickaël Salaün
2022-03-17 1:26 ` Paul Moore
2022-02-21 21:25 ` [PATCH v1 09/11] landlock: Document LANDLOCK_ACCESS_FS_REFER and ABI versioning Mickaël Salaün
2022-03-17 1:27 ` Paul Moore
2022-03-17 12:06 ` Mickaël Salaün
2022-02-21 21:25 ` [PATCH v1 10/11] landlock: Document good practices about filesystem policies Mickaël Salaün
2022-03-17 1:27 ` Paul Moore
2022-02-21 21:25 ` [PATCH v1 11/11] landlock: Add design choices documentation for filesystem access rights Mickaël Salaün
2022-03-17 1:27 ` Paul Moore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ed8467f2-dcd0-bc2f-8e98-1d9129fb2c30@digikod.net \
--to=mic@digikod.net \
--cc=jannh@google.com \
--cc=jmorris@namei.org \
--cc=keescook@chromium.org \
--cc=konstantin.meskhidze@huawei.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@linux.microsoft.com \
--cc=paul@paul-moore.com \
--cc=serge@hallyn.com \
--cc=shuah@kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).