[bug report] i40e: Fix race condition while adding/deleting MAC/VLAN filters

[bug report] i40e: Fix race condition while adding/deleting MAC/VLAN filters

[Dan Carpenter]

Hello Jedrzej Jagielski,

The patch 53a9e346e159: "i40e: Fix race condition while
adding/deleting MAC/VLAN filters" from Jan 14, 2022, leads to the
following Smatch static checker warning:

	drivers/net/ethernet/intel/i40e/i40e_main.c:2153 i40e_aqc_del_filters()
	error: uninitialized symbol 'aq_status'.

drivers/net/ethernet/intel/i40e/i40e_main.c
    2140 static
    2141 void i40e_aqc_del_filters(struct i40e_vsi *vsi, const char *vsi_name,
    2142                           struct i40e_aqc_remove_macvlan_element_data *list,
    2143                           int num_del, int *retval)
    2144 {
    2145         struct i40e_hw *hw = &vsi->back->hw;
    2146         enum i40e_admin_queue_err aq_status;
    2147         i40e_status aq_ret;
    2148 
    2149         aq_ret = i40e_aq_remove_macvlan_v2(hw, vsi->seid, list, num_del, NULL,
    2150                                            &aq_status);
    2151 
    2152         /* Explicitly ignore and do not report when firmware returns ENOENT */
--> 2153         if (aq_ret && !(aq_status == I40E_AQ_RC_ENOENT)) {

The error pathes doesn't set aq_status.

    2154                 *retval = -EIO;
    2155                 dev_info(&vsi->back->pdev->dev,
    2156                          "ignoring delete macvlan error on %s, err %s, aq_err %s\n",
    2157                          vsi_name, i40e_stat_str(hw, aq_ret),
    2158                          i40e_aq_str(hw, aq_status));
    2159         }
    2160 }

regards,
dan carpenter

Re: [bug report] i40e: Fix race condition while adding/deleting MAC/VLAN filters

[Dan Carpenter]

On Tue, Feb 08, 2022 at 10:20:04AM +0300, Dan Carpenter wrote:
> Hello Jedrzej Jagielski,
> 
> The patch 53a9e346e159: "i40e: Fix race condition while
> adding/deleting MAC/VLAN filters" from Jan 14, 2022, leads to the
> following Smatch static checker warning:
> 
> 	drivers/net/ethernet/intel/i40e/i40e_main.c:2153 i40e_aqc_del_filters()
> 	error: uninitialized symbol 'aq_status'.
> 
> drivers/net/ethernet/intel/i40e/i40e_main.c
>     2140 static
>     2141 void i40e_aqc_del_filters(struct i40e_vsi *vsi, const char *vsi_name,
>     2142                           struct i40e_aqc_remove_macvlan_element_data *list,
>     2143                           int num_del, int *retval)
>     2144 {
>     2145         struct i40e_hw *hw = &vsi->back->hw;
>     2146         enum i40e_admin_queue_err aq_status;
>     2147         i40e_status aq_ret;
>     2148 
>     2149         aq_ret = i40e_aq_remove_macvlan_v2(hw, vsi->seid, list, num_del, NULL,
>     2150                                            &aq_status);
>     2151 
>     2152         /* Explicitly ignore and do not report when firmware returns ENOENT */
> --> 2153         if (aq_ret && !(aq_status == I40E_AQ_RC_ENOENT)) {
> 
> The error pathes doesn't set aq_status.

In my head, I wrote "The first error path doesn't set aq_status".

regards,
dan carpenter

RE: [bug report] i40e: Fix race condition while adding/deleting MAC/VLAN filters

[Jagielski, Jedrzej]

Hello Dan,

>> The patch 53a9e346e159: "i40e: Fix race condition while 
>> adding/deleting MAC/VLAN filters" from Jan 14, 2022, leads to the 
>> following Smatch static checker warning:
>> 
>> 	drivers/net/ethernet/intel/i40e/i40e_main.c:2153 i40e_aqc_del_filters()
>> 	error: uninitialized symbol 'aq_status'.
>> 
>> drivers/net/ethernet/intel/i40e/i40e_main.c
>>     2140 static
>>     2141 void i40e_aqc_del_filters(struct i40e_vsi *vsi, const char *vsi_name,
>>     2142                           struct i40e_aqc_remove_macvlan_element_data *list,
>>     2143                           int num_del, int *retval)
>>     2144 {
>>     2145         struct i40e_hw *hw = &vsi->back->hw;
>>     2146         enum i40e_admin_queue_err aq_status;
>>     2147         i40e_status aq_ret;
>>     2148 
>>     2149         aq_ret = i40e_aq_remove_macvlan_v2(hw, vsi->seid, list, num_del, NULL,
>>     2150                                            &aq_status);
>>     2151 
>>     2152         /* Explicitly ignore and do not report when firmware returns ENOENT */
>> --> 2153         if (aq_ret && !(aq_status == I40E_AQ_RC_ENOENT)) {
>> 
>> The error pathes doesn't set aq_status.
>
>In my head, I wrote "The first error path doesn't set aq_status".

Thanks for that information. It is going to be investigated.

Regards,
Jedrzej