[PATCH net] ice: dpll: fix memory leak in ice_dpll_init_info error paths

[PATCH net] ice: dpll: fix memory leak in ice_dpll_init_info error paths

[ZhaoJinming]

Several error return paths in ice_dpll_init_info() directly return
without freeing previously allocated resources, causing memory leaks:

- When de->input_prio allocation fails, d->inputs is leaked
- When dp->input_prio allocation fails, d->inputs and de->input_prio
  are leaked
- When ice_get_cgu_rclk_pin_info() fails, all previously allocated
  inputs/outputs/input_prio are leaked
- When ice_dpll_init_pins_info(RCLK_INPUT) fails, same resources
  are leaked

Fix this by jumping to the deinit_info label which properly calls
ice_dpll_deinit_info() to free all allocated resources.

Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/net/ethernet/intel/ice/ice_dpll.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index 892bc7c2e28b..3cf098e67563 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -4341,12 +4341,16 @@ static int ice_dpll_init_info(struct ice_pf *pf, bool cgu)
 
 	alloc_size = sizeof(*de->input_prio) * d->num_inputs;
 	de->input_prio = kzalloc(alloc_size, GFP_KERNEL);
-	if (!de->input_prio)
-		return -ENOMEM;
+	if (!de->input_prio) {
+		ret = -ENOMEM;
+		goto deinit_info;
+	}
 
 	dp->input_prio = kzalloc(alloc_size, GFP_KERNEL);
-	if (!dp->input_prio)
-		return -ENOMEM;
+	if (!dp->input_prio) {
+		ret = -ENOMEM;
+		goto deinit_info;
+	}
 
 	ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_INPUT);
 	if (ret)
@@ -4371,12 +4375,12 @@ static int ice_dpll_init_info(struct ice_pf *pf, bool cgu)
 	ret = ice_get_cgu_rclk_pin_info(&pf->hw, &d->base_rclk_idx,
 					&pf->dplls.rclk.num_parents);
 	if (ret)
-		return ret;
+		goto deinit_info;
 	for (i = 0; i < pf->dplls.rclk.num_parents; i++)
 		pf->dplls.rclk.parent_idx[i] = d->base_rclk_idx + i;
 	ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_RCLK_INPUT);
 	if (ret)
-		return ret;
+		goto deinit_info;
 	de->mode = DPLL_MODE_AUTOMATIC;
 	dp->mode = DPLL_MODE_AUTOMATIC;
 
-- 
2.20.1

RE: [Intel-wired-lan] [PATCH net] ice: dpll: fix memory leak in ice_dpll_init_info error paths

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of ZhaoJinming
> Sent: Tuesday, May 26, 2026 4:12 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S . Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; ZhaoJinming <zhaojinming@uniontech.com>
> Subject: [Intel-wired-lan] [PATCH net] ice: dpll: fix memory leak in
> ice_dpll_init_info error paths
> 
> Several error return paths in ice_dpll_init_info() directly return
> without freeing previously allocated resources, causing memory leaks:
> 
> - When de->input_prio allocation fails, d->inputs is leaked
> - When dp->input_prio allocation fails, d->inputs and de->input_prio
>   are leaked
> - When ice_get_cgu_rclk_pin_info() fails, all previously allocated
>   inputs/outputs/input_prio are leaked
> - When ice_dpll_init_pins_info(RCLK_INPUT) fails, same resources
>   are leaked
> 
> Fix this by jumping to the deinit_info label which properly calls
> ice_dpll_deinit_info() to free all allocated resources.
> 
> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_dpll.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
> b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index 892bc7c2e28b..3cf098e67563 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> @@ -4341,12 +4341,16 @@ static int ice_dpll_init_info(struct ice_pf
> *pf, bool cgu)
> 
>  	alloc_size = sizeof(*de->input_prio) * d->num_inputs;
>  	de->input_prio = kzalloc(alloc_size, GFP_KERNEL);
> -	if (!de->input_prio)
> -		return -ENOMEM;
> +	if (!de->input_prio) {
> +		ret = -ENOMEM;
> +		goto deinit_info;
> +	}
> 
>  	dp->input_prio = kzalloc(alloc_size, GFP_KERNEL);
> -	if (!dp->input_prio)
> -		return -ENOMEM;
> +	if (!dp->input_prio) {
> +		ret = -ENOMEM;
> +		goto deinit_info;
> +	}
> 
>  	ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_INPUT);
>  	if (ret)
> @@ -4371,12 +4375,12 @@ static int ice_dpll_init_info(struct ice_pf
> *pf, bool cgu)
>  	ret = ice_get_cgu_rclk_pin_info(&pf->hw, &d->base_rclk_idx,
>  					&pf->dplls.rclk.num_parents);
>  	if (ret)
> -		return ret;
> +		goto deinit_info;
>  	for (i = 0; i < pf->dplls.rclk.num_parents; i++)
>  		pf->dplls.rclk.parent_idx[i] = d->base_rclk_idx + i;
>  	ret = ice_dpll_init_pins_info(pf,
> ICE_DPLL_PIN_TYPE_RCLK_INPUT);
>  	if (ret)
> -		return ret;
> +		goto deinit_info;
>  	de->mode = DPLL_MODE_AUTOMATIC;
>  	dp->mode = DPLL_MODE_AUTOMATIC;
> 
> --
> 2.20.1


Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Re: [PATCH net] ice: dpll: fix memory leak in ice_dpll_init_info error paths

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ice: dpll: fix memory leak in ice_dpll_init_info error paths

This commit fixes memory leaks in several error return paths of
ice_dpll_init_info() by routing them to the deinit_info label. This
ensures previously allocated resources are correctly freed.

> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
[ ... ]
> @@ -4341,12 +4341,16 @@ static int ice_dpll_init_info(struct ice_pf *pf, bool cgu)
>  
>  	alloc_size = sizeof(*de->input_prio) * d->num_inputs;
>  	de->input_prio = kzalloc(alloc_size, GFP_KERNEL);
> -	if (!de->input_prio)
> -		return -ENOMEM;
> +	if (!de->input_prio) {
> +		ret = -ENOMEM;
> +		goto deinit_info;
> +	}

Simon says: Although this is a pre-existing issue I believe it needs
            to be assessed in the context of this patch.

	    If it is valid then I would suggest a separate patch,
	    preceding this one in a patch-set, to address it.

[Severity: Critical]
This is a pre-existing issue, but does jumping to deinit_info here expose a
double-free risk during driver resets?

When a driver reset occurs, ice_dpll_deinit_info() frees the pointers in
pf->dplls (like d->inputs, d->outputs, de->input_prio, and dp->input_prio)
but does not set them to NULL.

During the subsequent reset recovery, ice_dpll_init_info() is called. If
allocation fails here at de->input_prio, the code jumps to deinit_info
and calls ice_dpll_deinit_info() again.

Because d->outputs and dp->input_prio have not yet been overwritten with
new allocations during this init sequence, they still hold the old dangling
pointers from before the reset. Will calling ice_dpll_deinit_info() again
attempt to kfree() these old pointers, resulting in a double-free?

[ ... ]