From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ido Schimmel Subject: [PATCH net 2/2] mlxsw: core: Fix devlink unregister flow Date: Mon, 29 Oct 2018 14:26:16 +0000 Message-ID: <20181029142517.1858-3-idosch@mellanox.com> References: <20181029142517.1858-1-idosch@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: "davem@davemloft.net" , Jiri Pirko , Petr Machata , Shalom Toledo , Alexander Petrovskiy , mlxsw , Ido Schimmel To: "netdev@vger.kernel.org" Return-path: Received: from mail-eopbgr50070.outbound.protection.outlook.com ([40.107.5.70]:29215 "EHLO EUR03-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726600AbeJ2XPO (ORCPT ); Mon, 29 Oct 2018 19:15:14 -0400 In-Reply-To: <20181029142517.1858-1-idosch@mellanox.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: From: Shalom Toledo 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 Reviewed-by: Jiri Pirko Signed-off-by: Ido Schimmel --- drivers/net/ethernet/mellanox/mlxsw/core.c | 24 +++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ether= net/mellanox/mlxsw/core.c index 937d0ace699a..30f751e69698 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/core.c +++ b/drivers/net/ethernet/mellanox/mlxsw/core.c @@ -943,8 +943,8 @@ static int mlxsw_devlink_core_bus_device_reload(struct = devlink *devlink, mlxsw_core->bus, mlxsw_core->bus_priv, true, devlink); - if (err) - mlxsw_core->reload_fail =3D true; + mlxsw_core->reload_fail =3D !!err; + return err; } =20 @@ -1083,8 +1083,15 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_c= ore *mlxsw_core, { struct devlink *devlink =3D priv_to_devlink(mlxsw_core); =20 - 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; + } =20 if (mlxsw_core->driver->fini) mlxsw_core->driver->fini(mlxsw_core); @@ -1098,9 +1105,12 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_c= ore *mlxsw_core, 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); --=20 2.17.2