[PATCH] HID: axff: add cleanup allocated struct axff_device heap

Jeongjun Park posted 1 patch 1 month, 2 weeks ago
drivers/hid/hid-axff.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
[PATCH] HID: axff: add cleanup allocated struct axff_device heap
Posted by Jeongjun Park 1 month, 2 weeks ago
Currently, acrux hid driver allocates heap memory equal to
sizeof(struct axff_device) to support force feedback, but there's no code
to free this memory except when initialization fails. This causes the
allocated memory to not be freed even if the driver is detached.

Therefore, to properly clean up and safely manage the allocated heap,
must be modified to use devm_kzalloc().

Fixes: c0dbcc33c652 ("HID: add ACRUX game controller force feedback support")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 drivers/hid/hid-axff.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-axff.c b/drivers/hid/hid-axff.c
index fbe4e16ab029..b8202737f4c8 100644
--- a/drivers/hid/hid-axff.c
+++ b/drivers/hid/hid-axff.c
@@ -96,7 +96,7 @@ static int axff_init(struct hid_device *hid)
 		return -ENODEV;
 	}
 
-	axff = kzalloc(sizeof(struct axff_device), GFP_KERNEL);
+	axff = devm_kzalloc(&hid->dev, sizeof(struct axff_device), GFP_KERNEL);
 	if (!axff)
 		return -ENOMEM;
 
@@ -104,7 +104,7 @@ static int axff_init(struct hid_device *hid)
 
 	error = input_ff_create_memless(dev, axff, axff_play);
 	if (error)
-		goto err_free_mem;
+		return error;
 
 	axff->report = report;
 	hid_hw_request(hid, axff->report, HID_REQ_SET_REPORT);
@@ -112,10 +112,6 @@ static int axff_init(struct hid_device *hid)
 	hid_info(hid, "Force Feedback for ACRUX game controllers by Sergei Kolzun <x0r@dv-life.ru>\n");
 
 	return 0;
-
-err_free_mem:
-	kfree(axff);
-	return error;
 }
 #else
 static inline int axff_init(struct hid_device *hid)
--
Re: [PATCH] HID: axff: add cleanup allocated struct axff_device heap
Posted by Dmitry Torokhov 1 month, 2 weeks ago
Hi Jeongjun,

On Tue, Aug 19, 2025 at 12:43:02AM +0900, Jeongjun Park wrote:
> Currently, acrux hid driver allocates heap memory equal to
> sizeof(struct axff_device) to support force feedback, but there's no code
> to free this memory except when initialization fails. This causes the
> allocated memory to not be freed even if the driver is detached.
> 
> Therefore, to properly clean up and safely manage the allocated heap,
> must be modified to use devm_kzalloc().

You have not tested this, have you? The private data that is passed to
input_ff_create_memless() is freed by ml_ff_destroy() which is invoked
when input core calls input_ff_destroy() as part of input device
teardown. Your change introduces double-free. 

> 
> Fixes: c0dbcc33c652 ("HID: add ACRUX game controller force feedback support")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  drivers/hid/hid-axff.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-axff.c b/drivers/hid/hid-axff.c
> index fbe4e16ab029..b8202737f4c8 100644
> --- a/drivers/hid/hid-axff.c
> +++ b/drivers/hid/hid-axff.c
> @@ -96,7 +96,7 @@ static int axff_init(struct hid_device *hid)
>  		return -ENODEV;
>  	}
>  
> -	axff = kzalloc(sizeof(struct axff_device), GFP_KERNEL);
> +	axff = devm_kzalloc(&hid->dev, sizeof(struct axff_device), GFP_KERNEL);
>  	if (!axff)
>  		return -ENOMEM;
>  
> @@ -104,7 +104,7 @@ static int axff_init(struct hid_device *hid)
>  
>  	error = input_ff_create_memless(dev, axff, axff_play);
>  	if (error)
> -		goto err_free_mem;
> +		return error;
>  
>  	axff->report = report;
>  	hid_hw_request(hid, axff->report, HID_REQ_SET_REPORT);
> @@ -112,10 +112,6 @@ static int axff_init(struct hid_device *hid)
>  	hid_info(hid, "Force Feedback for ACRUX game controllers by Sergei Kolzun <x0r@dv-life.ru>\n");
>  
>  	return 0;
> -
> -err_free_mem:
> -	kfree(axff);
> -	return error;
>  }
>  #else
>  static inline int axff_init(struct hid_device *hid)
> --
> 

Thanks.

-- 
Dmitry
Re: [PATCH] HID: axff: add cleanup allocated struct axff_device heap
Posted by Jeongjun Park 1 month, 2 weeks ago
Hello Dmitry,

Dmitry Torokhov wrote:
> Hi Jeongjun,
> 
> On Tue, Aug 19, 2025 at 12:43:02AM +0900, Jeongjun Park wrote:
> > Currently, acrux hid driver allocates heap memory equal to
> > sizeof(struct axff_device) to support force feedback, but there's no code
> > to free this memory except when initialization fails. This causes the
> > allocated memory to not be freed even if the driver is detached.
> > 
> > Therefore, to properly clean up and safely manage the allocated heap,
> > must be modified to use devm_kzalloc().
> 
> You have not tested this, have you? The private data that is passed to
> input_ff_create_memless() is freed by ml_ff_destroy() which is invoked
> when input core calls input_ff_destroy() as part of input device
> teardown. Your change introduces double-free. 

Oops, I checked again and realized my patch was a complete mess.

Thanks for letting me know!

Regards,
Jeongjun Park