devlink: disallow reload operation during device cleanup
There is a race between driver code that does setup/cleanup of device
and devlink reload operation that in some drivers works with the same
code. Use after free could we easily obtained by running:
while true; do
echo "0000:00:10.0" >/sys/bus/pci/drivers/mlxsw_spectrum2/bind
devlink dev reload pci/0000:00:10.0 &
echo "0000:00:10.0" >/sys/bus/pci/drivers/mlxsw_spectrum2/unbind
done
Fix this by enabling reload only after setup of device is complete and
disabling it at the beginning of the cleanup process.
Reported-by: Ido Schimmel <idosch@mellanox.com>
Fixes: 2d8dc5bbf4
("devlink: Add support for reload")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
0058b0a506
commit
5a508a254b
|
@ -4010,6 +4010,7 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
|
||||||
goto err_params_unregister;
|
goto err_params_unregister;
|
||||||
|
|
||||||
devlink_params_publish(devlink);
|
devlink_params_publish(devlink);
|
||||||
|
devlink_reload_enable(devlink);
|
||||||
pci_save_state(pdev);
|
pci_save_state(pdev);
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
@ -4121,6 +4122,8 @@ static void mlx4_remove_one(struct pci_dev *pdev)
|
||||||
struct devlink *devlink = priv_to_devlink(priv);
|
struct devlink *devlink = priv_to_devlink(priv);
|
||||||
int active_vfs = 0;
|
int active_vfs = 0;
|
||||||
|
|
||||||
|
devlink_reload_disable(devlink);
|
||||||
|
|
||||||
if (mlx4_is_slave(dev))
|
if (mlx4_is_slave(dev))
|
||||||
persist->interface_state |= MLX4_INTERFACE_STATE_NOWAIT;
|
persist->interface_state |= MLX4_INTERFACE_STATE_NOWAIT;
|
||||||
|
|
||||||
|
|
|
@ -1186,8 +1186,10 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
|
||||||
if (err)
|
if (err)
|
||||||
goto err_thermal_init;
|
goto err_thermal_init;
|
||||||
|
|
||||||
if (mlxsw_driver->params_register)
|
if (mlxsw_driver->params_register) {
|
||||||
devlink_params_publish(devlink);
|
devlink_params_publish(devlink);
|
||||||
|
devlink_reload_enable(devlink);
|
||||||
|
}
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
@ -1249,6 +1251,8 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
|
||||||
{
|
{
|
||||||
struct devlink *devlink = priv_to_devlink(mlxsw_core);
|
struct devlink *devlink = priv_to_devlink(mlxsw_core);
|
||||||
|
|
||||||
|
if (!reload)
|
||||||
|
devlink_reload_disable(devlink);
|
||||||
if (devlink_is_reload_failed(devlink)) {
|
if (devlink_is_reload_failed(devlink)) {
|
||||||
if (!reload)
|
if (!reload)
|
||||||
/* Only the parts that were not de-initialized in the
|
/* Only the parts that were not de-initialized in the
|
||||||
|
|
|
@ -708,6 +708,7 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
|
||||||
goto err_debugfs_exit;
|
goto err_debugfs_exit;
|
||||||
|
|
||||||
devlink_params_publish(devlink);
|
devlink_params_publish(devlink);
|
||||||
|
devlink_reload_enable(devlink);
|
||||||
return nsim_dev;
|
return nsim_dev;
|
||||||
|
|
||||||
err_debugfs_exit:
|
err_debugfs_exit:
|
||||||
|
@ -732,6 +733,7 @@ static void nsim_dev_destroy(struct nsim_dev *nsim_dev)
|
||||||
{
|
{
|
||||||
struct devlink *devlink = priv_to_devlink(nsim_dev);
|
struct devlink *devlink = priv_to_devlink(nsim_dev);
|
||||||
|
|
||||||
|
devlink_reload_disable(devlink);
|
||||||
nsim_bpf_dev_exit(nsim_dev);
|
nsim_bpf_dev_exit(nsim_dev);
|
||||||
nsim_dev_debugfs_exit(nsim_dev);
|
nsim_dev_debugfs_exit(nsim_dev);
|
||||||
nsim_dev_traps_exit(devlink);
|
nsim_dev_traps_exit(devlink);
|
||||||
|
|
|
@ -38,7 +38,8 @@ struct devlink {
|
||||||
struct device *dev;
|
struct device *dev;
|
||||||
possible_net_t _net;
|
possible_net_t _net;
|
||||||
struct mutex lock;
|
struct mutex lock;
|
||||||
bool reload_failed;
|
u8 reload_failed:1,
|
||||||
|
reload_enabled:1;
|
||||||
char priv[0] __aligned(NETDEV_ALIGN);
|
char priv[0] __aligned(NETDEV_ALIGN);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -774,6 +775,8 @@ struct ib_device;
|
||||||
struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size);
|
struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size);
|
||||||
int devlink_register(struct devlink *devlink, struct device *dev);
|
int devlink_register(struct devlink *devlink, struct device *dev);
|
||||||
void devlink_unregister(struct devlink *devlink);
|
void devlink_unregister(struct devlink *devlink);
|
||||||
|
void devlink_reload_enable(struct devlink *devlink);
|
||||||
|
void devlink_reload_disable(struct devlink *devlink);
|
||||||
void devlink_free(struct devlink *devlink);
|
void devlink_free(struct devlink *devlink);
|
||||||
int devlink_port_register(struct devlink *devlink,
|
int devlink_port_register(struct devlink *devlink,
|
||||||
struct devlink_port *devlink_port,
|
struct devlink_port *devlink_port,
|
||||||
|
|
|
@ -2699,7 +2699,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
|
||||||
struct devlink *devlink = info->user_ptr[0];
|
struct devlink *devlink = info->user_ptr[0];
|
||||||
int err;
|
int err;
|
||||||
|
|
||||||
if (!devlink_reload_supported(devlink))
|
if (!devlink_reload_supported(devlink) || !devlink->reload_enabled)
|
||||||
return -EOPNOTSUPP;
|
return -EOPNOTSUPP;
|
||||||
|
|
||||||
err = devlink_resources_validate(devlink, NULL, info);
|
err = devlink_resources_validate(devlink, NULL, info);
|
||||||
|
@ -6196,12 +6196,49 @@ EXPORT_SYMBOL_GPL(devlink_register);
|
||||||
void devlink_unregister(struct devlink *devlink)
|
void devlink_unregister(struct devlink *devlink)
|
||||||
{
|
{
|
||||||
mutex_lock(&devlink_mutex);
|
mutex_lock(&devlink_mutex);
|
||||||
|
WARN_ON(devlink_reload_supported(devlink) &&
|
||||||
|
devlink->reload_enabled);
|
||||||
devlink_notify(devlink, DEVLINK_CMD_DEL);
|
devlink_notify(devlink, DEVLINK_CMD_DEL);
|
||||||
list_del(&devlink->list);
|
list_del(&devlink->list);
|
||||||
mutex_unlock(&devlink_mutex);
|
mutex_unlock(&devlink_mutex);
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(devlink_unregister);
|
EXPORT_SYMBOL_GPL(devlink_unregister);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* devlink_reload_enable - Enable reload of devlink instance
|
||||||
|
*
|
||||||
|
* @devlink: devlink
|
||||||
|
*
|
||||||
|
* Should be called at end of device initialization
|
||||||
|
* process when reload operation is supported.
|
||||||
|
*/
|
||||||
|
void devlink_reload_enable(struct devlink *devlink)
|
||||||
|
{
|
||||||
|
mutex_lock(&devlink_mutex);
|
||||||
|
devlink->reload_enabled = true;
|
||||||
|
mutex_unlock(&devlink_mutex);
|
||||||
|
}
|
||||||
|
EXPORT_SYMBOL_GPL(devlink_reload_enable);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* devlink_reload_disable - Disable reload of devlink instance
|
||||||
|
*
|
||||||
|
* @devlink: devlink
|
||||||
|
*
|
||||||
|
* Should be called at the beginning of device cleanup
|
||||||
|
* process when reload operation is supported.
|
||||||
|
*/
|
||||||
|
void devlink_reload_disable(struct devlink *devlink)
|
||||||
|
{
|
||||||
|
mutex_lock(&devlink_mutex);
|
||||||
|
/* Mutex is taken which ensures that no reload operation is in
|
||||||
|
* progress while setting up forbidded flag.
|
||||||
|
*/
|
||||||
|
devlink->reload_enabled = false;
|
||||||
|
mutex_unlock(&devlink_mutex);
|
||||||
|
}
|
||||||
|
EXPORT_SYMBOL_GPL(devlink_reload_disable);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* devlink_free - Free devlink instance resources
|
* devlink_free - Free devlink instance resources
|
||||||
*
|
*
|
||||||
|
|
Loading…
Reference in New Issue