drivers/hid/hid-apple.c | 4 +--- drivers/hid/hid-asus.c | 15 +++++++++++---- drivers/hid/hid-magicmouse.c | 4 +--- 3 files changed, 13 insertions(+), 10 deletions(-)
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
Günther Noack (3):
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 +---
3 files changed, 13 insertions(+), 10 deletions(-)
--
2.53.0.335.g19a08e0c02-goog
On Feb 17 2026, Günther Noack wrote:
> 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.
No. Look at hid_close_report() in drivers/hid/hid-core.c.
HID still hasn't fully migrated to devm, so as a rule of thumb, if you
change a kzalloc into a devm_kzalloc, you are getting into troubles
unless you fix the all the kfree path.
>
> 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
If the memory is *not* managed, why would gemini converts kzalloc into
devm variants without changing the kfree paths????
> `report_fixup` implementation allocates a new buffer and returns that,
> that will not get freed by the caller.
This is wrong. See hid_close_report(): if the new rdesc (after fixup)
differs from the one initially set, there is an explicit call to
kfree().
-> there is no memleak AFAICT, and your prompt is wrong.
Cheers,
Benjamin
> 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
>
>
> Günther Noack (3):
> 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 +---
> 3 files changed, 13 insertions(+), 10 deletions(-)
>
> --
> 2.53.0.335.g19a08e0c02-goog
>
Hello!
On Tue, Feb 17, 2026 at 07:36:46PM +0100, Benjamin Tissoires wrote:
> On Feb 17 2026, 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)'
> >
> > The devm_* variants are safe in this context, because they tie the
> > allocated memory to the lifetime of the driver.
>
> No. Look at hid_close_report() in drivers/hid/hid-core.c.
>
> HID still hasn't fully migrated to devm, so as a rule of thumb, if you
> change a kzalloc into a devm_kzalloc, you are getting into troubles
> unless you fix the all the kfree path.
OK, I have not verified where the devm-allocated objects get freed up.
If devm_*() is not possible here, then the drivers hid-asus.c and
hid-gembird.c have two additional memory leaks, because they do that.
$ awk '/static.*report_fixup.*/,/^}/ { print FILENAME, $0 }' drivers/hid/hid-*.c | grep -E 'alloc'
drivers/hid/hid-asus.c new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
drivers/hid/hid-gembird.c new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
(That is without my threee patches)
> > 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
>
> If the memory is *not* managed, why would gemini converts kzalloc into
> devm variants without changing the kfree paths????
I'm not sure I understand the question, it's not clear to me what you
mean by the "kfree paths".
I have seen usages of devm in other HID drivers and I was under the
impression that devm_* allocations would work in the HID subsystem to
allocate objects which are then freed automatically at a later point
when the device gets removed. Is that inaccurate?
> > `report_fixup` implementation allocates a new buffer and returns that,
> > that will not get freed by the caller.
>
> This is wrong. See hid_close_report(): if the new rdesc (after fixup)
> differs from the one initially set, there is an explicit call to
> kfree().
>
> -> there is no memleak AFAICT, and your prompt is wrong.
See my discussion in [1]. The pointer returned by report_fixup() is
immediately discarded in the position marked with (4). This is still
in the hid_open_report() function where the leak happens.
Let me know whether this makes sense. I'm happy to be corrected, but
so far, I still have the feeling that my reasoning is sound.
—Günther
[1] https://lore.kernel.org/all/aZTEnPEHcWEkoTJR@google.com/
© 2016 - 2026 Red Hat, Inc.