On 28/11/2022 04:25, Konstantin Meskhidze (A) wrote:

11/17/2022 9:42 PM, Mickaël Salaün пишет:

quoted

On 21/10/2022 17:26, Konstantin Meskhidze wrote:

quoted

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

     Got it.

quoted

quoted

following commits.

Signed-off-by: Konstantin Meskhidze <redacted>
---

[...]

quoted

quoted

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.

    Yep. Will be fixed.

quoted

quoted

+
   /**
    * 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_"

    Do you mean ones which are shared between fs and net parts?

All helpers that ends up in the exported ELF symbols, so all implemented 
in the .c files with their signature defined in .h files. The static 
inlined .h helpers don't need to have such prefix if there is no conflict.

quoted

quoted

+		   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

     I got this warn, but cant get rid of it.
     Also think its a bug in checkpatck.pl

Please ignore it for now. It would be nice to have a checkpatch.pl fix 
though.

quoted

Any though Andy, Joe, Dwaipayan or Lukas?