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

[Bart Van Assche]

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?

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);