linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HID: axff: add cleanup allocated struct axff_device heap
@ 2025-08-18 15:43 Jeongjun Park
  2025-08-18 17:20 ` Dmitry Torokhov
  0 siblings, 1 reply; 3+ messages in thread
From: Jeongjun Park @ 2025-08-18 15:43 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: dtor, x0r, linux-input, linux-kernel, Jeongjun Park

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)
--

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: axff: add cleanup allocated struct axff_device heap
  2025-08-18 15:43 [PATCH] HID: axff: add cleanup allocated struct axff_device heap Jeongjun Park
@ 2025-08-18 17:20 ` Dmitry Torokhov
  2025-08-20  4:43   ` Jeongjun Park
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Torokhov @ 2025-08-18 17:20 UTC (permalink / raw)
  To: Jeongjun Park; +Cc: jikos, bentiss, x0r, linux-input, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: axff: add cleanup allocated struct axff_device heap
  2025-08-18 17:20 ` Dmitry Torokhov
@ 2025-08-20  4:43   ` Jeongjun Park
  0 siblings, 0 replies; 3+ messages in thread
From: Jeongjun Park @ 2025-08-20  4:43 UTC (permalink / raw)
  To: dtor; +Cc: aha310510, bentiss, jikos, linux-input, linux-kernel, x0r

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-08-20  4:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-18 15:43 [PATCH] HID: axff: add cleanup allocated struct axff_device heap Jeongjun Park
2025-08-18 17:20 ` Dmitry Torokhov
2025-08-20  4:43   ` Jeongjun Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).