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

["Günther Noack" <gnoack@google.com>]

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/