[PATCH] HID: wacom: fix memory leak on size mismatch in wacom_wac_queue_flush()

[PATCH] HID: wacom: fix memory leak on size mismatch in wacom_wac_queue_flush()

[Qasim Ijaz]

In wacom_wac_queue_flush() the code allocates zero initialised 
buffer which it uses as a storage buffer for copying data from 
a fifo via kfifo_out(). The kfifo_out() function returns the 
number of elements it has copied. The code checks if the number 
of copied elements does not equal the size of the fifo record, 
if it does not it simply skips the entry and continues to the
next iteration. However it does not release the storage buffer 
leading to a memory leak.
 
Fix the memory leak by freeing the buffer on size mismatch.

Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
 drivers/hid/wacom_sys.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 666b7eb0fdfe..3e4f823f8ee8 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -87,6 +87,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
 			// to flush seems reasonable enough, however.
 			hid_warn(hdev, "%s: removed fifo entry with unexpected size\n",
 				 __func__);
+			kfree(buf);
 			continue;
 		}
 		err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
-- 
2.39.5

Re: [PATCH] HID: wacom: fix memory leak on size mismatch in wacom_wac_queue_flush()

[Markus Elfring]

…
> number of elements it has copied. The code checks if the number
> of copied elements does not equal the size of the fifo record,
…
                     is?

Under which circumstances would you dare to use text lines
which would be longer than 63 characters?


Can a patch series be more appropriate for the affected software module?

Regards,
Markus

Re: [PATCH] HID: wacom: fix memory leak on size mismatch in wacom_wac_queue_flush()

[Jason Gerecke]

On Fri, Mar 28, 2025 at 5:20 PM Qasim Ijaz <qasdev00@gmail.com> wrote:
>
> In wacom_wac_queue_flush() the code allocates zero initialised
> buffer which it uses as a storage buffer for copying data from
> a fifo via kfifo_out(). The kfifo_out() function returns the
> number of elements it has copied. The code checks if the number
> of copied elements does not equal the size of the fifo record,
> if it does not it simply skips the entry and continues to the
> next iteration. However it does not release the storage buffer
> leading to a memory leak.
>
> Fix the memory leak by freeing the buffer on size mismatch.
>
> Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> ---
>  drivers/hid/wacom_sys.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 666b7eb0fdfe..3e4f823f8ee8 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -87,6 +87,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
>                         // to flush seems reasonable enough, however.
>                         hid_warn(hdev, "%s: removed fifo entry with unexpected size\n",
>                                  __func__);
> +                       kfree(buf);
>                         continue;
>                 }
>                 err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
> --
> 2.39.5
>

Patch looks good to me. With or without Markus' comments addressed,

Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>

Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....