| 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: <20230118152115.1113149-6-jiri@resnulli.us>
Date: Wed, 18 Jan 2023 16:21:08 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
edumazet@...gle.com, michael.chan@...adcom.com,
yisen.zhuang@...wei.com, salil.mehta@...wei.com,
jesse.brandeburg@...el.com, anthony.l.nguyen@...el.com,
tariqt@...dia.com, saeedm@...dia.com, leon@...nel.org,
idosch@...dia.com, petrm@...dia.com, mailhol.vincent@...adoo.fr,
jacob.e.keller@...el.com, gal@...dia.com
Subject: [patch net-next v5 05/12] devlink: protect health reporter operation with instance lock
From: Jiri Pirko <jiri@...dia.com>
Similar to other devlink objects, protect the reporters list
by devlink instance lock. Alongside add unlocked versions
of health reporter create/destroy functions and use them in drivers
on call paths where the instance lock is held.
Signed-off-by: Jiri Pirko <jiri@...dia.com>
---
v4->v5:
- changed mlx5 bits due to changed locking scheme
- added locked versions of port reporter create/destroy functions
v2->v3:
- split from v2 patch #4 - "devlink: remove reporters_lock", no change
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 8 +-
drivers/net/netdevsim/health.c | 20 ++---
include/net/devlink.h | 22 ++++-
net/devlink/leftover.c | 99 +++++++++++++++++-----
4 files changed, 110 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a0a06e2eff82..33ef726e4d54 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2051,8 +2051,8 @@ static int mlxsw_core_health_init(struct mlxsw_core *mlxsw_core)
if (!(mlxsw_core->bus->features & MLXSW_BUS_F_TXRX))
return 0;
- fw_fatal = devlink_health_reporter_create(devlink, &mlxsw_core_health_fw_fatal_ops,
- 0, mlxsw_core);
+ fw_fatal = devl_health_reporter_create(devlink, &mlxsw_core_health_fw_fatal_ops,
+ 0, mlxsw_core);
if (IS_ERR(fw_fatal)) {
dev_err(mlxsw_core->bus_info->dev, "Failed to create fw fatal reporter");
return PTR_ERR(fw_fatal);
@@ -2072,7 +2072,7 @@ static int mlxsw_core_health_init(struct mlxsw_core *mlxsw_core)
err_fw_fatal_config:
mlxsw_core_trap_unregister(mlxsw_core, &mlxsw_core_health_listener, mlxsw_core);
err_trap_register:
- devlink_health_reporter_destroy(mlxsw_core->health.fw_fatal);
+ devl_health_reporter_destroy(mlxsw_core->health.fw_fatal);
return err;
}
@@ -2085,7 +2085,7 @@ static void mlxsw_core_health_fini(struct mlxsw_core *mlxsw_core)
mlxsw_core_trap_unregister(mlxsw_core, &mlxsw_core_health_listener, mlxsw_core);
/* Make sure there is no more event work scheduled */
mlxsw_core_flush_owq();
- devlink_health_reporter_destroy(mlxsw_core->health.fw_fatal);
+ devl_health_reporter_destroy(mlxsw_core->health.fw_fatal);
}
static void mlxsw_core_irq_event_handler_init(struct mlxsw_core *mlxsw_core)
diff --git a/drivers/net/netdevsim/health.c b/drivers/net/netdevsim/health.c
index aa77af4a68df..eb04ed715d2d 100644
--- a/drivers/net/netdevsim/health.c
+++ b/drivers/net/netdevsim/health.c
@@ -233,16 +233,16 @@ int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink)
int err;
health->empty_reporter =
- devlink_health_reporter_create(devlink,
- &nsim_dev_empty_reporter_ops,
- 0, health);
+ devl_health_reporter_create(devlink,
+ &nsim_dev_empty_reporter_ops,
+ 0, health);
if (IS_ERR(health->empty_reporter))
return PTR_ERR(health->empty_reporter);
health->dummy_reporter =
- devlink_health_reporter_create(devlink,
- &nsim_dev_dummy_reporter_ops,
- 0, health);
+ devl_health_reporter_create(devlink,
+ &nsim_dev_dummy_reporter_ops,
+ 0, health);
if (IS_ERR(health->dummy_reporter)) {
err = PTR_ERR(health->dummy_reporter);
goto err_empty_reporter_destroy;
@@ -266,9 +266,9 @@ int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink)
return 0;
err_dummy_reporter_destroy:
- devlink_health_reporter_destroy(health->dummy_reporter);
+ devl_health_reporter_destroy(health->dummy_reporter);
err_empty_reporter_destroy:
- devlink_health_reporter_destroy(health->empty_reporter);
+ devl_health_reporter_destroy(health->empty_reporter);
return err;
}
@@ -278,6 +278,6 @@ void nsim_dev_health_exit(struct nsim_dev *nsim_dev)
debugfs_remove_recursive(health->ddir);
kfree(health->recovered_break_msg);
- devlink_health_reporter_destroy(health->dummy_reporter);
- devlink_health_reporter_destroy(health->empty_reporter);
+ devl_health_reporter_destroy(health->dummy_reporter);
+ devl_health_reporter_destroy(health->empty_reporter);
}
diff --git a/include/net/devlink.h b/include/net/devlink.h
index d7c9572e5bea..0d64feaef7cb 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1865,18 +1865,34 @@ int devlink_fmsg_binary_pair_put(struct devlink_fmsg *fmsg, const char *name,
const void *value, u32 value_len);
struct devlink_health_reporter *
-devlink_health_reporter_create(struct devlink *devlink,
- const struct devlink_health_reporter_ops *ops,
- u64 graceful_period, void *priv);
+devl_port_health_reporter_create(struct devlink_port *port,
+ const struct devlink_health_reporter_ops *ops,
+ u64 graceful_period, void *priv);
struct devlink_health_reporter *
devlink_port_health_reporter_create(struct devlink_port *port,
const struct devlink_health_reporter_ops *ops,
u64 graceful_period, void *priv);
+struct devlink_health_reporter *
+devl_health_reporter_create(struct devlink *devlink,
+ const struct devlink_health_reporter_ops *ops,
+ u64 graceful_period, void *priv);
+
+struct devlink_health_reporter *
+devlink_health_reporter_create(struct devlink *devlink,
+ const struct devlink_health_reporter_ops *ops,
+ u64 graceful_period, void *priv);
+
+void
+devl_health_reporter_destroy(struct devlink_health_reporter *reporter);
+
void
devlink_health_reporter_destroy(struct devlink_health_reporter *reporter);
+void
+devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter);
+
void
devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter);
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index c92bc04bc25c..0fc374140e6a 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -7337,8 +7337,8 @@ __devlink_health_reporter_create(struct devlink *devlink,
}
/**
- * devlink_port_health_reporter_create - create devlink health reporter for
- * specified port instance
+ * devl_port_health_reporter_create - create devlink health reporter for
+ * specified port instance
*
* @port: devlink_port which should contain the new reporter
* @ops: ops
@@ -7346,12 +7346,13 @@ __devlink_health_reporter_create(struct devlink *devlink,
* @priv: priv
*/
struct devlink_health_reporter *
-devlink_port_health_reporter_create(struct devlink_port *port,
- const struct devlink_health_reporter_ops *ops,
- u64 graceful_period, void *priv)
+devl_port_health_reporter_create(struct devlink_port *port,
+ const struct devlink_health_reporter_ops *ops,
+ u64 graceful_period, void *priv)
{
struct devlink_health_reporter *reporter;
+ devl_assert_locked(port->devlink);
mutex_lock(&port->reporters_lock);
if (__devlink_health_reporter_find_by_name(&port->reporter_list,
&port->reporters_lock, ops->name)) {
@@ -7370,10 +7371,26 @@ devlink_port_health_reporter_create(struct devlink_port *port,
mutex_unlock(&port->reporters_lock);
return reporter;
}
+EXPORT_SYMBOL_GPL(devl_port_health_reporter_create);
+
+struct devlink_health_reporter *
+devlink_port_health_reporter_create(struct devlink_port *port,
+ const struct devlink_health_reporter_ops *ops,
+ u64 graceful_period, void *priv)
+{
+ struct devlink_health_reporter *reporter;
+ struct devlink *devlink = port->devlink;
+
+ devl_lock(devlink);
+ reporter = devl_port_health_reporter_create(port, ops,
+ graceful_period, priv);
+ devl_unlock(devlink);
+ return reporter;
+}
EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create);
/**
- * devlink_health_reporter_create - create devlink health reporter
+ * devl_health_reporter_create - create devlink health reporter
*
* @devlink: devlink
* @ops: ops
@@ -7381,12 +7398,13 @@ EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create);
* @priv: priv
*/
struct devlink_health_reporter *
-devlink_health_reporter_create(struct devlink *devlink,
- const struct devlink_health_reporter_ops *ops,
- u64 graceful_period, void *priv)
+devl_health_reporter_create(struct devlink *devlink,
+ const struct devlink_health_reporter_ops *ops,
+ u64 graceful_period, void *priv)
{
struct devlink_health_reporter *reporter;
+ devl_assert_locked(devlink);
mutex_lock(&devlink->reporters_lock);
if (devlink_health_reporter_find_by_name(devlink, ops->name)) {
reporter = ERR_PTR(-EEXIST);
@@ -7403,6 +7421,21 @@ devlink_health_reporter_create(struct devlink *devlink,
mutex_unlock(&devlink->reporters_lock);
return reporter;
}
+EXPORT_SYMBOL_GPL(devl_health_reporter_create);
+
+struct devlink_health_reporter *
+devlink_health_reporter_create(struct devlink *devlink,
+ const struct devlink_health_reporter_ops *ops,
+ u64 graceful_period, void *priv)
+{
+ struct devlink_health_reporter *reporter;
+
+ devl_lock(devlink);
+ reporter = devl_health_reporter_create(devlink, ops,
+ graceful_period, priv);
+ devl_unlock(devlink);
+ return reporter;
+}
EXPORT_SYMBOL_GPL(devlink_health_reporter_create);
static void
@@ -7429,35 +7462,61 @@ __devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
}
/**
- * devlink_health_reporter_destroy - destroy devlink health reporter
+ * devl_health_reporter_destroy - destroy devlink health reporter
*
* @reporter: devlink health reporter to destroy
*/
void
-devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
+devl_health_reporter_destroy(struct devlink_health_reporter *reporter)
{
struct mutex *lock = &reporter->devlink->reporters_lock;
+ devl_assert_locked(reporter->devlink);
+
mutex_lock(lock);
__devlink_health_reporter_destroy(reporter);
mutex_unlock(lock);
}
+EXPORT_SYMBOL_GPL(devl_health_reporter_destroy);
+
+void
+devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
+{
+ struct devlink *devlink = reporter->devlink;
+
+ devl_lock(devlink);
+ devl_health_reporter_destroy(reporter);
+ devl_unlock(devlink);
+}
EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy);
/**
- * devlink_port_health_reporter_destroy - destroy devlink port health reporter
+ * devl_port_health_reporter_destroy - destroy devlink port health reporter
*
* @reporter: devlink health reporter to destroy
*/
void
-devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
+devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
{
struct mutex *lock = &reporter->devlink_port->reporters_lock;
+ devl_assert_locked(reporter->devlink);
+
mutex_lock(lock);
__devlink_health_reporter_destroy(reporter);
mutex_unlock(lock);
}
+EXPORT_SYMBOL_GPL(devl_port_health_reporter_destroy);
+
+void
+devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
+{
+ struct devlink *devlink = reporter->devlink;
+
+ devl_lock(devlink);
+ devl_port_health_reporter_destroy(reporter);
+ devl_unlock(devlink);
+}
EXPORT_SYMBOL_GPL(devlink_port_health_reporter_destroy);
static int
@@ -7805,12 +7864,11 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
unsigned long port_index;
int idx = 0;
+ devl_lock(devlink);
+ if (!devl_is_registered(devlink))
+ goto next_devlink;
+
mutex_lock(&devlink->reporters_lock);
- if (!devl_is_registered(devlink)) {
- mutex_unlock(&devlink->reporters_lock);
- devlink_put(devlink);
- continue;
- }
list_for_each_entry(reporter, &devlink->reporter_list,
list) {
@@ -7824,6 +7882,7 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI);
if (err) {
mutex_unlock(&devlink->reporters_lock);
+ devl_unlock(devlink);
devlink_put(devlink);
state->idx = idx;
goto out;
@@ -7832,10 +7891,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
}
mutex_unlock(&devlink->reporters_lock);
- devl_lock(devlink);
- if (!devl_is_registered(devlink))
- goto next_devlink;
-
xa_for_each(&devlink->ports, port_index, port) {
mutex_lock(&port->reporters_lock);
list_for_each_entry(reporter, &port->reporter_list, list) {
--
2.39.0
Powered by blists - more mailing lists