landlock: Use bit-fields for storing handled layer access masks

When defined using bit-fields, the compiler takes care of packing the
bits in a memory-efficient way and frees us from defining
LANDLOCK_SHIFT_ACCESS_* by hand.  The exact memory layout does not
matter in our use case.

The manual definition of LANDLOCK_SHIFT_ACCESS_* has resulted in bugs in
at least two recent patch sets [1] [2] where new kinds of handled access
rights were introduced.

Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
Cc: Tahera Fahimi <fahimitahera@gmail.com>
Link: https://lore.kernel.org/r/ebd680cc-25d6-ee14-4856-310f5e5e28e4@huawei-partners.com
Link: https://lore.kernel.org/r/ZmLEoBfHyUR3nKAV@google.com
Signed-off-by: Günther Noack <gnoack@google.com>
Link: https://lore.kernel.org/r/20240610082115.1693267-1-gnoack@google.com
Signed-off-by: Mickaël Salaün <mic@digikod.net>

diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 20fdb5ff351485989989403949e2ad24ee3fe1e3..4eb643077a2a6e31693d403af89be1046ad6337c 100644 (file)
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -21,12 +21,10 @@
 #define LANDLOCK_LAST_ACCESS_FS                LANDLOCK_ACCESS_FS_IOCTL_DEV
 #define LANDLOCK_MASK_ACCESS_FS                ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_FS         __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
-#define LANDLOCK_SHIFT_ACCESS_FS       0
 
 #define LANDLOCK_LAST_ACCESS_NET       LANDLOCK_ACCESS_NET_CONNECT_TCP
 #define LANDLOCK_MASK_ACCESS_NET       ((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_NET                __const_hweight64(LANDLOCK_MASK_ACCESS_NET)
-#define LANDLOCK_SHIFT_ACCESS_NET      LANDLOCK_NUM_ACCESS_FS
 
 /* clang-format on */
 

diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index e0a5fbf9201ade7036e8cb8203f5b99e18b94947..6ff232f586183d6b74c675a1f03555d7831b81f5 100644 (file)
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -169,13 +169,9 @@ static void build_check_ruleset(void)
                .num_rules = ~0,
                .num_layers = ~0,
        };
-       typeof(ruleset.access_masks[0]) access_masks = ~0;
 
        BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES);
        BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
-       BUILD_BUG_ON(access_masks <
-                    ((LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) |
-                     (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET)));
 }
 
 /**

diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index c7f1526784fd1043dc85a8ad20df865f56260e93..0f1b5b4c8f6b4136c4a02278cd46a0aef1e23065 100644 (file)
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -39,10 +39,10 @@ static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
 static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
 
 /* Ruleset access masks. */
-typedef u32 access_masks_t;
-/* Makes sure all ruleset access rights can be stored. */
-static_assert(BITS_PER_TYPE(access_masks_t) >=
-             LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET);
+struct access_masks {
+       access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
+       access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
+};
 
 typedef u16 layer_mask_t;
 /* Makes sure all layers can be checked. */
@@ -226,7 +226,7 @@ struct landlock_ruleset {
                         * layers are set once and never changed for the
                         * lifetime of the ruleset.
                         */
-                       access_masks_t access_masks[];
+                       struct access_masks access_masks[];
                };
        };
 };
@@ -265,8 +265,7 @@ landlock_add_fs_access_mask(struct landlock_ruleset *const ruleset,
 
        /* Should already be checked in sys_landlock_create_ruleset(). */
        WARN_ON_ONCE(fs_access_mask != fs_mask);
-       ruleset->access_masks[layer_level] |=
-               (fs_mask << LANDLOCK_SHIFT_ACCESS_FS);
+       ruleset->access_masks[layer_level].fs |= fs_mask;
 }
 
 static inline void
@@ -278,17 +277,14 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset,
 
        /* Should already be checked in sys_landlock_create_ruleset(). */
        WARN_ON_ONCE(net_access_mask != net_mask);
-       ruleset->access_masks[layer_level] |=
-               (net_mask << LANDLOCK_SHIFT_ACCESS_NET);
+       ruleset->access_masks[layer_level].net |= net_mask;
 }
 
 static inline access_mask_t
 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;
+       return ruleset->access_masks[layer_level].fs;
 }
 
 static inline access_mask_t
@@ -304,9 +300,7 @@ static inline access_mask_t
 landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset,
                             const u16 layer_level)
 {
-       return (ruleset->access_masks[layer_level] >>
-               LANDLOCK_SHIFT_ACCESS_NET) &
-              LANDLOCK_MASK_ACCESS_NET;
+       return ruleset->access_masks[layer_level].net;
 }
 
 bool landlock_unmask_layers(const struct landlock_rule *const rule,