All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Huang Wei <huangwei@kylinos.cn>
Cc: Minas Harutyunyan <hminas@synopsys.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	kakapapa2 <kakapapa2@gmail.com>
Subject: Re: [PATCH] usb: dwc2: debugfs: fix memory leak of hsotg->regset
Date: Sat, 5 Sep 2026 01:00:42 +0000	[thread overview]
Message-ID: <aptmNMpoQt-fL4OK@vbox> (raw)
In-Reply-To: <20260807100400.3179625-1-huangwei@kylinos.cn>

On Fri, Aug 07, 2026, Huang Wei wrote:
> hsotg->regset is allocated in dwc2_debugfs_init() using devm_kzalloc()
> but is never explicitly freed. While devres would eventually reclaim
> the memory, the regset is logically owned by the debugfs lifetime:
> dwc2_debugfs_exit() only removes the debugfs directory and leaves
> hsotg->regset dangling.
> 
> Switch to kzalloc() and free it explicitly in dwc2_debugfs_exit(),
> mirroring the equivalent fix already applied to dwc3 in commit
> e6bdf8195b4a ("usb: dwc3: fix memory leak of dwc->regset"). Also set
> the pointer to NULL after freeing to avoid a stale dangling pointer.

This 2nd paragraph sounds as if dwc3 did the same thing (changing
devm_kzalloc -> kzalloc). The mentioned commit is solving a separate
problem. That's misleading. Please remove this paragraph.

> 
> Reported-by: kakapapa2 <kakapapa2@gmail.com>
> Closes: https://urldefense.com/v3/__https://bugzilla.kernel.org/show_bug.cgi?id=219977__;!!A4F2R9G_pg!fZ_fOd6cc_gIWYTIuT9N9wCV-tBbojEsj1lrFMFZZHpEGRwC2EcvZzNOTtFndJI6z1GZYyQQQmn8L4pqnvVgioYL$ 

The report and the commit message are somewhat misleading because regset
is currently allocated with devm_kzalloc(), so it is not expected to be
freed from dwc2_debugfs_exit(). The issue is really about the allocation
lifetime with the debugfs lifetime rather than fixing a memory leak in
the existing implementation.

But overall, IMO, this change is good to have. If you send a v2 with a
fix up commit message and send a v2, you can add this:

Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

BR,
Thinh

> Signed-off-by: Huang Wei <huangwei@kylinos.cn>
> ---
>  drivers/usb/dwc2/debugfs.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
> index 3116ac72747f..2ecbf6523aaa 100644
> --- a/drivers/usb/dwc2/debugfs.c
> +++ b/drivers/usb/dwc2/debugfs.c
> @@ -9,6 +9,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/debugfs.h>
>  #include <linux/seq_file.h>
> +#include <linux/slab.h>
>  #include <linux/uaccess.h>
>  
>  #include "core.h"
> @@ -787,8 +788,7 @@ int dwc2_debugfs_init(struct dwc2_hsotg *hsotg)
>  	/* Add gadget debugfs nodes */
>  	dwc2_hsotg_create_debug(hsotg);
>  
> -	hsotg->regset = devm_kzalloc(hsotg->dev, sizeof(*hsotg->regset),
> -								GFP_KERNEL);
> +	hsotg->regset = kzalloc_obj(*hsotg->regset, GFP_KERNEL);
>  	if (!hsotg->regset) {
>  		ret = -ENOMEM;
>  		goto err;
> @@ -810,4 +810,6 @@ void dwc2_debugfs_exit(struct dwc2_hsotg *hsotg)
>  {
>  	debugfs_remove_recursive(hsotg->debug_root);
>  	hsotg->debug_root = NULL;
> +	kfree(hsotg->regset);
> +	hsotg->regset = NULL;
>  }
> -- 
> 2.25.1
> 
> 

  parent reply	other threads:[~2026-09-05  1:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 10:04 [PATCH] usb: dwc2: debugfs: fix memory leak of hsotg->regset Huang Wei
2026-09-02  9:05 ` Huang Wei
2026-09-02  9:25   ` Greg Kroah-Hartman
2026-09-03  6:44     ` Huang Wei
2026-09-05  1:00 ` Thinh Nguyen [this message]
2026-09-09  2:10 ` [PATCH v2] " Huang Wei

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=aptmNMpoQt-fL4OK@vbox \
    --to=thinh.nguyen@synopsys.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hminas@synopsys.com \
    --cc=huangwei@kylinos.cn \
    --cc=kakapapa2@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /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.