[bug report] net/mlx5: SD, Implement devcom communication and primary election

[bug report] net/mlx5: SD, Implement devcom communication and primary election

[Dan Carpenter]

Hello Tariq Toukan,

Commit d3d057666090 ("net/mlx5: SD, Implement devcom communication
and primary election") from Feb 14, 2024 (linux-next), leads to the
following Smatch static checker warning:

	drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c:221 sd_register()
	error: 'devcom' dereferencing possible ERR_PTR()

drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
    206 static int sd_register(struct mlx5_core_dev *dev)
    207 {
    208         struct mlx5_devcom_comp_dev *devcom, *pos;
    209         struct mlx5_core_dev *peer, *primary;
    210         struct mlx5_sd *sd, *primary_sd;
    211         int err, i;
    212 
    213         sd = mlx5_get_sd(dev);
    214         devcom = mlx5_devcom_register_component(dev->priv.devc, MLX5_DEVCOM_SD_GROUP,
    215                                                 sd->group_id, NULL, dev);
    216         if (!devcom)

The mlx5_devcom_register_component() function returns a mix of error
pointers and NULL.  It's not done really done correctly...  Here is an
explanation of how that normally works:

https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/

mlx5_devcom_register_component() is not optional so it should only
return error pointers.

    217                 return -ENOMEM;
    218 
    219         sd->devcom = devcom;
    220 
--> 221         if (mlx5_devcom_comp_get_size(devcom) != sd->host_buses)
                                              ^^^^^^
Dead.

    222                 return 0;

regards,
dan carpenter

Re: [bug report] net/mlx5: SD, Implement devcom communication and primary election

[Tariq Toukan]

On 15/03/2024 17:19, Dan Carpenter wrote:
> Hello Tariq Toukan,
> 
> Commit d3d057666090 ("net/mlx5: SD, Implement devcom communication
> and primary election") from Feb 14, 2024 (linux-next), leads to the
> following Smatch static checker warning:
> 
> 	drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c:221 sd_register()
> 	error: 'devcom' dereferencing possible ERR_PTR()
> 
> drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
>      206 static int sd_register(struct mlx5_core_dev *dev)
>      207 {
>      208         struct mlx5_devcom_comp_dev *devcom, *pos;
>      209         struct mlx5_core_dev *peer, *primary;
>      210         struct mlx5_sd *sd, *primary_sd;
>      211         int err, i;
>      212
>      213         sd = mlx5_get_sd(dev);
>      214         devcom = mlx5_devcom_register_component(dev->priv.devc, MLX5_DEVCOM_SD_GROUP,
>      215                                                 sd->group_id, NULL, dev);
>      216         if (!devcom)
> 
> The mlx5_devcom_register_component() function returns a mix of error
> pointers and NULL.  It's not done really done correctly...  Here is an
> explanation of how that normally works:
> 
> https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
> 
> mlx5_devcom_register_component() is not optional so it should only
> return error pointers.
> 
>      217                 return -ENOMEM;
>      218
>      219         sd->devcom = devcom;
>      220
> --> 221         if (mlx5_devcom_comp_get_size(devcom) != sd->host_buses)
>                                                ^^^^^^
> Dead.
> 
>      222                 return 0;
> 
> regards,
> dan carpenter

Hi Dan,

Thanks for your report!
I prepared a fix and will submit it soon.

Regards,
Tariq