Re: [PATCH] Revert "usb: typec: fusb302: Fix debugfs issue"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Also, sorry for the cc messup, I'll put my two previous
mails here, just so they'll appear in the archives.

As heikki just pointed out, there already is a fix for this problem.

=======

I do have a couple of questions on how to best make sure <rootdir>
is always NULL or pointing to a valid dentry.

Other drivers (e.g. drivers/usb/typec/tcpm.c) also seem
to handle this problem by leaving behind their debugfs rootdir.
This causes their debugfs entries to be unusable if the module
is reloaded once.

My next solution attempt would be, to check whether the rootdir
is empty, then remove it at set 'rootdir = NULL'. But attempting
to remove the rootdir everytime a link to a device is dropped seems
conceptually wrong to me since the rootdir is per driver, not per
device handled by this driver.

This leads to my third idea, the rootdir could be created and removed
in the register and unregister functions of the driver, but then
it could not use the module_i2c_driver macro to supply these functions.
Instead the modules register function would create the rootdir and then
call i2c_register_driver. However looking at other drivers using the
module_i2c_driver macro or an equivalent seems to be best practice.

=======

This reverts commit c9359f416207 ("usb: typec: fusb302: Fix debugfs issue")

Removing <rootdir> debugfs directory leaves <rootdir> pointing at
freed memory. The next subsequent call to fusb302_probe() causes a
NULL pointer derenference in debugfs_create_file() in
fusb302_debugfs_init(). This problem occurs, since <rootdir> is not
only removed when unloading the driver, but also when fusb302_probe()
fails or the link to a device is dropped.

The original reason for the reverted commit does not apply anymore
since commit bc9832c4a192 ("USB: typec: fsusb302: no need to check
return value of debugfs_create_dir()") removed error checking on
debugfs_create_dir(). The error check caused fusb302_probe() to fail,
if the "fusb302" debugfs directory still existed from previously
loading and unloading the fusb302 module.

Signed-off-by: Michael Gebhard <michael.gebhard@xxxxxx>
---
 drivers/usb/typec/fusb302/fusb302.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c
index 82bed9810be6..bd99bc947cb1 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -229,7 +229,6 @@ static void fusb302_debugfs_init(struct fusb302_chip *chip)
 static void fusb302_debugfs_exit(struct fusb302_chip *chip)
 {
 	debugfs_remove(chip->dentry);
-	debugfs_remove(rootdir);
 }

 #else
--
2.19.0



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux