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

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

[Yuho Choi]

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

[Greg KH]

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

[Sebastian Andrzej Siewior]

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