From: sashiko-bot@kernel.org
To: "Noah Weinberger" <NoahWeinberger2003@proton.me>
Cc: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: Re: [PATCH] usb: gadget: f_hid: free report list on release
Date: Sun, 08 Nov 2026 22:44:46 +0000 [thread overview]
Message-ID: <sashiko-outbox-204126@kernel.org> (raw)
In-Reply-To: <1QsGBy1k7rgvDtVuIrw8wkuScqkmKf-h2yKjARCmaLFH2TFGvlr8izECUG0Pq-FRMo3NqA6bmTOayxrZ8xc0xwSOKjLLpz9flCzQomrlnqk=@proton.me>
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
prev parent reply other threads:[~2026-11-08 22:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=sashiko-outbox-204126@kernel.org \
--to=sashiko-bot@kernel.org \
--cc=NoahWeinberger2003@proton.me \
--cc=linux-usb@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.