[PATCH 0/3] HID: Fix some memory leaks in drivers/hid

Günther Noack posted 3 patches 1 month, 2 weeks ago
There is a newer version of this series
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(-)
[PATCH 0/3] HID: Fix some memory leaks in drivers/hid
Posted by Günther Noack 1 month, 2 weeks ago
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
Re: [PATCH 0/3] HID: Fix some memory leaks in drivers/hid
Posted by Benjamin Tissoires 1 month, 2 weeks ago
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
> 
Re: [PATCH 0/3] HID: Fix some memory leaks in drivers/hid
Posted by Günther Noack 1 month, 2 weeks ago
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/