public inbox for linux-input@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid
@ 2026-02-19 15:43 Günther Noack
  2026-02-19 15:43 ` [PATCH v2 1/4] HID: Document memory allocation properties of report_fixup() Günther Noack
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Günther Noack @ 2026-02-19 15:43 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Günther Noack

Hello!

These patches fix a few memory leaks in HID report descriptor fixups.

FWIW, a good ad-hoc way to look for usages of allocation functions in
these is:

  awk '/static.*report_fixup.*/,/^}/ { print FILENAME, $0 }' drivers/hid/hid-*.c \
    | grep -E '(malloc|kzalloc|kcalloc|kmemdup)'

The devm_* variants are safe in this context, because they tie the
allocated memory to the lifetime of the driver.

For transparency, I generated these commits with Gemini-CLI,
starting with this prompt:

    We are working in the Linux kernel. In the HID drivers in
    `drivers/hid/hid-*.c`, the `report_fixup` driver hook is a function
    that gets a byte buffer (with size) as input and that may modify that
    byte buffer, and optionally return a pointer to a new byte buffer and
    update the size.  The returned value is *not* memory-managed by the
    caller though and will not be freed subsequently.  When the
    `report_fixup` implementation allocates a new buffer and returns that,
    that will not get freed by the caller.  Validate this assessment and
    fix up all HID drivers where that mistake is made.

(and then a little bit of additional nudging for the details).

—Günther

---

Revision history:

V2:

  * Add a commit documenting the memory allocation properties of report_fixup().
  * Reword the commit message for the three memory leak fixes.

V1: Original patch set

https://lore.kernel.org/all/20260217160125.1097578-1-gnoack@google.com/


Günther Noack (4):
  HID: Document memory allocation properties of report_fixup()
  HID: apple: avoid memory leak in apple_report_fixup()
  HID: magicmouse: avoid memory leak in magicmouse_report_fixup()
  HID: asus: avoid memory leak in asus_report_fixup()

 drivers/hid/hid-apple.c      |  4 +---
 drivers/hid/hid-asus.c       | 15 +++++++++++----
 drivers/hid/hid-magicmouse.c |  4 +---
 include/linux/hid.h          |  6 ++++++
 4 files changed, 19 insertions(+), 10 deletions(-)

-- 
2.53.0.371.g1d285c8824-goog


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

* [PATCH v2 1/4] HID: Document memory allocation properties of report_fixup()
  2026-02-19 15:43 [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Günther Noack
@ 2026-02-19 15:43 ` Günther Noack
  2026-02-19 15:43 ` [PATCH v2 2/4] HID: apple: avoid memory leak in apple_report_fixup() Günther Noack
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Günther Noack @ 2026-02-19 15:43 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Günther Noack

The memory pointer returned by the report_fixup() hook does not get
freed by the caller.  Instead, report_fixup() must return (in return
value and *rsize) a memory buffer with at least the same lifetime as
the input buffer (defined by rdesc and original *rsize).

This is usually achieved using one of the following techniques:

* Returning a pointer and size to a sub-portion of the input buffer
* Returning a pointer to a static buffer
* Allocating a buffer with a devm_*() function,
  which will automatically get freed when the device is removed.

Signed-off-by: Günther Noack <gnoack@google.com>
---
 include/linux/hid.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index dce862cafbbd..2990b9f94cb5 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -836,6 +836,12 @@ struct hid_usage_id {
  * raw_event and event should return negative on error, any other value will
  * pass the event on to .event() typically return 0 for success.
  *
+ * report_fixup must return a report descriptor pointer whose lifetime is at
+ * least that of the input rdesc.  This is usually done by mutating the input
+ * rdesc and returning it or a sub-portion of it.  In case a new buffer is
+ * allocated and returned, the implementation of report_fixup is responsible for
+ * freeing it later.
+ *
  * input_mapping shall return a negative value to completely ignore this usage
  * (e.g. doubled or invalid usage), zero to continue with parsing of this
  * usage by generic code (no special handling needed) or positive to skip
-- 
2.53.0.371.g1d285c8824-goog


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

* [PATCH v2 2/4] HID: apple: avoid memory leak in apple_report_fixup()
  2026-02-19 15:43 [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Günther Noack
  2026-02-19 15:43 ` [PATCH v2 1/4] HID: Document memory allocation properties of report_fixup() Günther Noack
@ 2026-02-19 15:43 ` Günther Noack
  2026-02-19 15:43 ` [PATCH v2 3/4] HID: magicmouse: avoid memory leak in magicmouse_report_fixup() Günther Noack
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Günther Noack @ 2026-02-19 15:43 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Günther Noack

The apple_report_fixup() function was returning a
newly kmemdup()-allocated buffer, but never freeing it.

The caller of report_fixup() does not take ownership of the returned
pointer, but it *is* permitted to return a sub-portion of the input
rdesc, whose lifetime is managed by the caller.

Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Günther Noack <gnoack@google.com>
---
 drivers/hid/hid-apple.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 233e367cce1d..894adc23367b 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -686,9 +686,7 @@ static const __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		hid_info(hdev,
 			 "fixing up Magic Keyboard battery report descriptor\n");
 		*rsize = *rsize - 1;
-		rdesc = kmemdup(rdesc + 1, *rsize, GFP_KERNEL);
-		if (!rdesc)
-			return NULL;
+		rdesc = rdesc + 1;
 
 		rdesc[0] = 0x05;
 		rdesc[1] = 0x01;
-- 
2.53.0.371.g1d285c8824-goog


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

* [PATCH v2 3/4] HID: magicmouse: avoid memory leak in magicmouse_report_fixup()
  2026-02-19 15:43 [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Günther Noack
  2026-02-19 15:43 ` [PATCH v2 1/4] HID: Document memory allocation properties of report_fixup() Günther Noack
  2026-02-19 15:43 ` [PATCH v2 2/4] HID: apple: avoid memory leak in apple_report_fixup() Günther Noack
@ 2026-02-19 15:43 ` Günther Noack
  2026-02-19 15:43 ` [PATCH v2 4/4] HID: asus: avoid memory leak in asus_report_fixup() Günther Noack
  2026-02-19 18:17 ` [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Benjamin Tissoires
  4 siblings, 0 replies; 6+ messages in thread
From: Günther Noack @ 2026-02-19 15:43 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Günther Noack

The magicmouse_report_fixup() function was returning a
newly kmemdup()-allocated buffer, but never freeing it.

The caller of report_fixup() does not take ownership of the returned
pointer, but it *is* permitted to return a sub-portion of the input
rdesc, whose lifetime is managed by the caller.

Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Günther Noack <gnoack@google.com>
---
 drivers/hid/hid-magicmouse.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 91f621ceb924..17908d52c027 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -994,9 +994,7 @@ static const __u8 *magicmouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		hid_info(hdev,
 			 "fixing up magicmouse battery report descriptor\n");
 		*rsize = *rsize - 1;
-		rdesc = kmemdup(rdesc + 1, *rsize, GFP_KERNEL);
-		if (!rdesc)
-			return NULL;
+		rdesc = rdesc + 1;
 
 		rdesc[0] = 0x05;
 		rdesc[1] = 0x01;
-- 
2.53.0.371.g1d285c8824-goog


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

* [PATCH v2 4/4] HID: asus: avoid memory leak in asus_report_fixup()
  2026-02-19 15:43 [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Günther Noack
                   ` (2 preceding siblings ...)
  2026-02-19 15:43 ` [PATCH v2 3/4] HID: magicmouse: avoid memory leak in magicmouse_report_fixup() Günther Noack
@ 2026-02-19 15:43 ` Günther Noack
  2026-02-19 18:17 ` [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Benjamin Tissoires
  4 siblings, 0 replies; 6+ messages in thread
From: Günther Noack @ 2026-02-19 15:43 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Günther Noack

The asus_report_fixup() function was returning a newly allocated
kmemdup()-allocated buffer, but never freeing it.  Switch to
devm_kzalloc() to ensure the memory is managed and freed automatically
when the device is removed.

The caller of report_fixup() does not take ownership of the returned
pointer, but it is permitted to return a pointer whose lifetime is at
least that of the input buffer.

Also fix a harmless out-of-bounds read by copying only the original
descriptor size.

Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Günther Noack <gnoack@google.com>
---
 drivers/hid/hid-asus.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 8ffcd12038e8..7a08e964b9cc 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1399,14 +1399,21 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		 */
 		if (*rsize == rsize_orig &&
 			rdesc[offs] == 0x09 && rdesc[offs + 1] == 0x76) {
-			*rsize = rsize_orig + 1;
-			rdesc = kmemdup(rdesc, *rsize, GFP_KERNEL);
-			if (!rdesc)
-				return NULL;
+			__u8 *new_rdesc;
+
+			new_rdesc = devm_kzalloc(&hdev->dev, rsize_orig + 1,
+						 GFP_KERNEL);
+			if (!new_rdesc)
+				return rdesc;
 
 			hid_info(hdev, "Fixing up %s keyb report descriptor\n",
 				drvdata->quirks & QUIRK_T100CHI ?
 				"T100CHI" : "T90CHI");
+
+			memcpy(new_rdesc, rdesc, rsize_orig);
+			*rsize = rsize_orig + 1;
+			rdesc = new_rdesc;
+
 			memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
 			rdesc[offs] = 0x19;
 			rdesc[offs + 1] = 0x00;
-- 
2.53.0.371.g1d285c8824-goog


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

* Re: [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid
  2026-02-19 15:43 [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Günther Noack
                   ` (3 preceding siblings ...)
  2026-02-19 15:43 ` [PATCH v2 4/4] HID: asus: avoid memory leak in asus_report_fixup() Günther Noack
@ 2026-02-19 18:17 ` Benjamin Tissoires
  4 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2026-02-19 18:17 UTC (permalink / raw)
  To: Jiri Kosina, Günther Noack; +Cc: linux-input, linux-kernel

On Thu, 19 Feb 2026 16:43:34 +0100, Günther Noack wrote:
> These patches fix a few memory leaks in HID report descriptor fixups.
> 
> FWIW, a good ad-hoc way to look for usages of allocation functions in
> these is:
> 
>   awk '/static.*report_fixup.*/,/^}/ { print FILENAME, $0 }' drivers/hid/hid-*.c \
>     | grep -E '(malloc|kzalloc|kcalloc|kmemdup)'
> 
> [...]

Applied to hid/hid.git (for-7.0/upstream-fixes), thanks!

[1/4] HID: Document memory allocation properties of report_fixup()
      https://git.kernel.org/hid/hid/c/6b3e458806e3
[2/4] HID: apple: avoid memory leak in apple_report_fixup()
      https://git.kernel.org/hid/hid/c/239c15116d80
[3/4] HID: magicmouse: avoid memory leak in magicmouse_report_fixup()
      https://git.kernel.org/hid/hid/c/91e8c6e601bd
[4/4] HID: asus: avoid memory leak in asus_report_fixup()
      https://git.kernel.org/hid/hid/c/2bad24c17742

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>


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

end of thread, other threads:[~2026-02-19 18:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-19 15:43 [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Günther Noack
2026-02-19 15:43 ` [PATCH v2 1/4] HID: Document memory allocation properties of report_fixup() Günther Noack
2026-02-19 15:43 ` [PATCH v2 2/4] HID: apple: avoid memory leak in apple_report_fixup() Günther Noack
2026-02-19 15:43 ` [PATCH v2 3/4] HID: magicmouse: avoid memory leak in magicmouse_report_fixup() Günther Noack
2026-02-19 15:43 ` [PATCH v2 4/4] HID: asus: avoid memory leak in asus_report_fixup() Günther Noack
2026-02-19 18:17 ` [PATCH v2 0/4] HID: Fix some memory leaks in drivers/hid Benjamin Tissoires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox