Unverified Commit 395a02d0 authored by Günther Noack's avatar Günther Noack Committed by Mickaël Salaün
Browse files

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 [1]
Link: https://lore.kernel.org/r/ZmLEoBfHyUR3nKAV@google.com

 [2]
Signed-off-by: default avatarGünther Noack <gnoack@google.com>
Link: https://lore.kernel.org/r/20240610082115.1693267-1-gnoack@google.com


Signed-off-by: default avatarMickaël Salaün <mic@digikod.net>
parent 256abd8e

security/landlock/limits.h

+0 −2
Original line number Diff line number Diff line
@@ -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 */

security/landlock/ruleset.c

+0 −4
Original line number Diff line number Diff line
@@ -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)));

}



/**

security/landlock/ruleset.h

+9 −15
Original line number Diff line number Diff line
@@ -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,