[v8,07/12] landlock: Add network rules support

Message ID	20221021152644.155136-8-konstantin.meskhidze@huawei.com (mailing list archive)
State	Not Applicable
Headers	show Return-Path: <netdev-owner@kernel.org> From: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> To: <mic@digikod.net> CC: <willemdebruijn.kernel@gmail.com>, <gnoack3000@gmail.com>, <linux-security-module@vger.kernel.org>, <netdev@vger.kernel.org>, <netfilter-devel@vger.kernel.org>, <artem.kuzin@huawei.com> Subject: [PATCH v8 07/12] landlock: Add network rules support Date: Fri, 21 Oct 2022 23:26:39 +0800 Message-ID: <20221021152644.155136-8-konstantin.meskhidze@huawei.com> In-Reply-To: <20221021152644.155136-1-konstantin.meskhidze@huawei.com> References: <20221021152644.155136-1-konstantin.meskhidze@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=US-ASCII Precedence: bulk
Series	Network support for Landlock \| expand [v8,00/12] Network support for Landlock [v8,01/12] landlock: Make ruleset's access masks more generic [v8,02/12] landlock: Refactor landlock_find_rule/insert_rule [v8,03/12] landlock: Refactor merge/inherit_ruleset functions [v8,04/12] landlock: Move unmask_layers() and init_layer_masks() [v8,05/12] landlock: Refactor unmask_layers() and init_layer_masks() [v8,06/12] landlock: Refactor landlock_add_rule() syscall [v8,07/12] landlock: Add network rules support [v8,08/12] landlock: Implement TCP network hooks [v8,09/12] selftests/landlock: Share enforce_ruleset() [v8,10/12] selftests/landlock: Add 10 new test suites dedicated to network [v8,11/12] samples/landlock: Add network demo [v8,12/12] landlock: Document Landlock's network support

Context	Check	Description
netdev/tree_selection	success	Not a local patch, async

Konstantin Meskhidze (A) Oct. 21, 2022, 3:26 p.m. UTC

This commit adds network rules support in internal landlock functions
(presented in ruleset.c) and landlock_create_ruleset syscall.
Refactors user space API to support network actions. Adds new network
access flags, network rule and network attributes. Increments Landlock
ABI version.

Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---

Changes since v7:
* Squashes commits.
* Increments ABI version to 4.
* Refactors commit message.
* Minor fixes.

Changes since v6:
* Renames landlock_set_net_access_mask() to landlock_add_net_access_mask()
  because it OR values.
* Makes landlock_add_net_access_mask() more resilient incorrect values.
* Refactors landlock_get_net_access_mask().
* Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use
  LANDLOCK_NUM_ACCESS_FS as value.
* Updates access_masks_t to u32 to support network access actions.
* Refactors landlock internal functions to support network actions with
  landlock_key/key_type/id types.

Changes since v5:
* Gets rid of partial revert from landlock_add_rule
syscall.
* Formats code with clang-format-14.

Changes since v4:
* Refactors landlock_create_ruleset() - splits ruleset and
masks checks.
* Refactors landlock_create_ruleset() and landlock mask
setters/getters to support two rule types.
* Refactors landlock_add_rule syscall add_rule_path_beneath
function by factoring out get_ruleset_from_fd() and
landlock_put_ruleset().

Changes since v3:
* Splits commit.
* Adds network rule support for internal landlock functions.
* Adds set_mask and get_mask for network.
* Adds rb_root root_net_port.

---
 include/uapi/linux/landlock.h                | 49 ++++++++++++++
 security/landlock/limits.h                   |  6 +-
 security/landlock/ruleset.c                  | 55 ++++++++++++++--
 security/landlock/ruleset.h                  | 68 ++++++++++++++++----
 security/landlock/syscalls.c                 | 13 +++-
 tools/testing/selftests/landlock/base_test.c |  2 +-
 6 files changed, 170 insertions(+), 23 deletions(-)

--
2.25.1

Mickaël Salaün Nov. 17, 2022, 6:43 p.m. UTC | #1

On 21/10/2022 17:26, Konstantin Meskhidze wrote:
> This commit adds network rules support in internal landlock functions
> (presented in ruleset.c) and landlock_create_ruleset syscall.

…in the ruleset management helpers and the landlock_create_ruleset syscall.


> Refactors user space API to support network actions. Adds new network

Refactor…

> access flags, network rule and network attributes. Increments Landlock

Increment…

> ABI version.
> 
> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> ---
> 
> Changes since v7:
> * Squashes commits.
> * Increments ABI version to 4.
> * Refactors commit message.
> * Minor fixes.
> 
> Changes since v6:
> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask()
>    because it OR values.
> * Makes landlock_add_net_access_mask() more resilient incorrect values.
> * Refactors landlock_get_net_access_mask().
> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use
>    LANDLOCK_NUM_ACCESS_FS as value.
> * Updates access_masks_t to u32 to support network access actions.
> * Refactors landlock internal functions to support network actions with
>    landlock_key/key_type/id types.
> 
> Changes since v5:
> * Gets rid of partial revert from landlock_add_rule
> syscall.
> * Formats code with clang-format-14.
> 
> Changes since v4:
> * Refactors landlock_create_ruleset() - splits ruleset and
> masks checks.
> * Refactors landlock_create_ruleset() and landlock mask
> setters/getters to support two rule types.
> * Refactors landlock_add_rule syscall add_rule_path_beneath
> function by factoring out get_ruleset_from_fd() and
> landlock_put_ruleset().
> 
> Changes since v3:
> * Splits commit.
> * Adds network rule support for internal landlock functions.
> * Adds set_mask and get_mask for network.
> * Adds rb_root root_net_port.
> 
> ---
>   include/uapi/linux/landlock.h                | 49 ++++++++++++++
>   security/landlock/limits.h                   |  6 +-
>   security/landlock/ruleset.c                  | 55 ++++++++++++++--
>   security/landlock/ruleset.h                  | 68 ++++++++++++++++----
>   security/landlock/syscalls.c                 | 13 +++-
>   tools/testing/selftests/landlock/base_test.c |  2 +-
>   6 files changed, 170 insertions(+), 23 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f3223f964691..096b683c6ff3 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr {
>   	 * this access right.
>   	 */
>   	__u64 handled_access_fs;
> +
> +	/**
> +	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
> +	 * that is handled by this ruleset and should then be forbidden if no
> +	 * rule explicitly allow them.
> +	 */
> +	__u64 handled_access_net;
>   };
> 
>   /*
> @@ -54,6 +61,11 @@ enum landlock_rule_type {
>   	 * landlock_path_beneath_attr .
>   	 */
>   	LANDLOCK_RULE_PATH_BENEATH = 1,
> +	/**
> +	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
> +	 * landlock_net_service_attr .
> +	 */
> +	LANDLOCK_RULE_NET_SERVICE = 2,
>   };
> 
>   /**
> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>   	 */
>   } __attribute__((packed));
> 
> +/**
> + * struct landlock_net_service_attr - TCP subnet definition
> + *
> + * Argument of sys_landlock_add_rule().
> + */
> +struct landlock_net_service_attr {
> +	/**
> +	 * @allowed_access: Bitmask of allowed access network for services
> +	 * (cf. `Network flags`_).
> +	 */
> +	__u64 allowed_access;
> +	/**
> +	 * @port: Network port.
> +	 */
> +	__u16 port;

 From an UAPI point of view, I think the port field should be __be16, as 
for sockaddr_in->port and other network-related APIs. This will require 
some kernel changes to please sparse: make C=2 security/landlock/ must 
not print any warning.

Using big-endian values as keys (casted to uintptr_t, not strictly 
__be16) in the rb-tree should not be an issue because there is no port 
range ordering (for now).

A dedicated test should check that endianness is correct, e.g. by using 
different port encoding. This should include passing and failing tests, 
but they should work on all architectures (i.e. big or little endian).


> +
> +} __attribute__((packed));
> +
>   /**
>    * DOC: fs_access
>    *
> @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr {
>   #define LANDLOCK_ACCESS_FS_TRUNCATE			(1ULL << 14)
>   /* clang-format on */
> 
> +/**
> + * DOC: net_access
> + *
> + * Network flags
> + * ~~~~~~~~~~~~~~~~
> + *
> + * These flags enable to restrict a sandboxed process to a set of network
> + * actions.
> + *
> + * TCP sockets with allowed actions:
> + *
> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
> + *   a remote port.
> + */
> +/* clang-format off */
> +#define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
> +/* clang-format on */
>   #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index bafb3b8dc677..8a1a6463c64e 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -23,6 +23,10 @@
>   #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>   #define LANDLOCK_SHIFT_ACCESS_FS	0
> 
> -/* clang-format on */
> +#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 */
>   #endif /* _SECURITY_LANDLOCK_LIMITS_H */
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index c7cf54ba4f6d..9277c1295114 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>   	refcount_set(&new_ruleset->usage, 1);
>   	mutex_init(&new_ruleset->lock);
>   	new_ruleset->root_inode = RB_ROOT;
> +#if IS_ENABLED(CONFIG_INET)
> +	new_ruleset->root_net_port = RB_ROOT;
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   	new_ruleset->num_layers = num_layers;
>   	/*
>   	 * hierarchy = NULL
> @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>   }
> 
>   struct landlock_ruleset *
> -landlock_create_ruleset(const access_mask_t fs_access_mask)
> +landlock_create_ruleset(const access_mask_t fs_access_mask,
> +			const access_mask_t net_access_mask)
>   {
>   	struct landlock_ruleset *new_ruleset;
> 
>   	/* Informs about useless ruleset. */
> -	if (!fs_access_mask)
> +	if (!fs_access_mask && !net_access_mask)
>   		return ERR_PTR(-ENOMSG);
>   	new_ruleset = create_ruleset(1);
> -	if (!IS_ERR(new_ruleset))
> +	if (IS_ERR(new_ruleset))
> +		return new_ruleset;
> +	if (fs_access_mask)
>   		landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
> +	if (net_access_mask)
> +		landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
>   	return new_ruleset;
>   }
> 
> @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type)
>   	switch (key_type) {
>   	case LANDLOCK_KEY_INODE:
>   		return true;
> +#if IS_ENABLED(CONFIG_INET)
> +	case LANDLOCK_KEY_NET_PORT:
> +		return false;
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   	}
>   	WARN_ON_ONCE(1);
>   	return false;
> @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset,
>   	case LANDLOCK_KEY_INODE:
>   		root = &ruleset->root_inode;
>   		break;
> +#if IS_ENABLED(CONFIG_INET)
> +	case LANDLOCK_KEY_NET_PORT:
> +		root = &ruleset->root_net_port;
> +		break;
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   	}
>   	if (WARN_ON_ONCE(!root))
>   		return ERR_PTR(-EINVAL);
> @@ -154,7 +171,9 @@ static void build_check_ruleset(void)
>   	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_FS << LANDLOCK_SHIFT_ACCESS_FS) +

This is correct but because we are dealing with bitmasks I would prefer 
to use "|" instead of "+".


> +			     (LANDLOCK_MASK_ACCESS_NET
> +			      << LANDLOCK_SHIFT_ACCESS_NET));
>   }
> 
>   /**
> @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
>   	err = merge_tree(dst, src, LANDLOCK_KEY_INODE);
>   	if (err)
>   		goto out_unlock;
> -

Please keep this newline.

> +#if IS_ENABLED(CONFIG_INET)
> +	/* Merges the @src network port tree. */
> +	err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT);
> +	if (err)
> +		goto out_unlock;
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   out_unlock:
>   	mutex_unlock(&src->lock);
>   	mutex_unlock(&dst->lock);
> @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
>   	err = inherit_tree(parent, child, LANDLOCK_KEY_INODE);
>   	if (err)
>   		goto out_unlock;
> -

newline

> +#if IS_ENABLED(CONFIG_INET)
> +	/* Copies the @parent network port tree. */
> +	err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT);
> +	if (err)
> +		goto out_unlock;
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
>   		err = -EINVAL;
>   		goto out_unlock;
> @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
>   	rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode,
>   					     node)
>   		free_rule(freeme, LANDLOCK_KEY_INODE);
> +#if IS_ENABLED(CONFIG_INET)
> +	rbtree_postorder_for_each_entry_safe(freeme, next,
> +					     &ruleset->root_net_port, node)
> +		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   	put_hierarchy(ruleset->hierarchy);
>   	kfree(ruleset);
>   }
> @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
>    * 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.
>    *
> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET
> + * elements according to @key_type.

Please include this sentence in the @layer_masks description below.

> + *
>    * @domain: The domain that defines the current restrictions.
>    * @access_request: The requested access rights to check.
>    * @layer_masks: The layer masks to populate.

"It must contain…"


> @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain,
>   		get_access_mask = landlock_get_fs_access_mask;
>   		num_access = LANDLOCK_NUM_ACCESS_FS;
>   		break;
> +#if IS_ENABLED(CONFIG_INET)
> +	case LANDLOCK_KEY_NET_PORT:
> +		get_access_mask = landlock_get_net_access_mask;
> +		num_access = LANDLOCK_NUM_ACCESS_NET;
> +		break;
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   	default:
>   		WARN_ON_ONCE(1);
>   		return 0;
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index d9eb79ea9a89..f272d2cd518c 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -19,16 +19,20 @@
>   #include "limits.h"
>   #include "object.h"
> 
> +/* Rule access mask. */
>   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 all network access rights can be stored. */
> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
>   /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
>   static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
> 
>   /* Ruleset access masks. */
> -typedef u16 access_masks_t;
> +typedef u32 access_masks_t;

This type change need to be explained in the commit message.


>   /* Makes sure all ruleset access rights can be stored. */
> -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS);
> +static_assert(BITS_PER_TYPE(access_masks_t) >=
> +	      LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET);
> 
>   typedef u16 layer_mask_t;
>   /* Makes sure all layers can be checked. */
> @@ -82,6 +86,13 @@ enum landlock_key_type {
>   	 * keys.
>   	 */
>   	LANDLOCK_KEY_INODE = 1,
> +#if IS_ENABLED(CONFIG_INET)
> +	/**
> +	 * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's
> +	 * node keys.
> +	 */
> +	LANDLOCK_KEY_NET_PORT = 2,
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   };
> 
>   /**
> @@ -156,6 +167,15 @@ struct landlock_ruleset {
>   	 * reaches zero.
>   	 */
>   	struct rb_root root_inode;
> +#if IS_ENABLED(CONFIG_INET)
> +	/**
> +	 * @root_net_port: Root of a red-black tree containing &struct
> +	 * landlock_rule nodes with network port. Once a ruleset is tied to a
> +	 * process (i.e. as a domain), this tree is immutable until @usage
> +	 * reaches zero.
> +	 */
> +	struct rb_root root_net_port;
> +#endif /* IS_ENABLED(CONFIG_INET) */
>   	/**
>   	 * @hierarchy: Enables hierarchy identification even when a parent
>   	 * domain vanishes.  This is needed for the ptrace protection.
> @@ -166,8 +186,8 @@ struct landlock_ruleset {
>   		 * @work_free: Enables to free a ruleset within a lockless
>   		 * section.  This is only used by
>   		 * landlock_put_ruleset_deferred() when @usage reaches zero.
> -		 * The fields @lock, @usage, @num_rules, @num_layers and
> -		 * @access_masks are then unused.
> +		 * The fields @lock, @usage, @num_rules, @num_layers,
> +		 * @net_access_mask and @access_masks are then unused.

There is no net_access_mask anymore.

Konstantin Meskhidze (A) Nov. 28, 2022, 4:01 a.m. UTC | #2

11/17/2022 9:43 PM, Mickaël Salaün пишет:
> 
> On 21/10/2022 17:26, Konstantin Meskhidze wrote:
>> This commit adds network rules support in internal landlock functions
>> (presented in ruleset.c) and landlock_create_ruleset syscall.
> 
> …in the ruleset management helpers and the landlock_create_ruleset syscall.
> 
> 
>> Refactors user space API to support network actions. Adds new network
> 
> Refactor…
> 
>> access flags, network rule and network attributes. Increments Landlock
> 
> Increment…

   The commit's message will be fixed. Thank you!
> 
>> ABI version.
>> 
>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> ---
>> 
>> Changes since v7:
>> * Squashes commits.
>> * Increments ABI version to 4.
>> * Refactors commit message.
>> * Minor fixes.
>> 
>> Changes since v6:
>> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask()
>>    because it OR values.
>> * Makes landlock_add_net_access_mask() more resilient incorrect values.
>> * Refactors landlock_get_net_access_mask().
>> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use
>>    LANDLOCK_NUM_ACCESS_FS as value.
>> * Updates access_masks_t to u32 to support network access actions.
>> * Refactors landlock internal functions to support network actions with
>>    landlock_key/key_type/id types.
>> 
>> Changes since v5:
>> * Gets rid of partial revert from landlock_add_rule
>> syscall.
>> * Formats code with clang-format-14.
>> 
>> Changes since v4:
>> * Refactors landlock_create_ruleset() - splits ruleset and
>> masks checks.
>> * Refactors landlock_create_ruleset() and landlock mask
>> setters/getters to support two rule types.
>> * Refactors landlock_add_rule syscall add_rule_path_beneath
>> function by factoring out get_ruleset_from_fd() and
>> landlock_put_ruleset().
>> 
>> Changes since v3:
>> * Splits commit.
>> * Adds network rule support for internal landlock functions.
>> * Adds set_mask and get_mask for network.
>> * Adds rb_root root_net_port.
>> 
>> ---
>>   include/uapi/linux/landlock.h                | 49 ++++++++++++++
>>   security/landlock/limits.h                   |  6 +-
>>   security/landlock/ruleset.c                  | 55 ++++++++++++++--
>>   security/landlock/ruleset.h                  | 68 ++++++++++++++++----
>>   security/landlock/syscalls.c                 | 13 +++-
>>   tools/testing/selftests/landlock/base_test.c |  2 +-
>>   6 files changed, 170 insertions(+), 23 deletions(-)
>> 
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index f3223f964691..096b683c6ff3 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr {
>>   	 * this access right.
>>   	 */
>>   	__u64 handled_access_fs;
>> +
>> +	/**
>> +	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
>> +	 * that is handled by this ruleset and should then be forbidden if no
>> +	 * rule explicitly allow them.
>> +	 */
>> +	__u64 handled_access_net;
>>   };
>> 
>>   /*
>> @@ -54,6 +61,11 @@ enum landlock_rule_type {
>>   	 * landlock_path_beneath_attr .
>>   	 */
>>   	LANDLOCK_RULE_PATH_BENEATH = 1,
>> +	/**
>> +	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
>> +	 * landlock_net_service_attr .
>> +	 */
>> +	LANDLOCK_RULE_NET_SERVICE = 2,
>>   };
>> 
>>   /**
>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>>   	 */
>>   } __attribute__((packed));
>> 
>> +/**
>> + * struct landlock_net_service_attr - TCP subnet definition
>> + *
>> + * Argument of sys_landlock_add_rule().
>> + */
>> +struct landlock_net_service_attr {
>> +	/**
>> +	 * @allowed_access: Bitmask of allowed access network for services
>> +	 * (cf. `Network flags`_).
>> +	 */
>> +	__u64 allowed_access;
>> +	/**
>> +	 * @port: Network port.
>> +	 */
>> +	__u16 port;
> 
>   From an UAPI point of view, I think the port field should be __be16, as
> for sockaddr_in->port and other network-related APIs. This will require
> some kernel changes to please sparse: make C=2 security/landlock/ must
> not print any warning.

   Is sparse a default checker?
> 
> Using big-endian values as keys (casted to uintptr_t, not strictly
> __be16) in the rb-tree should not be an issue because there is no port
> range ordering (for now).
> 
> A dedicated test should check that endianness is correct, e.g. by using
> different port encoding. This should include passing and failing tests,
> but they should work on all architectures (i.e. big or little endian).
> 
   I got it. Will be fixed.
> 
>> +
>> +} __attribute__((packed));
>> +
>>   /**
>>    * DOC: fs_access
>>    *
>> @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr {
>>   #define LANDLOCK_ACCESS_FS_TRUNCATE			(1ULL << 14)
>>   /* clang-format on */
>> 
>> +/**
>> + * DOC: net_access
>> + *
>> + * Network flags
>> + * ~~~~~~~~~~~~~~~~
>> + *
>> + * These flags enable to restrict a sandboxed process to a set of network
>> + * actions.
>> + *
>> + * TCP sockets with allowed actions:
>> + *
>> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
>> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
>> + *   a remote port.
>> + */
>> +/* clang-format off */
>> +#define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>> +/* clang-format on */
>>   #endif /* _UAPI_LINUX_LANDLOCK_H */
>> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
>> index bafb3b8dc677..8a1a6463c64e 100644
>> --- a/security/landlock/limits.h
>> +++ b/security/landlock/limits.h
>> @@ -23,6 +23,10 @@
>>   #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>>   #define LANDLOCK_SHIFT_ACCESS_FS	0
>> 
>> -/* clang-format on */
>> +#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 */
>>   #endif /* _SECURITY_LANDLOCK_LIMITS_H */
>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>> index c7cf54ba4f6d..9277c1295114 100644
>> --- a/security/landlock/ruleset.c
>> +++ b/security/landlock/ruleset.c
>> @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   	refcount_set(&new_ruleset->usage, 1);
>>   	mutex_init(&new_ruleset->lock);
>>   	new_ruleset->root_inode = RB_ROOT;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	new_ruleset->root_net_port = RB_ROOT;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	new_ruleset->num_layers = num_layers;
>>   	/*
>>   	 * hierarchy = NULL
>> @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   }
>> 
>>   struct landlock_ruleset *
>> -landlock_create_ruleset(const access_mask_t fs_access_mask)
>> +landlock_create_ruleset(const access_mask_t fs_access_mask,
>> +			const access_mask_t net_access_mask)
>>   {
>>   	struct landlock_ruleset *new_ruleset;
>> 
>>   	/* Informs about useless ruleset. */
>> -	if (!fs_access_mask)
>> +	if (!fs_access_mask && !net_access_mask)
>>   		return ERR_PTR(-ENOMSG);
>>   	new_ruleset = create_ruleset(1);
>> -	if (!IS_ERR(new_ruleset))
>> +	if (IS_ERR(new_ruleset))
>> +		return new_ruleset;
>> +	if (fs_access_mask)
>>   		landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
>> +	if (net_access_mask)
>> +		landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
>>   	return new_ruleset;
>>   }
>> 
>> @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type)
>>   	switch (key_type) {
>>   	case LANDLOCK_KEY_INODE:
>>   		return true;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		return false;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	}
>>   	WARN_ON_ONCE(1);
>>   	return false;
>> @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset,
>>   	case LANDLOCK_KEY_INODE:
>>   		root = &ruleset->root_inode;
>>   		break;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		root = &ruleset->root_net_port;
>> +		break;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	}
>>   	if (WARN_ON_ONCE(!root))
>>   		return ERR_PTR(-EINVAL);
>> @@ -154,7 +171,9 @@ static void build_check_ruleset(void)
>>   	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_FS << LANDLOCK_SHIFT_ACCESS_FS) +
> 
> This is correct but because we are dealing with bitmasks I would prefer
> to use "|" instead of "+".

   Ok. I will refactor it.
> 
> 
>> +			     (LANDLOCK_MASK_ACCESS_NET
>> +			      << LANDLOCK_SHIFT_ACCESS_NET));
>>   }
>> 
>>   /**
>> @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
>>   	err = merge_tree(dst, src, LANDLOCK_KEY_INODE);
>>   	if (err)
>>   		goto out_unlock;
>> -
> 
> Please keep this newline.

   Got it.
> 
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/* Merges the @src network port tree. */
>> +	err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT);
>> +	if (err)
>> +		goto out_unlock;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   out_unlock:
>>   	mutex_unlock(&src->lock);
>>   	mutex_unlock(&dst->lock);
>> @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
>>   	err = inherit_tree(parent, child, LANDLOCK_KEY_INODE);
>>   	if (err)
>>   		goto out_unlock;
>> -
> 
> newline

  Ok.
> 
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/* Copies the @parent network port tree. */
>> +	err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT);
>> +	if (err)
>> +		goto out_unlock;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
>>   		err = -EINVAL;
>>   		goto out_unlock;
>> @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
>>   	rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode,
>>   					     node)
>>   		free_rule(freeme, LANDLOCK_KEY_INODE);
>> +#if IS_ENABLED(CONFIG_INET)
>> +	rbtree_postorder_for_each_entry_safe(freeme, next,
>> +					     &ruleset->root_net_port, node)
>> +		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	put_hierarchy(ruleset->hierarchy);
>>   	kfree(ruleset);
>>   }
>> @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
>>    * 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.
>>    *
>> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET
>> + * elements according to @key_type.
> 
> Please include this sentence in the @layer_masks description below.

   Ok.
> 
>> + *
>>    * @domain: The domain that defines the current restrictions.
>>    * @access_request: The requested access rights to check.
>>    * @layer_masks: The layer masks to populate.
> 
> "It must contain…"
> 
> 
>> @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain,
>>   		get_access_mask = landlock_get_fs_access_mask;
>>   		num_access = LANDLOCK_NUM_ACCESS_FS;
>>   		break;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		get_access_mask = landlock_get_net_access_mask;
>> +		num_access = LANDLOCK_NUM_ACCESS_NET;
>> +		break;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	default:
>>   		WARN_ON_ONCE(1);
>>   		return 0;
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index d9eb79ea9a89..f272d2cd518c 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -19,16 +19,20 @@
>>   #include "limits.h"
>>   #include "object.h"
>> 
>> +/* Rule access mask. */
>>   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 all network access rights can be stored. */
>> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
>>   /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
>>   static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
>> 
>>   /* Ruleset access masks. */
>> -typedef u16 access_masks_t;
>> +typedef u32 access_masks_t;
> 
> This type change need to be explained in the commit message.
> 
   Ok. I will explain it.
> 
>>   /* Makes sure all ruleset access rights can be stored. */
>> -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS);
>> +static_assert(BITS_PER_TYPE(access_masks_t) >=
>> +	      LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET);
>> 
>>   typedef u16 layer_mask_t;
>>   /* Makes sure all layers can be checked. */
>> @@ -82,6 +86,13 @@ enum landlock_key_type {
>>   	 * keys.
>>   	 */
>>   	LANDLOCK_KEY_INODE = 1,
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/**
>> +	 * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's
>> +	 * node keys.
>> +	 */
>> +	LANDLOCK_KEY_NET_PORT = 2,
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   };
>> 
>>   /**
>> @@ -156,6 +167,15 @@ struct landlock_ruleset {
>>   	 * reaches zero.
>>   	 */
>>   	struct rb_root root_inode;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/**
>> +	 * @root_net_port: Root of a red-black tree containing &struct
>> +	 * landlock_rule nodes with network port. Once a ruleset is tied to a
>> +	 * process (i.e. as a domain), this tree is immutable until @usage
>> +	 * reaches zero.
>> +	 */
>> +	struct rb_root root_net_port;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	/**
>>   	 * @hierarchy: Enables hierarchy identification even when a parent
>>   	 * domain vanishes.  This is needed for the ptrace protection.
>> @@ -166,8 +186,8 @@ struct landlock_ruleset {
>>   		 * @work_free: Enables to free a ruleset within a lockless
>>   		 * section.  This is only used by
>>   		 * landlock_put_ruleset_deferred() when @usage reaches zero.
>> -		 * The fields @lock, @usage, @num_rules, @num_layers and
>> -		 * @access_masks are then unused.
>> +		 * The fields @lock, @usage, @num_rules, @num_layers,
>> +		 * @net_access_mask and @access_masks are then unused.
> 
> There is no net_access_mask anymore.
> .

Mickaël Salaün Nov. 28, 2022, 8:26 p.m. UTC | #3

On 28/11/2022 05:01, Konstantin Meskhidze (A) wrote:
> 
> 
> 11/17/2022 9:43 PM, Mickaël Salaün пишет:
>>
>> On 21/10/2022 17:26, Konstantin Meskhidze wrote:
>>> This commit adds network rules support in internal landlock functions
>>> (presented in ruleset.c) and landlock_create_ruleset syscall.
>>
>> …in the ruleset management helpers and the landlock_create_ruleset syscall.
>>
>>
>>> Refactors user space API to support network actions. Adds new network
>>
>> Refactor…
>>
>>> access flags, network rule and network attributes. Increments Landlock
>>
>> Increment…
> 
>     The commit's message will be fixed. Thank you!
>>
>>> ABI version.
>>>
>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>> ---
>>>
>>> Changes since v7:
>>> * Squashes commits.
>>> * Increments ABI version to 4.
>>> * Refactors commit message.
>>> * Minor fixes.
>>>
>>> Changes since v6:
>>> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask()
>>>     because it OR values.
>>> * Makes landlock_add_net_access_mask() more resilient incorrect values.
>>> * Refactors landlock_get_net_access_mask().
>>> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use
>>>     LANDLOCK_NUM_ACCESS_FS as value.
>>> * Updates access_masks_t to u32 to support network access actions.
>>> * Refactors landlock internal functions to support network actions with
>>>     landlock_key/key_type/id types.
>>>
>>> Changes since v5:
>>> * Gets rid of partial revert from landlock_add_rule
>>> syscall.
>>> * Formats code with clang-format-14.
>>>
>>> Changes since v4:
>>> * Refactors landlock_create_ruleset() - splits ruleset and
>>> masks checks.
>>> * Refactors landlock_create_ruleset() and landlock mask
>>> setters/getters to support two rule types.
>>> * Refactors landlock_add_rule syscall add_rule_path_beneath
>>> function by factoring out get_ruleset_from_fd() and
>>> landlock_put_ruleset().
>>>
>>> Changes since v3:
>>> * Splits commit.
>>> * Adds network rule support for internal landlock functions.
>>> * Adds set_mask and get_mask for network.
>>> * Adds rb_root root_net_port.
>>>
>>> ---
>>>    include/uapi/linux/landlock.h                | 49 ++++++++++++++
>>>    security/landlock/limits.h                   |  6 +-
>>>    security/landlock/ruleset.c                  | 55 ++++++++++++++--
>>>    security/landlock/ruleset.h                  | 68 ++++++++++++++++----
>>>    security/landlock/syscalls.c                 | 13 +++-
>>>    tools/testing/selftests/landlock/base_test.c |  2 +-
>>>    6 files changed, 170 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>>> index f3223f964691..096b683c6ff3 100644
>>> --- a/include/uapi/linux/landlock.h
>>> +++ b/include/uapi/linux/landlock.h
>>> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr {
>>>    	 * this access right.
>>>    	 */
>>>    	__u64 handled_access_fs;
>>> +
>>> +	/**
>>> +	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
>>> +	 * that is handled by this ruleset and should then be forbidden if no
>>> +	 * rule explicitly allow them.
>>> +	 */
>>> +	__u64 handled_access_net;
>>>    };
>>>
>>>    /*
>>> @@ -54,6 +61,11 @@ enum landlock_rule_type {
>>>    	 * landlock_path_beneath_attr .
>>>    	 */
>>>    	LANDLOCK_RULE_PATH_BENEATH = 1,
>>> +	/**
>>> +	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
>>> +	 * landlock_net_service_attr .
>>> +	 */
>>> +	LANDLOCK_RULE_NET_SERVICE = 2,
>>>    };
>>>
>>>    /**
>>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>>>    	 */
>>>    } __attribute__((packed));
>>>
>>> +/**
>>> + * struct landlock_net_service_attr - TCP subnet definition
>>> + *
>>> + * Argument of sys_landlock_add_rule().
>>> + */
>>> +struct landlock_net_service_attr {
>>> +	/**
>>> +	 * @allowed_access: Bitmask of allowed access network for services
>>> +	 * (cf. `Network flags`_).
>>> +	 */
>>> +	__u64 allowed_access;
>>> +	/**
>>> +	 * @port: Network port.
>>> +	 */
>>> +	__u16 port;
>>
>>    From an UAPI point of view, I think the port field should be __be16, as
>> for sockaddr_in->port and other network-related APIs. This will require
>> some kernel changes to please sparse: make C=2 security/landlock/ must
>> not print any warning.
> 
>     Is sparse a default checker?

You should be able to easily install it with your Linux distro.

Konstantin Meskhidze (A) Dec. 2, 2022, 2:54 a.m. UTC | #4

11/28/2022 11:26 PM, Mickaël Salaün пишет:
> 
> On 28/11/2022 05:01, Konstantin Meskhidze (A) wrote:
>> 
>> 
>> 11/17/2022 9:43 PM, Mickaël Salaün пишет:
>>>
>>> On 21/10/2022 17:26, Konstantin Meskhidze wrote:
>>>> This commit adds network rules support in internal landlock functions
>>>> (presented in ruleset.c) and landlock_create_ruleset syscall.
>>>
>>> …in the ruleset management helpers and the landlock_create_ruleset syscall.
>>>
>>>
>>>> Refactors user space API to support network actions. Adds new network
>>>
>>> Refactor…
>>>
>>>> access flags, network rule and network attributes. Increments Landlock
>>>
>>> Increment…
>> 
>>     The commit's message will be fixed. Thank you!
>>>
>>>> ABI version.
>>>>
>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>>> ---
>>>>
>>>> Changes since v7:
>>>> * Squashes commits.
>>>> * Increments ABI version to 4.
>>>> * Refactors commit message.
>>>> * Minor fixes.
>>>>
>>>> Changes since v6:
>>>> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask()
>>>>     because it OR values.
>>>> * Makes landlock_add_net_access_mask() more resilient incorrect values.
>>>> * Refactors landlock_get_net_access_mask().
>>>> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use
>>>>     LANDLOCK_NUM_ACCESS_FS as value.
>>>> * Updates access_masks_t to u32 to support network access actions.
>>>> * Refactors landlock internal functions to support network actions with
>>>>     landlock_key/key_type/id types.
>>>>
>>>> Changes since v5:
>>>> * Gets rid of partial revert from landlock_add_rule
>>>> syscall.
>>>> * Formats code with clang-format-14.
>>>>
>>>> Changes since v4:
>>>> * Refactors landlock_create_ruleset() - splits ruleset and
>>>> masks checks.
>>>> * Refactors landlock_create_ruleset() and landlock mask
>>>> setters/getters to support two rule types.
>>>> * Refactors landlock_add_rule syscall add_rule_path_beneath
>>>> function by factoring out get_ruleset_from_fd() and
>>>> landlock_put_ruleset().
>>>>
>>>> Changes since v3:
>>>> * Splits commit.
>>>> * Adds network rule support for internal landlock functions.
>>>> * Adds set_mask and get_mask for network.
>>>> * Adds rb_root root_net_port.
>>>>
>>>> ---
>>>>    include/uapi/linux/landlock.h                | 49 ++++++++++++++
>>>>    security/landlock/limits.h                   |  6 +-
>>>>    security/landlock/ruleset.c                  | 55 ++++++++++++++--
>>>>    security/landlock/ruleset.h                  | 68 ++++++++++++++++----
>>>>    security/landlock/syscalls.c                 | 13 +++-
>>>>    tools/testing/selftests/landlock/base_test.c |  2 +-
>>>>    6 files changed, 170 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>>>> index f3223f964691..096b683c6ff3 100644
>>>> --- a/include/uapi/linux/landlock.h
>>>> +++ b/include/uapi/linux/landlock.h
>>>> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr {
>>>>    	 * this access right.
>>>>    	 */
>>>>    	__u64 handled_access_fs;
>>>> +
>>>> +	/**
>>>> +	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
>>>> +	 * that is handled by this ruleset and should then be forbidden if no
>>>> +	 * rule explicitly allow them.
>>>> +	 */
>>>> +	__u64 handled_access_net;
>>>>    };
>>>>
>>>>    /*
>>>> @@ -54,6 +61,11 @@ enum landlock_rule_type {
>>>>    	 * landlock_path_beneath_attr .
>>>>    	 */
>>>>    	LANDLOCK_RULE_PATH_BENEATH = 1,
>>>> +	/**
>>>> +	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
>>>> +	 * landlock_net_service_attr .
>>>> +	 */
>>>> +	LANDLOCK_RULE_NET_SERVICE = 2,
>>>>    };
>>>>
>>>>    /**
>>>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>>>>    	 */
>>>>    } __attribute__((packed));
>>>>
>>>> +/**
>>>> + * struct landlock_net_service_attr - TCP subnet definition
>>>> + *
>>>> + * Argument of sys_landlock_add_rule().
>>>> + */
>>>> +struct landlock_net_service_attr {
>>>> +	/**
>>>> +	 * @allowed_access: Bitmask of allowed access network for services
>>>> +	 * (cf. `Network flags`_).
>>>> +	 */
>>>> +	__u64 allowed_access;
>>>> +	/**
>>>> +	 * @port: Network port.
>>>> +	 */
>>>> +	__u16 port;
>>>
>>>    From an UAPI point of view, I think the port field should be __be16, as
>>> for sockaddr_in->port and other network-related APIs. This will require
>>> some kernel changes to please sparse: make C=2 security/landlock/ must
>>> not print any warning.
>> 
>>     Is sparse a default checker?
> 
> You should be able to easily install it with your Linux distro.

  Ok. Thank you.
> .

Konstantin Meskhidze (A) Jan. 3, 2023, 12:44 p.m. UTC | #5

11/17/2022 9:43 PM, Mickaël Salaün пишет:
> 
> On 21/10/2022 17:26, Konstantin Meskhidze wrote:
>> This commit adds network rules support in internal landlock functions
>> (presented in ruleset.c) and landlock_create_ruleset syscall.
> 
> …in the ruleset management helpers and the landlock_create_ruleset syscall.
> 
> 
>> Refactors user space API to support network actions. Adds new network
> 
> Refactor…
> 
>> access flags, network rule and network attributes. Increments Landlock
> 
> Increment…
> 
>> ABI version.
>> 
>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> ---
>> 
>> Changes since v7:
>> * Squashes commits.
>> * Increments ABI version to 4.
>> * Refactors commit message.
>> * Minor fixes.
>> 
>> Changes since v6:
>> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask()
>>    because it OR values.
>> * Makes landlock_add_net_access_mask() more resilient incorrect values.
>> * Refactors landlock_get_net_access_mask().
>> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use
>>    LANDLOCK_NUM_ACCESS_FS as value.
>> * Updates access_masks_t to u32 to support network access actions.
>> * Refactors landlock internal functions to support network actions with
>>    landlock_key/key_type/id types.
>> 
>> Changes since v5:
>> * Gets rid of partial revert from landlock_add_rule
>> syscall.
>> * Formats code with clang-format-14.
>> 
>> Changes since v4:
>> * Refactors landlock_create_ruleset() - splits ruleset and
>> masks checks.
>> * Refactors landlock_create_ruleset() and landlock mask
>> setters/getters to support two rule types.
>> * Refactors landlock_add_rule syscall add_rule_path_beneath
>> function by factoring out get_ruleset_from_fd() and
>> landlock_put_ruleset().
>> 
>> Changes since v3:
>> * Splits commit.
>> * Adds network rule support for internal landlock functions.
>> * Adds set_mask and get_mask for network.
>> * Adds rb_root root_net_port.
>> 
>> ---
>>   include/uapi/linux/landlock.h                | 49 ++++++++++++++
>>   security/landlock/limits.h                   |  6 +-
>>   security/landlock/ruleset.c                  | 55 ++++++++++++++--
>>   security/landlock/ruleset.h                  | 68 ++++++++++++++++----
>>   security/landlock/syscalls.c                 | 13 +++-
>>   tools/testing/selftests/landlock/base_test.c |  2 +-
>>   6 files changed, 170 insertions(+), 23 deletions(-)
>> 
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index f3223f964691..096b683c6ff3 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr {
>>   	 * this access right.
>>   	 */
>>   	__u64 handled_access_fs;
>> +
>> +	/**
>> +	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
>> +	 * that is handled by this ruleset and should then be forbidden if no
>> +	 * rule explicitly allow them.
>> +	 */
>> +	__u64 handled_access_net;
>>   };
>> 
>>   /*
>> @@ -54,6 +61,11 @@ enum landlock_rule_type {
>>   	 * landlock_path_beneath_attr .
>>   	 */
>>   	LANDLOCK_RULE_PATH_BENEATH = 1,
>> +	/**
>> +	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
>> +	 * landlock_net_service_attr .
>> +	 */
>> +	LANDLOCK_RULE_NET_SERVICE = 2,
>>   };
>> 
>>   /**
>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>>   	 */
>>   } __attribute__((packed));
>> 
>> +/**
>> + * struct landlock_net_service_attr - TCP subnet definition
>> + *
>> + * Argument of sys_landlock_add_rule().
>> + */
>> +struct landlock_net_service_attr {
>> +	/**
>> +	 * @allowed_access: Bitmask of allowed access network for services
>> +	 * (cf. `Network flags`_).
>> +	 */
>> +	__u64 allowed_access;
>> +	/**
>> +	 * @port: Network port.
>> +	 */
>> +	__u16 port;
> 
>   From an UAPI point of view, I think the port field should be __be16, as
> for sockaddr_in->port and other network-related APIs. This will require
> some kernel changes to please sparse: make C=2 security/landlock/ must
> not print any warning.
> 
> Using big-endian values as keys (casted to uintptr_t, not strictly
> __be16) in the rb-tree should not be an issue because there is no port
> range ordering (for now).
> 
> A dedicated test should check that endianness is correct, e.g. by using
> different port encoding. This should include passing and failing tests,
> but they should work on all architectures (i.e. big or little endian).
> 
   Hi Mickaёl.
   Could you please give me a piece of advice about these kind of tests?
   I have not entirely understood this point.
> 
>> +
>> +} __attribute__((packed));
>> +
>>   /**
>>    * DOC: fs_access
>>    *
>> @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr {
>>   #define LANDLOCK_ACCESS_FS_TRUNCATE			(1ULL << 14)
>>   /* clang-format on */
>> 
>> +/**
>> + * DOC: net_access
>> + *
>> + * Network flags
>> + * ~~~~~~~~~~~~~~~~
>> + *
>> + * These flags enable to restrict a sandboxed process to a set of network
>> + * actions.
>> + *
>> + * TCP sockets with allowed actions:
>> + *
>> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
>> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
>> + *   a remote port.
>> + */
>> +/* clang-format off */
>> +#define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>> +/* clang-format on */
>>   #endif /* _UAPI_LINUX_LANDLOCK_H */
>> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
>> index bafb3b8dc677..8a1a6463c64e 100644
>> --- a/security/landlock/limits.h
>> +++ b/security/landlock/limits.h
>> @@ -23,6 +23,10 @@
>>   #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>>   #define LANDLOCK_SHIFT_ACCESS_FS	0
>> 
>> -/* clang-format on */
>> +#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 */
>>   #endif /* _SECURITY_LANDLOCK_LIMITS_H */
>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>> index c7cf54ba4f6d..9277c1295114 100644
>> --- a/security/landlock/ruleset.c
>> +++ b/security/landlock/ruleset.c
>> @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   	refcount_set(&new_ruleset->usage, 1);
>>   	mutex_init(&new_ruleset->lock);
>>   	new_ruleset->root_inode = RB_ROOT;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	new_ruleset->root_net_port = RB_ROOT;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	new_ruleset->num_layers = num_layers;
>>   	/*
>>   	 * hierarchy = NULL
>> @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   }
>> 
>>   struct landlock_ruleset *
>> -landlock_create_ruleset(const access_mask_t fs_access_mask)
>> +landlock_create_ruleset(const access_mask_t fs_access_mask,
>> +			const access_mask_t net_access_mask)
>>   {
>>   	struct landlock_ruleset *new_ruleset;
>> 
>>   	/* Informs about useless ruleset. */
>> -	if (!fs_access_mask)
>> +	if (!fs_access_mask && !net_access_mask)
>>   		return ERR_PTR(-ENOMSG);
>>   	new_ruleset = create_ruleset(1);
>> -	if (!IS_ERR(new_ruleset))
>> +	if (IS_ERR(new_ruleset))
>> +		return new_ruleset;
>> +	if (fs_access_mask)
>>   		landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
>> +	if (net_access_mask)
>> +		landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
>>   	return new_ruleset;
>>   }
>> 
>> @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type)
>>   	switch (key_type) {
>>   	case LANDLOCK_KEY_INODE:
>>   		return true;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		return false;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	}
>>   	WARN_ON_ONCE(1);
>>   	return false;
>> @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset,
>>   	case LANDLOCK_KEY_INODE:
>>   		root = &ruleset->root_inode;
>>   		break;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		root = &ruleset->root_net_port;
>> +		break;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	}
>>   	if (WARN_ON_ONCE(!root))
>>   		return ERR_PTR(-EINVAL);
>> @@ -154,7 +171,9 @@ static void build_check_ruleset(void)
>>   	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_FS << LANDLOCK_SHIFT_ACCESS_FS) +
> 
> This is correct but because we are dealing with bitmasks I would prefer
> to use "|" instead of "+".
> 
> 
>> +			     (LANDLOCK_MASK_ACCESS_NET
>> +			      << LANDLOCK_SHIFT_ACCESS_NET));
>>   }
>> 
>>   /**
>> @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
>>   	err = merge_tree(dst, src, LANDLOCK_KEY_INODE);
>>   	if (err)
>>   		goto out_unlock;
>> -
> 
> Please keep this newline.
> 
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/* Merges the @src network port tree. */
>> +	err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT);
>> +	if (err)
>> +		goto out_unlock;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   out_unlock:
>>   	mutex_unlock(&src->lock);
>>   	mutex_unlock(&dst->lock);
>> @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
>>   	err = inherit_tree(parent, child, LANDLOCK_KEY_INODE);
>>   	if (err)
>>   		goto out_unlock;
>> -
> 
> newline
> 
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/* Copies the @parent network port tree. */
>> +	err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT);
>> +	if (err)
>> +		goto out_unlock;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
>>   		err = -EINVAL;
>>   		goto out_unlock;
>> @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
>>   	rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode,
>>   					     node)
>>   		free_rule(freeme, LANDLOCK_KEY_INODE);
>> +#if IS_ENABLED(CONFIG_INET)
>> +	rbtree_postorder_for_each_entry_safe(freeme, next,
>> +					     &ruleset->root_net_port, node)
>> +		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	put_hierarchy(ruleset->hierarchy);
>>   	kfree(ruleset);
>>   }
>> @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
>>    * 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.
>>    *
>> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET
>> + * elements according to @key_type.
> 
> Please include this sentence in the @layer_masks description below.
> 
>> + *
>>    * @domain: The domain that defines the current restrictions.
>>    * @access_request: The requested access rights to check.
>>    * @layer_masks: The layer masks to populate.
> 
> "It must contain…"
> 
> 
>> @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain,
>>   		get_access_mask = landlock_get_fs_access_mask;
>>   		num_access = LANDLOCK_NUM_ACCESS_FS;
>>   		break;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		get_access_mask = landlock_get_net_access_mask;
>> +		num_access = LANDLOCK_NUM_ACCESS_NET;
>> +		break;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	default:
>>   		WARN_ON_ONCE(1);
>>   		return 0;
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index d9eb79ea9a89..f272d2cd518c 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -19,16 +19,20 @@
>>   #include "limits.h"
>>   #include "object.h"
>> 
>> +/* Rule access mask. */
>>   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 all network access rights can be stored. */
>> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
>>   /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
>>   static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
>> 
>>   /* Ruleset access masks. */
>> -typedef u16 access_masks_t;
>> +typedef u32 access_masks_t;
> 
> This type change need to be explained in the commit message.
> 
> 
>>   /* Makes sure all ruleset access rights can be stored. */
>> -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS);
>> +static_assert(BITS_PER_TYPE(access_masks_t) >=
>> +	      LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET);
>> 
>>   typedef u16 layer_mask_t;
>>   /* Makes sure all layers can be checked. */
>> @@ -82,6 +86,13 @@ enum landlock_key_type {
>>   	 * keys.
>>   	 */
>>   	LANDLOCK_KEY_INODE = 1,
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/**
>> +	 * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's
>> +	 * node keys.
>> +	 */
>> +	LANDLOCK_KEY_NET_PORT = 2,
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   };
>> 
>>   /**
>> @@ -156,6 +167,15 @@ struct landlock_ruleset {
>>   	 * reaches zero.
>>   	 */
>>   	struct rb_root root_inode;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/**
>> +	 * @root_net_port: Root of a red-black tree containing &struct
>> +	 * landlock_rule nodes with network port. Once a ruleset is tied to a
>> +	 * process (i.e. as a domain), this tree is immutable until @usage
>> +	 * reaches zero.
>> +	 */
>> +	struct rb_root root_net_port;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	/**
>>   	 * @hierarchy: Enables hierarchy identification even when a parent
>>   	 * domain vanishes.  This is needed for the ptrace protection.
>> @@ -166,8 +186,8 @@ struct landlock_ruleset {
>>   		 * @work_free: Enables to free a ruleset within a lockless
>>   		 * section.  This is only used by
>>   		 * landlock_put_ruleset_deferred() when @usage reaches zero.
>> -		 * The fields @lock, @usage, @num_rules, @num_layers and
>> -		 * @access_masks are then unused.
>> +		 * The fields @lock, @usage, @num_rules, @num_layers,
>> +		 * @net_access_mask and @access_masks are then unused.
> 
> There is no net_access_mask anymore.
> .

Konstantin Meskhidze (A) Jan. 4, 2023, 11:41 a.m. UTC | #6

11/17/2022 9:43 PM, Mickaël Salaün пишет:
> 
> On 21/10/2022 17:26, Konstantin Meskhidze wrote:
>> This commit adds network rules support in internal landlock functions
>> (presented in ruleset.c) and landlock_create_ruleset syscall.
> 
> …in the ruleset management helpers and the landlock_create_ruleset syscall.
> 
> 
>> Refactors user space API to support network actions. Adds new network
> 
> Refactor…
> 
>> access flags, network rule and network attributes. Increments Landlock
> 
> Increment…
> 
>> ABI version.
>> 
>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> ---
>> 
>> Changes since v7:
>> * Squashes commits.
>> * Increments ABI version to 4.
>> * Refactors commit message.
>> * Minor fixes.
>> 
>> Changes since v6:
>> * Renames landlock_set_net_access_mask() to landlock_add_net_access_mask()
>>    because it OR values.
>> * Makes landlock_add_net_access_mask() more resilient incorrect values.
>> * Refactors landlock_get_net_access_mask().
>> * Renames LANDLOCK_MASK_SHIFT_NET to LANDLOCK_SHIFT_ACCESS_NET and use
>>    LANDLOCK_NUM_ACCESS_FS as value.
>> * Updates access_masks_t to u32 to support network access actions.
>> * Refactors landlock internal functions to support network actions with
>>    landlock_key/key_type/id types.
>> 
>> Changes since v5:
>> * Gets rid of partial revert from landlock_add_rule
>> syscall.
>> * Formats code with clang-format-14.
>> 
>> Changes since v4:
>> * Refactors landlock_create_ruleset() - splits ruleset and
>> masks checks.
>> * Refactors landlock_create_ruleset() and landlock mask
>> setters/getters to support two rule types.
>> * Refactors landlock_add_rule syscall add_rule_path_beneath
>> function by factoring out get_ruleset_from_fd() and
>> landlock_put_ruleset().
>> 
>> Changes since v3:
>> * Splits commit.
>> * Adds network rule support for internal landlock functions.
>> * Adds set_mask and get_mask for network.
>> * Adds rb_root root_net_port.
>> 
>> ---
>>   include/uapi/linux/landlock.h                | 49 ++++++++++++++
>>   security/landlock/limits.h                   |  6 +-
>>   security/landlock/ruleset.c                  | 55 ++++++++++++++--
>>   security/landlock/ruleset.h                  | 68 ++++++++++++++++----
>>   security/landlock/syscalls.c                 | 13 +++-
>>   tools/testing/selftests/landlock/base_test.c |  2 +-
>>   6 files changed, 170 insertions(+), 23 deletions(-)
>> 
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index f3223f964691..096b683c6ff3 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -31,6 +31,13 @@ struct landlock_ruleset_attr {
>>   	 * this access right.
>>   	 */
>>   	__u64 handled_access_fs;
>> +
>> +	/**
>> +	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
>> +	 * that is handled by this ruleset and should then be forbidden if no
>> +	 * rule explicitly allow them.
>> +	 */
>> +	__u64 handled_access_net;
>>   };
>> 
>>   /*
>> @@ -54,6 +61,11 @@ enum landlock_rule_type {
>>   	 * landlock_path_beneath_attr .
>>   	 */
>>   	LANDLOCK_RULE_PATH_BENEATH = 1,
>> +	/**
>> +	 * @LANDLOCK_RULE_NET_SERVICE: Type of a &struct
>> +	 * landlock_net_service_attr .
>> +	 */
>> +	LANDLOCK_RULE_NET_SERVICE = 2,
>>   };
>> 
>>   /**
>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>>   	 */
>>   } __attribute__((packed));
>> 
>> +/**
>> + * struct landlock_net_service_attr - TCP subnet definition
>> + *
>> + * Argument of sys_landlock_add_rule().
>> + */
>> +struct landlock_net_service_attr {
>> +	/**
>> +	 * @allowed_access: Bitmask of allowed access network for services
>> +	 * (cf. `Network flags`_).
>> +	 */
>> +	__u64 allowed_access;
>> +	/**
>> +	 * @port: Network port.
>> +	 */
>> +	__u16 port;
> 
>   From an UAPI point of view, I think the port field should be __be16, as
> for sockaddr_in->port and other network-related APIs. This will require
> some kernel changes to please sparse: make C=2 security/landlock/ must
> not print any warning.

   I have this errors trying to launch sparse checking:

   DESCEND objtool
   DESCEND bpf/resolve_btfids
   CALL    scripts/checksyscalls.sh
   CHK     kernel/kheaders_data.tar.xz
   CC      security/landlock/setup.o
   CHECK   security/landlock/setup.c
./include/asm-generic/rwonce.h:67:16: error: typename in expression
./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call
./include/asm-generic/rwonce.h:67:16: error: got :
./include/linux/list.h:292:16: error: typename in expression
./include/linux/list.h:292:16: error: Expected ) in function call
./include/linux/list.h:292:16: error: got :

....

./include/linux/seqlock.h:682:16: error: Expected ) in function call
./include/linux/seqlock.h:682:16: error: got :
./include/linux/seqlock.h:695:16: error: typename in expression
./include/linux/seqlock.h:695:16: error: Expected ) in function call
./include/linux/seqlock.h:695:16: error: too many errors
Segmentation fault (core dumped)
make[3]: *** [scripts/Makefile.build:250: security/landlock/setup.o] 
Error 139
make[3]: *** Deleting file 'security/landlock/setup.o'
make[3]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[3]: *** [scripts/Makefile.build:250: security/landlock/syscalls.o] 
Error 139
make[3]: *** Deleting file 'security/landlock/syscalls.o'
make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2
make[1]: *** [scripts/Makefile.build:502: security] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1994: .] Error 2



> 
> Using big-endian values as keys (casted to uintptr_t, not strictly
> __be16) in the rb-tree should not be an issue because there is no port
> range ordering (for now).
> 
> A dedicated test should check that endianness is correct, e.g. by using
> different port encoding. This should include passing and failing tests,
> but they should work on all architectures (i.e. big or little endian).
> 
> 
>> +
>> +} __attribute__((packed));
>> +
>>   /**
>>    * DOC: fs_access
>>    *
>> @@ -173,4 +203,23 @@ struct landlock_path_beneath_attr {
>>   #define LANDLOCK_ACCESS_FS_TRUNCATE			(1ULL << 14)
>>   /* clang-format on */
>> 
>> +/**
>> + * DOC: net_access
>> + *
>> + * Network flags
>> + * ~~~~~~~~~~~~~~~~
>> + *
>> + * These flags enable to restrict a sandboxed process to a set of network
>> + * actions.
>> + *
>> + * TCP sockets with allowed actions:
>> + *
>> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
>> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
>> + *   a remote port.
>> + */
>> +/* clang-format off */
>> +#define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>> +/* clang-format on */
>>   #endif /* _UAPI_LINUX_LANDLOCK_H */
>> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
>> index bafb3b8dc677..8a1a6463c64e 100644
>> --- a/security/landlock/limits.h
>> +++ b/security/landlock/limits.h
>> @@ -23,6 +23,10 @@
>>   #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>>   #define LANDLOCK_SHIFT_ACCESS_FS	0
>> 
>> -/* clang-format on */
>> +#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 */
>>   #endif /* _SECURITY_LANDLOCK_LIMITS_H */
>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>> index c7cf54ba4f6d..9277c1295114 100644
>> --- a/security/landlock/ruleset.c
>> +++ b/security/landlock/ruleset.c
>> @@ -36,6 +36,9 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   	refcount_set(&new_ruleset->usage, 1);
>>   	mutex_init(&new_ruleset->lock);
>>   	new_ruleset->root_inode = RB_ROOT;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	new_ruleset->root_net_port = RB_ROOT;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	new_ruleset->num_layers = num_layers;
>>   	/*
>>   	 * hierarchy = NULL
>> @@ -46,16 +49,21 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   }
>> 
>>   struct landlock_ruleset *
>> -landlock_create_ruleset(const access_mask_t fs_access_mask)
>> +landlock_create_ruleset(const access_mask_t fs_access_mask,
>> +			const access_mask_t net_access_mask)
>>   {
>>   	struct landlock_ruleset *new_ruleset;
>> 
>>   	/* Informs about useless ruleset. */
>> -	if (!fs_access_mask)
>> +	if (!fs_access_mask && !net_access_mask)
>>   		return ERR_PTR(-ENOMSG);
>>   	new_ruleset = create_ruleset(1);
>> -	if (!IS_ERR(new_ruleset))
>> +	if (IS_ERR(new_ruleset))
>> +		return new_ruleset;
>> +	if (fs_access_mask)
>>   		landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
>> +	if (net_access_mask)
>> +		landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
>>   	return new_ruleset;
>>   }
>> 
>> @@ -73,6 +81,10 @@ static inline bool is_object_pointer(const enum landlock_key_type key_type)
>>   	switch (key_type) {
>>   	case LANDLOCK_KEY_INODE:
>>   		return true;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		return false;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	}
>>   	WARN_ON_ONCE(1);
>>   	return false;
>> @@ -126,6 +138,11 @@ static inline struct rb_root *get_root(struct landlock_ruleset *const ruleset,
>>   	case LANDLOCK_KEY_INODE:
>>   		root = &ruleset->root_inode;
>>   		break;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		root = &ruleset->root_net_port;
>> +		break;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	}
>>   	if (WARN_ON_ONCE(!root))
>>   		return ERR_PTR(-EINVAL);
>> @@ -154,7 +171,9 @@ static void build_check_ruleset(void)
>>   	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_FS << LANDLOCK_SHIFT_ACCESS_FS) +
> 
> This is correct but because we are dealing with bitmasks I would prefer
> to use "|" instead of "+".
> 
> 
>> +			     (LANDLOCK_MASK_ACCESS_NET
>> +			      << LANDLOCK_SHIFT_ACCESS_NET));
>>   }
>> 
>>   /**
>> @@ -370,7 +389,12 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
>>   	err = merge_tree(dst, src, LANDLOCK_KEY_INODE);
>>   	if (err)
>>   		goto out_unlock;
>> -
> 
> Please keep this newline.
> 
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/* Merges the @src network port tree. */
>> +	err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT);
>> +	if (err)
>> +		goto out_unlock;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   out_unlock:
>>   	mutex_unlock(&src->lock);
>>   	mutex_unlock(&dst->lock);
>> @@ -426,7 +450,12 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
>>   	err = inherit_tree(parent, child, LANDLOCK_KEY_INODE);
>>   	if (err)
>>   		goto out_unlock;
>> -
> 
> newline
> 
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/* Copies the @parent network port tree. */
>> +	err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT);
>> +	if (err)
>> +		goto out_unlock;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
>>   		err = -EINVAL;
>>   		goto out_unlock;
>> @@ -459,6 +488,11 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
>>   	rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root_inode,
>>   					     node)
>>   		free_rule(freeme, LANDLOCK_KEY_INODE);
>> +#if IS_ENABLED(CONFIG_INET)
>> +	rbtree_postorder_for_each_entry_safe(freeme, next,
>> +					     &ruleset->root_net_port, node)
>> +		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	put_hierarchy(ruleset->hierarchy);
>>   	kfree(ruleset);
>>   }
>> @@ -637,6 +671,9 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
>>    * 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.
>>    *
>> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET
>> + * elements according to @key_type.
> 
> Please include this sentence in the @layer_masks description below.
> 
>> + *
>>    * @domain: The domain that defines the current restrictions.
>>    * @access_request: The requested access rights to check.
>>    * @layer_masks: The layer masks to populate.
> 
> "It must contain…"
> 
> 
>> @@ -659,6 +696,12 @@ access_mask_t init_layer_masks(const struct landlock_ruleset *const domain,
>>   		get_access_mask = landlock_get_fs_access_mask;
>>   		num_access = LANDLOCK_NUM_ACCESS_FS;
>>   		break;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	case LANDLOCK_KEY_NET_PORT:
>> +		get_access_mask = landlock_get_net_access_mask;
>> +		num_access = LANDLOCK_NUM_ACCESS_NET;
>> +		break;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	default:
>>   		WARN_ON_ONCE(1);
>>   		return 0;
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index d9eb79ea9a89..f272d2cd518c 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -19,16 +19,20 @@
>>   #include "limits.h"
>>   #include "object.h"
>> 
>> +/* Rule access mask. */
>>   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 all network access rights can be stored. */
>> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
>>   /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
>>   static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
>> 
>>   /* Ruleset access masks. */
>> -typedef u16 access_masks_t;
>> +typedef u32 access_masks_t;
> 
> This type change need to be explained in the commit message.
> 
> 
>>   /* Makes sure all ruleset access rights can be stored. */
>> -static_assert(BITS_PER_TYPE(access_masks_t) >= LANDLOCK_NUM_ACCESS_FS);
>> +static_assert(BITS_PER_TYPE(access_masks_t) >=
>> +	      LANDLOCK_NUM_ACCESS_FS + LANDLOCK_NUM_ACCESS_NET);
>> 
>>   typedef u16 layer_mask_t;
>>   /* Makes sure all layers can be checked. */
>> @@ -82,6 +86,13 @@ enum landlock_key_type {
>>   	 * keys.
>>   	 */
>>   	LANDLOCK_KEY_INODE = 1,
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/**
>> +	 * @LANDLOCK_KEY_NET_PORT: Type of &landlock_ruleset.root_net_port's
>> +	 * node keys.
>> +	 */
>> +	LANDLOCK_KEY_NET_PORT = 2,
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   };
>> 
>>   /**
>> @@ -156,6 +167,15 @@ struct landlock_ruleset {
>>   	 * reaches zero.
>>   	 */
>>   	struct rb_root root_inode;
>> +#if IS_ENABLED(CONFIG_INET)
>> +	/**
>> +	 * @root_net_port: Root of a red-black tree containing &struct
>> +	 * landlock_rule nodes with network port. Once a ruleset is tied to a
>> +	 * process (i.e. as a domain), this tree is immutable until @usage
>> +	 * reaches zero.
>> +	 */
>> +	struct rb_root root_net_port;
>> +#endif /* IS_ENABLED(CONFIG_INET) */
>>   	/**
>>   	 * @hierarchy: Enables hierarchy identification even when a parent
>>   	 * domain vanishes.  This is needed for the ptrace protection.
>> @@ -166,8 +186,8 @@ struct landlock_ruleset {
>>   		 * @work_free: Enables to free a ruleset within a lockless
>>   		 * section.  This is only used by
>>   		 * landlock_put_ruleset_deferred() when @usage reaches zero.
>> -		 * The fields @lock, @usage, @num_rules, @num_layers and
>> -		 * @access_masks are then unused.
>> +		 * The fields @lock, @usage, @num_rules, @num_layers,
>> +		 * @net_access_mask and @access_masks are then unused.
> 
> There is no net_access_mask anymore.
> .

Mickaël Salaün Jan. 6, 2023, 7:22 p.m. UTC | #7

On 04/01/2023 12:41, Konstantin Meskhidze (A) wrote:
> 
> 
> 11/17/2022 9:43 PM, Mickaël Salaün пишет:

[...]

>>>    /**
>>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>>>    	 */
>>>    } __attribute__((packed));
>>>
>>> +/**
>>> + * struct landlock_net_service_attr - TCP subnet definition
>>> + *
>>> + * Argument of sys_landlock_add_rule().
>>> + */
>>> +struct landlock_net_service_attr {
>>> +	/**
>>> +	 * @allowed_access: Bitmask of allowed access network for services
>>> +	 * (cf. `Network flags`_).
>>> +	 */
>>> +	__u64 allowed_access;
>>> +	/**
>>> +	 * @port: Network port.
>>> +	 */
>>> +	__u16 port;
>>
>>    From an UAPI point of view, I think the port field should be __be16, as
>> for sockaddr_in->port and other network-related APIs. This will require
>> some kernel changes to please sparse: make C=2 security/landlock/ must
>> not print any warning.
> 
>     I have this errors trying to launch sparse checking:
> 
>     DESCEND objtool
>     DESCEND bpf/resolve_btfids
>     CALL    scripts/checksyscalls.sh
>     CHK     kernel/kheaders_data.tar.xz
>     CC      security/landlock/setup.o
>     CHECK   security/landlock/setup.c
> ./include/asm-generic/rwonce.h:67:16: error: typename in expression
> ./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call
> ./include/asm-generic/rwonce.h:67:16: error: got :
> ./include/linux/list.h:292:16: error: typename in expression
> ./include/linux/list.h:292:16: error: Expected ) in function call
> ./include/linux/list.h:292:16: error: got :
> 
> ....
> 
> ./include/linux/seqlock.h:682:16: error: Expected ) in function call
> ./include/linux/seqlock.h:682:16: error: got :
> ./include/linux/seqlock.h:695:16: error: typename in expression
> ./include/linux/seqlock.h:695:16: error: Expected ) in function call
> ./include/linux/seqlock.h:695:16: error: too many errors
> Segmentation fault (core dumped)
> make[3]: *** [scripts/Makefile.build:250: security/landlock/setup.o]
> Error 139
> make[3]: *** Deleting file 'security/landlock/setup.o'
> make[3]: *** Waiting for unfinished jobs....
> Segmentation fault (core dumped)
> make[3]: *** [scripts/Makefile.build:250: security/landlock/syscalls.o]
> Error 139
> make[3]: *** Deleting file 'security/landlock/syscalls.o'
> make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2
> make[1]: *** [scripts/Makefile.build:502: security] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1994: .] Error 2

I don't know about this error. Did you follow the documentation?
https://docs.kernel.org/dev-tools/sparse.html#getting-sparse



>>
>> Using big-endian values as keys (casted to uintptr_t, not strictly
>> __be16) in the rb-tree should not be an issue because there is no port
>> range ordering (for now).
>>
>> A dedicated test should check that endianness is correct, e.g. by using
>> different port encoding. This should include passing and failing tests,
>> but they should work on all architectures (i.e. big or little endian).

Konstantin Meskhidze (A) Jan. 9, 2023, 7:59 a.m. UTC | #8

1/6/2023 10:22 PM, Mickaël Salaün пишет:
> 
> On 04/01/2023 12:41, Konstantin Meskhidze (A) wrote:
>> 
>> 
>> 11/17/2022 9:43 PM, Mickaël Salaün пишет:
> 
> [...]
> 
>>>>    /**
>>>> @@ -79,6 +91,24 @@ struct landlock_path_beneath_attr {
>>>>    	 */
>>>>    } __attribute__((packed));
>>>>
>>>> +/**
>>>> + * struct landlock_net_service_attr - TCP subnet definition
>>>> + *
>>>> + * Argument of sys_landlock_add_rule().
>>>> + */
>>>> +struct landlock_net_service_attr {
>>>> +	/**
>>>> +	 * @allowed_access: Bitmask of allowed access network for services
>>>> +	 * (cf. `Network flags`_).
>>>> +	 */
>>>> +	__u64 allowed_access;
>>>> +	/**
>>>> +	 * @port: Network port.
>>>> +	 */
>>>> +	__u16 port;
>>>
>>>    From an UAPI point of view, I think the port field should be __be16, as
>>> for sockaddr_in->port and other network-related APIs. This will require
>>> some kernel changes to please sparse: make C=2 security/landlock/ must
>>> not print any warning.
>> 
>>     I have this errors trying to launch sparse checking:
>> 
>>     DESCEND objtool
>>     DESCEND bpf/resolve_btfids
>>     CALL    scripts/checksyscalls.sh
>>     CHK     kernel/kheaders_data.tar.xz
>>     CC      security/landlock/setup.o
>>     CHECK   security/landlock/setup.c
>> ./include/asm-generic/rwonce.h:67:16: error: typename in expression
>> ./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call
>> ./include/asm-generic/rwonce.h:67:16: error: got :
>> ./include/linux/list.h:292:16: error: typename in expression
>> ./include/linux/list.h:292:16: error: Expected ) in function call
>> ./include/linux/list.h:292:16: error: got :
>> 
>> ....
>> 
>> ./include/linux/seqlock.h:682:16: error: Expected ) in function call
>> ./include/linux/seqlock.h:682:16: error: got :
>> ./include/linux/seqlock.h:695:16: error: typename in expression
>> ./include/linux/seqlock.h:695:16: error: Expected ) in function call
>> ./include/linux/seqlock.h:695:16: error: too many errors
>> Segmentation fault (core dumped)
>> make[3]: *** [scripts/Makefile.build:250: security/landlock/setup.o]
>> Error 139
>> make[3]: *** Deleting file 'security/landlock/setup.o'
>> make[3]: *** Waiting for unfinished jobs....
>> Segmentation fault (core dumped)
>> make[3]: *** [scripts/Makefile.build:250: security/landlock/syscalls.o]
>> Error 139
>> make[3]: *** Deleting file 'security/landlock/syscalls.o'
>> make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2
>> make[1]: *** [scripts/Makefile.build:502: security] Error 2
>> make[1]: *** Waiting for unfinished jobs....
>> make: *** [Makefile:1994: .] Error 2
> 
> I don't know about this error. Did you follow the documentation?
> https://docs.kernel.org/dev-tools/sparse.html#getting-sparse
> 
   Yes, I did as in the documentation. that's strange.
If you dont mind can you please check it when I sent a new patch?

> 
> 
>>>
>>> Using big-endian values as keys (casted to uintptr_t, not strictly
>>> __be16) in the rb-tree should not be an issue because there is no port
>>> range ordering (for now).
>>>
>>> A dedicated test should check that endianness is correct, e.g. by using
>>> different port encoding. This should include passing and failing tests,
>>> but they should work on all architectures (i.e. big or little endian).
> .

Dan Carpenter Jan. 9, 2023, 8:58 a.m. UTC | #9

These warnings seem like something I have seen before.  Maybe it was an
issue with _Generic() support?

Are you really sure you're running the latest git version of Sparse?

I tested this patch with the latest version of Sparse on my system and
it worked fine.

regards,
dan carpenter

Konstantin Meskhidze (A) Jan. 9, 2023, 9:26 a.m. UTC | #10

1/9/2023 11:58 AM, Dan Carpenter пишет:
> These warnings seem like something I have seen before.  Maybe it was an
> issue with _Generic() support?
> 
> Are you really sure you're running the latest git version of Sparse?
> 
> I tested this patch with the latest version of Sparse on my system and
> it worked fine.

  Hi Dan,

  git is on the master branch now - hash ce1a6720 (dated 27 June 2022)

  Is this correct version?

  regards,
  Konstantin.
> 
> regards,
> dan carpenter
> 
> .

Dan Carpenter Jan. 9, 2023, 10:20 a.m. UTC | #11

On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote:
> 
> 
> 1/9/2023 11:58 AM, Dan Carpenter пишет:
> > These warnings seem like something I have seen before.  Maybe it was an
> > issue with _Generic() support?
> > 
> > Are you really sure you're running the latest git version of Sparse?
> > 
> > I tested this patch with the latest version of Sparse on my system and
> > it worked fine.
> 
>  Hi Dan,
> 
>  git is on the master branch now - hash ce1a6720 (dated 27 June 2022)
> 
>  Is this correct version?

Yes, that's correct.  What is your .config?

regards,
dan carpenter

Konstantin Meskhidze (A) Jan. 9, 2023, 11:39 a.m. UTC | #12

1/9/2023 1:20 PM, Dan Carpenter пишет:
> On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote:
>> 
>> 
>> 1/9/2023 11:58 AM, Dan Carpenter пишет:
>> > These warnings seem like something I have seen before.  Maybe it was an
>> > issue with _Generic() support?
>> > 
>> > Are you really sure you're running the latest git version of Sparse?
>> > 
>> > I tested this patch with the latest version of Sparse on my system and
>> > it worked fine.
>> 
>>  Hi Dan,
>> 
>>  git is on the master branch now - hash ce1a6720 (dated 27 June 2022)
>> 
>>  Is this correct version?
> 
> Yes, that's correct.  What is your .config?

   What parameters do I need to check in .config?
> 
> regards,
> dan carpenter
> 
> .

Dan Carpenter Jan. 9, 2023, 11:53 a.m. UTC | #13

On Mon, Jan 09, 2023 at 02:39:36PM +0300, Konstantin Meskhidze (A) wrote:
> 
> 
> 1/9/2023 1:20 PM, Dan Carpenter пишет:
> > On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote:
> > > 
> > > 
> > > 1/9/2023 11:58 AM, Dan Carpenter пишет:
> > > > These warnings seem like something I have seen before.  Maybe it was an
> > > > issue with _Generic() support?
> > > > > Are you really sure you're running the latest git version of
> > > Sparse?
> > > > > I tested this patch with the latest version of Sparse on my
> > > system and
> > > > it worked fine.
> > > 
> > >  Hi Dan,
> > > 
> > >  git is on the master branch now - hash ce1a6720 (dated 27 June 2022)
> > > 
> > >  Is this correct version?
> > 
> > Yes, that's correct.  What is your .config?
> 
>   What parameters do I need to check in .config?

I don't know.  I was hoping you could just email me the whole thing
and/or the results from make security/landlock/ruleset.i.  That way
we could see what line was making Sparse complain.

regards,
dan carpenter

Konstantin Meskhidze (A) Jan. 9, 2023, 12:18 p.m. UTC | #14

1/9/2023 2:53 PM, Dan Carpenter пишет:
> On Mon, Jan 09, 2023 at 02:39:36PM +0300, Konstantin Meskhidze (A) wrote:
>> 
>> 
>> 1/9/2023 1:20 PM, Dan Carpenter пишет:
>> > On Mon, Jan 09, 2023 at 12:26:52PM +0300, Konstantin Meskhidze (A) wrote:
>> > > 
>> > > 
>> > > 1/9/2023 11:58 AM, Dan Carpenter пишет:
>> > > > These warnings seem like something I have seen before.  Maybe it was an
>> > > > issue with _Generic() support?
>> > > > > Are you really sure you're running the latest git version of
>> > > Sparse?
>> > > > > I tested this patch with the latest version of Sparse on my
>> > > system and
>> > > > it worked fine.
>> > > 
>> > >  Hi Dan,
>> > > 
>> > >  git is on the master branch now - hash ce1a6720 (dated 27 June 2022)
>> > > 
>> > >  Is this correct version?
>> > 
>> > Yes, that's correct.  What is your .config?
>> 
>>   What parameters do I need to check in .config?
> 
> I don't know.  I was hoping you could just email me the whole thing
> and/or the results from make security/landlock/ruleset.i.  That way
> we could see what line was making Sparse complain.

   here is the whole error message:

   make C=2 security/landlock/
   CHECK   scripts/mod/empty.c
   CALL    scripts/checksyscalls.sh
   DESCEND objtool
   DESCEND bpf/resolve_btfids
   CHECK   security/landlock/setup.c
./include/asm-generic/rwonce.h:67:16: error: typename in expression
./include/asm-generic/rwonce.h:67:16: error: Expected ) in function call
./include/asm-generic/rwonce.h:67:16: error: got :
./include/linux/list.h:292:16: error: typename in expression
./include/linux/list.h:292:16: error: Expected ) in function call
./include/linux/list.h:292:16: error: got :
./include/linux/list.h:328:34: error: typename in expression
./include/linux/list.h:328:34: error: Expected ) in function call
./include/linux/list.h:328:34: error: got :
./include/linux/list.h:329:53: error: typename in expression
./include/linux/list.h:329:53: error: Expected ) in function call
./include/linux/list.h:329:53: error: got :
./include/linux/list.h:867:17: error: typename in expression
./include/linux/list.h:867:17: error: Expected ) in function call
./include/linux/list.h:867:17: error: got :
./include/linux/list.h:876:17: error: typename in expression
./include/linux/list.h:876:17: error: Expected ) in function call
./include/linux/list.h:876:17: error: got :
./arch/x86/include/asm/atomic.h:29:16: error: typename in expression
./arch/x86/include/asm/atomic.h:29:16: error: Expected ) in function call
./arch/x86/include/asm/atomic.h:29:16: error: got :
./arch/x86/include/asm/atomic64_64.h:22:16: error: typename in expression
./arch/x86/include/asm/atomic64_64.h:22:16: error: Expected ) in 
function call
./arch/x86/include/asm/atomic64_64.h:22:16: error: got :
./include/linux/atomic/atomic-arch-fallback.h:227:23: error: typename in 
expression
./include/linux/atomic/atomic-arch-fallback.h:227:23: error: Expected ) 
in function call
./include/linux/atomic/atomic-arch-fallback.h:227:23: error: got :
./include/linux/atomic/atomic-arch-fallback.h:1348:23: error: typename 
in expression
./include/linux/atomic/atomic-arch-fallback.h:1348:23: error: Expected ) 
in function call
./include/linux/atomic/atomic-arch-fallback.h:1348:23: error: got :
./include/linux/jump_label.h:286:9: error: Expected ; at end of statement
./include/linux/jump_label.h:286:9: error: got __flags
./include/linux/jump_label.h:302:9: error: Expected ; at end of statement
./include/linux/jump_label.h:302:9: error: got __flags
./include/linux/jump_label.h:319:9: error: Expected ; at end of statement
./include/linux/jump_label.h:319:9: error: got __flags
./include/linux/jump_label.h:322:17: error: Expected ; at end of statement
./include/linux/jump_label.h:322:17: error: got __flags
./include/linux/jump_label.h:330:9: error: Expected ; at end of statement
./include/linux/jump_label.h:330:9: error: got __flags
./include/linux/jump_label.h:333:17: error: Expected ; at end of statement
./include/linux/jump_label.h:333:17: error: got __flags
./include/asm-generic/bitops/generic-non-atomic.h:140:23: error: 
typename in expression
./include/asm-generic/bitops/generic-non-atomic.h:140:23: error: 
Expected ) in function call
./include/asm-generic/bitops/generic-non-atomic.h:140:23: error: got :
./include/linux/bitmap.h:268:17: error: Expected ; at end of statement
./include/linux/bitmap.h:268:17: error: got __flags
./include/linux/thread_info.h:127:16: error: typename in expression
./include/linux/thread_info.h:127:16: error: Expected ) in function call
./include/linux/thread_info.h:127:16: error: got :
./include/linux/thread_info.h:233:13: error: Expected ; at end of statement
./include/linux/thread_info.h:233:13: error: got __flags
./include/linux/llist.h:191:16: error: typename in expression
./include/linux/llist.h:191:16: error: Expected ) in function call
./include/linux/llist.h:191:16: error: got :
./include/linux/rcupdate.h:1073:31: error: typename in expression
./include/linux/rcupdate.h:1073:31: error: Expected ) in function call
./include/linux/rcupdate.h:1073:31: error: got :
./include/linux/rcupdate.h:1077:9: error: Expected ; at end of statement
./include/linux/rcupdate.h:1077:9: error: got __flags
./include/linux/key.h:453:16: error: typename in expression
./include/linux/key.h:453:16: error: Expected ) in function call
./include/linux/key.h:453:16: error: got :
./include/linux/list_bl.h:74:33: error: typename in expression
./include/linux/list_bl.h:74:33: error: Expected ) in function call
./include/linux/list_bl.h:74:33: error: got :
./include/linux/rculist_bl.h:24:33: error: typename in expression
./include/linux/rculist_bl.h:24:33: error: Expected ) in function call
./include/linux/rculist_bl.h:24:33: error: got :
./include/linux/seqlock.h:259:16: error: typename in expression
./include/linux/seqlock.h:259:16: error: Expected ) in function call
./include/linux/seqlock.h:259:16: error: got :
./include/linux/seqlock.h:274:1: error: typename in expression
./include/linux/seqlock.h:274:1: error: Expected ) in function call
./include/linux/seqlock.h:274:1: error: got :
./include/linux/seqlock.h:274:1: error: typename in expression
./include/linux/seqlock.h:274:1: error: Expected ) in function call
./include/linux/seqlock.h:274:1: error: got :
./include/linux/seqlock.h:275:1: error: typename in expression
./include/linux/seqlock.h:275:1: error: Expected ) in function call
./include/linux/seqlock.h:275:1: error: got :
./include/linux/seqlock.h:275:1: error: typename in expression
./include/linux/seqlock.h:275:1: error: Expected ) in function call
./include/linux/seqlock.h:275:1: error: got :
./include/linux/seqlock.h:276:1: error: typename in expression
./include/linux/seqlock.h:276:1: error: Expected ) in function call
./include/linux/seqlock.h:276:1: error: got :
./include/linux/seqlock.h:276:1: error: typename in expression
./include/linux/seqlock.h:276:1: error: Expected ) in function call
./include/linux/seqlock.h:276:1: error: got :
./include/linux/seqlock.h:277:1: error: typename in expression
./include/linux/seqlock.h:277:1: error: Expected ) in function call
./include/linux/seqlock.h:277:1: error: got :
./include/linux/seqlock.h:277:1: error: typename in expression
./include/linux/seqlock.h:277:1: error: Expected ) in function call
./include/linux/seqlock.h:277:1: error: got :
./include/linux/seqlock.h:429:16: error: typename in expression
./include/linux/seqlock.h:429:16: error: Expected ) in function call
./include/linux/seqlock.h:429:16: error: got :
./include/linux/seqlock.h:682:16: error: typename in expression
./include/linux/seqlock.h:682:16: error: Expected ) in function call
./include/linux/seqlock.h:682:16: error: too many errors
Segmentation fault (core dumped)
make[3]: *** [scripts/Makefile.build:251: security/landlock/setup.o] 
Error 139
make[2]: *** [scripts/Makefile.build:502: security/landlock] Error 2
make[1]: *** [scripts/Makefile.build:502: security] Error 2
make: *** [Makefile:1994: .] Error 2

Please tell me if you need some more info.

regards,
Konstantin

> 
> regards,
> dan carpenter
> 
> .

[v8,07/12] landlock: Add network rules support

Checks

Commit Message

Comments

Patch