linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Gebhard <michael.gebhard@fau.de>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org
Subject: Revert "usb: typec: fusb302: Fix debugfs issue"
Date: Fri, 21 Sep 2018 16:13:07 +0200	[thread overview]
Message-ID: <20180921141307.GA29636@fau.de> (raw)

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.
---
2.19.0

=======

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@fau.de>
---
 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

                 reply	other threads:[~2018-09-21 14:13 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20180921141307.GA29636@fau.de \
    --to=michael.gebhard@fau.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).