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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <61e3cc7a-af79-48e4-acb6-8ac7c8d2552c@intel.com>
Date: Thu, 6 Feb 2025 13:35:19 -0800
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Bart Van Assche <bvanassche@....org>, Peter Zijlstra
	<peterz@...radead.org>, "Dave.Ertman@...el.com" <Dave.Ertman@...el.com>
CC: Will Deacon <will@...nel.org>, Christoph Hellwig <hch@....de>, "Greg
 Kroah-Hartman" <gregkh@...uxfoundation.org>, Marco Elver <elver@...gle.com>,
	Nick Desaulniers <ndesaulniers@...gle.com>, Nathan Chancellor
	<nathan@...nel.org>, Kees Cook <kees@...nel.org>, Jann Horn
	<jannh@...gle.com>, <linux-kernel@...r.kernel.org>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>
Subject: Re: [PATCH RFC 15/33] ice: Fix a locking bug in an error path



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

> Cc: Tony Nguyen <anthony.l.nguyen@...el.com>
> Cc: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Fixes: 242b5e068b25 ("ice: Fix DCB rebuild after reset")
> Signed-off-by: Bart Van Assche <bvanassche@....org>
> ---
>   drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> index 69a4b84f935f..65545613e1e8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> @@ -563,7 +563,8 @@ void ice_dcb_rebuild(struct ice_pf *pf)
>   	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;
> +		disable_dcb(pf);
> +		return;
>   	}
>   
>   	mutex_lock(&pf->tc_mutex);