[PATCH v1] usb: typec: fusb302: Free log buffers on exit

Yuho Choi posted 1 patch 1 month, 3 weeks ago
drivers/usb/typec/tcpm/fusb302.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
[PATCH v1] usb: typec: fusb302: Free log buffers on exit
Posted by Yuho Choi 1 month, 3 weeks ago
fusb302_log() lazily allocates entries in chip->logbuffer[], but
fusb302_debugfs_exit() only removes the debugfs directory. The buffers are
not part of the devm-managed chip allocation, so they leak when the driver
is removed or probe fails after logging.

Free all log buffer entries during debugfs teardown.

Fixes: c034a43e72dd ("staging: typec: Fairchild FUSB302 Type-c chip driver")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
 drivers/usb/typec/tcpm/fusb302.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 3319f6a2b0c9..67ccbbd64caf 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -223,7 +223,16 @@ static void fusb302_debugfs_init(struct fusb302_chip *chip)
 
 static void fusb302_debugfs_exit(struct fusb302_chip *chip)
 {
+	int i;
+
 	debugfs_remove(chip->dentry);
+
+	mutex_lock(&chip->logbuffer_lock);
+	for (i = 0; i < LOG_BUFFER_ENTRIES; i++) {
+		kfree(chip->logbuffer[i]);
+		chip->logbuffer[i] = NULL;
+	}
+	mutex_unlock(&chip->logbuffer_lock);
 }
 
 #else
@@ -1784,8 +1793,8 @@ static int fusb302_probe(struct i2c_client *client)
 fwnode_put:
 	fwnode_handle_put(chip->tcpc_dev.fwnode);
 destroy_workqueue:
-	fusb302_debugfs_exit(chip);
 	destroy_workqueue(chip->wq);
+	fusb302_debugfs_exit(chip);
 
 	return ret;
 }
-- 
2.43.0
Re: [PATCH v1] usb: typec: fusb302: Free log buffers on exit
Posted by Greg KH 1 month, 3 weeks ago
On Fri, Aug 07, 2026 at 04:34:03PM -0400, Yuho Choi wrote:
> fusb302_log() lazily allocates entries in chip->logbuffer[], but
> fusb302_debugfs_exit() only removes the debugfs directory. The buffers are
> not part of the devm-managed chip allocation, so they leak when the driver
> is removed or probe fails after logging.
> 
> Free all log buffer entries during debugfs teardown.
> 
> Fixes: c034a43e72dd ("staging: typec: Fairchild FUSB302 Type-c chip driver")
> Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 3319f6a2b0c9..67ccbbd64caf 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -223,7 +223,16 @@ static void fusb302_debugfs_init(struct fusb302_chip *chip)
>  
>  static void fusb302_debugfs_exit(struct fusb302_chip *chip)
>  {
> +	int i;
> +
>  	debugfs_remove(chip->dentry);
> +
> +	mutex_lock(&chip->logbuffer_lock);
> +	for (i = 0; i < LOG_BUFFER_ENTRIES; i++) {
> +		kfree(chip->logbuffer[i]);
> +		chip->logbuffer[i] = NULL;
> +	}
> +	mutex_unlock(&chip->logbuffer_lock);

As you are tearing things down here, and there is no actual user, why is
the lock needed?  And if so, can you just use a guard() instead?

thanks,

greg k-h
Re: [PATCH v1] usb: typec: fusb302: Free log buffers on exit
Posted by Sebastian Andrzej Siewior 1 month ago
On 2026-08-08 09:10:28 [+0200], Greg KH wrote:
> > --- a/drivers/usb/typec/tcpm/fusb302.c
> > +++ b/drivers/usb/typec/tcpm/fusb302.c
> > @@ -223,7 +223,16 @@ static void fusb302_debugfs_init(struct fusb302_chip *chip)
> >  
> >  static void fusb302_debugfs_exit(struct fusb302_chip *chip)
> >  {
> > +	int i;
> > +
> >  	debugfs_remove(chip->dentry);
> > +
> > +	mutex_lock(&chip->logbuffer_lock);
> > +	for (i = 0; i < LOG_BUFFER_ENTRIES; i++) {
> > +		kfree(chip->logbuffer[i]);
> > +		chip->logbuffer[i] = NULL;
> > +	}
> > +	mutex_unlock(&chip->logbuffer_lock);
> 
> As you are tearing things down here, and there is no actual user, why is
> the lock needed?  And if so, can you just use a guard() instead?

That is correct. The whole thing is about vanish so locking is not
needed.
Looking at the actual user of that buffer, I'm curious if it wouldn't be
better to use dev_err()/ dev_info() for some of the output and other
which are just pure informative/ debug kind of information, hide behind
a trace event which can be enabled if needed.

> thanks,
> 
> greg k-h

Sebastian