| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5cd5b983-32a5-97ec-0835-f0c96d86e805@huawei.com>
Date: Mon, 7 Feb 2022 16:09:48 +0300
From: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
To: Mickaël Salaün <mic@...ikod.net>
CC: <linux-security-module@...r.kernel.org>, <netdev@...r.kernel.org>,
<netfilter@...r.kernel.org>, <yusongping@...wei.com>,
<artem.kuzin@...wei.com>
Subject: Re: [RFC PATCH 1/2] landlock: TCP network hooks implementation
2/1/2022 3:13 PM, Mickaël Salaün пишет:
>
> On 24/01/2022 09:02, Konstantin Meskhidze wrote:
>> Support of socket_bind() and socket_connect() hooks.
>> Current prototype can restrict binding and connecting of TCP
>> types of sockets. Its just basic idea how Landlock could support
>> network confinement.
>>
>> Changes:
>> 1. Access masks array refactored into 1D one and changed
>> to 32 bits. Filesystem masks occupy 16 lower bits and network
>> masks reside in 16 upper bits.
>> 2. Refactor API functions in ruleset.c:
>> 1. Add void *object argument.
>> 2. Add u16 rule_type argument.
>> 3. Use two rb_trees in ruleset structure:
>> 1. root_inode - for filesystem objects
>> 2. root_net_port - for network port objects
>
> It's good to add a changelog, but they must not be in commit messages
> that get copied by git am. Please use "---" to separate this additionnal
> info (but not the Signed-off-by). Please also include a version in the
> email subjects (this one should have been "[RFC PATCH v3 1/2] landlock:
> …"), e.g. using git format-patch --reroll-count=X .
>
> Please follow these rules:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> You can take some inspiration from this patch series:
> https://lore.kernel.org/lkml/20210422154123.13086-1-mic@digikod.net/
Ok. I will add patch vervison in next patch. So it will be "[RFC PATCH
v4 ../..] landlock: ..."
But the previous patches remain with no version, correct?
>
> The Kconfig is missing a "select SECURITY_NETWORK" to make it build. The
> network-related code (but not the Kconfig itself) should then check
> IS_ENABLED(CONFIG_INET) to make sure Landlock is usable even without
> network support. I think the best approach in this case would be to
> silently ignore parts of rulesets handling network access rights
> (because the kernel doesn't implement such network features, and then
> block them), but error out with EPROTONOSUPPORT when adding a new
> network rule. This way, user space can know that a request to allow an
> access is not possible (because such network protocol is not supported
> by the current kernel configuration). You need to check that a kernel
> without TCP support build and behave in a consistent way.
Ok. I got it. Thanks.
>
> This patch generates multiple compiler warnings (e.g. cast to pointer
> from integer of different size). Please make sure it doesn't happen for
> the next patches.
Sure. I will fix it.
>
> This patch is too big, try to split it in standalone patches (i.e. each
> of them must build without warning and have a meaningful description).
Ok. Will be split.
>
>>
>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
>> ---
>> include/uapi/linux/landlock.h | 52 ++++++++++
>> security/landlock/Makefile | 2 +-
>> security/landlock/fs.c | 12 ++-
>> security/landlock/limits.h | 6 ++
>> security/landlock/net.c | 175 ++++++++++++++++++++++++++++++++++
>> security/landlock/net.h | 21 ++++
>> security/landlock/ruleset.c | 167 +++++++++++++++++++++++---------
>> security/landlock/ruleset.h | 40 +++++---
>> security/landlock/setup.c | 3 +
>> security/landlock/syscalls.c | 142 +++++++++++++++++++--------
>> 10 files changed, 514 insertions(+), 106 deletions(-)
>> create mode 100644 security/landlock/net.c
>> create mode 100644 security/landlock/net.h
>>
>> diff --git a/include/uapi/linux/landlock.h
>> b/include/uapi/linux/landlock.h
>> index b3d952067f59..1745a3a2f7a9 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -25,6 +25,15 @@ struct landlock_ruleset_attr {
>> * compatibility reasons.
>> */
>> __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. This is needed for backward
>> + * compatibility reasons.
>> + */
>> + __u64 handled_access_net;
>> +
>> };
>> /*
>> @@ -46,6 +55,12 @@ 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,
>> };
>> /**
>> @@ -70,6 +85,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;
>> +
>> +} __attribute__((packed));
>> +
>> /**
>> * DOC: fs_access
>> *
>> @@ -134,4 +167,23 @@ struct landlock_path_beneath_attr {
>> #define LANDLOCK_ACCESS_FS_MAKE_BLOCK (1ULL << 11)
>> #define LANDLOCK_ACCESS_FS_MAKE_SYM (1ULL << 12)
>> +/**
>> + * 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 IP address.
>
> Which IP address? I suggested "to a local port" in my previous review.
>
My mistake. I will fix it.
>
>> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
>> + * a listening one.
>
> I suggested "Connect a TCP socket to a remote port." in my previous
> review. Please take them into account or explain why not.
Sorry. I missed it. Will be fixed.
>
>
>> + */
>> +#define LANDLOCK_ACCESS_NET_BIND_TCP (1ULL << 0)
>> +#define LANDLOCK_ACCESS_NET_CONNECT_TCP (1ULL << 1)
>> +
>> +
>> #endif /* _UAPI_LINUX_LANDLOCK_H */
>> diff --git a/security/landlock/Makefile b/security/landlock/Makefile
>> index 7bbd2f413b3e..afa44baaa83a 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -1,4 +1,4 @@
>> obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>> landlock-y := setup.o syscalls.o object.o ruleset.o \
>> - cred.o ptrace.o fs.o
>> + cred.o ptrace.o fs.o net.o
>> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
>> index 97b8e421f617..0cb2548157b5 100644
>> --- a/security/landlock/fs.c
>> +++ b/security/landlock/fs.c
>> @@ -163,12 +163,13 @@ int landlock_append_fs_rule(struct
>> landlock_ruleset *const ruleset,
>> return -EINVAL;
>> /* Transforms relative access rights to absolute ones. */
>> - access_rights |= LANDLOCK_MASK_ACCESS_FS &
>> ~ruleset->fs_access_masks[0];
>> + access_rights |= LANDLOCK_MASK_ACCESS_FS &
>> ~ruleset->access_masks[0];
>> object = get_inode_object(d_backing_inode(path->dentry));
>> if (IS_ERR(object))
>> return PTR_ERR(object);
>> mutex_lock(&ruleset->lock);
>> - err = landlock_insert_rule(ruleset, object, access_rights);
>> + err = landlock_insert_rule(ruleset, object, access_rights,
>> + LANDLOCK_RULE_PATH_BENEATH);
>
> The modifications of landlock_insert_rule() and landlock_find_rule()
> should be part of a standalone (and consistent) patch to ease review.
>
Thanks for noticing. I got it.
>
>> mutex_unlock(&ruleset->lock);
>> /*
>> * No need to check for an error because landlock_insert_rule()
>> @@ -195,7 +196,8 @@ static inline u64 unmask_layers(
>> inode = d_backing_inode(path->dentry);
>> rcu_read_lock();
>> rule = landlock_find_rule(domain,
>> - rcu_dereference(landlock_inode(inode)->object));
>> + rcu_dereference(landlock_inode(inode)->object),
>> + LANDLOCK_RULE_PATH_BENEATH);
>> rcu_read_unlock();
>> if (!rule)
>> return layer_mask;
>> @@ -229,6 +231,7 @@ static int check_access_path(const struct
>> landlock_ruleset *const domain,
>> struct path walker_path;
>> u64 layer_mask;
>> size_t i;
>> + u8 rule_fs_type;
>> /* Make sure all layers can be checked. */
>> BUILD_BUG_ON(BITS_PER_TYPE(layer_mask) < LANDLOCK_MAX_NUM_LAYERS);
>> @@ -249,10 +252,11 @@ static int check_access_path(const struct
>> landlock_ruleset *const domain,
>> if (WARN_ON_ONCE(domain->num_layers < 1))
>> return -EACCES;
>> + rule_fs_type = LANDLOCK_RULE_PATH_BENEATH - 1;
>> /* Saves all layers handling a subset of requested accesses. */
>> layer_mask = 0;
>> for (i = 0; i < domain->num_layers; i++) {
>> - if (domain->fs_access_masks[i] & access_request)
>> + if (domain->access_masks[i] & access_request)
>
> This fs_access_masks renaming should be part of a standalone patch.
I got it.
>
>
>> layer_mask |= BIT_ULL(i);
>> }
>> /* An access request not handled by the domain is allowed. */
>> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
>> index 2a0a1095ee27..fdbef85e4de0 100644
>> --- a/security/landlock/limits.h
>> +++ b/security/landlock/limits.h
>> @@ -18,4 +18,10 @@
>> #define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_MAKE_SYM
>> #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS <<
>> 1) - 1)
>> +#define LANDLOCK_LAST_ACCESS_NET LANDLOCK_ACCESS_NET_CONNECT_TCP
>> +#define LANDLOCK_MASK_ACCESS_NET ((LANDLOCK_LAST_ACCESS_NET << 1)
>> - 1)
>> +#define LANDLOCK_MASK_SHIFT_NET 16
>> +
>> +#define LANDLOCK_RULE_TYPE_NUM LANDLOCK_RULE_NET_SERVICE
>> +
>> #endif /* _SECURITY_LANDLOCK_LIMITS_H */
>> diff --git a/security/landlock/net.c b/security/landlock/net.c
>> new file mode 100644
>> index 000000000000..0b5323d254a7
>> --- /dev/null
>> +++ b/security/landlock/net.c
>> @@ -0,0 +1,175 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Landlock LSM - Filesystem management and hooks
>> + *
>> + * Copyright © 2016-2020 Mickaël Salaün <mic@...ikod.net>
>> + * Copyright © 2018-2020 ANSSI
>> + */
>> +
>> +#include <linux/socket.h>
>> +#include <linux/net.h>
>> +#include <linux/in.h>
>
> Why is linux/in.h required?
>
Struct sockaddr_in is described in this header.
A pointer to struct sockaddr_in is used in hook_socket_connect()
and hook_socket_bind() to get socket's family and port values.
>
>> +
>> +#include "cred.h"
>> +#include "limits.h"
>> +#include "net.h"
>> +
>> +int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
>> + u16 port, u32 access_rights)
>> +{
>> + int err;
>> +
>> + /* Transforms relative access rights to absolute ones. */
>> + access_rights |= LANDLOCK_MASK_ACCESS_NET &
>> ~(ruleset->access_masks[0] >>
>> + LANDLOCK_MASK_SHIFT_NET);
>> +
>> + mutex_lock(&ruleset->lock);
>> + err = landlock_insert_rule(ruleset, (void *)port, access_rights,
>> + LANDLOCK_RULE_NET_SERVICE);
>> + mutex_unlock(&ruleset->lock);
>> +
>> + return err;
>> +}
>> +
>> +/* Access-control management */
>> +static inline bool unmask_layers(
>> + const struct landlock_ruleset *const domain,
>> + const u16 port, const u32 access_request, u64 layer_mask)
>> +{
>> + const struct landlock_rule *rule;
>> + size_t i;
>> + bool allowed = false;
>> +
>> + rule = landlock_find_rule(domain, (void *)port,
>> + LANDLOCK_RULE_NET_SERVICE);
>> +
>> + /* Grant access if there is no rule for an oject */
>
> For consistency, please use the third person for such comments (e.g.
> "Grants access") and a full sentence ending with a period. This applies
> for all comments. Also check for typos.
Thanks for noticing. Will be fixed.
>
>
>> + if (!rule)
>> + return allowed = true;
>> +
>> + /*
>> + * An access is granted if, for each policy layer, at least one rule
>> + * encountered on network actions requested,
>
> This is a weird line cut.
Ok. Will be fixed.
>
>> + * regardless of their position in the layer stack. We must then
>> check
>> + * the remaining layers, from the first added layer to
>> + * the last one.
>> + */
>> + for (i = 0; i < rule->num_layers; i++) {
>> + const struct landlock_layer *const layer = &rule->layers[i];
>> + const u64 layer_level = BIT_ULL(layer->level - 1);
>> +
>> + /* Checks that the layer grants access to the request. */
>> + if ((layer->access & access_request) == access_request) {
>> + layer_mask &= ~layer_level;
>> + allowed = true;
>> +
>> + if (layer_mask == 0)
>> + return allowed;
>> + } else {
>> + layer_mask &= ~layer_level;
>> +
>> + if (layer_mask == 0)
>> + return allowed;
>> + }
>> + }
>> + return allowed;
>> +}
>
> We should not copy-paste such code. I'm working on a different patch
> series involving modifications to this function. I'll move this function
> in a separate file to ease code sharing. In the meantime, please create
> a standalone patch that moves this function to
> security/landlock/ruleset.c .
>
I got it. I will move this function. Thanks.
>
>> +
>> +static int check_socket_access(const struct landlock_ruleset *const
>> domain,
>> + u16 port, u32 access_request)
>> +{
>> + bool allowed = false;
>> + u64 layer_mask;
>> + size_t i;
>> +
>> + /* Make sure all layers can be checked. */
>> + BUILD_BUG_ON(BITS_PER_TYPE(layer_mask) < LANDLOCK_MAX_NUM_LAYERS);
>> +
>> + if (WARN_ON_ONCE(!domain))
>> + return 0;
>> + if (WARN_ON_ONCE(domain->num_layers < 1))
>> + return -EACCES;
>> +
>> + /* Saves all layers handling a subset of requested
>
> Please follow the kernel practices about comments: if there is multiple
> lines, starts with "/*\n".
Ok. I got it.
>
>
>> + * socket access rules.
>> + */
>> + layer_mask = 0;
>> + for (i = 0; i < domain->num_layers; i++) {
>> + if ((domain->access_masks[i] >> LANDLOCK_MASK_SHIFT_NET) &
>> access_request)
>
> We should use an helper to access the underlying masks instead of
> manually shifting access_masks[], e.g. get_fs_access_masks(domain), and
> document it with the access_masks comments.
Got it. Will be refactored.
>
>
>> + layer_mask |= BIT_ULL(i);
>> + }
>> + /* An access request not handled by the domain is allowed. */
>> + if (layer_mask == 0)
>> + return 0;
>> +
>> + /*
>> + * We need to walk through all the hierarchy to not miss any
>> relevant
>> + * restriction.
>> + */
>> + allowed = unmask_layers(domain, port, access_request, layer_mask);
>> +
>> + return allowed ? 0 : -EACCES;
>> +}
>> +
>> +static int hook_socket_bind(struct socket *sock, struct sockaddr
>> *address, int addrlen)
>> +{
>> + short socket_type;
>> + struct sockaddr_in *sockaddr;
>> + u16 port;
>> + const struct landlock_ruleset *const dom =
>> landlock_get_current_domain();
>> +
>> + /* Check if the hook is AF_INET* socket's action */
>> + if ((address->sa_family != AF_INET) && (address->sa_family !=
>> AF_INET6))
>
> Using a switch/case would be better.
Ok. Will be refactored.
>
>
>> + return 0;
>> +
>> + socket_type = sock->type;
>> + /* Check if it's a TCP socket */
>> + if (socket_type != SOCK_STREAM)
>> + return 0;
>> +
>> + if (!dom)
>> + return 0;
>
> This must be at the top of *each* hook to make it clear that they don't
> impact non-landlocked processes.
>
They don't impact. It does not matter what to check first socket
family/type or landlocked process.
>
>> +
>> + /* Get port value in host byte order */
>> + sockaddr = (struct sockaddr_in *)address;
>> + port = ntohs(sockaddr->sin_port);
>> +
>> + return check_socket_access(dom, port, LANDLOCK_ACCESS_NET_BIND_TCP);
>> +}
>> +
>> +static int hook_socket_connect(struct socket *sock, struct sockaddr
>> *address, int addrlen)
>> +{
>> + short socket_type;
>> + struct sockaddr_in *sockaddr;
>> + u16 port;
>> + const struct landlock_ruleset *const dom =
>> landlock_get_current_domain();
>> +
>> + /* Check if the hook is AF_INET* socket's action */
>> + if ((address->sa_family != AF_INET) && (address->sa_family !=
>> AF_INET6))
>> + return 0;
>> +
>> + socket_type = sock->type;
>> + /* Check if it's a TCP socket */
>> + if (socket_type != SOCK_STREAM)
>> + return 0;
>> +
>> + if (!dom)
>> + return 0;
>> +
>> + /* Get port value in host byte order */
>> + sockaddr = (struct sockaddr_in *)address;
>> + port = ntohs(sockaddr->sin_port);
>> +
>> + return check_socket_access(dom, port,
>> LANDLOCK_ACCESS_NET_CONNECT_TCP);
>> +}
>> +
>> +static struct security_hook_list landlock_hooks[] __lsm_ro_after_init
>> = {
>> + LSM_HOOK_INIT(socket_bind, hook_socket_bind),
>> + LSM_HOOK_INIT(socket_connect, hook_socket_connect),
>> +};
>> +
>> +__init void landlock_add_net_hooks(void)
>> +{
>> + security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
>> + LANDLOCK_NAME);
>> +}
>> diff --git a/security/landlock/net.h b/security/landlock/net.h
>> new file mode 100644
>> index 000000000000..cd081808716a
>> --- /dev/null
>> +++ b/security/landlock/net.h
>> @@ -0,0 +1,21 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Landlock LSM - Network management and hooks
>> + *
>> + * Copyright © 2017-2020 Mickaël Salaün <mic@...ikod.net>
>> + * Copyright © 2018-2020 ANSSI
>> + */
>> +
>> +#ifndef _SECURITY_LANDLOCK_NET_H
>> +#define _SECURITY_LANDLOCK_NET_H
>> +
>> +#include "common.h"
>> +#include "ruleset.h"
>> +#include "setup.h"
>> +
>> +__init void landlock_add_net_hooks(void);
>> +
>> +int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
>> + u16 port, u32 access_hierarchy);
>> +
>> +#endif /* _SECURITY_LANDLOCK_NET_H */
>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>> index ec72b9262bf3..d7e49842b299 100644
>> --- a/security/landlock/ruleset.c
>> +++ b/security/landlock/ruleset.c
>> @@ -28,32 +28,41 @@ static struct landlock_ruleset
>> *create_ruleset(const u32 num_layers)
>> {
>> struct landlock_ruleset *new_ruleset;
>> - new_ruleset = kzalloc(struct_size(new_ruleset, fs_access_masks,
>> + new_ruleset = kzalloc(struct_size(new_ruleset, access_masks,
>> num_layers), GFP_KERNEL_ACCOUNT);
>> +
>> if (!new_ruleset)
>> return ERR_PTR(-ENOMEM);
>> refcount_set(&new_ruleset->usage, 1);
>> mutex_init(&new_ruleset->lock);
>> - new_ruleset->root = RB_ROOT;
>> + new_ruleset->root_inode = RB_ROOT;
>> + new_ruleset->root_net_port = RB_ROOT;
>> new_ruleset->num_layers = num_layers;
>> /*
>> * hierarchy = NULL
>> * num_rules = 0
>> - * fs_access_masks[] = 0
>> + * access_masks[] = 0
>> */
>> return new_ruleset;
>> }
>> -struct landlock_ruleset *landlock_create_ruleset(const u32
>> fs_access_mask)
>> +struct landlock_ruleset *landlock_create_ruleset(const u32
>> fs_access_mask,
>> + const u32 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))
>> - new_ruleset->fs_access_masks[0] = fs_access_mask;
>> +
>> + if (!IS_ERR(new_ruleset) && fs_access_mask)
>> + new_ruleset->access_masks[0] = fs_access_mask;
>> +
>> + /* Add network mask by shifting it to upper 16 bits of
>> access_masks */
>> + if (!IS_ERR(new_ruleset) && net_access_mask)
>> + new_ruleset->access_masks[0] |= (net_access_mask <<
>> LANDLOCK_MASK_SHIFT_NET);
>> +
>> return new_ruleset;
>> }
>> @@ -67,10 +76,11 @@ static void build_check_rule(void)
>> }
>> static struct landlock_rule *create_rule(
>> - struct landlock_object *const object,
>> + void *const object,
>
> Instead of shoehorning two different types into one (and then loosing
> the typing), you should rename object to object_ptr and add a new
> object_data argument. Only one of these should be set according to the
> rule_type. However, if there is no special action performed on one of
> these type (e.g. landlock_get_object), only one uintptr_t argument
> should be enough.
>
Do you mean using 2 object arguments in create_rule():
1. create_rule( object_ptr = landlock_object , object_data = 0,
..., fs_rule_type);
2. create_rule( object_ptr = NULL , object_data = port, .... ,
net_rule_type);
>
>> const struct landlock_layer (*const layers)[],
>> const u32 num_layers,
>> - const struct landlock_layer *const new_layer)
>> + const struct landlock_layer *const new_layer,
>> + const u16 rule_type)
>> {
>> struct landlock_rule *new_rule;
>> u32 new_num_layers;
>> @@ -89,8 +99,11 @@ static struct landlock_rule *create_rule(
>> if (!new_rule)
>> return ERR_PTR(-ENOMEM);
>> RB_CLEAR_NODE(&new_rule->node);
>> - landlock_get_object(object);
>> - new_rule->object = object;
>> +
>> + if (rule_type == LANDLOCK_RULE_PATH_BENEATH)
>
> Please use a switch/case when checking LANDLOCK_RULE_* to make sure that
> every case is handled.
Ok. I got it.
>
>
>> + landlock_get_object(object);
>> +
>> + new_rule->object.ptr = object;
>> new_rule->num_layers = new_num_layers;
>> /* Copies the original layer stack. */
>> memcpy(new_rule->layers, layers,
>> @@ -101,12 +114,13 @@ static struct landlock_rule *create_rule(
>> return new_rule;
>> }
>> -static void free_rule(struct landlock_rule *const rule)
>> +static void free_rule(struct landlock_rule *const rule, const u16
>> rule_type)
>> {
>> might_sleep();
>> if (!rule)
>> return;
>> - landlock_put_object(rule->object);
>> + if (rule_type == LANDLOCK_RULE_PATH_BENEATH)
>> + landlock_put_object(rule->object.ptr);
>> kfree(rule);
>> }
>> @@ -116,11 +130,14 @@ static void build_check_ruleset(void)
>> .num_rules = ~0,
>> .num_layers = ~0,
>> };
>> - typeof(ruleset.fs_access_masks[0]) fs_access_mask = ~0;
>> +
>> + typeof(ruleset.access_masks[0]) fs_access_mask = ~0;
>> + typeof(ruleset.access_masks[0]) net_access_mask = ~0;
>> BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES);
>> BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
>> BUILD_BUG_ON(fs_access_mask < LANDLOCK_MASK_ACCESS_FS);
>> + BUILD_BUG_ON(net_access_mask < LANDLOCK_MASK_ACCESS_NET);
>> }
>> /**
>> @@ -142,26 +159,36 @@ static void build_check_ruleset(void)
>> * access rights.
>> */
>> static int insert_rule(struct landlock_ruleset *const ruleset,
>> - struct landlock_object *const object,
>> - const struct landlock_layer (*const layers)[],
>> - size_t num_layers)
>> + void *const obj, const struct landlock_layer (*const layers)[],
>
> same here
Do you mean using 2 object arguments in insert_rule():
1. insert_rule( object_ptr = landlock_object , object_data = 0,
..., fs_rule_type);
2. insert_rule( object_ptr = NULL , object_data = port, .... ,
net_rule_type);
>
>
>> + size_t num_layers, const u16 rule_type)
>> {
>> struct rb_node **walker_node;
>> struct rb_node *parent_node = NULL;
>> struct landlock_rule *new_rule;
>> + struct landlock_object *object;
>> + struct rb_root *root;
>> might_sleep();
>> lockdep_assert_held(&ruleset->lock);
>> - if (WARN_ON_ONCE(!object || !layers))
>> + if (WARN_ON_ONCE(!obj || !layers))
>> return -ENOENT;
>> - walker_node = &(ruleset->root.rb_node);
>> + object = (struct landlock_object *)obj;
>> + /* Choose rb_tree structure depending on a rule type */
>> + if (rule_type == LANDLOCK_RULE_PATH_BENEATH)
>> + root = &ruleset->root_inode;
>> + else if (rule_type == LANDLOCK_RULE_NET_SERVICE)
>> + root = &ruleset->root_net_port;
>> + else
>> + return -EINVAL;
>
> ditto
Ok. I will refactor to switch/case.
>
>
>> +
>> + walker_node = &root->rb_node;
>> while (*walker_node) {
>> struct landlock_rule *const this = rb_entry(*walker_node,
>> struct landlock_rule, node);
>> - if (this->object != object) {
>> + if (this->object.ptr != object) {
>> parent_node = *walker_node;
>> - if (this->object < object)
>> + if (this->object.ptr < object)
>> walker_node = &((*walker_node)->rb_right);
>> else
>> walker_node = &((*walker_node)->rb_left);
>> @@ -194,11 +221,11 @@ static int insert_rule(struct landlock_ruleset
>> *const ruleset,
>> * ruleset and a domain.
>> */
>> new_rule = create_rule(object, &this->layers, this->num_layers,
>> - &(*layers)[0]);
>> + &(*layers)[0], rule_type);
>> if (IS_ERR(new_rule))
>> return PTR_ERR(new_rule);
>> - rb_replace_node(&this->node, &new_rule->node, &ruleset->root);
>> - free_rule(this);
>> + rb_replace_node(&this->node, &new_rule->node, root);
>> + free_rule(this, rule_type);
>> return 0;
>> }
>> @@ -206,11 +233,11 @@ static int insert_rule(struct landlock_ruleset
>> *const ruleset,
>> build_check_ruleset();
>> if (ruleset->num_rules >= LANDLOCK_MAX_NUM_RULES)
>> return -E2BIG;
>> - new_rule = create_rule(object, layers, num_layers, NULL);
>> + new_rule = create_rule(object, layers, num_layers, NULL, rule_type);
>> if (IS_ERR(new_rule))
>> return PTR_ERR(new_rule);
>> rb_link_node(&new_rule->node, parent_node, walker_node);
>> - rb_insert_color(&new_rule->node, &ruleset->root);
>> + rb_insert_color(&new_rule->node, root);
>> ruleset->num_rules++;
>> return 0;
>> }
>> @@ -228,7 +255,8 @@ static void build_check_layer(void)
>> /* @ruleset must be locked by the caller. */
>> int landlock_insert_rule(struct landlock_ruleset *const ruleset,
>> - struct landlock_object *const object, const u32 access)
>> + void *const object, const u32 access,
>> + const u16 rule_type)
>> {
>> struct landlock_layer layers[] = {{
>> .access = access,
>> @@ -237,7 +265,7 @@ int landlock_insert_rule(struct landlock_ruleset
>> *const ruleset,
>> }};
>> build_check_layer();
>> - return insert_rule(ruleset, object, &layers, ARRAY_SIZE(layers));
>> + return insert_rule(ruleset, object, &layers, ARRAY_SIZE(layers),
>> rule_type);
>> }
>> static inline void get_hierarchy(struct landlock_hierarchy *const
>> hierarchy)
>> @@ -279,11 +307,13 @@ static int merge_ruleset(struct landlock_ruleset
>> *const dst,
>> err = -EINVAL;
>> goto out_unlock;
>> }
>> - dst->fs_access_masks[dst->num_layers - 1] = src->fs_access_masks[0];
>> +
>> + /* Copy access masks. */
>> + dst->access_masks[dst->num_layers - 1] = src->access_masks[0];
>> /* Merges the @src tree. */
>> rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
>> - &src->root, node) {
>> + &src->root_inode, node) {
>> struct landlock_layer layers[] = {{
>> .level = dst->num_layers,
>> }};
>> @@ -297,8 +327,30 @@ static int merge_ruleset(struct landlock_ruleset
>> *const dst,
>> goto out_unlock;
>> }
>> layers[0].access = walker_rule->layers[0].access;
>> - err = insert_rule(dst, walker_rule->object, &layers,
>> - ARRAY_SIZE(layers));
>> + err = insert_rule(dst, walker_rule->object.ptr, &layers,
>> + ARRAY_SIZE(layers), LANDLOCK_RULE_PATH_BENEATH);
>> + if (err)
>> + goto out_unlock;
>> + }
>> +
>> + /* Merges the @src tree. */
>> + rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
>> + &src->root_net_port, node) {
>> + struct landlock_layer layers[] = {{
>> + .level = dst->num_layers,
>> + }};
>> +
>> + if (WARN_ON_ONCE(walker_rule->num_layers != 1)) {
>> + err = -EINVAL;
>> + goto out_unlock;
>> + }
>> + if (WARN_ON_ONCE(walker_rule->layers[0].level != 0)) {
>> + err = -EINVAL;
>> + goto out_unlock;
>> + }
>> + layers[0].access = walker_rule->layers[0].access;
>> + err = insert_rule(dst, walker_rule->object.ptr, &layers,
>> + ARRAY_SIZE(layers), LANDLOCK_RULE_NET_SERVICE);
>> if (err)
>> goto out_unlock;
>> }
>> @@ -325,9 +377,20 @@ static int inherit_ruleset(struct
>> landlock_ruleset *const parent,
>> /* Copies the @parent tree. */
>> rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
>> - &parent->root, node) {
>> - err = insert_rule(child, walker_rule->object,
>> - &walker_rule->layers, walker_rule->num_layers);
>> + &parent->root_inode, node) {
>> + err = insert_rule(child, walker_rule->object.ptr,
>> + &walker_rule->layers, walker_rule->num_layers,
>> + LANDLOCK_RULE_PATH_BENEATH);
>> + if (err)
>> + goto out_unlock;
>> + }
>> +
>> + /* Copies the @parent tree. */
>> + rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
>> + &parent->root_net_port, node) {
>> + err = insert_rule(child, walker_rule->object.ptr,
>> + &walker_rule->layers, walker_rule->num_layers,
>> + LANDLOCK_RULE_NET_SERVICE);
>> if (err)
>> goto out_unlock;
>> }
>> @@ -336,9 +399,11 @@ static int inherit_ruleset(struct
>> landlock_ruleset *const parent,
>> err = -EINVAL;
>> goto out_unlock;
>> }
>> - /* Copies the parent layer stack and leaves a space for the new
>> layer. */
>> - memcpy(child->fs_access_masks, parent->fs_access_masks,
>> - flex_array_size(parent, fs_access_masks,
>> parent->num_layers));
>> + /* Copies the parent layer stack and leaves a space for the new
>> layer.
>
> ditto
Do you mean comments style here?
>
>
>> + * Remember to copy num_layers*num_tule_types size.
>> + */
>> + memcpy(child->access_masks, parent->access_masks,
>> + flex_array_size(parent, access_masks, parent->num_layers));
>> if (WARN_ON_ONCE(!parent->hierarchy)) {
>> err = -EINVAL;
>> @@ -358,9 +423,13 @@ static void free_ruleset(struct landlock_ruleset
>> *const ruleset)
>> struct landlock_rule *freeme, *next;
>> might_sleep();
>> - rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root,
>> - node)
>> - free_rule(freeme);
>> + rbtree_postorder_for_each_entry_safe(freeme, next,
>> &ruleset->root_inode,
>> + node)
>> + free_rule(freeme, LANDLOCK_RULE_PATH_BENEATH);
>> + rbtree_postorder_for_each_entry_safe(freeme, next,
>> &ruleset->root_net_port,
>> + node)
>> + free_rule(freeme, LANDLOCK_RULE_NET_SERVICE);
>> +
>> put_hierarchy(ruleset->hierarchy);
>> kfree(ruleset);
>> }
>> @@ -451,20 +520,26 @@ struct landlock_ruleset *landlock_merge_ruleset(
>> */
>> const struct landlock_rule *landlock_find_rule(
>> const struct landlock_ruleset *const ruleset,
>> - const struct landlock_object *const object)
>> + const void *const obj, const u16 rule_type)
>
> Only an uintptr_t is needed here.
>
Ok. I got it.
>
>> {
>> const struct rb_node *node;
>> + const struct landlock_object *object;
>> - if (!object)
>> + if (!obj)
>> return NULL;
>> - node = ruleset->root.rb_node;
>> + object = (struct landlock_object *)obj;
>> + if (rule_type == LANDLOCK_RULE_PATH_BENEATH)
>> + node = ruleset->root_inode.rb_node;
>> + else if (rule_type == LANDLOCK_RULE_NET_SERVICE)
>> + node = ruleset->root_net_port.rb_node;
>> +
>> while (node) {
>> struct landlock_rule *this = rb_entry(node,
>> struct landlock_rule, node);
>> - if (this->object == object)
>> + if (this->object.ptr == object)
>> return this;
>> - if (this->object < object)
>> + if (this->object.ptr < object)
>> node = node->rb_right;
>> else
>> node = node->rb_left;
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index 2d3ed7ec5a0a..831e47ac2467 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -45,7 +45,13 @@ struct landlock_rule {
>> * and never modified. It always points to an allocated object
>> because
>> * each rule increments the refcount of its object.
>> */
>> - struct landlock_object *object;
>> + //struct landlock_object *object;
>
> You need to remove such code comments.
>
Sorry. Missed it while was cleaning the code. I will fix it.
Thanks.
>
>> +
>> + union {
>> + struct landlock_object *ptr;
>> + uintptr_t data;
>> + } object;
>> +
>> /**
>> * @num_layers: Number of entries in @layers.
>> */
>> @@ -85,7 +91,13 @@ struct landlock_ruleset {
>> * nodes. 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;
>> + struct rb_root root_inode;
>> + /**
>> + * @root_net_port: Root of a red-black tree containing object nodes
>> + * for 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;
>> /**
>> * @hierarchy: Enables hierarchy identification even when a parent
>> * domain vanishes. This is needed for the ptrace protection.
>> @@ -124,29 +136,31 @@ struct landlock_ruleset {
>> */
>> u32 num_layers;
>> /**
>> - * @fs_access_masks: Contains the subset of filesystem
>> - * actions that are restricted by a ruleset. A domain
>> - * saves all layers of merged rulesets in a stack
>> - * (FAM), starting from the first layer to the last
>> - * one. These layers are used when merging rulesets,
>> - * for user space backward compatibility (i.e.
>> - * future-proof), and to properly handle merged
>> + * @access_masks: Contains the subset of filesystem
>> + * or network actions that are restricted by a ruleset.
>> + * A domain saves all layers of merged rulesets in a
>> + * stack(FAM), starting from the first layer to the
>> + * last one. These layers are used when merging
>> + * rulesets, for user space backward compatibility
>> + * (i.e. future-proof), and to properly handle merged
>> * rulesets without overlapping access rights. These
>> * layers are set once and never changed for the
>> * lifetime of the ruleset.
>> */
>> - u16 fs_access_masks[];
>> + u32 access_masks[];
>> };
>> };
>> };
>> -struct landlock_ruleset *landlock_create_ruleset(const u32
>> fs_access_mask);
>> +struct landlock_ruleset *landlock_create_ruleset(const u32
>> fs_access_mask,
>> + const u32 net_access_mask);
>
> To make it easier and avoid mistakes, you could use a dedicated struct
> to properly manage masks passing and conversions:
> struct landlock_access_mask {
> u16 fs; // TODO: make sure at build-time that all access rights fit
> in.
> u16 net; // TODO: ditto for network access rights.
> }
>
> get_access_masks(const struct landlock_ruleset *, struct
> landlock_access_mask *);
> set_access_masks(struct landlock_ruleset *, const struct
> landlock_access_mask *);
>
> This should also be part of a standalone patch.
Ok. Thanks for noticing. I will refactor this part.
>
>
>> void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
>> void landlock_put_ruleset_deferred(struct landlock_ruleset *const
>> ruleset);
>> int landlock_insert_rule(struct landlock_ruleset *const ruleset,
>> - struct landlock_object *const object, const u32 access);
>> + void *const object, const u32 access,
>> + const u16 rule_type);
>> struct landlock_ruleset *landlock_merge_ruleset(
>> struct landlock_ruleset *const parent,
>> @@ -154,7 +168,7 @@ struct landlock_ruleset *landlock_merge_ruleset(
>> const struct landlock_rule *landlock_find_rule(
>> const struct landlock_ruleset *const ruleset,
>> - const struct landlock_object *const object);
>> + const void *const obj, const u16 rule_type);
>> static inline void landlock_get_ruleset(struct landlock_ruleset
>> *const ruleset)
>> {
>> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
>> index f8e8e980454c..91ab06ec8ce0 100644
>> --- a/security/landlock/setup.c
>> +++ b/security/landlock/setup.c
>> @@ -14,6 +14,7 @@
>> #include "fs.h"
>> #include "ptrace.h"
>> #include "setup.h"
>> +#include "net.h"
>> bool landlock_initialized __lsm_ro_after_init = false;
>> @@ -21,6 +22,7 @@ struct lsm_blob_sizes landlock_blob_sizes
>> __lsm_ro_after_init = {
>> .lbs_cred = sizeof(struct landlock_cred_security),
>> .lbs_inode = sizeof(struct landlock_inode_security),
>> .lbs_superblock = sizeof(struct landlock_superblock_security),
>> + .lbs_task = sizeof(struct landlock_task_security),
>
> This patch doesn't build. For the next patches, double check that
> everything build and all tests pass.
>
Sorry. Its my fault. After some refactoring I did not check kernel
building. I will fix it.
>
>> };
>> static int __init landlock_init(void)
>> @@ -28,6 +30,7 @@ static int __init landlock_init(void)
>> landlock_add_cred_hooks();
>> landlock_add_ptrace_hooks();
>> landlock_add_fs_hooks();
>> + landlock_add_net_hooks();
>> landlock_initialized = true;
>> pr_info("Up and running.\n");
>> return 0;
>> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
>> index 32396962f04d..e0d7eb07dd76 100644
>> --- a/security/landlock/syscalls.c
>> +++ b/security/landlock/syscalls.c
>> @@ -31,6 +31,7 @@
>> #include "limits.h"
>> #include "ruleset.h"
>> #include "setup.h"
>> +#include "net.h"
>> /**
>> * copy_min_struct_from_user - Safe future-proof argument copying
>> @@ -73,7 +74,8 @@ static void build_check_abi(void)
>> {
>> struct landlock_ruleset_attr ruleset_attr;
>> struct landlock_path_beneath_attr path_beneath_attr;
>> - size_t ruleset_size, path_beneath_size;
>> + struct landlock_net_service_attr net_service_attr;
>> + size_t ruleset_size, path_beneath_size, net_service_size;
>> /*
>> * For each user space ABI structures, first checks that there
>> is no
>> @@ -81,17 +83,22 @@ static void build_check_abi(void)
>> * struct size.
>> */
>> ruleset_size = sizeof(ruleset_attr.handled_access_fs);
>> + ruleset_size += sizeof(ruleset_attr.handled_access_net);
>> BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
>> - BUILD_BUG_ON(sizeof(ruleset_attr) != 8);
>> + BUILD_BUG_ON(sizeof(ruleset_attr) != 16);
>> path_beneath_size = sizeof(path_beneath_attr.allowed_access);
>> path_beneath_size += sizeof(path_beneath_attr.parent_fd);
>> BUILD_BUG_ON(sizeof(path_beneath_attr) != path_beneath_size);
>> BUILD_BUG_ON(sizeof(path_beneath_attr) != 12);
>> +
>> + net_service_size = sizeof(net_service_attr.allowed_access);
>> + net_service_size += sizeof(net_service_attr.port);
>> + BUILD_BUG_ON(sizeof(net_service_attr) != net_service_size);
>> + BUILD_BUG_ON(sizeof(net_service_attr) != 10);
>> }
>> /* Ruleset handling */
>> -
>> static int fop_ruleset_release(struct inode *const inode,
>> struct file *const filp)
>> {
>> @@ -176,18 +183,24 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
>> /* Copies raw user space buffer. */
>> err = copy_min_struct_from_user(&ruleset_attr,
>> sizeof(ruleset_attr),
>> - offsetofend(typeof(ruleset_attr), handled_access_fs),
>> + offsetofend(typeof(ruleset_attr), handled_access_net),
>
> Please read the documentation of copy_min_struct_from_user(). This
> breaks backward compatibility…
>
Ok. I will.
>
>> attr, size);
>> if (err)
>> return err;
>> - /* Checks content (and 32-bits cast). */
>> + /* Checks fs content (and 32-bits cast). */
>> if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
>> LANDLOCK_MASK_ACCESS_FS)
>> return -EINVAL;
>> + /* Checks network content (and 32-bits cast). */
>> + if ((ruleset_attr.handled_access_net | LANDLOCK_MASK_ACCESS_NET) !=
>> + LANDLOCK_MASK_ACCESS_NET)
>> + return -EINVAL;
>> +
>> /* Checks arguments and transforms to kernel struct. */
>> - ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs);
>> + ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs,
>> + ruleset_attr.handled_access_net);
>> if (IS_ERR(ruleset))
>> return PTR_ERR(ruleset);
>> @@ -306,6 +319,7 @@ SYSCALL_DEFINE4(landlock_add_rule,
>> const void __user *const, rule_attr, const __u32, flags)
>> {
>> struct landlock_path_beneath_attr path_beneath_attr;
>> + struct landlock_net_service_attr net_service_attr;
>> struct path path;
>> struct landlock_ruleset *ruleset;
>> int res, err;
>> @@ -317,47 +331,91 @@ SYSCALL_DEFINE4(landlock_add_rule,
>> if (flags)
>> return -EINVAL;
>> - if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
>> + if ((rule_type != LANDLOCK_RULE_PATH_BENEATH) &&
>> + (rule_type != LANDLOCK_RULE_NET_SERVICE))
>
> Please replace with a switch/case.
Ok. I got it.
>
>
>> return -EINVAL;
>> - /* Copies raw user space buffer, only one type for now. */
>> - res = copy_from_user(&path_beneath_attr, rule_attr,
>> - sizeof(path_beneath_attr));
>> - if (res)
>> - return -EFAULT;
>> -
>> - /* Gets and checks the ruleset. */
>> - ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
>> - if (IS_ERR(ruleset))
>> - return PTR_ERR(ruleset);
>> -
>> - /*
>> - * Informs about useless rule: empty allowed_access (i.e. deny
>> rules)
>> - * are ignored in path walks.
>> - */
>> - if (!path_beneath_attr.allowed_access) {
>> - err = -ENOMSG;
>> - goto out_put_ruleset;
>> - }
>> - /*
>> - * Checks that allowed_access matches the @ruleset constraints
>> - * (ruleset->fs_access_masks[0] is automatically upgraded to
>> 64-bits).
>> - */
>> - if ((path_beneath_attr.allowed_access |
>> ruleset->fs_access_masks[0]) !=
>> - ruleset->fs_access_masks[0]) {
>> - err = -EINVAL;
>> - goto out_put_ruleset;
>> + switch (rule_type) {
>> + case LANDLOCK_RULE_PATH_BENEATH:
>> + /* Copies raw user space buffer, for fs rule type. */
>> + res = copy_from_user(&path_beneath_attr, rule_attr,
>> + sizeof(path_beneath_attr));
>> + if (res)
>> + return -EFAULT;
>> + break;
>> +
>> + case LANDLOCK_RULE_NET_SERVICE:
>> + /* Copies raw user space buffer, for net rule type. */
>> + res = copy_from_user(&net_service_attr, rule_attr,
>> + sizeof(net_service_attr));
>> + if (res)
>> + return -EFAULT;
>> + break;
>> }
>> - /* Gets and checks the new rule. */
>> - err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
>> - if (err)
>> - goto out_put_ruleset;
>> + if (rule_type == LANDLOCK_RULE_PATH_BENEATH) {
>> + /* Gets and checks the ruleset. */
>> + ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
>> + if (IS_ERR(ruleset))
>> + return PTR_ERR(ruleset);
>> +
>> + /*
>> + * Informs about useless rule: empty allowed_access (i.e.
>> deny rules)
>> + * are ignored in path walks.
>> + */
>> + if (!path_beneath_attr.allowed_access) {
>> + err = -ENOMSG;
>> + goto out_put_ruleset;
>> + }
>> + /*
>> + * Checks that allowed_access matches the @ruleset constraints
>> + * (ruleset->access_masks[0] is automatically upgraded to
>> 64-bits).
>> + */
>> + if ((path_beneath_attr.allowed_access |
>> ruleset->access_masks[0]) !=
>> + ruleset->access_masks[0]) {
>> + err = -EINVAL;
>> + goto out_put_ruleset;
>> + }
>> +
>> + /* Gets and checks the new rule. */
>> + err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
>> + if (err)
>> + goto out_put_ruleset;
>> +
>> + /* Imports the new rule. */
>> + err = landlock_append_fs_rule(ruleset, &path,
>> + path_beneath_attr.allowed_access);
>> + path_put(&path);
>> + }
>> - /* Imports the new rule. */
>> - err = landlock_append_fs_rule(ruleset, &path,
>> - path_beneath_attr.allowed_access);
>> - path_put(&path);
>> + if (rule_type == LANDLOCK_RULE_NET_SERVICE) {
>> + /* Gets and checks the ruleset. */
>> + ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
>
> You need to factor out more code.
Sorry. I did not get you here. Please could you explain more detailed?
>
>
>> + if (IS_ERR(ruleset))
>> + return PTR_ERR(ruleset);
>> +
>> + /*
>> + * Informs about useless rule: empty allowed_access (i.e.
>> deny rules)
>> + * are ignored in network actions
>> + */
>> + if (!net_service_attr.allowed_access) {
>> + err = -ENOMSG;
>> + goto out_put_ruleset;
>> + }
>> + /*
>> + * Checks that allowed_access matches the @ruleset constraints
>> + * (ruleset->access_masks[0] is automatically upgraded to
>> 64-bits).
>> + */
>> + if (((net_service_attr.allowed_access <<
>> LANDLOCK_MASK_SHIFT_NET) |
>> + ruleset->access_masks[0]) != ruleset->access_masks[0]) {
>> + err = -EINVAL;
>> + goto out_put_ruleset;
>> + }
>> +
>> + /* Imports the new rule. */
>> + err = landlock_append_net_rule(ruleset, net_service_attr.port,
>> + net_service_attr.allowed_access);
>> + }
>> out_put_ruleset:
>> landlock_put_ruleset(ruleset);
> .
Powered by blists - more mailing lists