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: [email protected]
>
> Signed-off-by: Anurag Mandal <[email protected]>
> ---
> 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
>