Message-ID: <m1zkxj27xp.fsf_-_@fess.ebiederm.org>
Date:	Thu, 22 Jul 2010 02:16:02 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Greg KH <gregkh@...e.de>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Greg KH <greg@...ah.com>, "Rafael J. Wysocki" <rjw@...k.pl>,
	"Maciej W. Rozycki" <macro@...ux-mips.org>,
	Kay Sievers <kay.sievers@...y.org>,
	Johannes Berg <johannes@...solutions.net>,
	netdev <netdev@...r.kernel.org>
Subject: [PATCH] Driver-core: Fix bluetooth network device rename regression


With CONFIG_SYSFS_DEPRECATED_V2 enabled I can rename any network device
anything as long as the new name does not conflict with another network
device.

With CONFIG_SYSFS_DEPRECATED_V2 disabled without this fix bluetooth benp
devices, and the mac80211_hwsim driver can not be renamed to any arbitrary
name that happens to conflict with any other name that is used in their
parent devices directory.

The device model usage of the bluetooth bnep driver has not changed since
the current device model sysfs laytout was introduced.   Making this a latent
and very annoying regression.

This regression was reported at the time it was introduced and apprently a
few cases were missed by:

commit 864062457a2e444227bd368ca5f2a2b740de604f
Author: Kay Sievers <kay.sievers@...y.org>
Date:   Wed Mar 14 03:25:56 2007 +0100

    driver core: fix namespace issue with devices assigned to classes

      - uses a kset in "struct class" to keep track of all directories
        belonging to this class
      - merges with the /sys/devices/virtual logic.
      - removes the namespace-dir if the last member of that class
        leaves the directory.

    There may be locking or refcounting fixes left, I stopped when it seemed
    to work with network and sound modules. :)

    From: Kay Sievers <kay.sievers@...y.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>

That bug fix had a completely undocumented and apparently deliberate omission
where it does not apply in some cases.  Those omitted cases cover the
bluetooth network driver.

What makes this regression a serious issue now is the introduction of network
namespace support in sysfs took this from a mild bug to complete driver
non-function.

Several reasons have been put forward not to use this one line bug fix in
the driver core.

- We don't have special cases in the driver core.

  This is non-sense we currently have special cases for block devices
  scattered all throughout the driver core.

- The driver is at fault.

  The bluetooth driver's driver core usage predates the introduction of the
  current sysfs layout.  It has been 3 years and no one has bothered
  to change the driver in all of this time.

  If this was really a driver issue that someone cared about and not simply
  an academic issue the driver should have been fixed long since.

I offer these reasons to make the change.

- There is not enough information available for sysfs to support rename
  or delete of symlinks when only the source directory but not the destination
  directory is tagged.

- All of the proposals put forth will change the sysfs layout slightly in
  one way or another.

- A network device special case makes sense as network devices are unique
  in placing a user choosen name sysfs.

- The driver that are affected are different in sysfs from all other
  network devices which likely already consuses any userspace software
  that cares about the exact device layout.

- The change is one line that is obviously correct and has no chance
  of affecting anything that it is not a network device.

- Real users have hit this problem and reported this bug and those users
  deserve drivers that work.

The fix is a trivial change to get_device_parent to always create
the network class directory in any parent of a network device,
Ignoring the incorrect, non-obvious and esoteric rules that the
driver core is trying to impose on the creation of class directories.

Ideally I would remove those crazy special cases get_device_parent for all
devices but in testing it was observed there are other devices such as the
input layer that don't create the class directories today, and applying the
change broadly is likely to break something in userspace and there is no
need to take that risk.

Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>
---
 drivers/base/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 9630fbd..ffb8443 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -673,7 +673,7 @@ static struct kobject *get_device_parent(struct device *dev,
 		 */
 		if (parent == NULL)
 			parent_kobj = virtual_device_parent(dev);
-		else if (parent->class)
+		else if (parent->class && (strcmp(dev->class->name, "net") != 0))
 			return &parent->kobj;
 		else
 			parent_kobj = &parent->kobj;
-- 
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists