On 21/10/2022 17:26, Konstantin Meskhidze wrote:
This patch moves unmask_layers() and init_layer_masks() helpers
to ruleset.c to share with landlock network implementation in
…to share them with the Landlock network implementation in
following commits.
Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@xxxxxxxxxx>
---
Changes since v7:
* Refactors commit message.
Changes since v6:
* Moves get_handled_accesses() helper from ruleset.c back to fs.c,
cause it's not used in coming network commits.
Changes since v5:
* Splits commit.
* Moves init_layer_masks() and get_handled_accesses() helpers
to ruleset.c and makes then non-static.
* Formats code with clang-format-14.
---
security/landlock/fs.c | 103 ------------------------------------
security/landlock/ruleset.c | 102 +++++++++++++++++++++++++++++++++++
security/landlock/ruleset.h | 20 +++++++
3 files changed, 122 insertions(+), 103 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 710cfa1306de..240e42a8f788 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -226,60 +226,6 @@ find_rule(const struct landlock_ruleset *const domain,
return rule;
}
-/*
- * @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 false;
-
- /*
- * An access is granted if, for each policy layer, at least one rule
- * 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. 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;
-
- /*
- * 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 false;
-}
-
/*
* Allows access to pseudo filesystems that will never be mountable (e.g.
* sockfs, pipefs), but can still be reachable through
@@ -303,55 +249,6 @@ get_handled_accesses(const struct landlock_ruleset *const domain)
return access_dom & LANDLOCK_MASK_ACCESS_FS;
}
-/**
- * init_layer_masks - Initialize layer masks from an access request
- *
- * Populates @layer_masks such that for each access right in @access_request,
- * the bits for all the layers are set where this access right is handled.
- *
- * @domain: The domain that defines the current restrictions.
- * @access_request: The requested access rights to check.
- * @layer_masks: The layer masks to populate.
- *
- * Returns: An access mask where each access right bit is set which is handled
- * in any of the active layers in @domain.
- */
-static inline access_mask_t
-init_layer_masks(const struct landlock_ruleset *const domain,
- const access_mask_t access_request,
- layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
-{
- access_mask_t handled_accesses = 0;
- size_t layer_level;
-
- memset(layer_masks, 0, sizeof(*layer_masks));
- /* An empty access request can happen because of O_WRONLY | O_RDWR. */
- if (!access_request)
- return 0;
-
- /* Saves all handled accesses per layer. */
- for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
- const unsigned long access_req = access_request;
- unsigned long access_bit;
-
- 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)) {
- (*layer_masks)[access_bit] |=
- BIT_ULL(layer_level);
- handled_accesses |= BIT_ULL(access_bit);
- }
- }
- }
- return handled_accesses;
-}
-
/*
* Check that a destination file hierarchy has more restrictions than a source
* file hierarchy. This is only used for link and rename actions.
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 961ffe0c709e..02ab14439c43 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -572,3 +572,105 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
}
return NULL;
}
+
+/*
+ * @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).
+ */
+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 false;
+
+ /*
+ * An access is granted if, for each policy layer, at least one rule
+ * 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. 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;
+
+ /*
+ * 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 false;
+}
+
+/**
+ * init_layer_masks - Initialize layer masks from an access request
+ *
+ * Populates @layer_masks such that for each access right in @access_request,
+ * the bits for all the layers are set where this access right is handled.
+ *
+ * @domain: The domain that defines the current restrictions.
+ * @access_request: The requested access rights to check.
+ * @layer_masks: The layer masks to populate.
+ *
+ * Returns: An access mask where each access right bit is set which is handled
+ * in any of the active layers in @domain.
+ */
+access_mask_t
+init_layer_masks(const struct landlock_ruleset *const domain,
+ const access_mask_t access_request,
+ layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
+{
+ access_mask_t handled_accesses = 0;
+ size_t layer_level;
+
+ memset(layer_masks, 0, sizeof(*layer_masks));
+ /* An empty access request can happen because of O_WRONLY | O_RDWR. */
+ if (!access_request)
+ return 0;
+
+ /* Saves all handled accesses per layer. */
+ for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
+ const unsigned long access_req = access_request;
+ unsigned long access_bit;
+
+ 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)) {
This is a future bug in the a next commit when
landlock_get_fs_access_mask() is changed to get the network access mask.
My patch will fix this part but you'll need to do a new copy of this
function.
+ (*layer_masks)[access_bit] |=
+ BIT_ULL(layer_level);
+ handled_accesses |= BIT_ULL(access_bit);
+ }
+ }
+ }
+ return handled_accesses;
+}
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 608ab356bc3e..50baff4fcbb4 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -34,6 +34,16 @@ typedef u16 layer_mask_t;
/* Makes sure all layers can be checked. */
static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
+/*
+ * 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 */
This ACCESS_INITIALLY_DENIED definition must be moved, not copied. You
can rename ACCESS_INITIALLY_DENIED to ACCESS_FS_INITIALLY_DENIED and
move this hunk before the access_mask_t definition.
+
/**
* struct landlock_layer - Access rights for a given layer
*/
@@ -246,4 +256,14 @@ landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset,
LANDLOCK_SHIFT_ACCESS_FS) &
LANDLOCK_MASK_ACCESS_FS;
}
+
+bool unmask_layers(const struct landlock_rule *const rule,
All public Landlock helpers must be prefixed with "landlock_"
+ const access_mask_t access_request,
+ layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]);
+
+access_mask_t
+init_layer_masks(const struct landlock_ruleset *const domain,
+ const access_mask_t access_request,
+ layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]);
There is a warning generated by checkpatch.pl about this line:
WARNING: function definition argument 'layer_mask_t' should also have
an identifier name
I think this is a bug in checkpatch.pl
Any though Andy, Joe, Dwaipayan or Lukas?
+
#endif /* _SECURITY_LANDLOCK_RULESET_H */
--
2.25.1
