* [PATCH] usb: gadget: f_hid: free report list on release
@ 2026-11-08 22:32 Noah Weinberger
2026-11-08 22:44 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Noah Weinberger @ 2026-11-08 22:32 UTC (permalink / raw)
To: nkapron@google.com, gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Hello,
Entries added to hidg->report_list by the GADGET_HID_WRITE_GET_REPORT
ioctl are never freed: hidg_release() releases report_desc,
set_report_buf and hidg itself, but never walks the list (and
hidg_unbind() does not either). Each distinct report_id used over the
lifetime of a function instance therefore leaks one entry (~72 bytes),
which adds up across gadget reconfiguration cycles.
Free the list when the device's last reference is dropped.
Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/usb/gadget/function/f_hid.c:679-682 (list add), 121-128 (hidg_release).
The report-list mechanism is newer; the leak is present at HEAD.
This was found during an assisted (AI-supported) static source review of
mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
reporter against that tree. The patch below has NOT been compiled and has
NOT been runtime tested, and no reproducer was produced - the triggering
conditions are described above instead. Please treat this as a report for
verification rather than a battle-tested fix.
Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
---
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 3c6b43d..dcf6dce 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -121,6 +121,13 @@ static inline struct f_hidg *func_to_hidg(struct usb_function *f)
static void hidg_release(struct device *dev)
{
struct f_hidg *hidg = container_of(dev, struct f_hidg, dev);
+ struct report_entry *entry, *tmp;
+
+ list_for_each_entry_safe(entry, tmp, &hidg->report_list, node) {
+ list_del(&entry->node);
+ kfree(entry);
+ }
+
kfree(hidg->report_desc);
kfree(hidg->set_report_buf);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] usb: gadget: f_hid: free report list on release
2026-11-08 22:32 [PATCH] usb: gadget: f_hid: free report list on release Noah Weinberger
@ 2026-11-08 22:44 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-11-08 22:44 UTC (permalink / raw)
To: Noah Weinberger; +Cc: linux-usb@vger.kernel.org
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] usb: gadget: f_hid: missing Fixes/Signed-off-by tags and
changelog/style nits
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] usb: gadget: f_hid: memory leak of hidg->get_req in
hidg_unbind() [1]
--
commit 6b437be3613ff7b4c0c239461caa731cf0c00ade
Author: Noah Weinberger <NoahWeinberger2003@proton.me>
> usb: gadget: f_hid: free report list on release
>
> Hello,
>
> Entries added to hidg->report_list by the GADGET_HID_WRITE_GET_REPORT
> ioctl are never freed: hidg_release() releases report_desc,
> set_report_buf and hidg itself, but never walks the list (and
> hidg_unbind() does not either). Each distinct report_id used over the
> lifetime of a function instance therefore leaks one entry (~72 bytes),
> which adds up across gadget reconfiguration cycles.
>
> Free the list when the device's last reference is dropped.
>
> Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/usb/gadget/function/f_hid.c:679-682 (list add), 121-128 (hidg_release).
> The report-list mechanism is newer; the leak is present at HEAD.
>
> This was found during an assisted (AI-supported) static source review of
> mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
> reporter against that tree. The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
> conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
>
> Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
[Severity: Low]
This commit fixes the report_list leak introduced in a139c98f760e ("USB:
gadget: f_hid: Add GET_REPORT via userspace IOCTL"), so it should include:
Fixes: a139c98f760e ("USB: gadget: f_hid: Add GET_REPORT via userspace IOCTL")
Note that 4e0a88254ad5 ("usb: gadget: f_hid: move list and spinlock inits
from bind to alloc") is a prerequisite for stable backports so that
hidg->report_list is initialized in hidg_alloc() before device_initialize().
The commit message is also missing a Signed-off-by: trailer, and the
greeting and testing disclaimers belong below the "---" tear line rather
than in the permanent changelog. Finally, the diff adds a second
consecutive blank line after the list_for_each_entry_safe() loop in
hidg_release().
> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index 3c6b43d06a6d1..dcf6dce1e1d6c 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -121,6 +121,13 @@ static inline struct f_hidg *func_to_hidg(struct usb_function *f)
> static void hidg_release(struct device *dev)
> {
> struct f_hidg *hidg = container_of(dev, struct f_hidg, dev);
> + struct report_entry *entry, *tmp;
> +
> + list_for_each_entry_safe(entry, tmp, &hidg->report_list, node) {
> + list_del(&entry->node);
> + kfree(entry);
> + }
> +
>
> kfree(hidg->report_desc);
> kfree(hidg->set_report_buf);
[Severity: Medium]
This problem wasn't introduced by this patch, but does hidg_unbind() also
leak hidg->get_req when no GET_REPORT request is pending [1]?
hidg_bind() unconditionally allocates hidg->get_req via
usb_ep_alloc_request(c->cdev->gadget->ep0, GFP_ATOMIC), whereas
hidg_disable() only frees it when a GET_REPORT request is in flight:
drivers/usb/gadget/function/f_hid.c:hidg_disable() {
...
spin_lock_irqsave(&hidg->get_report_spinlock, flags);
if (!hidg->get_report_returned) {
usb_ep_free_request(f->config->cdev->gadget->ep0, hidg->get_req);
hidg->get_req = NULL;
hidg->get_report_returned = true;
}
spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
...
}
When the gadget is unbound while idle, hidg->get_report_returned is true
and hidg_unbind() never frees hidg->get_req, leaking the usb_request on
every bind/unbind cycle.
[1] https://sashiko.dev/#/bug/linux-b7d8a0a0-a788-48be-a1a4-1106b659764e
--
Sashiko AI review · https://sashiko.dev/#/patchset/1QsGBy1k7rgvDtVuIrw8wkuScqkmKf-h2yKjARCmaLFH2TFGvlr8izECUG0Pq-FRMo3NqA6bmTOayxrZ8xc0xwSOKjLLpz9flCzQomrlnqk=@proton.me?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-08 22:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-08 22:32 [PATCH] usb: gadget: f_hid: free report list on release Noah Weinberger
2026-11-08 22:44 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox