June 2022 - Linux-stable-mirror

WTF: patch "[PATCH] landlock: Fix same-layer rule unions" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 8ba0005ff418ec356e176b26eaa04a6ac755d05b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:10:54 +0200 Subject: [PATCH] landlock: Fix same-layer rule unions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original behavior was to check if the full set of requested accesses was allowed by at least a rule of every relevant layer. This didn't take into account requests for multiple accesses and same-layer rules allowing the union of these accesses in a complementary way. As a result, multiple accesses requested on a file hierarchy matching rules that, together, allowed these accesses, but without a unique rule allowing all of them, was illegitimately denied. This case should be rare in practice and it can only be triggered by the path_rename or file_open hook implementations. For instance, if, for the same layer, a rule allows execution beneath /a/b and another rule allows read beneath /a, requesting access to read and execute at the same time for /a/b should be allowed for this layer. This was an inconsistency because the union of same-layer rule accesses was already allowed if requested once at a time anyway. This fix changes the way allowed accesses are gathered over a path walk. To take into account all these rule accesses, we store in a matrix all layer granting the set of requested accesses, according to the handled accesses. To avoid heap allocation, we use an array on the stack which is 2*13 bytes. A following commit bringing the LANDLOCK_ACCESS_FS_REFER access right will increase this size to reach 112 bytes (2*14*4) in case of link or rename actions. Add a new layout1.layer_rule_unions test to check that accesses from different rules pertaining to the same layer are ORed in a file hierarchy. Also test that it is not the case for rules from different layers. Reviewed-by: Paul Moore <paul(a)paul-moore.com> Link: https://lore.kernel.org/r/20220506161102.525323-5-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 20953bff8fd5..c5749301b37d 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -207,45 +207,67 @@ find_rule(const struct landlock_ruleset *const domain, return rule; } -static inline layer_mask_t unmask_layers(const struct landlock_rule *const rule, - const access_mask_t access_request, - layer_mask_t layer_mask) +/* + * @layer_masks is read and may be updated according to the access request and + * the matching rule. + * + * Returns true if the request is allowed (i.e. relevant layer masks for the + * request are empty). + */ +static inline bool +unmask_layers(const struct landlock_rule *const rule, + const access_mask_t access_request, + layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]) { size_t layer_level; + if (!access_request || !layer_masks) + return true; if (!rule) - return layer_mask; + return false; /* * An access is granted if, for each policy layer, at least one rule - * encountered on the pathwalk grants the requested accesses, - * regardless of their position in the layer stack. We must then check + * encountered on the pathwalk grants the requested access, + * regardless of its position in the layer stack. We must then check * the remaining layers for each inode, from the first added layer to - * the last one. + * the last one. When there is multiple requested accesses, for each + * policy layer, the full set of requested accesses may not be granted + * by only one rule, but by the union (binary OR) of multiple rules. + * E.g. /a/b <execute> + /a <read> => /a/b <execute + read> */ for (layer_level = 0; layer_level < rule->num_layers; layer_level++) { const struct landlock_layer *const layer = &rule->layers[layer_level]; const layer_mask_t layer_bit = BIT_ULL(layer->level - 1); + const unsigned long access_req = access_request; + unsigned long access_bit; + bool is_empty; - /* Checks that the layer grants access to the full request. */ - if ((layer->access & access_request) == access_request) { - layer_mask &= ~layer_bit; - - if (layer_mask == 0) - return layer_mask; + /* + * Records in @layer_masks which layer grants access to each + * requested access. + */ + is_empty = true; + for_each_set_bit(access_bit, &access_req, + ARRAY_SIZE(*layer_masks)) { + if (layer->access & BIT_ULL(access_bit)) + (*layer_masks)[access_bit] &= ~layer_bit; + is_empty = is_empty && !(*layer_masks)[access_bit]; } + if (is_empty) + return true; } - return layer_mask; + return false; } static int check_access_path(const struct landlock_ruleset *const domain, const struct path *const path, const access_mask_t access_request) { - bool allowed = false; + layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {}; + bool allowed = false, has_access = false; struct path walker_path; - layer_mask_t layer_mask; size_t i; if (!access_request) @@ -265,13 +287,20 @@ static int check_access_path(const struct landlock_ruleset *const domain, return -EACCES; /* Saves all layers handling a subset of requested accesses. */ - layer_mask = 0; for (i = 0; i < domain->num_layers; i++) { - if (domain->fs_access_masks[i] & access_request) - layer_mask |= BIT_ULL(i); + const unsigned long access_req = access_request; + unsigned long access_bit; + + for_each_set_bit(access_bit, &access_req, + ARRAY_SIZE(layer_masks)) { + if (domain->fs_access_masks[i] & BIT_ULL(access_bit)) { + layer_masks[access_bit] |= BIT_ULL(i); + has_access = true; + } + } } /* An access request not handled by the domain is allowed. */ - if (layer_mask == 0) + if (!has_access) return 0; walker_path = *path; @@ -283,14 +312,11 @@ static int check_access_path(const struct landlock_ruleset *const domain, while (true) { struct dentry *parent_dentry; - layer_mask = - unmask_layers(find_rule(domain, walker_path.dentry), - access_request, layer_mask); - if (layer_mask == 0) { + allowed = unmask_layers(find_rule(domain, walker_path.dentry), + access_request, &layer_masks); + if (allowed) /* Stops when a rule from each layer grants access. */ - allowed = true; break; - } jump_up: if (walker_path.dentry == walker_path.mnt->mnt_root) { diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index 521af2848951..d43231b783e4 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -22,6 +22,8 @@ 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 for_each_set_bit() and for_each_clear_bit() calls are OK. */ +static_assert(sizeof(unsigned long) >= sizeof(access_mask_t)); typedef u16 layer_mask_t; /* Makes sure all layers can be checked. */ diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index e13f046a172a..a4fdcda62bde 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -758,6 +758,113 @@ TEST_F_FORK(layout1, ruleset_overlap) ASSERT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY)); } +TEST_F_FORK(layout1, layer_rule_unions) +{ + const struct rule layer1[] = { + { + .path = dir_s1d2, + .access = LANDLOCK_ACCESS_FS_READ_FILE, + }, + /* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */ + { + .path = dir_s1d3, + .access = LANDLOCK_ACCESS_FS_WRITE_FILE, + }, + {}, + }; + const struct rule layer2[] = { + /* Doesn't change anything from layer1. */ + { + .path = dir_s1d2, + .access = LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_WRITE_FILE, + }, + {}, + }; + const struct rule layer3[] = { + /* Only allows write (but not read) to dir_s1d3. */ + { + .path = dir_s1d2, + .access = LANDLOCK_ACCESS_FS_WRITE_FILE, + }, + {}, + }; + int ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1); + + ASSERT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + ASSERT_EQ(0, close(ruleset_fd)); + + /* Checks s1d1 hierarchy with layer1. */ + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Checks s1d2 hierarchy with layer1. */ + ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Checks s1d3 hierarchy with layer1. */ + ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY)); + ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY)); + /* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */ + ASSERT_EQ(0, test_open(file1_s1d3, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Doesn't change anything from layer1. */ + ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer2); + ASSERT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + ASSERT_EQ(0, close(ruleset_fd)); + + /* Checks s1d1 hierarchy with layer2. */ + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Checks s1d2 hierarchy with layer2. */ + ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Checks s1d3 hierarchy with layer2. */ + ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY)); + ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY)); + /* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */ + ASSERT_EQ(0, test_open(file1_s1d3, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Only allows write (but not read) to dir_s1d3. */ + ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer3); + ASSERT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + ASSERT_EQ(0, close(ruleset_fd)); + + /* Checks s1d1 hierarchy with layer3. */ + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Checks s1d2 hierarchy with layer3. */ + ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY)); + ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); + + /* Checks s1d3 hierarchy with layer3. */ + ASSERT_EQ(EACCES, test_open(file1_s1d3, O_RDONLY)); + ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY)); + /* dir_s1d3 should now deny READ_FILE and WRITE_FILE (O_RDWR). */ + ASSERT_EQ(EACCES, test_open(file1_s1d3, O_RDWR)); + ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY)); +} + TEST_F_FORK(layout1, non_overlapping_accesses) { const struct rule layer1[] = {

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] landlock: Create find_rule() from unmask_layers()" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 2cd7cd6eed88b8383cfddce589afe9c0ae1d19b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:10:53 +0200 Subject: [PATCH] landlock: Create find_rule() from unmask_layers() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This refactoring will be useful in a following commit. Reviewed-by: Paul Moore <paul(a)paul-moore.com> Link: https://lore.kernel.org/r/20220506161102.525323-4-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/security/landlock/fs.c b/security/landlock/fs.c index f48c0a3b1e75..20953bff8fd5 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -183,23 +183,36 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset, /* Access-control management */ -static inline layer_mask_t -unmask_layers(const struct landlock_ruleset *const domain, - const struct path *const path, const access_mask_t access_request, - layer_mask_t layer_mask) +/* + * The lifetime of the returned rule is tied to @domain. + * + * Returns NULL if no rule is found or if @dentry is negative. + */ +static inline const struct landlock_rule * +find_rule(const struct landlock_ruleset *const domain, + const struct dentry *const dentry) { const struct landlock_rule *rule; const struct inode *inode; - size_t i; - if (d_is_negative(path->dentry)) - /* Ignore nonexistent leafs. */ - return layer_mask; - inode = d_backing_inode(path->dentry); + /* Ignores nonexistent leafs. */ + if (d_is_negative(dentry)) + return NULL; + + inode = d_backing_inode(dentry); rcu_read_lock(); rule = landlock_find_rule( domain, rcu_dereference(landlock_inode(inode)->object)); rcu_read_unlock(); + return rule; +} + +static inline layer_mask_t unmask_layers(const struct landlock_rule *const rule, + const access_mask_t access_request, + layer_mask_t layer_mask) +{ + size_t layer_level; + if (!rule) return layer_mask; @@ -210,8 +223,9 @@ unmask_layers(const struct landlock_ruleset *const domain, * the remaining layers for each inode, from the first added layer to * the last one. */ - for (i = 0; i < rule->num_layers; i++) { - const struct landlock_layer *const layer = &rule->layers[i]; + for (layer_level = 0; layer_level < rule->num_layers; layer_level++) { + const struct landlock_layer *const layer = + &rule->layers[layer_level]; const layer_mask_t layer_bit = BIT_ULL(layer->level - 1); /* Checks that the layer grants access to the full request. */ @@ -269,8 +283,9 @@ static int check_access_path(const struct landlock_ruleset *const domain, while (true) { struct dentry *parent_dentry; - layer_mask = unmask_layers(domain, &walker_path, access_request, - layer_mask); + layer_mask = + unmask_layers(find_rule(domain, walker_path.dentry), + access_request, layer_mask); if (layer_mask == 0) { /* Stops when a rule from each layer grants access. */ allowed = true;

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] landlock: Change landlock_restrict_self(2) check ordering" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From eba39ca4b155c54adf471a69e91799cc1727873f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:08:19 +0200 Subject: [PATCH] landlock: Change landlock_restrict_self(2) check ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the Landlock goal to be a security feature available to unprivileges processes, it makes more sense to first check for no_new_privs before checking anything else (i.e. syscall arguments). Merge inval_fd_enforce and unpriv_enforce_without_no_new_privs tests into the new restrict_self_checks_ordering. This is similar to the previous commit checking other syscalls. Link: https://lore.kernel.org/r/20220506160820.524344-10-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index a7396220c9d4..507d43827afe 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -405,10 +405,6 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, if (!landlock_initialized) return -EOPNOTSUPP; - /* No flag for now. */ - if (flags) - return -EINVAL; - /* * Similar checks as for seccomp(2), except that an -EPERM may be * returned. @@ -417,6 +413,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN)) return -EPERM; + /* No flag for now. */ + if (flags) + return -EINVAL; + /* Gets and checks the ruleset. */ ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ); if (IS_ERR(ruleset)) diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c index 18b779471dcb..21fb33581419 100644 --- a/tools/testing/selftests/landlock/base_test.c +++ b/tools/testing/selftests/landlock/base_test.c @@ -168,22 +168,49 @@ TEST(add_rule_checks_ordering) ASSERT_EQ(0, close(ruleset_fd)); } -TEST(inval_fd_enforce) +/* Tests ordering of syscall argument and permission checks. */ +TEST(restrict_self_checks_ordering) { + const struct landlock_ruleset_attr ruleset_attr = { + .handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE, + }; + struct landlock_path_beneath_attr path_beneath_attr = { + .allowed_access = LANDLOCK_ACCESS_FS_EXECUTE, + .parent_fd = -1, + }; + const int ruleset_fd = + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); + + ASSERT_LE(0, ruleset_fd); + path_beneath_attr.parent_fd = + open("/tmp", O_PATH | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC); + ASSERT_LE(0, path_beneath_attr.parent_fd); + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, + &path_beneath_attr, 0)); + ASSERT_EQ(0, close(path_beneath_attr.parent_fd)); + + /* Checks unprivileged enforcement without no_new_privs. */ + drop_caps(_metadata); + ASSERT_EQ(-1, landlock_restrict_self(-1, -1)); + ASSERT_EQ(EPERM, errno); + ASSERT_EQ(-1, landlock_restrict_self(-1, 0)); + ASSERT_EQ(EPERM, errno); + ASSERT_EQ(-1, landlock_restrict_self(ruleset_fd, 0)); + ASSERT_EQ(EPERM, errno); + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)); + /* Checks invalid flags. */ + ASSERT_EQ(-1, landlock_restrict_self(-1, -1)); + ASSERT_EQ(EINVAL, errno); + + /* Checks invalid ruleset FD. */ ASSERT_EQ(-1, landlock_restrict_self(-1, 0)); ASSERT_EQ(EBADF, errno); -} - -TEST(unpriv_enforce_without_no_new_privs) -{ - int err; - drop_caps(_metadata); - err = landlock_restrict_self(-1, 0); - ASSERT_EQ(EPERM, errno); - ASSERT_EQ(err, -1); + /* Checks valid call. */ + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0)); + ASSERT_EQ(0, close(ruleset_fd)); } TEST(ruleset_fd_io)

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] landlock: Define access_mask_t to enforce a consistent access" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 5f2ff33e10843ef51275c8611bdb7b49537aba5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:10:51 +0200 Subject: [PATCH] landlock: Define access_mask_t to enforce a consistent access mask size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Reviewed-by: Paul Moore <paul(a)paul-moore.com> Link: https://lore.kernel.org/r/20220506161102.525323-2-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/security/landlock/fs.c b/security/landlock/fs.c index eeecf5b2fa89..d4006add8bdf 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -152,7 +152,8 @@ 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; @@ -184,7 +185,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 access_mask_t access_request, + u64 layer_mask) { const struct landlock_rule *rule; const struct inode *inode; @@ -224,7 +226,8 @@ static inline u64 unmask_layers(const struct landlock_ruleset *const domain, } 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; @@ -309,7 +312,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(); @@ -512,7 +515,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: @@ -565,7 +568,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; @@ -635,9 +638,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 03f746e74e9e..8db7acf9109b 100644 --- a/security/landlock/fs.h +++ b/security/landlock/fs.h @@ -66,6 +66,6 @@ __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); + access_mask_t access_hierarchy); #endif /* _SECURITY_LANDLOCK_FS_H */ diff --git a/security/landlock/limits.h b/security/landlock/limits.h index a274ae6b5570..41372f22837f 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> @@ -19,6 +20,7 @@ #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) /* clang-format on */ diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index 4d33359addbd..996484f98bfd 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -45,7 +45,8 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) return new_ruleset; } -struct landlock_ruleset *landlock_create_ruleset(const u32 fs_access_mask) +struct landlock_ruleset * +landlock_create_ruleset(const access_mask_t fs_access_mask) { struct landlock_ruleset *new_ruleset; @@ -228,7 +229,8 @@ static void build_check_layer(void) /* @ruleset must be locked by the caller. */ int landlock_insert_rule(struct landlock_ruleset *const ruleset, - struct landlock_object *const object, const u32 access) + struct landlock_object *const object, + const access_mask_t access) { struct landlock_layer layers[] = { { .access = access, diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index e9ba47045aca..8d5717594931 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); + /** * struct landlock_layer - Access rights for a given layer */ @@ -28,7 +35,7 @@ struct landlock_layer { * @access: Bitfield of allowed actions on the kernel object. They are * relative to the object type (e.g. %LANDLOCK_ACTION_FS_READ). */ - u16 access; + access_mask_t access; }; /** @@ -135,19 +142,20 @@ struct landlock_ruleset { * layers are set once and never changed for the * lifetime of the ruleset. */ - u16 fs_access_masks[]; + access_mask_t fs_access_masks[]; }; }; }; -struct landlock_ruleset *landlock_create_ruleset(const u32 fs_access_mask); +struct landlock_ruleset * +landlock_create_ruleset(const access_mask_t fs_access_mask); void landlock_put_ruleset(struct landlock_ruleset *const ruleset); void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset); int landlock_insert_rule(struct landlock_ruleset *const ruleset, struct landlock_object *const object, - const u32 access); + const access_mask_t access); struct landlock_ruleset * landlock_merge_ruleset(struct landlock_ruleset *const parent,

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] selftests/landlock: Add tests for O_PATH" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From d1788ad990874734341b05ab8ccb6448c09c6422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:08:17 +0200 Subject: [PATCH] selftests/landlock: Add tests for O_PATH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The O_PATH flag is currently not handled by Landlock. Let's make sure this behavior will remain consistent with the same ruleset over time. Cc: Shuah Khan <shuah(a)kernel.org> Link: https://lore.kernel.org/r/20220506160820.524344-8-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 9165f6adf7b9..a8f54c4462eb 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -654,17 +654,23 @@ TEST_F_FORK(layout1, effective_access) enforce_ruleset(_metadata, ruleset_fd); ASSERT_EQ(0, close(ruleset_fd)); - /* Tests on a directory. */ + /* Tests on a directory (with or without O_PATH). */ ASSERT_EQ(EACCES, test_open("/", O_RDONLY)); + ASSERT_EQ(0, test_open("/", O_RDONLY | O_PATH)); ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY)); + ASSERT_EQ(0, test_open(dir_s1d1, O_RDONLY | O_PATH)); ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY)); + ASSERT_EQ(0, test_open(file1_s1d1, O_RDONLY | O_PATH)); + ASSERT_EQ(0, test_open(dir_s1d2, O_RDONLY)); ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY)); ASSERT_EQ(0, test_open(dir_s1d3, O_RDONLY)); ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY)); - /* Tests on a file. */ + /* Tests on a file (with or without O_PATH). */ ASSERT_EQ(EACCES, test_open(dir_s2d2, O_RDONLY)); + ASSERT_EQ(0, test_open(dir_s2d2, O_RDONLY | O_PATH)); + ASSERT_EQ(0, test_open(file1_s2d2, O_RDONLY)); /* Checks effective read and write actions. */

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] landlock: Reduce the maximum number of layers to 16" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 75c542d6c6cc48720376862d5496d51509160dfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:10:52 +0200 Subject: [PATCH] landlock: Reduce the maximum number of layers to 16 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The maximum number of nested Landlock domains is currently 64. Because of the following fix and to help reduce the stack size, let's reduce it to 16. This seems large enough for a lot of use cases (e.g. sandboxed init service, spawning a sandboxed SSH service, in nested sandboxed containers). Reducing the number of nested domains may also help to discover misuse of Landlock (e.g. creating a domain per rule). Add and use a dedicated layer_mask_t typedef to fit with the number of layers. This might be useful when changing it and to keep it consistent with the maximum number of layers. Reviewed-by: Paul Moore <paul(a)paul-moore.com> Link: https://lore.kernel.org/r/20220506161102.525323-3-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst index f35552ff19ba..b68e7a51009f 100644 --- a/Documentation/userspace-api/landlock.rst +++ b/Documentation/userspace-api/landlock.rst @@ -267,8 +267,8 @@ restrict such paths with dedicated ruleset flags. Ruleset layers -------------- -There is a limit of 64 layers of stacked rulesets. This can be an issue for a -task willing to enforce a new ruleset in complement to its 64 inherited +There is a limit of 16 layers of stacked rulesets. This can be an issue for a +task willing to enforce a new ruleset in complement to its 16 inherited rulesets. Once this limit is reached, sys_landlock_restrict_self() returns E2BIG. It is then strongly suggested to carefully build rulesets once in the life of a thread, especially for applications able to launch other applications diff --git a/security/landlock/fs.c b/security/landlock/fs.c index d4006add8bdf..f48c0a3b1e75 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -183,10 +183,10 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset, /* Access-control management */ -static inline u64 unmask_layers(const struct landlock_ruleset *const domain, - const struct path *const path, - const access_mask_t access_request, - u64 layer_mask) +static inline layer_mask_t +unmask_layers(const struct landlock_ruleset *const domain, + const struct path *const path, const access_mask_t access_request, + layer_mask_t layer_mask) { const struct landlock_rule *rule; const struct inode *inode; @@ -212,11 +212,11 @@ static inline u64 unmask_layers(const struct landlock_ruleset *const domain, */ for (i = 0; i < rule->num_layers; i++) { const struct landlock_layer *const layer = &rule->layers[i]; - const u64 layer_level = BIT_ULL(layer->level - 1); + const layer_mask_t layer_bit = BIT_ULL(layer->level - 1); /* Checks that the layer grants access to the full request. */ if ((layer->access & access_request) == access_request) { - layer_mask &= ~layer_level; + layer_mask &= ~layer_bit; if (layer_mask == 0) return layer_mask; @@ -231,12 +231,9 @@ static int check_access_path(const struct landlock_ruleset *const domain, { bool allowed = false; struct path walker_path; - u64 layer_mask; + layer_mask_t layer_mask; size_t i; - /* Make sure all layers can be checked. */ - BUILD_BUG_ON(BITS_PER_TYPE(layer_mask) < LANDLOCK_MAX_NUM_LAYERS); - if (!access_request) return 0; if (WARN_ON_ONCE(!domain || !path)) diff --git a/security/landlock/limits.h b/security/landlock/limits.h index 41372f22837f..17c2a2e7fe1e 100644 --- a/security/landlock/limits.h +++ b/security/landlock/limits.h @@ -15,7 +15,7 @@ /* clang-format off */ -#define LANDLOCK_MAX_NUM_LAYERS 64 +#define LANDLOCK_MAX_NUM_LAYERS 16 #define LANDLOCK_MAX_NUM_RULES U32_MAX #define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_MAKE_SYM diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h index 8d5717594931..521af2848951 100644 --- a/security/landlock/ruleset.h +++ b/security/landlock/ruleset.h @@ -23,6 +23,10 @@ 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); +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 */ diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index a8f54c4462eb..e13f046a172a 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -1159,7 +1159,7 @@ TEST_F_FORK(layout1, max_layers) const int ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules); ASSERT_LE(0, ruleset_fd); - for (i = 0; i < 64; i++) + for (i = 0; i < 16; i++) enforce_ruleset(_metadata, ruleset_fd); for (i = 0; i < 2; i++) {

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] selftests/landlock: Make tests build with old libc" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 87129ef13603ae46c82bcd09eed948acf0506dbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:08:12 +0200 Subject: [PATCH] selftests/landlock: Make tests build with old libc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace SYS_<syscall> with __NR_<syscall>. Using the __NR_<syscall> notation, provided by UAPI, is useful to build tests on systems without the SYS_<syscall> definitions. Replace SYS_pivot_root with __NR_pivot_root, and SYS_move_mount with __NR_move_mount. Define renameat2() and RENAME_EXCHANGE if they are unknown to old build systems. Cc: Shuah Khan <shuah(a)kernel.org> Link: https://lore.kernel.org/r/20220506160820.524344-3-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 28b01cb30c78..cc7fa7b17578 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -22,6 +22,19 @@ #include "common.h" +#ifndef renameat2 +int renameat2(int olddirfd, const char *oldpath, int newdirfd, + const char *newpath, unsigned int flags) +{ + return syscall(__NR_renameat2, olddirfd, oldpath, newdirfd, newpath, + flags); +} +#endif + +#ifndef RENAME_EXCHANGE +#define RENAME_EXCHANGE (1 << 1) +#endif + #define TMP_DIR "tmp" #define BINARY_PATH "./true" @@ -1279,7 +1292,7 @@ TEST_F_FORK(layout1, rule_inside_mount_ns) int ruleset_fd; set_cap(_metadata, CAP_SYS_ADMIN); - ASSERT_EQ(0, syscall(SYS_pivot_root, dir_s3d2, dir_s3d3)) + ASSERT_EQ(0, syscall(__NR_pivot_root, dir_s3d2, dir_s3d3)) { TH_LOG("Failed to pivot root: %s", strerror(errno)); }; @@ -1313,7 +1326,7 @@ TEST_F_FORK(layout1, mount_and_pivot) set_cap(_metadata, CAP_SYS_ADMIN); ASSERT_EQ(-1, mount(NULL, dir_s3d2, NULL, MS_RDONLY, NULL)); ASSERT_EQ(EPERM, errno); - ASSERT_EQ(-1, syscall(SYS_pivot_root, dir_s3d2, dir_s3d3)); + ASSERT_EQ(-1, syscall(__NR_pivot_root, dir_s3d2, dir_s3d3)); ASSERT_EQ(EPERM, errno); clear_cap(_metadata, CAP_SYS_ADMIN); } @@ -1332,13 +1345,13 @@ TEST_F_FORK(layout1, move_mount) ASSERT_LE(0, ruleset_fd); set_cap(_metadata, CAP_SYS_ADMIN); - ASSERT_EQ(0, syscall(SYS_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD, + ASSERT_EQ(0, syscall(__NR_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD, dir_s1d2, 0)) { TH_LOG("Failed to move mount: %s", strerror(errno)); } - ASSERT_EQ(0, syscall(SYS_move_mount, AT_FDCWD, dir_s1d2, AT_FDCWD, + ASSERT_EQ(0, syscall(__NR_move_mount, AT_FDCWD, dir_s1d2, AT_FDCWD, dir_s3d2, 0)); clear_cap(_metadata, CAP_SYS_ADMIN); @@ -1346,7 +1359,7 @@ TEST_F_FORK(layout1, move_mount) ASSERT_EQ(0, close(ruleset_fd)); set_cap(_metadata, CAP_SYS_ADMIN); - ASSERT_EQ(-1, syscall(SYS_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD, + ASSERT_EQ(-1, syscall(__NR_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD, dir_s1d2, 0)); ASSERT_EQ(EPERM, errno); clear_cap(_metadata, CAP_SYS_ADMIN);

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] landlock: Change landlock_add_rule(2) argument check ordering" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 589172e5636c4d16c40b90e87543d43defe2d968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:08:18 +0200 Subject: [PATCH] landlock: Change landlock_add_rule(2) argument check ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes more sense to first check the ruleset FD and then the rule attribute. It will be useful to factor out code for other rule types. Add inval_add_rule_arguments tests, extension of empty_path_beneath_attr tests, to also check error ordering for landlock_add_rule(2). Link: https://lore.kernel.org/r/20220506160820.524344-9-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index 7edc1d50e2bf..a7396220c9d4 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -318,20 +318,24 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd, if (flags) return -EINVAL; - if (rule_type != LANDLOCK_RULE_PATH_BENEATH) - return -EINVAL; - - /* Copies raw user space buffer, only one type for now. */ - res = copy_from_user(&path_beneath_attr, rule_attr, - sizeof(path_beneath_attr)); - if (res) - return -EFAULT; - /* Gets and checks the ruleset. */ ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE); if (IS_ERR(ruleset)) return PTR_ERR(ruleset); + if (rule_type != LANDLOCK_RULE_PATH_BENEATH) { + err = -EINVAL; + goto out_put_ruleset; + } + + /* Copies raw user space buffer, only one type for now. */ + res = copy_from_user(&path_beneath_attr, rule_attr, + sizeof(path_beneath_attr)); + if (res) { + err = -EFAULT; + goto out_put_ruleset; + } + /* * Informs about useless rule: empty allowed_access (i.e. deny rules) * are ignored in path walks. diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c index be9b937256ac..18b779471dcb 100644 --- a/tools/testing/selftests/landlock/base_test.c +++ b/tools/testing/selftests/landlock/base_test.c @@ -121,20 +121,50 @@ TEST(inval_create_ruleset_flags) ASSERT_EQ(EINVAL, errno); } -TEST(empty_path_beneath_attr) +/* Tests ordering of syscall argument checks. */ +TEST(add_rule_checks_ordering) { const struct landlock_ruleset_attr ruleset_attr = { .handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE, }; + struct landlock_path_beneath_attr path_beneath_attr = { + .allowed_access = LANDLOCK_ACCESS_FS_EXECUTE, + .parent_fd = -1, + }; const int ruleset_fd = landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); ASSERT_LE(0, ruleset_fd); - /* Similar to struct landlock_path_beneath_attr.parent_fd = 0 */ + /* Checks invalid flags. */ + ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 1)); + ASSERT_EQ(EINVAL, errno); + + /* Checks invalid ruleset FD. */ + ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 0)); + ASSERT_EQ(EBADF, errno); + + /* Checks invalid rule type. */ + ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, 0, NULL, 0)); + ASSERT_EQ(EINVAL, errno); + + /* Checks invalid rule attr. */ ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, NULL, 0)); ASSERT_EQ(EFAULT, errno); + + /* Checks invalid path_beneath.parent_fd. */ + ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, + &path_beneath_attr, 0)); + ASSERT_EQ(EBADF, errno); + + /* Checks valid call. */ + path_beneath_attr.parent_fd = + open("/tmp", O_PATH | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC); + ASSERT_LE(0, path_beneath_attr.parent_fd); + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, + &path_beneath_attr, 0)); + ASSERT_EQ(0, close(path_beneath_attr.parent_fd)); ASSERT_EQ(0, close(ruleset_fd)); }

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] selftests/landlock: Test landlock_create_ruleset(2) argument" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 6533d0c3a86ee1cc74ff37ac92ca597deb87015c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:08:20 +0200 Subject: [PATCH] selftests/landlock: Test landlock_create_ruleset(2) argument check ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add inval_create_ruleset_arguments, extension of inval_create_ruleset_flags, to also check error ordering for landlock_create_ruleset(2). This is similar to the previous commit checking landlock_add_rule(2). Test coverage for security/landlock is 94.4% of 504 lines accorging to gcc/gcov-11. Link: https://lore.kernel.org/r/20220506160820.524344-11-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c index 21fb33581419..35f64832b869 100644 --- a/tools/testing/selftests/landlock/base_test.c +++ b/tools/testing/selftests/landlock/base_test.c @@ -97,14 +97,17 @@ TEST(abi_version) ASSERT_EQ(EINVAL, errno); } -TEST(inval_create_ruleset_flags) +/* Tests ordering of syscall argument checks. */ +TEST(create_ruleset_checks_ordering) { const int last_flag = LANDLOCK_CREATE_RULESET_VERSION; const int invalid_flag = last_flag << 1; + int ruleset_fd; const struct landlock_ruleset_attr ruleset_attr = { .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE, }; + /* Checks priority for invalid flags. */ ASSERT_EQ(-1, landlock_create_ruleset(NULL, 0, invalid_flag)); ASSERT_EQ(EINVAL, errno); @@ -119,6 +122,22 @@ TEST(inval_create_ruleset_flags) landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), invalid_flag)); ASSERT_EQ(EINVAL, errno); + + /* Checks too big ruleset_attr size. */ + ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, -1, 0)); + ASSERT_EQ(E2BIG, errno); + + /* Checks too small ruleset_attr size. */ + ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0, 0)); + ASSERT_EQ(EINVAL, errno); + ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 1, 0)); + ASSERT_EQ(EINVAL, errno); + + /* Checks valid call. */ + ruleset_fd = + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); + ASSERT_LE(0, ruleset_fd); + ASSERT_EQ(0, close(ruleset_fd)); } /* Tests ordering of syscall argument checks. */

2 years, 7 months

1
0
0 0

WTF: patch "[PATCH] selftests/landlock: Add tests for unknown access rights" was seriously submitted to be applied to the 5.18-stable tree?

by gregkh＠linuxfoundation.org

The patch below was submitted to be applied to the 5.18-stable tree. I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst. I could be totally wrong, and if so, please respond to <stable(a)vger.kernel.org> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From c56b3bf566da5a0dd3b58ad97a614b0928b06ebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic(a)digikod.net> Date: Fri, 6 May 2022 18:08:14 +0200 Subject: [PATCH] selftests/landlock: Add tests for unknown access rights MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure that trying to use unknown access rights returns an error. Cc: Shuah Khan <shuah(a)kernel.org> Link: https://lore.kernel.org/r/20220506160820.524344-5-mic@digikod.net Cc: stable(a)vger.kernel.org Signed-off-by: Mickaël Salaün <mic(a)digikod.net> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index cc7fa7b17578..f293b7e2a1a7 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -448,6 +448,22 @@ TEST_F_FORK(layout1, file_access_rights) ASSERT_EQ(0, close(path_beneath.parent_fd)); } +TEST_F_FORK(layout1, unknown_access_rights) +{ + __u64 access_mask; + + for (access_mask = 1ULL << 63; access_mask != ACCESS_LAST; + access_mask >>= 1) { + struct landlock_ruleset_attr ruleset_attr = { + .handled_access_fs = access_mask, + }; + + ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, + sizeof(ruleset_attr), 0)); + ASSERT_EQ(EINVAL, errno); + } +} + static void add_path_beneath(struct __test_metadata *const _metadata, const int ruleset_fd, const __u64 allowed_access, const char *const path)

2 years, 7 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror June 2022