mlxsw: core: Fix devlink unregister flow
authorShalom Toledo <shalomt@mellanox.com>
Mon, 29 Oct 2018 14:26:16 +0000 (14:26 +0000)
committerDavid S. Miller <davem@davemloft.net>
Tue, 30 Oct 2018 03:48:00 +0000 (20:48 -0700)
After a failed reload, the driver is still registered to devlink, its
devlink instance is still allocated and the 'reload_fail' flag is set.
Then, in the next reload try, the driver's allocated devlink instance will
be freed without unregistering from devlink and its components (e.g,
resources). This scenario can cause a use-after-free if the user tries to
execute command via devlink user-space tool.

Fix by not freeing the devlink instance during reload (failed or not).

Fixes: 24cc68ad6c46 ("mlxsw: core: Add support for reload")
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ethernet/mellanox/mlxsw/core.c

index 937d0ace699a7eeb4e04af3bf54eebde5dd5d459..30f751e696980d727a86200e1748adda13bb8a22 100644 (file)
@@ -943,8 +943,8 @@ static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink,
                                             mlxsw_core->bus,
                                             mlxsw_core->bus_priv, true,
                                             devlink);
                                             mlxsw_core->bus,
                                             mlxsw_core->bus_priv, true,
                                             devlink);
-       if (err)
-               mlxsw_core->reload_fail = true;
+       mlxsw_core->reload_fail = !!err;
+
        return err;
 }
 
        return err;
 }
 
@@ -1083,8 +1083,15 @@ 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 (mlxsw_core->reload_fail)
-               goto reload_fail;
+       if (mlxsw_core->reload_fail) {
+               if (!reload)
+                       /* Only the parts that were not de-initialized in the
+                        * failed reload attempt need to be de-initialized.
+                        */
+                       goto reload_fail_deinit;
+               else
+                       return;
+       }
 
        if (mlxsw_core->driver->fini)
                mlxsw_core->driver->fini(mlxsw_core);
 
        if (mlxsw_core->driver->fini)
                mlxsw_core->driver->fini(mlxsw_core);
@@ -1098,9 +1105,12 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
        if (!reload)
                devlink_resources_unregister(devlink, NULL);
        mlxsw_core->bus->fini(mlxsw_core->bus_priv);
        if (!reload)
                devlink_resources_unregister(devlink, NULL);
        mlxsw_core->bus->fini(mlxsw_core->bus_priv);
-       if (reload)
-               return;
-reload_fail:
+
+       return;
+
+reload_fail_deinit:
+       devlink_unregister(devlink);
+       devlink_resources_unregister(devlink, NULL);
        devlink_free(devlink);
 }
 EXPORT_SYMBOL(mlxsw_core_bus_device_unregister);
        devlink_free(devlink);
 }
 EXPORT_SYMBOL(mlxsw_core_bus_device_unregister);