Received-SPF: pass (google.com: domain of linux-kernel+bounces-175342-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Fri, 10 May 2024 08:44:26 +0200
From: Larysa Zaremba <larysa.zaremba@intel.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
CC: Shay Drory <shayd@nvidia.com>, Saeed Mahameed <saeedm@nvidia.com>, "Leon
 Romanovsky" <leon@kernel.org>, Tariq Toukan <tariqt@nvidia.com>, "David S.
 Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, "Jakub
 Kicinski" <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Moshe Shemesh
	<moshe@nvidia.com>, <netdev@vger.kernel.org>, <linux-rdma@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <kernel-janitors@vger.kernel.org>
Subject: Re: [PATCH net] net/mlx5: Fix error handling in mlx5_init_one_light()
Message-ID: <Zj3CShR731VUdEUA@lzaremba-mobl.ger.corp.intel.com>
References: <a2bb6a55-5415-4c15-bee9-9e63f4b6a339@moroto.mountain>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <a2bb6a55-5415-4c15-bee9-9e63f4b6a339@moroto.mountain>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?iyxBsQbRQT/rU268DZkXFuuuauShhY8r6M08a+hKeuF00w1etUMAtoBvPxhE?=
 =?us-ascii?Q?ZTqyjs90VYVFYBAs2230QsXnVjbNZG4Jj1Qp8sNvHyEVgfwKNsOgmqjeh5Pw?=
 =?us-ascii?Q?7odOgXpHFqCXABFNjgEBS9yiVgMcIiDfn/o7qeouX0aNCbTXklLGrifJAvAD?=
 =?us-ascii?Q?y8d+M/fMsmm5lneopVaOJzwKyCIiwK4YC2hVI7JAvFVjFXEAAq0YOt5QEINN?=
 =?us-ascii?Q?C4zKZN0XCZkWiyLP9+Mgl9IkKfFfTi2SqOLPxwePfFe2sz7uiF6poCXwqCS5?=
 =?us-ascii?Q?RPB3Q6VV5S0fBraN34kRRMw1hVj4EdOfBFKWHVtUTxyfcdDMjU8X/CPnT8I2?=
 =?us-ascii?Q?mD1B2+cUpwvix8MOEi5HcLOgmN5YCH0k6PgVaX8ZZ9xMU5M8qr+Ga8aN5cJj?=
 =?us-ascii?Q?gpVtNLNk4QLSmsAbWkdwVgqamJKLl8QvQNS64SZQEheFLyd1OjUYCcLubXWb?=
 =?us-ascii?Q?7Vo58yzFxYaiCrqdAJqxJmmvKqWfX4ZHCDmdFxz/OI2AVhvFMKVqUMKhh7IP?=
 =?us-ascii?Q?qgrweWMJGyxYfUrD4Acj/PfNH3/xApgnEeFumWv1aCqxYh2ltMH6/txLsuQD?=
 =?us-ascii?Q?Z1pw2pgSjQiJkbTI95DRr/14kkl+W/XIyfQDFQOzoRZtcs5EZb9TftONOHVH?=
 =?us-ascii?Q?idHPfXqf/aVfm+ubPU2HthnVBVf99pog9ofQH9hf3hrF/Fpsp3E78DWw84+T?=
 =?us-ascii?Q?cxaKOHWlqNYlpKLuGXoAc0FxX2oSvddUBtQpFKSF6ciIQp2EjDH0eZel0qzy?=
 =?us-ascii?Q?6HeADQVYyBn4/Ulcp8QTBEL1qlD6A92B97+i855lr9KgiU8xhSCniZQB+JSG?=
 =?us-ascii?Q?JVBFopcid1/YQYW0PbdjIyRfZmsMeVoNbk6mhkid/cFdUElTe1unqjvg6B6A?=
 =?us-ascii?Q?gPTlKxfRTiM3x2pfT2PYVDkIn05uXv6Cva5d/7W9sn3W3SjIeuZI5E42EUdG?=
 =?us-ascii?Q?5PyhpVpbU/uOuf7k8a3b88cI9P4SG0qjMOV5UO9qteZsTPLCDQXRpLtrALli?=
 =?us-ascii?Q?Fb00XW5HZcHYS7FnngrrArYSNpBMta9k3NytVyttcfd/1wLxbGXEBT1ykKzE?=
 =?us-ascii?Q?R5S1rlyzu3AWucTVkEQbQNyZDZGM/x1jU2ksfJWrURHwALlW101LY46tzc6k?=
 =?us-ascii?Q?gdO18s+gub39RcH2fCdFOf5ph119Kpo8xdtxogCSazmdjCpKqPTP5zCDoRTb?=
 =?us-ascii?Q?AnfL9/TeGyjvk0x20z+CV+vQq/K6SwX+iyirUhpiVp9kgTiWwkHkUE7FVJM2?=
 =?us-ascii?Q?gWw1cXFlRvWa++yVEngBfGshkNUUe2QidCqiszJLrMZxmOJWPS1Lx+JtdAig?=
 =?us-ascii?Q?FxoS+Gih8YocKqMSACntHeY7FMKkaLD5yS4v+t5TufRBDPFdymQW7x9RbZrx?=
 =?us-ascii?Q?6XadNw1BFmQQGPFBxfWINBvCB4pPFp7+HUwPW0UrWf4CdPbyv1nLkZ3SBQp1?=
 =?us-ascii?Q?L5okCZnLPZ7goaZxukkcI36ZKbGq4bVYzj5LDKWmpVf79RH35LFsKjGU/tE8?=
 =?us-ascii?Q?KTPaZ67iJHfNztHuxLPoGlBzVsksDn3XPMRXM81FoPzCsHdZLDYu6RDc6H9q?=
 =?us-ascii?Q?fiStKWQzFL2Ki+xQw40BQ474nM7jPEiN/Mha1DgdsmdwUFMixAFo3CAg2fsZ?=
 =?us-ascii?Q?kw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 7dced2b8-9345-4ec4-d77d-08dc70bca63b
X-MS-Exchange-CrossTenant-AuthSource: SN7PR11MB7540.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2024 06:44:32.6261
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ITJHTy28bC3h7qt8/Z0U07/UFemv0gkVeFFuxuhyUhI5CYmgYD7g+HIdbFhQNri/QCMw/URWi33mC+s/L8OpT853X5d1quQeNzTHpifflLI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB8106
X-OriginatorOrg: intel.com

On Thu, May 09, 2024 at 02:00:18PM +0300, Dan Carpenter wrote:
> If mlx5_query_hca_caps_light() fails then calling devl_unregister() or
> devl_unlock() is a bug.  It's not registered and it's not locked.  That
> will trigger a stack trace in this case because devl_unregister() checks
> both those things at the start of the function.
> 
> If mlx5_devlink_params_register() fails then this code will call
> devl_unregister() and devl_unlock() twice which will again lead to a
> stack trace or possibly something worse as well.
> 
> Fixes: bf729988303a ("net/mlx5: Restore mistakenly dropped parts in register devlink flow")
> Fixes: c6e77aa9dd82 ("net/mlx5: Register devlink first under devlink lock")

Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>

> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/main.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 331ce47f51a1..105c98160327 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -1690,7 +1690,7 @@ int mlx5_init_one_light(struct mlx5_core_dev *dev)
>  	err = mlx5_query_hca_caps_light(dev);
>  	if (err) {
>  		mlx5_core_warn(dev, "mlx5_query_hca_caps_light err=%d\n", err);
> -		goto query_hca_caps_err;
> +		goto err_function_disable;
>  	}
>  
>  	devl_lock(devlink);
> @@ -1699,18 +1699,16 @@ int mlx5_init_one_light(struct mlx5_core_dev *dev)
>  	err = mlx5_devlink_params_register(priv_to_devlink(dev));
>  	if (err) {
>  		mlx5_core_warn(dev, "mlx5_devlink_param_reg err = %d\n", err);
> -		goto params_reg_err;
> +		goto err_unregister;
>  	}
>  
>  	devl_unlock(devlink);
>  	return 0;
>  
> -params_reg_err:
> -	devl_unregister(devlink);
> -	devl_unlock(devlink);
> -query_hca_caps_err:
> +err_unregister:
>  	devl_unregister(devlink);
>  	devl_unlock(devlink);
> +err_function_disable:
>  	mlx5_function_disable(dev, true);
>  out:
>  	dev->state = MLX5_DEVICE_STATE_INTERNAL_ERROR;
> -- 
> 2.43.0
> 
>