Re: [PATCH RFC 15/33] ice: Fix a locking bug in an error path

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 2/6/2025 1:44 PM, Bart Van Assche wrote:
> On 2/6/25 1:35 PM, Tony Nguyen wrote:
>> On 2/6/2025 9:50 AM, Bart Van Assche wrote:
>>> Do not unlock pf->tc_mutex if it has not been locked. Jumping to
>>> 'dcb_error' causes pf->tc_mutex to be unlocked. This bug has been 
>>> detected
>>> by the Clang thread-safety analyzer.
>>
>> Thanks for catching this Bart. I think it would be better to move the 
>> tc_mutex up to cover this call. Though unlikely, the DCB settings 
>> could change after this call so it would be better to protect this 
>> under the mutex. Also, as the error path is changing configuration, 
>> that should be done under this mutex as well.
> 
> Thanks Tony for having taken a look. Is this perhaps the change that you
> want me to make?

Yes, looks good to me.

Thanks,
Tony

> Thanks,
> 
> Bart.
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ 
> ethernet/intel/ice/ice_dcb_lib.c
> index a7c510832824..d185b1aba7a4 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> @@ -537,14 +537,14 @@ void ice_dcb_rebuild(struct ice_pf *pf)
>       struct ice_dcbx_cfg *err_cfg;
>       int ret;
> 
> +    mutex_lock(&pf->tc_mutex);
> +
>       ret = ice_query_port_ets(pf->hw.port_info, &buf, sizeof(buf), NULL);
>       if (ret) {
>           dev_err(dev, "Query Port ETS failed\n");
>           goto dcb_error;
>       }
> 
> -    mutex_lock(&pf->tc_mutex);
> -
>       if (!pf->hw.port_info->qos_cfg.is_sw_lldp)
>           ice_cfg_etsrec_defaults(pf->hw.port_info);
> 
>