devlink: disallow reload operation during device cleanup

BugLink: https://bugs.launchpad.net/bugs/1853475
[ Upstream commit 5a508a254bed9a2e36a5fb96c9065532a6bf1e9c ]

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: 2d8dc5bbf4e7 ("devlink: Add support for reload")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 309470ec02190471476744602f98be74ab02f57b..d3654c35d2ddadf023b4e6f05f379db76a5a6037 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3982,6 +3982,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;
 
@@ -4093,6 +4094,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;
 

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index b94cdbd7bb18bdbe7901d9ee4975424a3e700eb1..92ea4a2894298930ec1ffebbb5bc5b9683205ae2 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1128,8 +1128,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;
 
@@ -1191,6 +1193,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 (mlxsw_core->reload_fail) {
                if (!reload)
                        /* Only the parts that were not de-initialized in the

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index bcc40a2366248be1954f4ba7d67184009b58e548..b2fe271a4f5d5a3a990cf581204623003f574f00 100644 (file)
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -297,6 +297,7 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
        if (err)
                goto err_debugfs_exit;
 
+       devlink_reload_enable(devlink);
        return nsim_dev;
 
 err_debugfs_exit:
@@ -314,6 +315,7 @@ static void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 {
        struct devlink *devlink = priv_to_devlink(nsim_dev);
 
+       devlink_reload_disable(devlink);
        nsim_bpf_dev_exit(nsim_dev);
        nsim_dev_debugfs_exit(nsim_dev);
        devlink_unregister(devlink);

diff --git a/include/net/devlink.h b/include/net/devlink.h
index bc36f942a7d533e66d39394a91df3e62606517bb..ffa506ae501839b53259cbd07cdccb766ff99a97 100644 (file)
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -35,6 +35,7 @@ struct devlink {
        struct device *dev;
        possible_net_t _net;
        struct mutex lock;
+       u8 reload_enabled:1;
        char priv[0] __aligned(NETDEV_ALIGN);
 };
 
@@ -594,6 +595,8 @@ struct ib_device;
 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,

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4f40aeace9022672b86662176e55fd61de7430d1..707d160cc6ea040946a36e563670a5ad9b125c5a 100644 (file)
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2677,7 +2677,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
        struct devlink *devlink = info->user_ptr[0];
        int err;
 
-       if (!devlink->ops->reload)
+       if (!devlink->ops->reload || !devlink->reload_enabled)
                return -EOPNOTSUPP;
 
        err = devlink_resources_validate(devlink, NULL, info);
@@ -5559,12 +5559,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
  *