[bug report] net/mlx5e: Generalize IPsec work structs

[bug report] net/mlx5e: Generalize IPsec work structs

[Dan Carpenter]

Hello Leon Romanovsky,

This is a semi-automatic email about new static checker warnings.

The patch 4562116f8a56: "net/mlx5e: Generalize IPsec work structs"
from Mar 30, 2023, leads to the following Smatch complaint:

    drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:755 mlx5e_xfrm_free_state()
    error: we previously assumed 'sa_entry->work' could be null (see line 746)

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
   745	
   746		if (sa_entry->work)
                    ^^^^^^^^^^^^^^

   747			cancel_work_sync(&sa_entry->work->work);
   748	
   749		if (sa_entry->dwork)
                    ^^^^^^^^^^^^^^^
These checks can be deleted, right?

   750			cancel_delayed_work_sync(&sa_entry->dwork->dwork);
   751	
   752		mlx5e_accel_ipsec_fs_del_rule(sa_entry);
   753		mlx5_ipsec_free_sa_ctx(sa_entry);
   754		kfree(sa_entry->dwork);
   755		kfree(sa_entry->work->data);
                      ^^^^^^^^^^^^^^^^^^^^
Unchecked dereference.

   756		kfree(sa_entry->work);
   757	sa_entry_free:

regards,
dan carpenter

Re: [bug report] net/mlx5e: Generalize IPsec work structs

[Leon Romanovsky]

On Thu, Apr 20, 2023 at 11:24:09AM +0300, Dan Carpenter wrote:
> Hello Leon Romanovsky,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 4562116f8a56: "net/mlx5e: Generalize IPsec work structs"
> from Mar 30, 2023, leads to the following Smatch complaint:
> 
>     drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:755 mlx5e_xfrm_free_state()
>     error: we previously assumed 'sa_entry->work' could be null (see line 746)

Thanks for the report,
I fixed it in commit 94edec448479 ("net/mlx5e: Properly release work data structure")

> 
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>    745	
>    746		if (sa_entry->work)
>                     ^^^^^^^^^^^^^^
> 
>    747			cancel_work_sync(&sa_entry->work->work);
>    748	
>    749		if (sa_entry->dwork)
>                     ^^^^^^^^^^^^^^^
> These checks can be deleted, right?
> 
>    750			cancel_delayed_work_sync(&sa_entry->dwork->dwork);
>    751	
>    752		mlx5e_accel_ipsec_fs_del_rule(sa_entry);
>    753		mlx5_ipsec_free_sa_ctx(sa_entry);
>    754		kfree(sa_entry->dwork);
>    755		kfree(sa_entry->work->data);
>                       ^^^^^^^^^^^^^^^^^^^^
> Unchecked dereference.
> 
>    756		kfree(sa_entry->work);
>    757	sa_entry_free:
> 
> regards,
> dan carpenter