Re: [PATCH 3/7] net/ice: fix DDP filename leak

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Oct 27, 2026 at 10:51:33AM +0000, Anurag Mandal wrote:
> The DDP package filename given with the ddp_pkg_file devarg is
> duplicated with strdup() during devargs parsing, but only freed on
> close. It leaks when ice_dev_init() fails at or after devargs
> parsing, and when the devarg is given more than once.
> 
> Free the previous string before storing a new one, and free it on all
> init error paths.
> 
> Fixes: 9dd6dd1db834 ("net/ice: add option to choose DDP package file")
> Fixes: 603beeb970b5 ("net/ice: add safe mode devarg")
> Fixes: f9cf4f864150 ("net/ice: support device initialization")

Cc: stable@dpdk.org

> 
> Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
> ---
>  drivers/net/intel/ice/ice_ethdev.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 27f992aefa..ec4d021ec6 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -805,6 +805,7 @@ handle_ddp_filename_arg(__rte_unused const char *key, const char *value, void *n
>  		PMD_DRV_LOG(ERR, "The DDP package filename is too long : '%s'", value);
>  		return -1;
>  	}
> +	free((void *)(uintptr_t)*filename);

You can avoid the casting by changing the type of filename to char * in
this function only.

>  	*filename = strdup(value);
>  	return 0;
>  }
> @@ -2738,7 +2739,8 @@ ice_dev_init(struct rte_eth_dev *dev)
>  	ret = ice_parse_devargs(dev);
>  	if (ret) {
>  		PMD_INIT_LOG(ERR, "Failed to parse devargs");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err_free_devargs;
>  	}
>  
>  	ice_init_controlq_parameter(hw);
> @@ -2746,7 +2748,8 @@ ice_dev_init(struct rte_eth_dev *dev)
>  	ret = ice_init_hw(hw);
>  	if (ret) {
>  		PMD_INIT_LOG(ERR, "Failed to initialize HW");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err_free_devargs;
>  	}
>  
>  	/*
> @@ -2761,7 +2764,8 @@ ice_dev_init(struct rte_eth_dev *dev)
>  		PMD_INIT_LOG(ERR, "Invalid rl_burst_size %u bytes",
>  			     ad->devargs.rl_burst_size);
>  		ice_deinit_hw(hw);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err_free_devargs;
>  	}
>  
>  #ifndef RTE_EXEC_ENV_WINDOWS
> @@ -2912,6 +2916,9 @@ ice_dev_init(struct rte_eth_dev *dev)
>  err_init_fw:
>  #endif
>  	ice_deinit_hw(hw);
> +err_free_devargs:
> +	free((void *)(uintptr_t)ad->devargs.ddp_filename);

I think this would be better using RTE_CAST_PTR, rather than using explicit
casts.

> +	ad->devargs.ddp_filename = NULL;
>  
>  	return ret;
>  }
> -- 
> 2.25.1
>