Commit a0c76345 authored by Jiri Pirko's avatar Jiri Pirko Committed by David S. Miller
Browse files

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 10 > /sys/bus/netdevsim/new_device
        devlink dev reload netdevsim/netdevsim10 &
        echo 10 > /sys/bus/netdevsim/del_device
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: default avatarIdo Schimmel <idosch@mellanox.com>
Fixes: 2d8dc5bb ("devlink: Add support for reload")
Signed-off-by: default avatarJiri Pirko <jiri@mellanox.com>
Acked-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent f95e6c9c

drivers/net/ethernet/mellanox/mlx4/main.c

+3 −0
Original line number Diff line number Diff line
@@ -4015,6 +4015,7 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id) 
		goto err_params_unregister;



	devlink_params_publish(devlink);

	devlink_reload_enable(devlink);

	pci_save_state(pdev);

	return 0;
@@ -4126,6 +4127,8 @@ static void mlx4_remove_one(struct pci_dev *pdev) 
	struct devlink *devlink = priv_to_devlink(priv);

	int active_vfs = 0;



	devlink_reload_disable(devlink);



	if (mlx4_is_slave(dev))

		persist->interface_state |= MLX4_INTERFACE_STATE_NOWAIT;

drivers/net/ethernet/mellanox/mlxsw/core.c

+5 −1
Original line number Diff line number Diff line
@@ -1198,8 +1198,10 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info, 
	if (err)

		goto err_thermal_init;



	if (mlxsw_driver->params_register)

	if (mlxsw_driver->params_register) {

		devlink_params_publish(devlink);

		devlink_reload_enable(devlink);

	}



	return 0;
@@ -1263,6 +1265,8 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core, 
{

	struct devlink *devlink = priv_to_devlink(mlxsw_core);



	if (!reload)

		devlink_reload_disable(devlink);

	if (devlink_is_reload_failed(devlink)) {

		if (!reload)

			/* Only the parts that were not de-initialized in the

drivers/net/netdevsim/dev.c

+3 −0
Original line number Diff line number Diff line
@@ -820,6 +820,7 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev) 
		goto err_bpf_dev_exit;



	devlink_params_publish(devlink);

	devlink_reload_enable(devlink);

	return 0;



err_bpf_dev_exit:
@@ -865,6 +866,8 @@ void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev) 
	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);

	struct devlink *devlink = priv_to_devlink(nsim_dev);



	devlink_reload_disable(devlink);



	nsim_dev_reload_destroy(nsim_dev);



	nsim_bpf_dev_exit(nsim_dev);

include/net/devlink.h

+5 −2
Original line number Diff line number Diff line
@@ -38,8 +38,9 @@ struct devlink { 
	struct device *dev;

	possible_net_t _net;

	struct mutex lock;

	bool reload_failed;

	bool registered;

	u8 reload_failed:1,

	   reload_enabled:1,

	   registered:1;

	char priv[0] __aligned(NETDEV_ALIGN);

};
@@ -824,6 +825,8 @@ void devlink_net_set(struct devlink *devlink, struct net *net); 
struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size);

int devlink_register(struct devlink *devlink, struct device *dev);

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);

int devlink_port_register(struct devlink *devlink,

			  struct devlink_port *devlink_port,

net/core/devlink.c

+41 −1
Original line number Diff line number Diff line
@@ -2791,6 +2791,9 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net, 
{

	int err;



	if (!devlink->reload_enabled)

		return -EOPNOTSUPP;



	err = devlink->ops->reload_down(devlink, !!dest_net, extack);

	if (err)

		return err;
@@ -6308,12 +6311,49 @@ EXPORT_SYMBOL_GPL(devlink_register); 
void devlink_unregister(struct devlink *devlink)

{

	mutex_lock(&devlink_mutex);

	WARN_ON(devlink_reload_supported(devlink) &&

		devlink->reload_enabled);

	devlink_notify(devlink, DEVLINK_CMD_DEL);

	list_del(&devlink->list);

	mutex_unlock(&devlink_mutex);

}

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

 *
@@ -8201,7 +8241,7 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net) 
			if (WARN_ON(!devlink_reload_supported(devlink)))

				continue;

			err = devlink_reload(devlink, &init_net, NULL);

			if (err)

			if (err && err != -EOPNOTSUPP)

				pr_warn("Failed to reload devlink instance into init_net\n");

		}

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE