Re: [PATCH] HID: logitech-hidpp: Fix FF device cleanup on init failure

On Tue, 2026-06-23 at 14:52 +0800, Haoxiang Li wrote:
> hidpp_ff_init() creates the input force-feedback device with
> input_ff_create(), then allocates the HID++ FF private data,
> effect ID array, and workqueue.
> 
> If any of those allocations fail after input_ff_create() succeeds,
> the function returns an error without destroying the FF device.
> Add an unwind path that frees the private allocations made by
> hidpp_ff_init() and calls input_ff_destroy() for failures after
> input_ff_create() succeeds.
> 
> Fixes: ff21a635dd1a ("HID: logitech-hidpp: Force feedback support for
> the Logitech G920")
> Signed-off-by: Haoxiang Li <[email protected]>

Looks correct. I wish there was an easier way to handle this.

It looks like hid-bigbenff.c doesn't handle post-input_ff_create*
failure properly either.

Reviewed-by: Bastien Nocera <[email protected]>

> ---
>  drivers/hid/hid-logitech-hidpp.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 90b0184df777..fb2062233df2 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -2861,18 +2861,19 @@ static int hidpp_ff_init(struct hidpp_device
> *hidpp,
>  	 * ownership to FF core
>  	 */
>  	data = kmemdup(data, sizeof(*data), GFP_KERNEL);
> -	if (!data)
> -		return -ENOMEM;
> +	if (!data) {
> +		error = -ENOMEM;
> +		goto err_destroy_ff;
> +	}
>  	data->effect_ids = kzalloc_objs(int, num_slots);
>  	if (!data->effect_ids) {
> -		kfree(data);
> -		return -ENOMEM;
> +		error = -ENOMEM;
> +		goto err_free_data;
>  	}
>  	data->wq = create_singlethread_workqueue("hidpp-ff-
> sendqueue");
>  	if (!data->wq) {
> -		kfree(data->effect_ids);
> -		kfree(data);
> -		return -ENOMEM;
> +		error = -ENOMEM;
> +		goto err_free_effect_ids;
>  	}
>  
>  	data->hidpp = hidpp;
> @@ -2902,6 +2903,14 @@ static int hidpp_ff_init(struct hidpp_device
> *hidpp,
>  		 version);
>  
>  	return 0;
> +
> +err_free_effect_ids:
> +	kfree(data->effect_ids);
> +err_free_data:
> +	kfree(data);
> +err_destroy_ff:
> +	input_ff_destroy(dev);
> +	return error;
>  }
>  
>  /*
> *********************************************************************
> ***** */