| 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: <461d1e1b3d0399d6cab1117a796f0b02b634597f.1636390483.git.leonro@nvidia.com>
Date: Mon, 8 Nov 2021 19:05:30 +0200
From: Leon Romanovsky <leon@...nel.org>
To: "David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>
Cc: Leon Romanovsky <leonro@...dia.com>,
Ido Schimmel <idosch@...dia.com>, Jiri Pirko <jiri@...dia.com>,
netdev <netdev@...r.kernel.org>
Subject: [RFC PATCH 08/16] devlink: Protect resource list with specific lock
From: Leon Romanovsky <leonro@...dia.com>
Devlink resource flows relied on devlink instance lock to protect
from parallel addition and deletion from resource_list. So instead
of overloading devlink->lock, let's introduce new lock with much more
clear scope.
Signed-off-by: Leon Romanovsky <leonro@...dia.com>
---
net/core/devlink.c | 100 +++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 44 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 86db7cf1f3ca..97154219fca2 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -46,6 +46,7 @@ struct devlink {
struct list_head sb_list;
struct list_head dpipe_table_list;
struct list_head resource_list;
+ struct mutex resource_list_lock; /* protects resource_list */
struct list_head param_list;
struct list_head region_list;
struct list_head reporter_list;
@@ -3587,12 +3588,15 @@ devlink_resource_find(struct devlink *devlink,
}
static void
-devlink_resource_validate_children(struct devlink_resource *resource)
+devlink_resource_validate_children(struct devlink *devlink,
+ struct devlink_resource *resource)
{
struct devlink_resource *child_resource;
bool size_valid = true;
u64 parts_size = 0;
+ lockdep_assert_held(&devlink->lock);
+
if (list_empty(&resource->resource_list))
goto out;
@@ -3645,20 +3649,25 @@ static int devlink_nl_cmd_resource_set(struct sk_buff *skb,
return -EINVAL;
resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
- if (!resource)
- return -EINVAL;
+ if (!resource) {
+ err = -EINVAL;
+ goto out;
+ }
size = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]);
err = devlink_resource_validate_size(resource, size, info->extack);
if (err)
- return err;
+ goto out;
resource->size_new = size;
- devlink_resource_validate_children(resource);
+ devlink_resource_validate_children(devlink, resource);
if (resource->parent)
- devlink_resource_validate_children(resource->parent);
- return 0;
+ devlink_resource_validate_children(devlink, resource->parent);
+out:
+ mutex_unlock(&devlink->resource_list_lock);
+ return err;
}
static int
@@ -3742,31 +3751,36 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
return -EMSGSIZE;
}
-static int devlink_resource_fill(struct genl_info *info,
- enum devlink_command cmd, int flags)
+static int devlink_nl_cmd_resource_dump(struct sk_buff *sk,
+ struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
struct devlink_resource *resource;
struct nlattr *resources_attr;
struct sk_buff *skb = NULL;
+ int err = -EOPNOTSUPP;
struct nlmsghdr *nlh;
bool incomplete;
void *hdr;
int i;
- int err;
+
+ mutex_lock(&devlink->resource_list_lock);
+ if (list_empty(&devlink->resource_list))
+ goto out;
resource = list_first_entry(&devlink->resource_list,
struct devlink_resource, list);
start_again:
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
- return err;
+ goto out;
hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq,
- &devlink_nl_family, NLM_F_MULTI, cmd);
+ &devlink_nl_family, NLM_F_MULTI,
+ DEVLINK_CMD_RESOURCE_DUMP);
if (!hdr) {
- nlmsg_free(skb);
- return -EMSGSIZE;
+ err = -EMSGSIZE;
+ goto err_resource_put;
}
if (devlink_nl_put_handle(skb, devlink))
@@ -3793,9 +3807,10 @@ static int devlink_resource_fill(struct genl_info *info,
genlmsg_end(skb, hdr);
if (incomplete)
goto start_again;
+ mutex_unlock(&devlink->resource_list_lock);
send_done:
- nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq,
- NLMSG_DONE, 0, flags | NLM_F_MULTI);
+ nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq, NLMSG_DONE, 0,
+ NLM_F_MULTI);
if (!nlh) {
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
@@ -3808,28 +3823,20 @@ static int devlink_resource_fill(struct genl_info *info,
err = -EMSGSIZE;
err_resource_put:
nlmsg_free(skb);
+out:
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
-static int devlink_nl_cmd_resource_dump(struct sk_buff *skb,
- struct genl_info *info)
-{
- struct devlink *devlink = info->user_ptr[0];
-
- if (list_empty(&devlink->resource_list))
- return -EOPNOTSUPP;
-
- return devlink_resource_fill(info, DEVLINK_CMD_RESOURCE_DUMP, 0);
-}
-
static int
devlink_resources_validate(struct devlink *devlink,
- struct devlink_resource *resource,
- struct genl_info *info)
+ struct devlink_resource *resource)
{
struct list_head *resource_list;
int err = 0;
+ lockdep_assert_held(&devlink->lock);
+
if (resource)
resource_list = &resource->resource_list;
else
@@ -3838,9 +3845,10 @@ devlink_resources_validate(struct devlink *devlink,
list_for_each_entry(resource, resource_list, list) {
if (!resource->size_valid)
return -EINVAL;
- err = devlink_resources_validate(devlink, resource, info);
+
+ err = devlink_resources_validate(devlink, resource);
if (err)
- return err;
+ break;
}
return err;
}
@@ -4064,7 +4072,9 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
if (!(devlink->features & DEVLINK_F_RELOAD))
return -EOPNOTSUPP;
- err = devlink_resources_validate(devlink, NULL, info);
+ mutex_lock(&devlink->resource_list_lock);
+ err = devlink_resources_validate(devlink, NULL);
+ mutex_unlock(&devlink->resource_list_lock);
if (err) {
NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
return err;
@@ -8955,7 +8965,10 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
INIT_LIST_HEAD(&devlink->rate_list);
INIT_LIST_HEAD(&devlink->sb_list);
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
+
INIT_LIST_HEAD(&devlink->resource_list);
+ mutex_init(&devlink->resource_list_lock);
+
INIT_LIST_HEAD(&devlink->param_list);
INIT_LIST_HEAD(&devlink->region_list);
INIT_LIST_HEAD(&devlink->reporter_list);
@@ -9108,6 +9121,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->reporters_lock);
mutex_destroy(&devlink->port_list_lock);
+ mutex_destroy(&devlink->resource_list_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -9825,7 +9839,7 @@ int devlink_resource_register(struct devlink *devlink,
sizeof(resource->size_params));
INIT_LIST_HEAD(&resource->resource_list);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
if (parent_resource_id == DEVLINK_RESOURCE_ID_PARENT_TOP) {
resource_list = &devlink->resource_list;
} else {
@@ -9845,7 +9859,7 @@ int devlink_resource_register(struct devlink *devlink,
list_add_tail(&resource->list, resource_list);
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_resource_register);
@@ -9872,16 +9886,14 @@ void devlink_resources_unregister(struct devlink *devlink)
{
struct devlink_resource *tmp, *child_resource;
- mutex_lock(&devlink->lock);
-
+ mutex_lock(&devlink->resource_list_lock);
list_for_each_entry_safe(child_resource, tmp, &devlink->resource_list,
list) {
devlink_resource_unregister(devlink, child_resource);
list_del(&child_resource->list);
kfree(child_resource);
}
-
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resources_unregister);
@@ -9899,7 +9911,7 @@ int devlink_resource_size_get(struct devlink *devlink,
struct devlink_resource *resource;
int err = 0;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (!resource) {
err = -EINVAL;
@@ -9908,7 +9920,7 @@ int devlink_resource_size_get(struct devlink *devlink,
*p_resource_size = resource->size_new;
resource->size = resource->size_new;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_resource_size_get);
@@ -9959,7 +9971,7 @@ void devlink_resource_occ_get_register(struct devlink *devlink,
{
struct devlink_resource *resource;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (WARN_ON(!resource))
goto out;
@@ -9968,7 +9980,7 @@ void devlink_resource_occ_get_register(struct devlink *devlink,
resource->occ_get = occ_get;
resource->occ_get_priv = occ_get_priv;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
@@ -9983,7 +9995,7 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,
{
struct devlink_resource *resource;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (WARN_ON(!resource))
goto out;
@@ -9992,7 +10004,7 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,
resource->occ_get = NULL;
resource->occ_get_priv = NULL;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
--
2.33.1
Powered by blists - more mailing lists